Skip to content

Commit

Permalink
Harmonize punctuation in issue messages
Browse files Browse the repository at this point in the history
  • Loading branch information
rrrene committed Feb 12, 2024
1 parent 0181f40 commit e9797ca
Show file tree
Hide file tree
Showing 41 changed files with 126 additions and 95 deletions.
2 changes: 1 addition & 1 deletion lib/credo/check/design/skip_test_without_comment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ defmodule Credo.Check.Design.SkipTestWithoutComment do
defp issue_for(issue_meta, line_no) do
format_issue(
issue_meta,
message: "Tests tagged to be skipped should have a comment preceding the `@tag :skip`",
message: "Tests tagged to be skipped should have a comment preceding the `@tag :skip`.",
trigger: "@tag :skip",
line_no: line_no
)
Expand Down
11 changes: 9 additions & 2 deletions lib/credo/check/readability/alias_as.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ defmodule Credo.Check.Readability.AliasAs do
tags: [:experimental],
explanations: [
check: """
Aliases which are not completely renamed using the `:as` option are easier to follow.
Aliases can be "renamed" using the `:as` option, but that sometimes
makes the code more difficult to read.
# preferred
Expand All @@ -28,6 +29,12 @@ defmodule Credo.Check.Readability.AliasAs do
end
end
Please note that you might want to deactivate this check for cases in which you have an alias that
is used tons throughout your codebase.
If, for example, you are using a third-party module named `FlupsyTopsyDataRetentionServiceServer`
in half your modules, it is of course reasonable to alias it to `Server`.
Like all `Readability` issues, this one is not a technical concern.
But you can improve the odds of others reading and liking your code by making
it easier to follow.
Expand Down Expand Up @@ -59,7 +66,7 @@ defmodule Credo.Check.Readability.AliasAs do
defp issue_for(issue_meta, line_no) do
format_issue(
issue_meta,
message: "Avoid using the :as option with alias.",
message: "Avoid using the `:as` option with `alias`.",
trigger: "as:",
line_no: line_no
)
Expand Down
3 changes: 2 additions & 1 deletion lib/credo/check/readability/alias_order.ex
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ defmodule Credo.Check.Readability.AliasOrder do
Options
- `:alpha` - Alphabetical case-insensitive sorting.
- `:ascii` - Case-sensitive sorting where upper case characters are ordered before their lower case equivalent.
- `:ascii` - Case-sensitive sorting where upper case characters are ordered
before their lower case equivalent.
"""
]
]
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/readability/block_pipe.ex
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ defmodule Credo.Check.Readability.BlockPipe do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "Use a variable or create a new function instead of piping to a block",
message: "Use a variable or create a new function instead of piping to a block.",
trigger: trigger,
line_no: line_no
)
Expand Down
28 changes: 17 additions & 11 deletions lib/credo/check/readability/impl_true.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,28 @@ defmodule Credo.Check.Readability.ImplTrue do
base_priority: :normal,
explanations: [
check: """
When implementing behaviour callbacks, `@impl true` indicates that a function implements a callback, but
a better way is to note the actual behaviour being implemented, for example `@impl MyBehaviour`. This
not only improves readability, but adds extra validation in cases where multiple behaviours are implemented
in a single module.
`@impl true` is a shortform so you don't have to write the actual behaviour that is being implemented.
This can make code harder to comprehend.
Instead of:
# preferred
@impl true
@impl MyBehaviour
def my_funcion() do
...
# ...
end
use:
# NOT preferred
@impl MyBehaviour
@impl true
def my_funcion() do
...
# ...
end
When implementing behaviour callbacks, `@impl true` indicates that a function implements a callback, but
a more explicit way is to use the actual behaviour being implemented, for example `@impl MyBehaviour`.
This not only improves readability, but adds extra validation in cases where multiple behaviours are
implemented in a single module.
Like all `Readability` issues, this one is not a technical concern.
But you can improve the odds of others reading and liking your code by making
Expand All @@ -46,7 +52,7 @@ defmodule Credo.Check.Readability.ImplTrue do
defp issue_for(issue_meta, line_no) do
format_issue(
issue_meta,
message: "@impl true should be @impl MyBehaviour",
message: "`@impl true` should be `@impl MyBehaviour`.",
trigger: "@impl",
line_no: line_no
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/readability/nested_function_calls.ex
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ defmodule Credo.Check.Readability.NestedFunctionCalls do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "Use a pipeline when there are nested function calls",
message: "Use a pipeline instead of nested function calls.",
trigger: trigger,
line_no: line_no
)
Expand Down
8 changes: 4 additions & 4 deletions lib/credo/check/readability/one_arity_function_in_pipe.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ defmodule Credo.Check.Readability.OneArityFunctionInPipe do
end

defp traverse({:|>, _, [_, {name, meta, nil}]} = ast, issues, issue_meta) when is_atom(name) do
{ast, [issue(issue_meta, meta[:line], name) | issues]}
{ast, [issue_for(issue_meta, meta[:line], name) | issues]}
end

defp traverse(ast, issues, _) do
{ast, issues}
end

defp issue(meta, line, name) do
defp issue_for(issue_meta, line, name) do
format_issue(
meta,
message: "One arity functions should have parentheses in pipes",
issue_meta,
message: "One arity functions should have parentheses in pipes.",
line_no: line,
trigger: name
)
Expand Down
23 changes: 14 additions & 9 deletions lib/credo/check/readability/one_pipe_per_line.ex
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,17 @@ defmodule Credo.Check.Readability.OnePipePerLine do
"""
]

