Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add correct column metadata to all warning checks #1127

Merged
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
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 @@ -36,13 +36,13 @@ defmodule Credo.Check.Warning.ApplicationConfigInModuleAttribute do
Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
end

defp traverse({:@, meta, [attribute_definition]} = ast, issues, issue_meta) do
defp traverse({:@, _meta, [attribute_definition]} = ast, issues, issue_meta) do
case traverse_attribute(attribute_definition) do
nil ->
{ast, issues}

{attribute, call} ->
{ast, issues_for_call(attribute, call, meta, issue_meta, issues)}
{ast, issues_for_call(attribute, call, issue_meta, issues)}
end
end

Expand All @@ -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, {meta, call, trigger}, 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: meta[:line],
column: 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
19 changes: 13 additions & 6 deletions lib/credo/check/warning/expensive_empty_enum_check.ex
Original file line number Diff line number Diff line change
Expand Up @@ -50,34 +50,41 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck do
for {lhs, rhs, trigger} <- @comparisons,
operator <- @operators do
defp traverse(
{unquote(operator), meta, [unquote(lhs), unquote(rhs)]} = ast,
{unquote(operator), _meta, [unquote(lhs), unquote(rhs)]} = ast,
issues,
issue_meta
) do
{ast, issues_for_call(meta, unquote(trigger), issues, issue_meta, ast)}
{ast, issues_for_call(unquote(trigger), issues, issue_meta, ast)}
end
end

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

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

defp suggest({_op, _, [0, {_pattern, _, args}]}), do: suggest_for_arity(Enum.count(args))
defp suggest({_op, _, [{_pattern, _, args}, 0]}), do: suggest_for_arity(Enum.count(args))

defp get_meta({_op, _, [0, {{:., _, [{:__aliases__, meta, _}, _]}, _, _}]}), do: meta
defp get_meta({_op, _, [0, {_, meta, _}]}), do: meta
defp get_meta({_op, _, [{{:., _, [{:__aliases__, meta, _}, _]}, _, _}, 0]}), do: meta
defp get_meta({_op, _, [{_, meta, _}, 0]}), do: meta

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
23 changes: 10 additions & 13 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, module)

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

issues =
Enum.reduce(modules, issues, fn {module, line}, issues ->
put_issue_if_forbidden(issues, issue_meta, line, module, forbidden_modules)
Enum.reduce(aliases, issues, fn {:__aliases__, meta, module}, issues ->
full_module = Name.full([base_alias, module])
module = Name.full(module)
put_issue_if_forbidden(issues, issue_meta, meta, full_module, forbidden_modules, module)
end)

{ast, issues}
end

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

defp put_issue_if_forbidden(issues, issue_meta, line_no, module, forbidden_modules) do
defp put_issue_if_forbidden(issues, issue_meta, meta, module, forbidden_modules, trigger) do
forbidden_module_names = Enum.map(forbidden_modules, &elem(&1, 0))

if found_module?(forbidden_module_names, module) do
[issue_for(issue_meta, line_no, module, forbidden_modules) | issues]
[issue_for(issue_meta, meta, module, forbidden_modules, trigger) | issues]
else
issues
end
Expand All @@ -83,15 +80,15 @@ defmodule Credo.Check.Warning.ForbiddenModule do
Enum.member?(forbidden_module_names, module)
end

defp issue_for(issue_meta, line_no, module, forbidden_modules) do
trigger = Name.full(module)
defp issue_for(issue_meta, meta, module, forbidden_modules, trigger) do
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
18 changes: 10 additions & 8 deletions lib/credo/check/warning/lazy_logging.ex
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ 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
)
when fun_name in @logger_functions do
issue = find_issue(fun_name, arguments, meta, issue_meta)
trigger = "Logger.#{fun_name}"
issue = find_issue(fun_name, arguments, meta, issue_meta, trigger)

{ast, add_issue_to_state(state, issue)}
end
Expand All @@ -62,7 +63,7 @@ defmodule Credo.Check.Warning.LazyLogging do
issue_meta
)
when fun_name in @logger_functions do
issue = find_issue(fun_name, arguments, meta, issue_meta)
issue = find_issue(fun_name, arguments, meta, issue_meta, fun_name)

{ast, add_issue_to_state(state, issue)}
end
Expand All @@ -89,17 +90,17 @@ defmodule Credo.Check.Warning.LazyLogging do
{module_contains_import?, [issue | issues]}
end

defp find_issue(fun_name, arguments, meta, issue_meta) do
defp find_issue(fun_name, arguments, meta, issue_meta, trigger) do
params = IssueMeta.params(issue_meta)
ignored_functions = Params.get(params, :ignore, __MODULE__)

unless Enum.member?(ignored_functions, fun_name) do
issue_for_call(arguments, meta, fun_name, issue_meta)
issue_for_call(arguments, meta, trigger, issue_meta)
end
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 +110,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
24 changes: 17 additions & 7 deletions lib/credo/check/warning/leaky_environment.ex
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,37 @@ defmodule Credo.Check.Warning.LeakyEnvironment do
Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
end

@colon_and_dot_length 2
defp traverse({{:., _, call}, meta, args} = ast, issues, issue_meta) do
case get_forbidden_call(call, args) do
nil ->
{ast, issues}

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

trigger ->
{ast, [issue_for(issue_meta, meta[:line], trigger) | issues]}
[module, _function] = call
len = module |> Atom.to_string() |> String.length()
column = meta[:column] - len - @colon_and_dot_length
meta = Keyword.put(meta, :column, column)

{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 +72,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
Loading
Loading