diff --git a/.formatter.exs b/.formatter.exs index dac196f8..76f376bf 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -4,6 +4,5 @@ "{config,lib,test}/**/*.{ex,exs}" ], plugins: [Styler], - styler: [dictate: true], line_length: 122 ] diff --git a/lib/mix/tasks/speedo.ex b/lib/mix/tasks/speedo.ex index 3ef2de8f..53683816 100644 --- a/lib/mix/tasks/speedo.ex +++ b/lib/mix/tasks/speedo.ex @@ -36,11 +36,8 @@ defmodule Mix.Tasks.Speedo do |> Path.wildcard(match_dot: true) |> Enum.filter(&String.ends_with?(&1, [".ex", "exs"])) end) - |> Task.async_stream(fn file -> - input = file |> File.read!() |> String.trim() - IO.inspect file - Styler.lint(input, file) - end, + |> Task.async_stream( + fn file -> file |> File.read!() |> Styler.lint(file) end, ordered: false, timeout: :infinity ) @@ -54,12 +51,17 @@ defmodule Mix.Tasks.Speedo do defp check!(errors) do errors + |> Enum.reject(&is_nil/1) |> Enum.group_by(& &1.check) + |> Enum.sort_by(fn {check, _} -> check end) |> Enum.each(fn {check, errors} -> - Mix.shell().error("#{check} violations") + check = check |> to_string() |> String.trim_leading("Elixir.") + Mix.shell().error("\n#{check} violations") Mix.shell().error("--------------------------------------------------------------") - for %{file: file, line: line} <- errors do - Mix.shell().error("#{Path.relative_to_cwd(file)}:#{line}") + + for %{file: file, line: line, message: message} <- errors do + Mix.shell().info([IO.ANSI.light_yellow(), message, IO.ANSI.reset()]) + Mix.shell().info(" #{Path.relative_to_cwd(file)}:#{line}") end end) diff --git a/lib/style.ex b/lib/style.ex index 3829dde5..c0c15e51 100644 --- a/lib/style.ex +++ b/lib/style.ex @@ -64,21 +64,22 @@ defmodule Styler.Style do |> Zipper.root() end + def in_block?(zipper) do + case Zipper.up(zipper) do + {{:__block__, _, _}, _} -> true + {{:->, _, _}, _} -> true + {{_, _}, _} -> true + nil -> true + _ -> false + end + end + @doc """ Returns the current node (wrapped in a `__block__` if necessary) if it's a valid place to insert additional nodes """ @spec ensure_block_parent(Zipper.t()) :: {:ok, Zipper.t()} | :error def ensure_block_parent(zipper) do - valid_block_location? = - case Zipper.up(zipper) do - {{:__block__, _, _}, _} -> true - {{:->, _, _}, _} -> true - {{_, _}, _} -> true - nil -> true - _ -> false - end - - if valid_block_location? do + if in_block?(zipper) do {:ok, find_nearest_block(zipper)} else :error diff --git a/lib/style/speedo.ex b/lib/style/speedo.ex index 56ae7677..2837b97d 100644 --- a/lib/style/speedo.ex +++ b/lib/style/speedo.ex @@ -11,49 +11,68 @@ defmodule Styler.Speedo do alias Credo.Check.Readability.WithCustomTaggedTuple alias Styler.Zipper - def run(zipper, context) do - case run!(Zipper.node(zipper), context.file) do - nil -> {zipper, context} - [] -> {zipper, context} - errors -> {zipper, Map.update!(context, :errors, &[errors | &1])} - end - end - @definer ~w(def defp defmacro defmacrop defguard defguardp)a - defp run!({def, _, [{name, m, _} | _]}, file) when def in @definer do + def run({{def, _, [{name, m, _} | _]}, _} = zipper, context) when def in @definer and is_atom(name) do line = m[:line] name = to_string(name) - # Credo.Check.Readability.FunctionNames + error = %{file: context.file, line: line, check: nil, message: nil} + snake_error = - unless snake_case?(name), do: %{file: file, check: FunctionNames, message: "#{def} #{name} is not snake case"} + unless snake_case?(name), do: %{error | check: FunctionNames, message: "`#{def} #{name}` is not snake case"} + + predicate_error = %{error | check: PredicateFunctionNames, message: nil} - predicate_error = %{file: file, line: line, check: PredicateFunctionNames, message: nil} - # Credo.Check.Readability.PredicateFunctionNames predicate_error = cond do String.starts_with?(name, "is_") && String.ends_with?(name, "?") -> - [%{predicate_error | message: "#{def} #{name} wow choose `?` or `is_`, you monster"}] + %{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"}] + %{predicate_error | message: "`#{def} #{name}` is invalid -- use `?` not `is_` for defs"} def in ~w(defmacro defmacrop defguard defguardp)a and String.ends_with?(name, "?") -> - [%{predicate_error | message: "#{def} #{name}: use `is_*` not `*?` for things that can be used in guards"}] + %{predicate_error | message: "`#{def} #{name}`: use `is_*` not `*?` for things that can be used in guards"} true -> [] end - [snake_error | predicate_error] + context = Map.update!(context, :errors, &[snake_error, predicate_error | &1]) + {zipper, context} + end + + # Credo.Check.Readability.VariableNames + def run({{name, m, nil}, _} = zipper, ctx) do + # probably get false positives here if people haven't run their pipes thru first + # also, when we start reporting multiple errors this'll need updating to only report at the place a var is created 🤔 + error = + if not Styler.Style.in_block?(zipper) or snake_case?(name) or + name in [:__STACKTRACE__, :__CALLER__, :__DIR__, :__ENV__, :__MODULE__] do + [] + else + %{file: ctx.file, line: m[:line], check: VariableNames, message: "`#{name}`: variables must be snake case"} + end + + ctx = Map.update!(ctx, :errors, &[error | &1]) + {zipper, ctx} + end + + def run(zipper, context) do + case run!(Zipper.node(zipper), context.file) do + nil -> {zipper, context} + [] -> {zipper, context} + errors -> {zipper, Map.update!(context, :errors, &[errors | &1])} + end end # Credo.Check.Readability.ImplTru defp run!({:@, _, [{:impl, m, [true]}]}, file), do: %{file: file, line: m[:line], check: ImplTrue, message: "`@impl true` not allowed"} + # Credo.Check.Readability.ModuleAttributeNames defp run!({:@, _, [{name, m, _}]}, file) do unless snake_case?(name), - do: %{file: file, line: m[:line], check: ModuleAttributeNames, message: "@#{name} is not snake case"} + do: %{file: file, line: m[:line], check: ModuleAttributeNames, message: "`@#{inspect(name)}` is not snake case"} end # Credo.Check.Readability.ModuleNames @@ -61,36 +80,34 @@ defmodule Styler.Speedo do name = Enum.map_join(aliases, ".", &to_string/1) unless pascal_case?(name), - do: %{file: file, line: m[:line], check: ModuleNames, message: "defmodule #{name} is not pascal case"} + do: %{file: file, line: m[:line], check: ModuleNames, message: "`defmodule #{inspect(name)}` is not pascal case"} end # Credo.Check.Readability.StringSigils - defp run!({:__block__, [{:delimiter, ~s|"|} | _] = m, [string]}, file) do + 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.VariableNames - defp run!({name, m, nil}, file) do - # probably get false positives here if people haven't run their pipes thru first - # also, when we start reporting multiple errors this'll need updating to only report at the place a var is created 🤔 - unless snake_case?(name), - do: %{file: file, line: m[:line], check: VariableNames, message: "#{name}: variables must be snake case"} - end - # Credo.Check.Readability.WithCustomTaggedTuple - defp run!({:<-, m, [{tag, _}, {tag, _}]}, file) do - msg = "tagging tuples with things like #{tag} is known to be evil" + 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 - # - # def naughtyFun do - # raise "nooooo" - # end + defp run!(_, _), do: [] + + def naughtyFun(naughtyVar) do + with {:ugh, naughty_var} <- {:ugh, naughtyVar} do + naughty_var + end + end 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]*$/ + defp pascal_case?(name), do: to_string(name) =~ ~r/^[A-Z][a-zA-Z0-9\.]*$/ end diff --git a/lib/styler.ex b/lib/styler.ex index 19313d45..eb6aada3 100644 --- a/lib/styler.ex +++ b/lib/styler.ex @@ -71,6 +71,7 @@ defmodule Styler do def lint(input, file) do {ast, comments} = string_to_quoted_with_comments(input, file) context = %{comments: comments, file: file, errors: []} + {_, %{errors: errors}} = ast |> Zipper.zip()