@impl Credo.Check
@doc false
@impl true
def run(%SourceFile{} = source_file, params \\ []) do
issue_meta = IssueMeta.for(source_file, params)

Credo.Code.to_tokens(source_file)
|> Enum.filter(&filter_pipes/1)
|> Enum.group_by(fn {_, {line, _, _}, :|>} -> line end)
|> Enum.filter(&filter_tokens/1)
|> Enum.map(fn {_, [{_, {line_no, column_no, _}, _} | _]} ->
format_issue(
issue_meta,
message: "Don't use multiple |> in the same line",
line_no: line_no,
column: column_no,
trigger: "|>"
)
|> Enum.map(fn {_, [{_, {line_no, column, _}, _} | _]} ->
issue_for(issue_meta, line_no, column)
end)
end

Expand All @@ -58,4 +53,14 @@ defmodule Credo.Check.Readability.OnePipePerLine do
defp filter_tokens({_, [_]}), do: false
defp filter_tokens({_, [_ | _]}), do: true
defp filter_tokens(_), do: false

defp issue_for(issue_meta, line_no, column) do
format_issue(
issue_meta,
message: "Avoid using multiple pipes (`|>`) on the same line.",
line_no: line_no,
column: column,
trigger: "|>"
)
end
end
6 changes: 3 additions & 3 deletions lib/credo/check/readability/parentheses_on_zero_arity_defs.ex
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefs do

cond do
parens? and not enforce_parens? ->
issues ++ [issue_for(issue_meta, name, line_no, :present)]
[issue_for(issue_meta, name, line_no, :present) | issues]

not parens? and enforce_parens? ->
issues ++ [issue_for(issue_meta, name, line_no, :missing)]
[issue_for(issue_meta, name, line_no, :missing) | issues]

true ->
issues
Expand All @@ -99,7 +99,7 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefs do
"Do not use parentheses when defining a function which has no arguments."

:missing ->
"Use parentheses () when defining a function which has no arguments."
"Use parentheses when defining a function which has no arguments."
end

format_issue(issue_meta, message: message, line_no: line_no, trigger: name)
Expand Down
10 changes: 9 additions & 1 deletion lib/credo/check/readability/pipe_into_anonymous_functions.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ defmodule Credo.Check.Readability.PipeIntoAnonymousFunctions do
defp timex_2(i), do: i * 2
... or use `then/1`:
def my_fun(foo) do
foo
|> then(fn i -> i * 2 end)
|> my_other_fun()
end
Like all `Readability` issues, this one is not a technical concern.
But you can improve the odds of others reading and liking your code by making
it easier to follow.
Expand Down Expand Up @@ -52,7 +60,7 @@ defmodule Credo.Check.Readability.PipeIntoAnonymousFunctions do
defp issue_for(issue_meta, line_no) do
format_issue(
issue_meta,
message: "Avoid piping into anonymous function calls",
message: "Avoid piping into anonymous function calls.",
trigger: "|>",
line_no: line_no
)
Expand Down
4 changes: 4 additions & 0 deletions lib/credo/check/readability/prefer_implicit_try.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ defmodule Credo.Check.Readability.PreferImplicitTry do
_ -> :rescued
end
This emphazises that you really want to try/rescue anything the function does,
which might be important for other contributors so they can reason about adding
code to the function.
Like all `Readability` issues, this one is not a technical concern.
But you can improve the odds of others reading and liking your code by making
it easier to follow.
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/readability/semicolons.ex
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ defmodule Credo.Check.Readability.Semicolons do
defp issue_for(issue_meta, line_no, column) do
format_issue(
issue_meta,
message: "Don't use ; to separate statements and expressions",
message: "Don't use `;` to separate statements and expressions.",
line_no: line_no,
column: column,
trigger: ";"
Expand Down
4 changes: 2 additions & 2 deletions lib/credo/check/readability/separate_alias_require.ex
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,6 @@ defmodule Credo.Check.Readability.SeparateAliasRequire do
)
end

def message(:alias), do: "aliases should be consecutive within a file"
def message(:require), do: "requires should be consecutive within a file"
def message(:alias), do: "`alias` calls should be consecutive within a module."
def message(:require), do: "`require` calls should be consecutive within a module."
end
2 changes: 1 addition & 1 deletion lib/credo/check/readability/single_pipe.ex
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ defmodule Credo.Check.Readability.SinglePipe do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "Use a function call when a pipeline is only one function long",
message: "Use a function call when a pipeline is only one function long.",
trigger: trigger,
line_no: line_no
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/readability/space_after_commas.ex
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ defmodule Credo.Check.Readability.SpaceAfterCommas do
defp issue_for(issue_meta, trigger, line_no, column) do
format_issue(
issue_meta,
message: "Space missing after comma",
message: "Space missing after comma.",
trigger: trigger,
line_no: line_no,
column: column
Expand Down
34 changes: 17 additions & 17 deletions lib/credo/check/readability/with_custom_tagged_tuple.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule Credo.Check.Readability.WithCustomTaggedTuple do
check: """
Avoid using custom tags for error reporting from `with` macros.
This code injects placeholder tags such as `:resource` and `:authz` for the purpose of error
This code injects tuple_tag tags such as `:resource` and `:authz` for the purpose of error
reporting.
with {:resource, {:ok, resource}} <- {:resource, Resource.fetch(user)},
Expand Down Expand Up @@ -45,39 +45,39 @@ defmodule Credo.Check.Readability.WithCustomTaggedTuple do
@impl true
def run(%SourceFile{} = source_file, params \\ []) do
source_file
|> errors()
|> Enum.map(&credo_error(&1, IssueMeta.for(source_file, params)))
|> find_issues()
|> Enum.map(&issue_for(&1, IssueMeta.for(source_file, params)))
end

defp errors(source_file) do
{_ast, errors} = Macro.prewalk(Credo.Code.ast(source_file), MapSet.new(), &traverse/2)
defp find_issues(source_file) do
{_ast, issues} = Macro.prewalk(Credo.Code.ast(source_file), MapSet.new(), &traverse/2)

Enum.sort_by(errors, &{&1.line, &1.column})
Enum.sort_by(issues, &{&1.line, &1.column})
end

defp traverse({:with, _meta, args}, errors) do
errors =
defp traverse({:with, _meta, args}, issues) do
issues =
args
|> Stream.map(&placeholder/1)
|> Stream.map(&tuple_tag/1)
|> Enum.reject(&is_nil/1)
|> Enum.into(errors)
|> Enum.into(issues)

{args, errors}
{args, issues}
end

defp traverse(ast, state), do: {ast, state}

defp placeholder({:<-, meta, [{placeholder, _}, {placeholder, _}]}) when is_atom(placeholder),
do: %{placeholder: placeholder, line: meta[:line], column: meta[:column]}
defp tuple_tag({:<-, meta, [{tuple_tag, _}, {tuple_tag, _}]}) when is_atom(tuple_tag),
do: %{tuple_tag: tuple_tag, line: meta[:line], column: meta[:column]}

defp placeholder(_), do: nil
defp tuple_tag(_), do: nil

defp credo_error(error, issue_meta) do
defp issue_for(error, issue_meta) do
format_issue(
issue_meta,
message: "Invalid usage of placeholder `#{inspect(error.placeholder)}` in with",
message: "Avoid using tagged tuples as placeholders in `with` (found: `#{inspect(error.tuple_tag)}`).",
line_no: error.line,
trigger: inspect(error.placeholder)
trigger: inspect(error.tuple_tag)
)
end
end
3 changes: 1 addition & 2 deletions lib/credo/check/refactor/append_single_item.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ defmodule Credo.Check.Refactor.AppendSingleItem do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "Appending a single item to a list is inefficient, use [head | tail]
notation (and Enum.reverse/1 when order matters)",
message: "Appending a single item to a list is inefficient, use `[head | tail]` notation (and `Enum.reverse/1` when order matters).",
trigger: trigger,
line_no: line_no
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/refactor/apply.ex
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ defmodule Credo.Check.Refactor.Apply do
defp issue_for(meta, issue_meta) do
format_issue(
issue_meta,
message: "Avoid `apply/2` and `apply/3` when the number of arguments is known",
message: "Avoid `apply/2` and `apply/3` when the number of arguments is known.",
line_no: meta[:line],
trigger: "apply"
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/refactor/filter_count.ex
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ defmodule Credo.Check.Refactor.FilterCount do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "`Enum.count/2` is more efficient than `Enum.filter/2 |> Enum.count/1`",
message: "`Enum.count/2` is more efficient than `Enum.filter/2 |> Enum.count/1`.",
trigger: trigger,
line_no: line_no
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/refactor/io_puts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ defmodule Credo.Check.Refactor.IoPuts do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "There should be no calls to IO.puts/1.",
message: "There should be no calls to `IO.puts/1`.",
trigger: trigger,
line_no: line_no
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/refactor/map_into.ex
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ defmodule Credo.Check.Refactor.MapInto do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "`Enum.into/3` is more efficient than `Enum.map/2 |> Enum.into/2`",
message: "`Enum.into/3` is more efficient than `Enum.map/2 |> Enum.into/2`.",
trigger: trigger,
line_no: line_no
)
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/refactor/map_join.ex
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ defmodule Credo.Check.Refactor.MapJoin do
defp issue_for(issue_meta, line_no, trigger) do
format_issue(
issue_meta,
message: "`Enum.map_join/3` is more efficient than `Enum.map/2 |> Enum.join/2`",
message: "`Enum.map_join/3` is more efficient than `Enum.map/2 |> Enum.join/2`.",
trigger: trigger,
line_no: line_no
)
Expand Down
Loading

0 comments on commit e9797ca

Please sign in to comment.