tests

.formatter.exs

@@ -3,6 +3,12 @@
     "{mix,.formatter}.exs",
     "{config,lib,test}/**/*.{ex,exs}"
   ],
+  locals_without_parens: [
+    assert_error: 2,
+    refute_errors: 1,
+    assert_style: 2,
+    assert_style: 1
+  ],
   plugins: [Styler],
   line_length: 122
 ]

lib/style/speedo.ex

@@ -13,6 +13,7 @@ defmodule Styler.Speedo do
 
   @definer ~w(def defp defmacro defmacrop defguard defguardp)a
   def run({{def, _, [{name, m, args} | _]}, _} = zipper, ctx) when def in @definer and is_atom(name) do
+    {name, m, args} = if name == :when, do: hd(args), else: {name, m, args}
     line = m[:line]
     name = to_string(name)
     error = %{file: ctx.file, line: line, check: nil, message: nil}
@@ -24,9 +25,6 @@ defmodule Styler.Speedo do
 
     predicate_error =
       cond do
-        String.starts_with?(name, "is_") && String.ends_with?(name, "?") ->
-          %{predicate_error | message: "`#{def} #{name}` wow choose `?` or `is_`, you monster"}
-
         def in ~w(def defp)a and String.starts_with?(name, "is_") ->
           %{predicate_error | message: "`#{def} #{name}` is invalid -- use `?` not `is_` for defs"}
 
@@ -39,7 +37,7 @@ defmodule Styler.Speedo do
 
     var_errors =
       Enum.map(args || [], fn arg ->
-        {_, var_errors} = arg |> Zipper.zip() |> Zipper.traverse([], &find_vars_with_bad_names(&1, &2, ctx.file))
+        {_, var_errors} = arg |> Zipper.zip() |> Zipper.traverse([], &readability_variable_names(&1, &2, ctx.file))
         var_errors
       end)
 
@@ -63,6 +61,9 @@ defmodule Styler.Speedo do
       |> Zipper.down()
       |> Zipper.down()
       |> Zipper.right()
+      # coerce single-child defmodules to have the same shape as multi-child
+      |> Styler.Style.find_nearest_block()
+      |> Zipper.up()
       |> Zipper.children()
       |> Enum.flat_map(fn
         {:defexception, _, _} ->
@@ -76,7 +77,7 @@ defmodule Styler.Speedo do
               }
             ]
 
-        {:@, _, [{:impl, m, [true]}]} ->
+        {:@, m, [{:impl, _, [{:__block__, _, [true]}]}]} ->
           [%{error | line: m[:line], check: ImplTrue, message: "`@impl true` not allowed"}]
 
         {:@, _, [{name, m, _}]} ->
@@ -91,13 +92,39 @@ defmodule Styler.Speedo do
     {zipper, Map.update!(ctx, :errors, &[[pascal | errors] | &1])}
   end
 
+  def run({{:<-, m, [lhs, _] = args}, _} = zipper, ctx) do
+    # Credo.Check.Readability.WithCustomTaggedTuple
+    tag_error =
+      case args do
+        [{:__block__, _, [{{:__block__, _, [tag]}, _}]}, {:__block__, _, [{{:__block__, _, [tag]}, _}]}] ->
+          msg = "tagging tuples with things like `#{inspect(tag)}` is known to be evil"
+          %{file: ctx.file, line: m[:line], check: WithCustomTaggedTuple, message: msg}
+        _ -> []
+      end
+
+    {_, assignment_errors} = lhs |> Zipper.zip() |> Zipper.traverse([], &readability_variable_names(&1, &2, ctx.file))
+    {zipper, Map.update!(ctx, :errors, &[[tag_error | assignment_errors] | &1])}
+  end
+
   # Credo.Check.Readability.VariableNames
   # the `=` here will double report when nested in a case. need to move it to its own clause w/ "in block"
-  def run({{assignment_op, _, [lhs, _]}, _} = zipper, ctx) when assignment_op in ~w(= <- ->)a do
-    {_, errors} = lhs |> Zipper.zip() |> Zipper.traverse([], &find_vars_with_bad_names(&1, &2, ctx.file))
+  def run({{assignment_op, _, [lhs, _]}, _} = zipper, ctx) when assignment_op in ~w(= ->)a do
+    {_, errors} = lhs |> Zipper.zip() |> Zipper.traverse([], &readability_variable_names(&1, &2, ctx.file))
     {zipper, Map.update!(ctx, :errors, &[errors | &1])}
   end
 
