Skip to content

Commit

Permalink
Add correct column metadata to all warning checks
Browse files Browse the repository at this point in the history
  • Loading branch information
NJichev committed Apr 29, 2024
1 parent 35abc4d commit 47f20e4
Show file tree
Hide file tree
Showing 35 changed files with 272 additions and 108 deletions.
9 changes: 5 additions & 4 deletions lib/credo/check/refactor/io_puts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ defmodule Credo.Check.Refactor.IoPuts do
end

defp traverse(
{{:., _, [{:__aliases__, _, [:IO]}, :puts]}, meta, _arguments} = ast,
{{:., _, [{:__aliases__, meta, [:IO]}, :puts]}, _, _arguments} = ast,
issues,
issue_meta
) do
Expand All @@ -36,15 +36,16 @@ defmodule Credo.Check.Refactor.IoPuts do
end

defp issues_for_call(meta, issues, issue_meta) do
[issue_for(issue_meta, meta[:line], @call_string) | issues]
[issue_for(issue_meta, meta, @call_string) | issues]
end

defp issue_for(issue_meta, line_no, trigger) do
defp issue_for(issue_meta, meta, trigger) do
format_issue(
issue_meta,
message: "There should be no calls to `IO.puts/1`.",
trigger: trigger,
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end
end
21 changes: 11 additions & 10 deletions lib/credo/check/warning/application_config_in_module_attribute.ex
Original file line number Diff line number Diff line change
Expand Up @@ -65,44 +65,45 @@ defmodule Credo.Check.Warning.ApplicationConfigInModuleAttribute do
end

defp get_forbidden_call(
{{:., _, [{:__aliases__, _, [:Application]}, :fetch_env]}, _meta, _args} = ast,
{{:., _, [{:__aliases__, meta, [:Application]}, :fetch_env]}, _meta, _args} = ast,
_acc
) do
{ast, {"Application.fetch_env/2", "Application.fetch_env"}}
{ast, {meta, "Application.fetch_env/2", "Application.fetch_env"}}
end

defp get_forbidden_call(
{{:., _, [{:__aliases__, _, [:Application]}, :fetch_env!]}, _meta, _args} = ast,
{{:., _, [{:__aliases__, meta, [:Application]}, :fetch_env!]}, _meta, _args} = ast,
_acc
) do
{ast, {"Application.fetch_env!/2", "Application.fetch_env"}}
{ast, {meta, "Application.fetch_env!/2", "Application.fetch_env"}}
end

defp get_forbidden_call(
{{:., _, [{:__aliases__, _, [:Application]}, :get_all_env]}, _meta, _args} = ast,
{{:., _, [{:__aliases__, meta, [:Application]}, :get_all_env]}, _meta, _args} = ast,
_acc
) do
{ast, {"Application.get_all_env/1", "Application.get_all_env"}}
{ast, {meta, "Application.get_all_env/1", "Application.get_all_env"}}
end

defp get_forbidden_call(
{{:., _, [{:__aliases__, _, [:Application]}, :get_env]}, _meta, args} = ast,
{{:., _, [{:__aliases__, meta, [:Application]}, :get_env]}, _meta, args} = ast,
_acc
) do
{ast, {"Application.get_env/#{length(args)}", "Application.get_env"}}
{ast, {meta, "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, trigger}, meta, issue_meta, issues) do
defp issues_for_call(attribute, {call_meta, 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]
line_no: call_meta[:line],
column: call_meta[:column]
)
| issues
]
Expand Down
9 changes: 5 additions & 4 deletions lib/credo/check/warning/bool_operation_on_same_values.ex
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ defmodule Credo.Check.Warning.BoolOperationOnSameValues do
op_not_redefined? = unquote(op) not in redefined_ops

if op_not_redefined? && Credo.Code.remove_metadata(lhs) === Credo.Code.remove_metadata(rhs) do
new_issue = issue_for(issue_meta, meta[:line], unquote(op))
{ast, issues ++ [new_issue]}
new_issue = issue_for(issue_meta, meta, unquote(op))
{ast, [new_issue | issues]}
else
{ast, issues}
end
Expand Down Expand Up @@ -86,13 +86,14 @@ defmodule Credo.Check.Warning.BoolOperationOnSameValues do
{ast, acc}
end

defp issue_for(issue_meta, line_no, trigger) do
defp issue_for(issue_meta, meta, trigger) do
format_issue(
issue_meta,
message:
"There are identical sub-expressions to the left and to the right of the '#{trigger}' operator.",
trigger: trigger,
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end
end
7 changes: 4 additions & 3 deletions lib/credo/check/warning/expensive_empty_enum_check.ex
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do
end

defp issues_for_call(meta, trigger, issues, issue_meta, ast) do
[issue_for(issue_meta, meta[:line], trigger, suggest(ast)) | issues]
[issue_for(issue_meta, meta, trigger, suggest(ast)) | issues]
end

defp suggest({_op, _, [0, {_pattern, _, args}]}), do: suggest_for_arity(Enum.count(args))
Expand All @@ -72,12 +72,13 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do
defp suggest_for_arity(2), do: "`not Enum.any?/2`"
defp suggest_for_arity(1), do: "`Enum.empty?/1` or `list == []`"

defp issue_for(issue_meta, line_no, trigger, suggestion) do
defp issue_for(issue_meta, meta, trigger, suggestion) do
format_issue(
issue_meta,
message: "#{trigger} is expensive, prefer #{suggestion}.",
trigger: trigger,
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end
end
13 changes: 7 additions & 6 deletions lib/credo/check/warning/forbidden_module.ex
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ defmodule Credo.Check.Warning.ForbiddenModule do
defp traverse({:__aliases__, meta, modules} = ast, issues, forbidden_modules, issue_meta) do
module = Name.full(modules)

issues = put_issue_if_forbidden(issues, issue_meta, meta[:line], module, forbidden_modules)
issues = put_issue_if_forbidden(issues, issue_meta, meta, module, forbidden_modules)

{ast, issues}
end
Expand All @@ -56,12 +56,12 @@ defmodule Credo.Check.Warning.ForbiddenModule do
) do
modules =
Enum.map(aliases, fn {:__aliases__, meta, module} ->
{Name.full([base_alias, module]), meta[:line]}
{Name.full([base_alias, module]), meta}
end)

issues =
Enum.reduce(modules, issues, fn {module, line}, issues ->
put_issue_if_forbidden(issues, issue_meta, line, module, forbidden_modules)
Enum.reduce(modules, issues, fn {module, meta}, issues ->
put_issue_if_forbidden(issues, issue_meta, meta, module, forbidden_modules)
end)

{ast, issues}
Expand All @@ -83,15 +83,16 @@ defmodule Credo.Check.Warning.ForbiddenModule do
Enum.member?(forbidden_module_names, module)
end

defp issue_for(issue_meta, line_no, module, forbidden_modules) do
defp issue_for(issue_meta, meta, module, forbidden_modules) do
trigger = Name.full(module)
message = message(forbidden_modules, module) || "The `#{trigger}` module is not allowed."

format_issue(
issue_meta,
message: message,
trigger: trigger,
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end

Expand Down
7 changes: 4 additions & 3 deletions lib/credo/check/warning/iex_pry.ex
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ defmodule Credo.Check.Warning.IExPry do

defp traverse(
{
{:., _, [{:__aliases__, _, [:IEx]}, :pry]},
meta,
{:., _, [{:__aliases__, meta, [:IEx]}, :pry]},
_,
_arguments
} = ast,
issues,
Expand All @@ -44,7 +44,8 @@ defmodule Credo.Check.Warning.IExPry do
issue_meta,
message: "There should be no calls to `IEx.pry/0`.",
trigger: @call_string,
line_no: meta[:line]
line_no: meta[:line],
column: meta[:column]
)

[new_issue | issues]
Expand Down
9 changes: 5 additions & 4 deletions lib/credo/check/warning/lazy_logging.ex
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ defmodule Credo.Check.Warning.LazyLogging do
end

defp traverse(
{{:., _, [{:__aliases__, _, [:Logger]}, fun_name]}, meta, arguments} = ast,
{{:., _, [{:__aliases__, meta, [:Logger]}, fun_name]}, _, arguments} = ast,
state,
issue_meta
)
Expand Down Expand Up @@ -99,7 +99,7 @@ defmodule Credo.Check.Warning.LazyLogging do
end

defp issue_for_call([{:<<>>, _, [_ | _]} | _] = _args, meta, fun_name, issue_meta) do
issue_for(issue_meta, meta[:line], fun_name)
issue_for(issue_meta, meta, fun_name)
end

defp issue_for_call(_args, _meta, _trigger, _issue_meta) do
Expand All @@ -109,11 +109,12 @@ defmodule Credo.Check.Warning.LazyLogging do
defp logger_import?([{:__aliases__, _meta, [:Logger]}]), do: true
defp logger_import?(_), do: false

defp issue_for(issue_meta, line_no, trigger) do
defp issue_for(issue_meta, meta, trigger) do
format_issue(
issue_meta,
message: "Prefer lazy Logger calls.",
line_no: line_no,
line_no: meta[:line],
column: meta[:column],
trigger: trigger
)
end
Expand Down
18 changes: 11 additions & 7 deletions lib/credo/check/warning/leaky_environment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,26 @@ defmodule Credo.Check.Warning.LeakyEnvironment do
nil ->
{ast, issues}

{trigger, meta} ->
{ast, [issue_for(issue_meta, meta, trigger) | issues]}

trigger ->
{ast, [issue_for(issue_meta, meta[:line], trigger) | issues]}
{ast, [issue_for(issue_meta, meta, trigger) | issues]}
end
end

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

defp get_forbidden_call([{:__aliases__, _, [:System]}, :cmd], [_, _]) do
"System.cmd"
defp get_forbidden_call([{:__aliases__, meta, [:System]}, :cmd], [_, _]) do
{"System.cmd", meta}
end

defp get_forbidden_call([{:__aliases__, _, [:System]}, :cmd], [_, _, opts])
defp get_forbidden_call([{:__aliases__, meta, [:System]}, :cmd], [_, _, opts])
when is_list(opts) do
if not Keyword.has_key?(opts, :env) do
"System.cmd"
{"System.cmd", meta}
end
end

Expand All @@ -63,12 +66,13 @@ defmodule Credo.Check.Warning.LeakyEnvironment do
nil
end

defp issue_for(issue_meta, line_no, trigger) do
defp issue_for(issue_meta, meta, trigger) do
format_issue(
issue_meta,
message: "When using #{trigger}, clear or overwrite sensitive environment variables.",
trigger: trigger,
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end
end
9 changes: 5 additions & 4 deletions lib/credo/check/warning/map_get_unsafe_pass.ex
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ defmodule Credo.Check.Warning.MapGetUnsafePass do
{next_expr, _} = Enum.at(pipe, idx + 1, {nil, nil})

case {expr, nil_safe?(next_expr)} do
{{{{:., meta, [{_, _, [:Map]}, :get]}, _, args}, _}, false}
{{{{:., _, [{_, meta, [:Map]}, :get]}, _, args}, _}, false}
when length(args) != required_length ->
acc ++ [issue_for(issue_meta, meta[:line], @call_string)]
[issue_for(issue_meta, meta, @call_string) | acc]

_ ->
acc
Expand All @@ -80,13 +80,14 @@ defmodule Credo.Check.Warning.MapGetUnsafePass do
end
end

defp issue_for(issue_meta, line_no, trigger) do
defp issue_for(issue_meta, meta, trigger) do
format_issue(
issue_meta,
message:
"`Map.get` with no default return value is potentially unsafe in pipes, use `Map.get/3` instead.",
trigger: trigger,
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfig do
end

defp traverse(
{{:., _, [{:__aliases__, _, [:Logger]}, fun_name]}, meta, arguments} = ast,
{{:., _, [{:__aliases__, meta, [:Logger]}, fun_name]}, _, arguments} = ast,
state,
issue_meta,
metadata_keys
Expand Down Expand Up @@ -151,7 +151,7 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfig do
nil

missed ->
issue_for(issue_meta, meta[:line], Keyword.keys(missed))
issue_for(issue_meta, meta, Keyword.keys(missed))
end
end
end
Expand All @@ -176,10 +176,11 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfig do
|> Keyword.get(:metadata)
end

defp issue_for(issue_meta, line_no, [trigger | _] = missed_keys) do
defp issue_for(issue_meta, meta, [trigger | _] = missed_keys) do
format_issue(issue_meta,
message: "Logger metadata key #{Enum.join(missed_keys, ", ")} not found in Logger config.",
line_no: line_no,
line_no: meta[:line],
column: meta[:column],
trigger: trigger
)
end
Expand Down
9 changes: 5 additions & 4 deletions lib/credo/check/warning/mix_env.ex
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ defmodule Credo.Check.Warning.MixEnv do
end

defp traverse_defs(
{{:., _, [{:__aliases__, _, [:Mix]}, :env]}, meta, _arguments} = ast,
{{:., _, [{:__aliases__, meta, [:Mix]}, :env]}, _, _arguments} = ast,
issues,
issue_meta
) do
Expand All @@ -78,15 +78,16 @@ defmodule Credo.Check.Warning.MixEnv do
end

defp issues_for_call(meta, issues, issue_meta) do
[issue_for(issue_meta, meta[:line]) | issues]
[issue_for(issue_meta, meta) | issues]
end

defp issue_for(issue_meta, line_no) do
defp issue_for(issue_meta, meta) do
format_issue(
issue_meta,
message: "There should be no calls to Mix.env in application code.",
trigger: "Mix.env",
line_no: line_no
line_no: meta[:line],
column: meta[:column]
)
end
end

0 comments on commit 47f20e4

Please sign in to comment.