From e4b1993704ae99a373e0d34d88fc81e9de4866b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20F=C3=B6hring?= Date: Sun, 4 Feb 2024 12:53:25 +0100 Subject: [PATCH] Fix more triggers --- lib/credo/check.ex | 7 ++- .../readability/one_arity_function_in_pipe.ex | 2 +- .../parentheses_on_zero_arity_defs.ex | 2 +- .../readability/separate_alias_require.ex | 2 +- .../single_function_to_block_pipe.ex | 2 +- .../check/refactor/cyclomatic_complexity.ex | 2 +- .../application_config_in_module_attribute.ex | 25 +++++----- .../warning/expensive_empty_enum_check.ex | 16 +++---- lib/credo/check/warning/lazy_logging.ex | 2 +- .../missed_metadata_key_in_logger_config.ex | 2 +- lib/credo/check/warning/spec_with_struct.ex | 2 +- lib/credo/check/warning/unsafe_exec.ex | 4 +- lib/credo/check/warning/unsafe_to_atom.ex | 48 ++++++++++--------- .../parameter_pattern_matching_test.exs | 4 +- test/credo/check/housekeeping_trigger.exs | 5 +- .../check/readability/function_names_test.exs | 4 +- .../check/refactor/reject_filter_test.exs | 4 +- .../check/refactor/reject_reject_test.exs | 4 +- .../check/refactor/unless_with_else_test.exs | 5 +- .../refactor/variable_rebinding_test.exs | 5 +- .../check/refactor/with_clauses_test.exs | 5 +- ...cation_config_in_module_attribute_test.exs | 42 +++++++++++----- .../bool_operation_on_same_values_test.exs | 17 +++++++ .../expensive_empty_enum_check_test.exs | 9 +++- .../warning/map_get_unsafe_pass_test.exs | 5 +- .../warning/operation_on_same_values_test.exs | 4 +- .../operation_with_constant_result_test.exs | 4 +- .../check/warning/spec_with_struct_test.exs | 6 ++- test/credo/check/warning/unsafe_exec_test.exs | 10 +++- .../check/warning/unsafe_to_atom_test.exs | 5 +- 30 files changed, 170 insertions(+), 84 deletions(-) diff --git a/lib/credo/check.ex b/lib/credo/check.ex index 101fb3e4e..7aeb557f4 100644 --- a/lib/credo/check.ex +++ b/lib/credo/check.ex @@ -711,10 +711,15 @@ defmodule Credo.Check do exit_status = Check.to_exit_status(exit_status_or_category) line_no = opts[:line_no] - trigger = opts[:trigger] column = opts[:column] severity = opts[:severity] || Severity.default_value() + trigger = + case opts[:trigger] do + {:__no_trigger__} -> {:__no_trigger__} + trigger -> to_string(trigger) + end + %Issue{ priority: priority, filename: source_file.filename, 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 f9e0c4474..95f8d5600 100644 --- a/lib/credo/check/readability/one_arity_function_in_pipe.ex +++ b/lib/credo/check/readability/one_arity_function_in_pipe.ex @@ -37,7 +37,7 @@ defmodule Credo.Check.Readability.OneArityFunctionInPipe do meta, message: "One arity functions should have parentheses in pipes", line_no: line, - trigger: to_string(name) + trigger: name ) 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 662e3fb2f..b10424d7f 100644 --- a/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex +++ b/lib/credo/check/readability/parentheses_on_zero_arity_defs.ex @@ -102,6 +102,6 @@ defmodule Credo.Check.Readability.ParenthesesOnZeroArityDefs do "Use parentheses () when defining a function which has no arguments." end - format_issue(issue_meta, message: message, line_no: line_no, trigger: to_string(name)) + format_issue(issue_meta, message: message, line_no: line_no, trigger: name) end end diff --git a/lib/credo/check/readability/separate_alias_require.ex b/lib/credo/check/readability/separate_alias_require.ex index 6368b11ed..a176d3c29 100644 --- a/lib/credo/check/readability/separate_alias_require.ex +++ b/lib/credo/check/readability/separate_alias_require.ex @@ -80,7 +80,7 @@ defmodule Credo.Check.Readability.SeparateAliasRequire do format_issue(issue_meta, message: message(macro_name), line_no: line_no, - trigger: to_string(macro_name) + trigger: macro_name ) end diff --git a/lib/credo/check/readability/single_function_to_block_pipe.ex b/lib/credo/check/readability/single_function_to_block_pipe.ex index 7690e6482..3857c8799 100644 --- a/lib/credo/check/readability/single_function_to_block_pipe.ex +++ b/lib/credo/check/readability/single_function_to_block_pipe.ex @@ -79,7 +79,7 @@ defmodule Credo.Check.Readability.SingleFunctionToBlockPipe do issue_meta, message: "Avoid single pipes to a block", line_no: line_no, - trigger: to_string(marker) + trigger: marker ) end end diff --git a/lib/credo/check/refactor/cyclomatic_complexity.ex b/lib/credo/check/refactor/cyclomatic_complexity.ex index 7d57d006f..5809422fc 100644 --- a/lib/credo/check/refactor/cyclomatic_complexity.ex +++ b/lib/credo/check/refactor/cyclomatic_complexity.ex @@ -169,7 +169,7 @@ defmodule Credo.Check.Refactor.CyclomaticComplexity do issue_meta, message: "Function is too complex (cyclomatic complexity is #{actual_value}, max is #{max_value}).", - trigger: to_string(trigger), + trigger: trigger, line_no: line_no, severity: Severity.compute(actual_value, max_value) ) diff --git a/lib/credo/check/warning/application_config_in_module_attribute.ex b/lib/credo/check/warning/application_config_in_module_attribute.ex index 1a16ce188..cdedcc899 100644 --- a/lib/credo/check/warning/application_config_in_module_attribute.ex +++ b/lib/credo/check/warning/application_config_in_module_attribute.ex @@ -68,42 +68,43 @@ defmodule Credo.Check.Warning.ApplicationConfigInModuleAttribute do {{:., _, [{:__aliases__, _, [:Application]}, :fetch_env]}, _meta, _args} = ast, _acc ) do - {ast, "Application.fetch_env/2"} + {ast, {"Application.fetch_env/2", "Application.fetch_env"}} end defp get_forbidden_call( {{:., _, [{:__aliases__, _, [:Application]}, :fetch_env!]}, _meta, _args} = ast, _acc ) do - {ast, "Application.fetch_env!/2"} + {ast, {"Application.fetch_env!/2", "Application.fetch_env"}} end defp get_forbidden_call( {{:., _, [{:__aliases__, _, [:Application]}, :get_all_env]}, _meta, _args} = ast, _acc ) do - {ast, "Application.get_all_env/1"} + {ast, {"Application.get_all_env/1", "Application.get_all_env"}} end defp get_forbidden_call( {{:., _, [{:__aliases__, _, [:Application]}, :get_env]}, _meta, args} = ast, _acc ) do - {ast, "Application.get_env/#{length(args)}"} + {ast, {"Application.get_env/#{length(args)}", "Application.get_env"}} end defp get_forbidden_call(ast, acc) do {ast, acc} end - defp issues_for_call(attribute, call, meta, issue_meta, issues) do - options = [ - message: - "Module attribute @#{Atom.to_string(attribute)} makes use of unsafe Application configuration call #{call}", - trigger: call, - line_no: meta[:line] + defp issues_for_call(attribute, {call, trigger}, meta, issue_meta, issues) do + [ + format_issue(issue_meta, + message: + "Module attribute @#{Atom.to_string(attribute)} makes use of unsafe Application configuration call #{call}", + trigger: trigger, + line_no: meta[:line] + ) + | issues ] - - [format_issue(issue_meta, options) | issues] end end diff --git a/lib/credo/check/warning/expensive_empty_enum_check.ex b/lib/credo/check/warning/expensive_empty_enum_check.ex index a79b1edff..d6a4385d9 100644 --- a/lib/credo/check/warning/expensive_empty_enum_check.ex +++ b/lib/credo/check/warning/expensive_empty_enum_check.ex @@ -40,21 +40,21 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do } @length_pattern quote do: {:length, _, [_]} @comparisons [ - {@enum_count_pattern, 0}, - {0, @enum_count_pattern}, - {@length_pattern, 0}, - {0, @length_pattern} + {@enum_count_pattern, 0, "Enum.count"}, + {0, @enum_count_pattern, "Enum.count"}, + {@length_pattern, 0, "length"}, + {0, @length_pattern, "length"} ] @operators [:==, :===] - for {lhs, rhs} <- @comparisons, + for {lhs, rhs, trigger} <- @comparisons, operator <- @operators do defp traverse( {unquote(operator), meta, [unquote(lhs), unquote(rhs)]} = ast, issues, issue_meta ) do - {ast, issues_for_call(meta, issues, issue_meta, ast)} + {ast, issues_for_call(meta, unquote(trigger), issues, issue_meta, ast)} end end @@ -62,8 +62,8 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do {ast, issues} end - defp issues_for_call(meta, issues, issue_meta, ast) do - [issue_for(issue_meta, meta[:line], Macro.to_string(ast), suggest(ast)) | issues] + defp issues_for_call(meta, trigger, issues, issue_meta, ast) do + [issue_for(issue_meta, meta[:line], trigger, suggest(ast)) | issues] end defp suggest({_op, _, [0, {_pattern, _, args}]}), do: suggest_for_arity(Enum.count(args)) diff --git a/lib/credo/check/warning/lazy_logging.ex b/lib/credo/check/warning/lazy_logging.ex index a7633ea6c..a0240abc8 100644 --- a/lib/credo/check/warning/lazy_logging.ex +++ b/lib/credo/check/warning/lazy_logging.ex @@ -114,7 +114,7 @@ defmodule Credo.Check.Warning.LazyLogging do issue_meta, message: "Prefer lazy Logger calls.", line_no: line_no, - trigger: to_string(trigger) + trigger: trigger ) end end 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 9f36df390..2a90fe72e 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 @@ -163,7 +163,7 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfig do defp issue_for(issue_meta, line_no, [trigger | _] = missed_keys) do message = "Logger metadata key #{Enum.join(missed_keys, ", ")} not found in Logger config" - format_issue(issue_meta, message: message, line_no: line_no, trigger: to_string(trigger)) + format_issue(issue_meta, message: message, line_no: line_no, trigger: trigger) end defp find_metadata_keys(params) do diff --git a/lib/credo/check/warning/spec_with_struct.ex b/lib/credo/check/warning/spec_with_struct.ex index 322f80e93..a326e74d2 100644 --- a/lib/credo/check/warning/spec_with_struct.ex +++ b/lib/credo/check/warning/spec_with_struct.ex @@ -39,7 +39,7 @@ defmodule Credo.Check.Warning.SpecWithStruct do Enum.reduce(structs, issues, fn curr, acc -> options = [ message: "Struct %#{curr}{} found in @spec", - trigger: "%#{curr}{}", + trigger: "%#{curr}{", line_no: meta[:line] ] diff --git a/lib/credo/check/warning/unsafe_exec.ex b/lib/credo/check/warning/unsafe_exec.ex index 8f9963832..92b49a945 100644 --- a/lib/credo/check/warning/unsafe_exec.ex +++ b/lib/credo/check/warning/unsafe_exec.ex @@ -49,11 +49,11 @@ defmodule Credo.Check.Warning.UnsafeExec do end defp get_forbidden_call([:os, :cmd], [_]) do - {":os.cmd/1", "System.cmd/2,3", "System.cmd"} + {":os.cmd/1", "System.cmd/2,3", ":os.cmd"} end defp get_forbidden_call([:os, :cmd], [_, _]) do - {":os.cmd/2", "System.cmd/2,3", "System.cmd"} + {":os.cmd/2", "System.cmd/2,3", ":os.cmd"} end defp get_forbidden_call([:erlang, :open_port], [{:spawn, _}, _]) do diff --git a/lib/credo/check/warning/unsafe_to_atom.ex b/lib/credo/check/warning/unsafe_to_atom.ex index 2f7c0dc73..f7f30bc66 100644 --- a/lib/credo/check/warning/unsafe_to_atom.ex +++ b/lib/credo/check/warning/unsafe_to_atom.ex @@ -54,8 +54,8 @@ defmodule Credo.Check.Warning.UnsafeToAtom do issue_meta ) do case get_forbidden_pipe(call, args) do - {bad, suggestion} -> - {ast, issues_for_call(bad, suggestion, meta, issue_meta, issues)} + {bad, suggestion, trigger} -> + {ast, issues_for_call(bad, suggestion, trigger, meta, issue_meta, issues)} nil -> {ast, issues} @@ -64,8 +64,8 @@ defmodule Credo.Check.Warning.UnsafeToAtom do defp traverse({{:., _loc, call}, meta, args} = ast, issues, issue_meta) do case get_forbidden_call(call, args) do - {bad, suggestion} -> - {ast, issues_for_call(bad, suggestion, meta, issue_meta, issues)} + {bad, suggestion, trigger} -> + {ast, issues_for_call(bad, suggestion, trigger, meta, issue_meta, issues)} nil -> {ast, issues} @@ -77,27 +77,27 @@ defmodule Credo.Check.Warning.UnsafeToAtom do end defp get_forbidden_call([:erlang, :list_to_atom], [_]) do - {":erlang.list_to_atom/1", ":erlang.list_to_existing_atom/1"} + {":erlang.list_to_atom/1", ":erlang.list_to_existing_atom/1", ":erlang.list_to_atom"} end defp get_forbidden_call([:erlang, :binary_to_atom], [_, _]) do - {":erlang.binary_to_atom/2", ":erlang.binary_to_existing_atom/2"} + {":erlang.binary_to_atom/2", ":erlang.binary_to_existing_atom/2", ":erlang.binary_to_atom"} end defp get_forbidden_call([{:__aliases__, _, [:String]}, :to_atom], [_]) do - {"String.to_atom/1", "String.to_existing_atom/1"} + {"String.to_atom/1", "String.to_existing_atom/1", "String.to_atom"} end defp get_forbidden_call([{:__aliases__, _, [:List]}, :to_atom], [_]) do - {"List.to_atom/1", "List.to_existing_atom/1"} + {"List.to_atom/1", "List.to_existing_atom/1", "List.to_atom"} end defp get_forbidden_call([{:__aliases__, _, [:Module]}, :concat], [_]) do - {"Module.concat/1", "Module.safe_concat/1"} + {"Module.concat/1", "Module.safe_concat/1", "Module.concat"} end defp get_forbidden_call([{:__aliases__, _, [:Module]}, :concat], [_, _]) do - {"Module.concat/2", "Module.safe_concat/2"} + {"Module.concat/2", "Module.safe_concat/2", "Module.concat"} end defp get_forbidden_call([{:__aliases__, _, [:Jason]}, decode], args) @@ -105,7 +105,8 @@ defmodule Credo.Check.Warning.UnsafeToAtom do args |> Enum.any?(fn arg -> Keyword.keyword?(arg) and Keyword.get(arg, :keys) == :atoms end) |> if do - {"Jason.#{decode}(..., keys: :atoms)", "Jason.#{decode}(..., keys: :atoms!)"} + {"Jason.#{decode}(..., keys: :atoms)", "Jason.#{decode}(..., keys: :atoms!)", + "Jason.#{decode}"} else nil end @@ -116,36 +117,37 @@ defmodule Credo.Check.Warning.UnsafeToAtom do end defp get_forbidden_pipe([:erlang, :list_to_atom], []) do - {":erlang.list_to_atom/1", ":erlang.list_to_existing_atom/1"} + {":erlang.list_to_atom/1", ":erlang.list_to_existing_atom/1", ":erlang.list_to_atom"} end defp get_forbidden_pipe([:erlang, :binary_to_atom], [_]) do - {":erlang.binary_to_atom/2", ":erlang.binary_to_existing_atom/2"} + {":erlang.binary_to_atom/2", ":erlang.binary_to_existing_atom/2", ":erlang.binary_to_atom"} end defp get_forbidden_pipe([{:__aliases__, _, [:String]}, :to_atom], []) do - {"String.to_atom/1", "String.to_existing_atom/1"} + {"String.to_atom/1", "String.to_existing_atom/1", "String.to_atom"} end defp get_forbidden_pipe([{:__aliases__, _, [:List]}, :to_atom], []) do - {"List.to_atom/1", "List.to_existing_atom/1"} + {"List.to_atom/1", "List.to_existing_atom/1", "List.to_atom"} end defp get_forbidden_pipe([{:__aliases__, _, [:Module]}, :concat], []) do - {"Module.concat/1", "Module.safe_concat/1"} + {"Module.concat/1", "Module.safe_concat/1", "Module.concat"} end defp get_forbidden_pipe(_, _) do nil end - defp issues_for_call(call, suggestion, meta, issue_meta, issues) do - options = [ - message: "Prefer #{suggestion} over #{call} to avoid creating atoms at runtime", - trigger: call, - line_no: meta[:line] + 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", + trigger: trigger, + line_no: meta[:line] + ) + | issues ] - - [format_issue(issue_meta, options) | issues] end end diff --git a/test/credo/check/consistency/parameter_pattern_matching_test.exs b/test/credo/check/consistency/parameter_pattern_matching_test.exs index 6ecf06829..b601e4277 100644 --- a/test/credo/check/consistency/parameter_pattern_matching_test.exs +++ b/test/credo/check/consistency/parameter_pattern_matching_test.exs @@ -120,11 +120,11 @@ defmodule Credo.Check.Consistency.ParameterPatternMatchingTest do |> run_check(@described_check) |> assert_issues(fn issues -> assert Enum.any?(issues, fn issue -> - issue.trigger == :foo_left && issue.line_no == 5 + issue.trigger == "foo_left" && issue.line_no == 5 end) assert Enum.any?(issues, fn issue -> - issue.trigger == :foo_left && issue.line_no == 8 + issue.trigger == "foo_left" && issue.line_no == 8 end) assert 2 == Enum.count(issues) diff --git a/test/credo/check/housekeeping_trigger.exs b/test/credo/check/housekeeping_trigger.exs index e0c94f6e1..050661974 100644 --- a/test/credo/check/housekeeping_trigger.exs +++ b/test/credo/check/housekeeping_trigger.exs @@ -7,6 +7,9 @@ defmodule Credo.Check.HousekeepingHeredocsInTestsTest do Path.join(__DIR__, "*/**/*_test.exs") |> Path.wildcard() |> Enum.reject(&String.match?(&1, ~r/(collector|helper)/)) + |> Enum.reject( + &String.match?(&1, ~r/(wrong_test_file|unreachable_code|regex_multiple_spaces)/) + ) |> Enum.map(&{&1, File.read!(&1)}) |> Enum.flat_map(fn {filename, source} -> ast = Code.string_to_quoted!(source) @@ -40,7 +43,7 @@ defmodule Credo.Check.HousekeepingHeredocsInTestsTest do if acc == [] do [ - "- #{Credo.Code.Module.name(ast) |> String.replace(~r/Test$/, "")}" + "- #{Path.relative_to_cwd(filename)}:1" ] else [] diff --git a/test/credo/check/readability/function_names_test.exs b/test/credo/check/readability/function_names_test.exs index 95e81d700..5f7c1b7e3 100644 --- a/test/credo/check/readability/function_names_test.exs +++ b/test/credo/check/readability/function_names_test.exs @@ -281,6 +281,8 @@ defmodule Credo.Check.Readability.FunctionNamesTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "clean_HTTP_url" + end) end end diff --git a/test/credo/check/refactor/reject_filter_test.exs b/test/credo/check/refactor/reject_filter_test.exs index 7576444ea..a745a9aef 100644 --- a/test/credo/check/refactor/reject_filter_test.exs +++ b/test/credo/check/refactor/reject_filter_test.exs @@ -111,6 +111,8 @@ defmodule Credo.Check.Refactor.RejectFilterTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "|>" + end) end end diff --git a/test/credo/check/refactor/reject_reject_test.exs b/test/credo/check/refactor/reject_reject_test.exs index e37434d46..fad1a7547 100644 --- a/test/credo/check/refactor/reject_reject_test.exs +++ b/test/credo/check/refactor/reject_reject_test.exs @@ -111,6 +111,8 @@ defmodule Credo.Check.Refactor.RejectRejectTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "|>" + end) end end diff --git a/test/credo/check/refactor/unless_with_else_test.exs b/test/credo/check/refactor/unless_with_else_test.exs index fac35cb36..1dabf5f78 100644 --- a/test/credo/check/refactor/unless_with_else_test.exs +++ b/test/credo/check/refactor/unless_with_else_test.exs @@ -45,6 +45,9 @@ defmodule Credo.Check.Refactor.UnlessWithElseTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 3 + assert issue.trigger == "unless" + end) end end diff --git a/test/credo/check/refactor/variable_rebinding_test.exs b/test/credo/check/refactor/variable_rebinding_test.exs index 99b6cf7a7..e184766c3 100644 --- a/test/credo/check/refactor/variable_rebinding_test.exs +++ b/test/credo/check/refactor/variable_rebinding_test.exs @@ -154,6 +154,9 @@ defmodule Credo.Check.Refactor.VariableRebindingTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 4 + assert issue.trigger == "a!" + end) end end diff --git a/test/credo/check/refactor/with_clauses_test.exs b/test/credo/check/refactor/with_clauses_test.exs index 20cb70dc4..393c9d8f8 100644 --- a/test/credo/check/refactor/with_clauses_test.exs +++ b/test/credo/check/refactor/with_clauses_test.exs @@ -80,6 +80,9 @@ defmodule Credo.Check.Refactor.WithClausesTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 2 + assert issue.trigger == "with" + end) end end diff --git a/test/credo/check/warning/application_config_in_module_attribute_test.exs b/test/credo/check/warning/application_config_in_module_attribute_test.exs index 91fa3539f..b4b0e07b9 100644 --- a/test/credo/check/warning/application_config_in_module_attribute_test.exs +++ b/test/credo/check/warning/application_config_in_module_attribute_test.exs @@ -29,8 +29,21 @@ defmodule Credo.Check.Warning.ApplicationConfigInModuleAttributeTest do # cases raising issues # + test "it should report a violation" do + """ + defmodule CredoSampleModule do + @config_1 Application.fetch_env(:my_app, :key) + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue(fn issue -> + assert issue.trigger == "Application.fetch_env" + end) + end + test "it should report a violation when bad calls are made" do - errors = + issues = """ defmodule CredoSampleModule do @config_1 Application.fetch_env(:my_app, :key) @@ -51,39 +64,44 @@ defmodule Credo.Check.Warning.ApplicationConfigInModuleAttributeTest do |> to_source_file |> run_check(@described_check) - assert_errors = [ + assert_issues = [ { "Module attribute @config_1 makes use of unsafe Application configuration call Application.fetch_env/2", - 2 + 2, + "Application.fetch_env" }, { "Module attribute @config_3 makes use of unsafe Application configuration call Application.fetch_env!/2", - 4 + 4, + "Application.fetch_env" }, { "Module attribute @config_5 makes use of unsafe Application configuration call Application.get_all_env/1", - 6 + 6, + "Application.get_all_env" }, { "Module attribute @config_7 makes use of unsafe Application configuration call Application.get_env/2", - 8 + 8, + "Application.get_env" }, { "Module attribute @config_9 makes use of unsafe Application configuration call Application.get_env/3", - 10 + 10, + "Application.get_env" } ] - assert length(errors) == 5 + assert length(issues) == 5 - Enum.each(assert_errors, fn {error_message, line_number} -> - assert error_exists?(errors, error_message, line_number) + Enum.each(assert_issues, fn {error_message, line_no, trigger} -> + assert error_exists?(issues, error_message, line_no, trigger) end) end - defp error_exists?(errors, error_message, line_number) do + defp error_exists?(errors, error_message, line_no, trigger) do Enum.any?(errors, fn - %Credo.Issue{message: ^error_message, line_no: ^line_number} -> true + %Credo.Issue{message: ^error_message, line_no: ^line_no, trigger: ^trigger} -> true _ -> false end) end diff --git a/test/credo/check/warning/bool_operation_on_same_values_test.exs b/test/credo/check/warning/bool_operation_on_same_values_test.exs index 2070a03fa..4bdc839e2 100644 --- a/test/credo/check/warning/bool_operation_on_same_values_test.exs +++ b/test/credo/check/warning/bool_operation_on_same_values_test.exs @@ -83,4 +83,21 @@ defmodule Credo.Check.Warning.BoolOperationOnSameValuesTest do assert 5 == Enum.count(issues) end) end + + test "it should report a violation for `and`" do + """ + defmodule CredoSampleModule do + use ExUnit.Case + + def some_fun do + x and x + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issue(fn issue -> + assert issue.trigger == "and" + end) + end end diff --git a/test/credo/check/warning/expensive_empty_enum_check_test.exs b/test/credo/check/warning/expensive_empty_enum_check_test.exs index fc2362fa6..ec4f4336b 100644 --- a/test/credo/check/warning/expensive_empty_enum_check_test.exs +++ b/test/credo/check/warning/expensive_empty_enum_check_test.exs @@ -129,7 +129,9 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheckTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "length" + end) end test "it should report when checking if length is 0 backwards" do @@ -163,7 +165,10 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheckTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue(fn issue -> assert issue.message =~ "Enum.empty" end) + |> assert_issue(fn issue -> + assert issue.trigger == "Enum.count" + assert issue.message =~ "Enum.empty" + end) end test "it should report when checking if Enum.count/1 is 0 backwards" do diff --git a/test/credo/check/warning/map_get_unsafe_pass_test.exs b/test/credo/check/warning/map_get_unsafe_pass_test.exs index 084144cfd..250f5f3a8 100644 --- a/test/credo/check/warning/map_get_unsafe_pass_test.exs +++ b/test/credo/check/warning/map_get_unsafe_pass_test.exs @@ -114,6 +114,9 @@ defmodule Credo.Check.Warning.MapGetUnsafePassTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 7 + assert issue.trigger == "Map.get" + end) end end diff --git a/test/credo/check/warning/operation_on_same_values_test.exs b/test/credo/check/warning/operation_on_same_values_test.exs index 6b85097dc..6c5eae444 100644 --- a/test/credo/check/warning/operation_on_same_values_test.exs +++ b/test/credo/check/warning/operation_on_same_values_test.exs @@ -80,7 +80,9 @@ defmodule Credo.Check.Warning.OperationOnSameValuesTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "-" + end) end test "it should report a violation for all defined operations" do diff --git a/test/credo/check/warning/operation_with_constant_result_test.exs b/test/credo/check/warning/operation_with_constant_result_test.exs index ba84bb8b9..7957793c3 100644 --- a/test/credo/check/warning/operation_with_constant_result_test.exs +++ b/test/credo/check/warning/operation_with_constant_result_test.exs @@ -56,7 +56,9 @@ defmodule Credo.Check.Warning.OperationWithConstantResultTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "*" + end) end test "it should report a violation for all defined operations" do diff --git a/test/credo/check/warning/spec_with_struct_test.exs b/test/credo/check/warning/spec_with_struct_test.exs index 770bf6e74..956396da2 100644 --- a/test/credo/check/warning/spec_with_struct_test.exs +++ b/test/credo/check/warning/spec_with_struct_test.exs @@ -98,6 +98,7 @@ defmodule Credo.Check.Warning.SpecWithStructTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert %{line_no: 2, message: "Struct %MyApp.MyStruct{} found in @spec"} = issue + assert issue.trigger == "%MyApp.MyStruct{" end) end @@ -115,7 +116,10 @@ defmodule Credo.Check.Warning.SpecWithStructTest do ] |> to_source_files() |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 2 + assert issue.trigger == "%AStruct{" + end) end test "it should report an issue if a struct is part of a union in a spec" do diff --git a/test/credo/check/warning/unsafe_exec_test.exs b/test/credo/check/warning/unsafe_exec_test.exs index 217496e9e..1c6fbb4d2 100644 --- a/test/credo/check/warning/unsafe_exec_test.exs +++ b/test/credo/check/warning/unsafe_exec_test.exs @@ -55,7 +55,10 @@ defmodule Credo.Check.Warning.UnsafeExecTest do """ |> to_source_file() |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 3 + assert issue.trigger == ":os.cmd" + end) end test "it should report a violation /3" do @@ -68,6 +71,9 @@ defmodule Credo.Check.Warning.UnsafeExecTest do """ |> to_source_file() |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 3 + assert issue.trigger == ":erlang.open_port" + end) end end diff --git a/test/credo/check/warning/unsafe_to_atom_test.exs b/test/credo/check/warning/unsafe_to_atom_test.exs index 29230671c..9bead880a 100644 --- a/test/credo/check/warning/unsafe_to_atom_test.exs +++ b/test/credo/check/warning/unsafe_to_atom_test.exs @@ -421,6 +421,9 @@ defmodule Credo.Check.Warning.UnsafeToAtomTest do """ |> to_source_file() |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 3 + assert issue.trigger == "Jason.decode!" + end) end end