diff --git a/lib/credo/check/design/skip_test_without_comment.ex b/lib/credo/check/design/skip_test_without_comment.ex index 22592142b..824f3462f 100644 --- a/lib/credo/check/design/skip_test_without_comment.ex +++ b/lib/credo/check/design/skip_test_without_comment.ex @@ -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 ) diff --git a/lib/credo/check/readability/alias_as.ex b/lib/credo/check/readability/alias_as.ex index bb366f3d5..337405000 100644 --- a/lib/credo/check/readability/alias_as.ex +++ b/lib/credo/check/readability/alias_as.ex @@ -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 @@ -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. @@ -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 ) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index c129860a8..82fdae4f7 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -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. """ ] ] diff --git a/lib/credo/check/readability/block_pipe.ex b/lib/credo/check/readability/block_pipe.ex index 36b8a7b5f..40987a85d 100644 --- a/lib/credo/check/readability/block_pipe.ex +++ b/lib/credo/check/readability/block_pipe.ex @@ -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 ) diff --git a/lib/credo/check/readability/impl_true.ex b/lib/credo/check/readability/impl_true.ex index eb5307799..b90edf764 100644 --- a/lib/credo/check/readability/impl_true.ex +++ b/lib/credo/check/readability/impl_true.ex @@ -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 @@ -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 ) diff --git a/lib/credo/check/readability/nested_function_calls.ex b/lib/credo/check/readability/nested_function_calls.ex index daedb2b30..13e76c880 100644 --- a/lib/credo/check/readability/nested_function_calls.ex +++ b/lib/credo/check/readability/nested_function_calls.ex @@ -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 ) diff --git a/lib/credo/check/readability/one_arity_function_in_pipe.ex b/lib/credo/check/readability/one_arity_function_in_pipe.ex index 95f8d5600..29d7c4806 100644 --- a/lib/credo/check/readability/one_arity_function_in_pipe.ex +++ b/lib/credo/check/readability/one_arity_function_in_pipe.ex @@ -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 ) diff --git a/lib/credo/check/readability/one_pipe_per_line.ex b/lib/credo/check/readability/one_pipe_per_line.ex index cc07ca422..b5fc8a3c0 100644 --- a/lib/credo/check/readability/one_pipe_per_line.ex +++ b/lib/credo/check/readability/one_pipe_per_line.ex @@ -33,7 +33,8 @@ 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) @@ -41,14 +42,8 @@ defmodule Credo.Check.Readability.OnePipePerLine do |> 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 @@ -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 diff --git a/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex b/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex index b10424d7f..e70ebf4f0 100644 --- a/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex +++ b/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex @@ -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 @@ -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) diff --git a/lib/credo/check/readability/pipe_into_anonymous_functions.ex b/lib/credo/check/readability/pipe_into_anonymous_functions.ex index fc92c981a..132678028 100644 --- a/lib/credo/check/readability/pipe_into_anonymous_functions.ex +++ b/lib/credo/check/readability/pipe_into_anonymous_functions.ex @@ -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. @@ -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 ) diff --git a/lib/credo/check/readability/prefer_implicit_try.ex b/lib/credo/check/readability/prefer_implicit_try.ex index 2069120c8..319dd255f 100644 --- a/lib/credo/check/readability/prefer_implicit_try.ex +++ b/lib/credo/check/readability/prefer_implicit_try.ex @@ -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. diff --git a/lib/credo/check/readability/semicolons.ex b/lib/credo/check/readability/semicolons.ex index 18faf59f9..b98ae1632 100644 --- a/lib/credo/check/readability/semicolons.ex +++ b/lib/credo/check/readability/semicolons.ex @@ -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: ";" diff --git a/lib/credo/check/readability/separate_alias_require.ex b/lib/credo/check/readability/separate_alias_require.ex index a176d3c29..7f5788ed9 100644 --- a/lib/credo/check/readability/separate_alias_require.ex +++ b/lib/credo/check/readability/separate_alias_require.ex @@ -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 diff --git a/lib/credo/check/readability/single_pipe.ex b/lib/credo/check/readability/single_pipe.ex index ac4075758..add6cf771 100644 --- a/lib/credo/check/readability/single_pipe.ex +++ b/lib/credo/check/readability/single_pipe.ex @@ -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 ) diff --git a/lib/credo/check/readability/space_after_commas.ex b/lib/credo/check/readability/space_after_commas.ex index 966c5af4d..04e076615 100644 --- a/lib/credo/check/readability/space_after_commas.ex +++ b/lib/credo/check/readability/space_after_commas.ex @@ -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 diff --git a/lib/credo/check/readability/with_custom_tagged_tuple.ex b/lib/credo/check/readability/with_custom_tagged_tuple.ex index 2e61c0dbc..d28a40c1b 100644 --- a/lib/credo/check/readability/with_custom_tagged_tuple.ex +++ b/lib/credo/check/readability/with_custom_tagged_tuple.ex @@ -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)}, @@ -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 diff --git a/lib/credo/check/refactor/append_single_item.ex b/lib/credo/check/refactor/append_single_item.ex index 62873310b..cacb9de58 100644 --- a/lib/credo/check/refactor/append_single_item.ex +++ b/lib/credo/check/refactor/append_single_item.ex @@ -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 ) diff --git a/lib/credo/check/refactor/apply.ex b/lib/credo/check/refactor/apply.ex index 8d7db635e..a4c0dba00 100644 --- a/lib/credo/check/refactor/apply.ex +++ b/lib/credo/check/refactor/apply.ex @@ -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" ) diff --git a/lib/credo/check/refactor/filter_count.ex b/lib/credo/check/refactor/filter_count.ex index 046e991ce..755277189 100644 --- a/lib/credo/check/refactor/filter_count.ex +++ b/lib/credo/check/refactor/filter_count.ex @@ -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 ) diff --git a/lib/credo/check/refactor/io_puts.ex b/lib/credo/check/refactor/io_puts.ex index fdd0dba12..352b0acb9 100644 --- a/lib/credo/check/refactor/io_puts.ex +++ b/lib/credo/check/refactor/io_puts.ex @@ -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 ) diff --git a/lib/credo/check/refactor/map_into.ex b/lib/credo/check/refactor/map_into.ex index 40776f2cc..30b03c047 100644 --- a/lib/credo/check/refactor/map_into.ex +++ b/lib/credo/check/refactor/map_into.ex @@ -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 ) diff --git a/lib/credo/check/refactor/map_join.ex b/lib/credo/check/refactor/map_join.ex index a6aa8f8ee..0957989a4 100644 --- a/lib/credo/check/refactor/map_join.ex +++ b/lib/credo/check/refactor/map_join.ex @@ -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 ) diff --git a/lib/credo/check/refactor/match_in_condition.ex b/lib/credo/check/refactor/match_in_condition.ex index 80e2fac00..48189c37b 100644 --- a/lib/credo/check/refactor/match_in_condition.ex +++ b/lib/credo/check/refactor/match_in_condition.ex @@ -134,7 +134,7 @@ defmodule Credo.Check.Refactor.MatchInCondition do defp issue_for(op, line_no, issue_meta) do format_issue( issue_meta, - message: "There should be no matches in `#{op}` conditions.", + message: "Avoid matches in `#{op}` conditions.", trigger: @trigger, line_no: line_no ) diff --git a/lib/credo/check/refactor/negated_is_nil.ex b/lib/credo/check/refactor/negated_is_nil.ex index 14643a47d..65638b18a 100644 --- a/lib/credo/check/refactor/negated_is_nil.ex +++ b/lib/credo/check/refactor/negated_is_nil.ex @@ -58,7 +58,7 @@ defmodule Credo.Check.Refactor.NegatedIsNil do issue = format_issue( issue_meta, - message: "Negated is_nil in guard clause found", + message: "Avoid negated `is_nil/1` in guard clauses.", trigger: trigger, line_no: meta[:line] ) diff --git a/lib/credo/check/refactor/pass_async_in_test_cases.ex b/lib/credo/check/refactor/pass_async_in_test_cases.ex index 92d7bb3f5..339054efd 100644 --- a/lib/credo/check/refactor/pass_async_in_test_cases.ex +++ b/lib/credo/check/refactor/pass_async_in_test_cases.ex @@ -58,7 +58,7 @@ defmodule Credo.Check.Refactor.PassAsyncInTestCases do defp issue_for(line_no, issue_meta) do format_issue( issue_meta, - message: "Pass an `:async` boolean option to `use` a test case module", + message: "Pass an `:async` boolean option to `use` a test case module.", trigger: "use", line_no: line_no ) diff --git a/lib/credo/check/refactor/redundant_with_clause_result.ex b/lib/credo/check/refactor/redundant_with_clause_result.ex index 0039367b5..c3a6c081a 100644 --- a/lib/credo/check/refactor/redundant_with_clause_result.ex +++ b/lib/credo/check/refactor/redundant_with_clause_result.ex @@ -29,8 +29,8 @@ defmodule Credo.Check.Refactor.RedundantWithClauseResult do require Logger - @redundant_with "`with` statement is redundant" - @redundant_clause "Last clause in `with` is redundant" + @redundant_with "`with` statement is redundant." + @redundant_clause "Last clause in `with` is redundant." @doc false @impl true diff --git a/lib/credo/check/refactor/with_clauses.ex b/lib/credo/check/refactor/with_clauses.ex index ed93b9065..3e5b4702e 100644 --- a/lib/credo/check/refactor/with_clauses.ex +++ b/lib/credo/check/refactor/with_clauses.ex @@ -37,8 +37,8 @@ defmodule Credo.Check.Refactor.WithClauses do """ ] - @message_first_clause_not_pattern "`with` doesn't start with a <- clause, move the non-pattern <- clauses outside of the `with`" - @message_last_clause_not_pattern "`with` doesn't end with a <- clause, move the non-pattern <- clauses inside the body of the `with`" + @message_first_clause_not_pattern "`with` doesn't start with a <- clause, move the non-pattern <- clauses outside of the `with`." + @message_last_clause_not_pattern "`with` doesn't end with a <- clause, move the non-pattern <- clauses inside the body of the `with`." @doc false @impl true diff --git a/lib/credo/check/warning/dbg.ex b/lib/credo/check/warning/dbg.ex index fca0e2cd6..7c60313e6 100644 --- a/lib/credo/check/warning/dbg.ex +++ b/lib/credo/check/warning/dbg.ex @@ -82,7 +82,7 @@ defmodule Credo.Check.Warning.Dbg do defp issue_for(issue_meta, line_no) do format_issue( issue_meta, - message: "There should be no calls to dbg.", + message: "There should be no calls to `dbg/1`.", trigger: "dbg", line_no: line_no ) diff --git a/lib/credo/check/warning/expensive_empty_enum_check.ex b/lib/credo/check/warning/expensive_empty_enum_check.ex index d6a4385d9..537831ba0 100644 --- a/lib/credo/check/warning/expensive_empty_enum_check.ex +++ b/lib/credo/check/warning/expensive_empty_enum_check.ex @@ -70,12 +70,12 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do defp suggest({_op, _, [{_pattern, _, args}, 0]}), do: suggest_for_arity(Enum.count(args)) defp suggest_for_arity(2), do: "`not Enum.any?/2`" - defp suggest_for_arity(1), do: "Enum.empty?/1 or list == []" + defp suggest_for_arity(1), do: "`Enum.empty?/1` or `list == []`" defp issue_for(issue_meta, line_no, trigger, suggestion) do format_issue( issue_meta, - message: "#{trigger} is expensive. Prefer #{suggestion}", + message: "#{trigger} is expensive, prefer #{suggestion}.", trigger: trigger, line_no: line_no ) diff --git a/lib/credo/check/warning/iex_pry.ex b/lib/credo/check/warning/iex_pry.ex index 205447075..f908251ce 100644 --- a/lib/credo/check/warning/iex_pry.ex +++ b/lib/credo/check/warning/iex_pry.ex @@ -42,7 +42,7 @@ defmodule Credo.Check.Warning.IExPry do new_issue = format_issue( issue_meta, - message: "There should be no calls to IEx.pry/0.", + message: "There should be no calls to `IEx.pry/0`.", trigger: @call_string, line_no: meta[:line] ) diff --git a/lib/credo/check/warning/io_inspect.ex b/lib/credo/check/warning/io_inspect.ex index 438f01aea..c2a3528d9 100644 --- a/lib/credo/check/warning/io_inspect.ex +++ b/lib/credo/check/warning/io_inspect.ex @@ -49,7 +49,7 @@ defmodule Credo.Check.Warning.IoInspect do defp issue_for(issue_meta, line_no, trigger) do format_issue( issue_meta, - message: "There should be no calls to IO.inspect/1.", + message: "There should be no calls to `IO.inspect/1`.", trigger: trigger, line_no: line_no ) diff --git a/lib/credo/check/warning/leaky_environment.ex b/lib/credo/check/warning/leaky_environment.ex index eff02f901..dda853845 100644 --- a/lib/credo/check/warning/leaky_environment.ex +++ b/lib/credo/check/warning/leaky_environment.ex @@ -66,7 +66,7 @@ defmodule Credo.Check.Warning.LeakyEnvironment do defp issue_for(issue_meta, line_no, trigger) do format_issue( issue_meta, - message: "When using #{trigger}, clear or overwrite sensitive environment variables", + message: "When using #{trigger}, clear or overwrite sensitive environment variables.", trigger: trigger, line_no: line_no ) diff --git a/lib/credo/check/warning/map_get_unsafe_pass.ex b/lib/credo/check/warning/map_get_unsafe_pass.ex index f8dd63755..eaa3db390 100644 --- a/lib/credo/check/warning/map_get_unsafe_pass.ex +++ b/lib/credo/check/warning/map_get_unsafe_pass.ex @@ -83,8 +83,7 @@ defmodule Credo.Check.Warning.MapGetUnsafePass do defp issue_for(issue_meta, line_no, trigger) do format_issue( issue_meta, - message: "Map.get with no default return value is potentially unsafe - in pipes, use Map.get/3 instead", + message: "`Map.get` with no default return value is potentially unsafe in pipes, use `Map.get/3` instead.", trigger: trigger, line_no: line_no ) diff --git a/lib/credo/check/warning/missed_metadata_key_in_logger_config.ex b/lib/credo/check/warning/missed_metadata_key_in_logger_config.ex index 165cd8834..b4a8a617f 100644 --- a/lib/credo/check/warning/missed_metadata_key_in_logger_config.ex +++ b/lib/credo/check/warning/missed_metadata_key_in_logger_config.ex @@ -178,7 +178,7 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfig do defp issue_for(issue_meta, line_no, [trigger | _] = missed_keys) do format_issue(issue_meta, - message: "Logger metadata key #{Enum.join(missed_keys, ", ")} not found in Logger config", + message: "Logger metadata key #{Enum.join(missed_keys, ", ")} not found in Logger config.", line_no: line_no, trigger: trigger ) diff --git a/lib/credo/check/warning/spec_with_struct.ex b/lib/credo/check/warning/spec_with_struct.ex index a326e74d2..021740abe 100644 --- a/lib/credo/check/warning/spec_with_struct.ex +++ b/lib/credo/check/warning/spec_with_struct.ex @@ -37,13 +37,7 @@ defmodule Credo.Check.Warning.SpecWithStruct do {ast, structs} -> issues = Enum.reduce(structs, issues, fn curr, acc -> - options = [ - message: "Struct %#{curr}{} found in @spec", - trigger: "%#{curr}{", - line_no: meta[:line] - ] - - [format_issue(issue_meta, options) | acc] + [issue_for(issue_meta, meta[:line], curr) | acc] end) {ast, issues} @@ -61,4 +55,12 @@ defmodule Credo.Check.Warning.SpecWithStruct do defp find_structs(ast, acc) do {ast, acc} end + + defp issue_for(issue_meta, line_no, struct) do + format_issue(issue_meta, + message: "Struct %#{struct}{} found in `@spec`.", + trigger: "%#{struct}{", + line_no: line_no + ) + end end diff --git a/lib/credo/check/warning/unsafe_exec.ex b/lib/credo/check/warning/unsafe_exec.ex index 92b49a945..1b8a18528 100644 --- a/lib/credo/check/warning/unsafe_exec.ex +++ b/lib/credo/check/warning/unsafe_exec.ex @@ -67,7 +67,7 @@ defmodule Credo.Check.Warning.UnsafeExec do defp issue_for(call, suggestion, trigger, line_no, issue_meta) do format_issue(issue_meta, - message: "Prefer #{suggestion} over #{call} to prevent command injection", + message: "Prefer #{suggestion} over #{call} to prevent command injection.", trigger: trigger, line_no: line_no ) diff --git a/lib/credo/check/warning/unsafe_to_atom.ex b/lib/credo/check/warning/unsafe_to_atom.ex index f7f30bc66..afeaa8c8d 100644 --- a/lib/credo/check/warning/unsafe_to_atom.ex +++ b/lib/credo/check/warning/unsafe_to_atom.ex @@ -143,7 +143,7 @@ defmodule Credo.Check.Warning.UnsafeToAtom do defp issues_for_call(call, suggestion, trigger, meta, issue_meta, issues) do [ format_issue(issue_meta, - message: "Prefer #{suggestion} over #{call} to avoid creating atoms at runtime", + message: "Prefer #{suggestion} over #{call} to avoid creating atoms at runtime.", trigger: trigger, line_no: meta[:line] ) diff --git a/lib/credo/check/warning/wrong_test_file_extension.ex b/lib/credo/check/warning/wrong_test_file_extension.ex index 13b9e70b4..5e87296f0 100644 --- a/lib/credo/check/warning/wrong_test_file_extension.ex +++ b/lib/credo/check/warning/wrong_test_file_extension.ex @@ -29,7 +29,7 @@ defmodule Credo.Check.Warning.WrongTestFileExtension do defp issue_for(issue_meta) do format_issue( issue_meta, - message: "Test files should end with .exs", + message: "Test files should end with `_test.exs`.", line_no: 1, trigger: Issue.no_trigger() ) diff --git a/test/credo/check/readability/with_custom_tagged_tuple_test.exs b/test/credo/check/readability/with_custom_tagged_tuple_test.exs index 68e7f7f97..4dc6c9eba 100644 --- a/test/credo/check/readability/with_custom_tagged_tuple_test.exs +++ b/test/credo/check/readability/with_custom_tagged_tuple_test.exs @@ -45,10 +45,10 @@ defmodule Credo.Check.Readability.WithCustomTaggedTupleTest do |> assert_issues(fn issues -> [issue1, issue2] = issues - assert issue1.message == "Invalid usage of placeholder `:resource` in with" + assert issue1.message == "Avoid using tagged tuples as placeholders in `with` (found: `:resource`)." assert issue1.trigger == ":resource" - assert issue2.message == "Invalid usage of placeholder `:authz` in with" + assert issue2.message == "Avoid using tagged tuples as placeholders in `with` (found: `:authz`)." assert issue2.trigger == ":authz" end) end diff --git a/test/credo/check/refactor/redundant_with_clause_result_test.exs b/test/credo/check/refactor/redundant_with_clause_result_test.exs index 67aee33d9..d9dce4a59 100644 --- a/test/credo/check/refactor/redundant_with_clause_result_test.exs +++ b/test/credo/check/refactor/redundant_with_clause_result_test.exs @@ -94,7 +94,7 @@ defmodule Credo.Check.Refactor.RedundantWithClauseResultTest do |> to_source_file |> run_check(@described_check) |> assert_issue(fn issue -> - assert issue.message == "Last clause in `with` is redundant" + assert issue.message == "Last clause in `with` is redundant." end) end @@ -111,7 +111,7 @@ defmodule Credo.Check.Refactor.RedundantWithClauseResultTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 2 - assert issue.message == "Last clause in `with` is redundant" + assert issue.message == "Last clause in `with` is redundant." end) end @@ -127,7 +127,7 @@ defmodule Credo.Check.Refactor.RedundantWithClauseResultTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 2 - assert issue.message == "`with` statement is redundant" + assert issue.message == "`with` statement is redundant." assert issue.trigger == "with" end) end diff --git a/test/credo/check/warning/spec_with_struct_test.exs b/test/credo/check/warning/spec_with_struct_test.exs index 956396da2..f0108c898 100644 --- a/test/credo/check/warning/spec_with_struct_test.exs +++ b/test/credo/check/warning/spec_with_struct_test.exs @@ -97,7 +97,7 @@ defmodule Credo.Check.Warning.SpecWithStructTest do |> to_source_files() |> run_check(@described_check) |> assert_issue(fn issue -> - assert %{line_no: 2, message: "Struct %MyApp.MyStruct{} found in @spec"} = issue + assert %{line_no: 2, message: "Struct %MyApp.MyStruct{} found in `@spec`."} = issue assert issue.trigger == "%MyApp.MyStruct{" end) end