+
+  # Credo.Check.Readability.StringSigils
+  def run({{:__block__, [{:delimiter, ~s|"|} | _] = m, [string]}, _} = zipper, ctx) when is_binary(string) do
+    if string =~ ~r/".*".*".*"/ do
+      msg = "use a sigil for #{inspect(string)}, it has too many quotes"
+      error = %{file: ctx.file, line: m[:line], check: StringSigils, message: msg}
+      {zipper, Map.update!(ctx, :errors, &[error | &1])}
+    else
+      {zipper, ctx}
+    end
+  end
+
   def run(zipper, context) do
     case run!(Zipper.node(zipper), context.file) do
       nil -> {zipper, context}
@@ -106,7 +133,7 @@ defmodule Styler.Speedo do
     end
   end
 
-  defp find_vars_with_bad_names({{name, m, nil}, _} = zipper, errors, file) do
+  defp readability_variable_names({{name, m, nil}, _} = zipper, errors, file) do
     if name in [:__CALLER__, :__DIR__, :__ENV__, :__MODULE__] or snake_case?(name) do
       {zipper, errors}
     else
@@ -115,45 +142,13 @@ defmodule Styler.Speedo do
     end
   end
 
-  defp find_vars_with_bad_names(zipper, errors, _) do
+  defp readability_variable_names(zipper, errors, _) do
     {zipper, errors}
   end
 
-  # Credo.Check.Readability.StringSigils
-  defp run!({:__block__, [{:delimiter, ~s|"|} | _] = m, [string]}, file) when is_binary(string) do
-    if string =~ ~r/".*".*"/ do
-      msg = "use a sigil for #{inspect(string)}, it has too many quotes"
-      %{file: file, line: m[:line], check: StringSigils, message: msg}
-    end
-  end
-
-  # Credo.Check.Readability.WithCustomTaggedTuple
-  defp run!(
-         {:<-, m, [{:__block__, _, [{{:__block__, _, [tag]}, _}]}, {:__block__, _, [{{:__block__, _, [tag]}, _}]}]},
-         file
-       ) do
-    msg = "tagging tuples with things like `#{inspect(tag)}` is known to be evil"
-    %{file: file, line: m[:line], check: WithCustomTaggedTuple, message: msg}
-  end
 
   defp run!(_, _), do: []
 
-  @badName :bad
-  def naughtyFun(naughtyParam) do
-    IO.inspect(@badName)
-
-    naughtyAssignment = :ok
-
-    with {:ugh, naughtyVar} <- {:ugh, naughtyParam} do
-      naughtyVar
-    end
-  end
-
-  def foo(naughtyParam2, %{bar: :x = naughtyParam3}) do
-  end
-
-  defexception [:foo]
-
   defp snake_case?(name), do: to_string(name) =~ ~r/^[[:lower:]\d\_\!\?]+$/u
   defp pascal_case?(name), do: to_string(name) =~ ~r/^[A-Z][a-zA-Z0-9\.]*$/
 end

lib/styler.ex

@@ -68,7 +68,7 @@ defmodule Styler do
     quoted_to_string(ast, comments, formatter_opts)
   end
 
-  def lint(input, file) do
+  def lint(input, file \\ "nofile") do
     {ast, comments} = string_to_quoted_with_comments(input, file)
     context = %{comments: comments, file: file, errors: []}
 

test/speedo/readability_test.exs

@@ -0,0 +1,114 @@
+defmodule Styler.Speedo.ReadabilityTest do
+  use Styler.SpeedoCase
+
+  alias Credo.Check.Readability.FunctionNames
+  alias Credo.Check.Readability.ImplTrue
+  alias Credo.Check.Readability.ModuleAttributeNames
+  alias Credo.Check.Readability.ModuleNames
+  alias Credo.Check.Readability.PredicateFunctionNames
+  alias Credo.Check.Readability.StringSigils
+  alias Credo.Check.Readability.VariableNames
+  alias Credo.Check.Readability.WithCustomTaggedTuple
+
+  describe "Credo.Check.Readability.FunctionNames" do
+    test "positives" do
+      for def <- ~w(def defp defmacro defmacrop defguard defguardp) do
+        assert_error "#{def} ooF", FunctionNames
+      end
+    end
+
+    test "negatives" do
+      for def <- ~w(def defp defmacro defmacrop defguard defguardp) do
+        refute_errors "#{def} snake(), do: :ok"
+        refute_errors "#{def} snake_case(), do: :ok"
+      end
+
+      refute_errors "def unquote(foo)(), do: :ok"
+    end
+  end
+
+  describe "Credo.Check.Readability.ImplTrue" do
+    test "this one's pretty simple ngl" do
+      assert_error "defmodule Foo do @impl true end", ImplTrue
+      refute_errors "defmodule Foo do @impl Blue end"
+    end
+  end
+
+  describe "Credo.Check.Readability.ModuleAttributeNames" do
+    test "snake s'il vous plaît" do
+      assert_error "defmodule Foo do @weEeEe :foo end", ModuleAttributeNames
+      refute_errors "defmodule Foo do @snake :always_snakey end"
+      refute_errors "defmodule Foo do bar = @wEeEeE end"
+    end
+  end
+
+  describe "Credo.Check.Readability.ModuleNames" do
+    test "pascal por favor" do
+      assert_error "defmodule Snake_Kinda do end", ModuleNames
+      refute_errors "defmodule OkayName do end"
+      refute_errors "defmodule Okay.Name do end"
+    end
+  end
+
+  describe "Credo.Check.Readability.PredicateFunctionNames" do
+    test "defs dont get `is_` prefix" do
+      for def <- ~w(def defp) do
+        assert_error "#{def} is_foo?", PredicateFunctionNames
+        assert_error "#{def} is_foo?(bar)", PredicateFunctionNames
+        assert_error "#{def} is_foo?(bar) when bar != :baz, do: :bop", PredicateFunctionNames
+        refute_errors "#{def} foo?"
+        refute_errors "#{def} foo?(bar)"
+        refute_errors "#{def} foo?(bar) when bar != :baz, do: :bop"
+      end
+    end
+
+    test "macros and guards dont get `?` suffix" do
+      for def <- ~w(defmacro defmacrop defguard defguardp) do
+        assert_error "#{def} is_foo?", PredicateFunctionNames
+        assert_error "#{def} is_foo?(bar)", PredicateFunctionNames
+        assert_error "#{def} is_foo?(bar) when bar != :baz, do: :bop", PredicateFunctionNames
+        refute_errors "#{def} is_foo"
+        refute_errors "#{def} is_foo(bar)"
+        refute_errors "#{def} is_foo(bar) when bar != :baz, do: :bop"
+      end
+    end
+  end
+
+  describe "Credo.Check.Readability.StringSigils" do
+    test "3 escaped quotes tops" do
+      assert_error ~s|x = "\\"1\\"2\\"3\\"4"|, StringSigils
+      refute_errors ~s|x = ~s{"1"2"3"4}|
+      refute_errors ~s|x = "\\"1\\"2\\"3"|
+    end
+  end
+
+  describe "Credo.Check.Readability.VariableNames" do
+    test "reports violations on variable creation only " do
+      errors =
+        lint("""
+        def foo(badName) do
+          with {:ok, anotherBadName} <- badName do
+            [stopCamelCase | camelsTail] = anotherBadName
+            lovely_var_name = functionNotYourFault()
+          end
+        end
+        """)
+
+      assert Enum.count(errors) == 4
+
+      errors = errors |> Enum.group_by(& &1.line) |> Map.new()
+
+      assert [%{check: VariableNames, message: "badName"}] = errors[1]
+      assert [%{check: VariableNames, message: "anotherBadName"}] = errors[2]
+      assert [one, two] = errors[3]
+      assert "stopCamelCase" in [one.message, two.message]
+      assert "camelsTail" in [one.message, two.message]
+    end
+  end
+
+  describe "Credo.Check.Readability.WithCustomTaggedTuple" do
+    test "shames tagged tuples" do
+      assert_error "with {:ooph, result} <- {:ooph, call()}, do: :ok", WithCustomTaggedTuple
+    end
+  end
+end

test/support/speedo_case.ex

@@ -0,0 +1,50 @@
+# Copyright 2023 Adobe. All rights reserved.
+# This file is licensed to you under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License. You may obtain a copy
+# of the License at http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software distributed under
+# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+# OF ANY KIND, either express or implied. See the License for the specific language
+# governing permissions and limitations under the License.
+
+defmodule Styler.SpeedoCase do
+  @moduledoc """
+  Helpers around testing Style rules.
+  """
+  use ExUnit.CaseTemplate
+
+  using do
+    quote do
+      import unquote(__MODULE__), only: [assert_error: 2, refute_errors: 1, lint: 1]
+    end
+  end
+
+  defmacro assert_error(code, check) do
+    quote location: :keep, bind_quoted: [code: code, check: check] do
+      errors = lint(code)
+
+      if Enum.empty?(errors) and ExUnit.configuration()[:trace] do
+        {ast, comments} = Styler.string_to_quoted_with_comments(code)
+        dbg(ast)
+        dbg(comments)
+      end
+
+      assert [%{check: ^check, message: m, line: l, file: f}] = errors
+      assert m, "message was nil for #{check} in #{inspect(errors)}"
+      assert l, "line was nil for #{check} in #{inspect(errors)}"
+      assert f, "file was nil for #{check} in #{inspect(errors)}"
+    end
+  end
+
+  def refute_errors(code) do
+    assert [] = lint(code)
+  end
+
+  def lint(code) do
+    code
+    |> Styler.lint()
+    |> List.flatten()
+    |> Enum.reject(&is_nil/1)
+  end
+end