Skip to content

Commit

Permalink
Fix more triggers
Browse files Browse the repository at this point in the history
  • Loading branch information
rrrene committed Feb 4, 2024
1 parent baa0c80 commit e4b1993
Show file tree
Hide file tree
Showing 30 changed files with 170 additions and 84 deletions.
7 changes: 6 additions & 1 deletion lib/credo/check.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/readability/one_arity_function_in_pipe.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/credo/check/readability/separate_alias_require.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion lib/credo/check/refactor/cyclomatic_complexity.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
Expand Down
25 changes: 13 additions & 12 deletions lib/credo/check/warning/application_config_in_module_attribute.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 8 additions & 8 deletions lib/credo/check/warning/expensive_empty_enum_check.ex
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,30 @@ 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

defp traverse(ast, issues, _issue_meta) 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))
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/warning/lazy_logging.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/credo/check/warning/spec_with_struct.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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]
]

Expand Down
4 changes: 2 additions & 2 deletions lib/credo/check/warning/unsafe_exec.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 25 additions & 23 deletions lib/credo/check/warning/unsafe_to_atom.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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}
Expand All @@ -77,35 +77,36 @@ 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)
when decode in [:decode, :decode!] 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
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion test/credo/check/housekeeping_trigger.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
[]
Expand Down
4 changes: 3 additions & 1 deletion test/credo/check/readability/function_names_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 3 additions & 1 deletion test/credo/check/refactor/reject_filter_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 3 additions & 1 deletion test/credo/check/refactor/reject_reject_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion test/credo/check/refactor/unless_with_else_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 4 additions & 1 deletion test/credo/check/refactor/variable_rebinding_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit e4b1993

Please sign in to comment.