diff --git a/lib/credo/check/refactor/io_puts.ex b/lib/credo/check/refactor/io_puts.ex index 352b0acb9..af89bd8a9 100644 --- a/lib/credo/check/refactor/io_puts.ex +++ b/lib/credo/check/refactor/io_puts.ex @@ -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 @@ -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 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 cdedcc899..e1e36f5a3 100644 --- a/lib/credo/check/warning/application_config_in_module_attribute.ex +++ b/lib/credo/check/warning/application_config_in_module_attribute.ex @@ -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 @@ -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 ] diff --git a/lib/credo/check/warning/bool_operation_on_same_values.ex b/lib/credo/check/warning/bool_operation_on_same_values.ex index 341f866ca..579c911be 100644 --- a/lib/credo/check/warning/bool_operation_on_same_values.ex +++ b/lib/credo/check/warning/bool_operation_on_same_values.ex @@ -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 @@ -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 diff --git a/lib/credo/check/warning/expensive_empty_enum_check.ex b/lib/credo/check/warning/expensive_empty_enum_check.ex index 537831ba0..afa53991e 100644 --- a/lib/credo/check/warning/expensive_empty_enum_check.ex +++ b/lib/credo/check/warning/expensive_empty_enum_check.ex @@ -50,11 +50,11 @@ 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 @@ -62,22 +62,29 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheck 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 diff --git a/lib/credo/check/warning/forbidden_module.ex b/lib/credo/check/warning/forbidden_module.ex index ebedd1161..ab9a747d1 100644 --- a/lib/credo/check/warning/forbidden_module.ex +++ b/lib/credo/check/warning/forbidden_module.ex @@ -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 @@ -54,14 +54,11 @@ 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} @@ -69,11 +66,11 @@ defmodule Credo.Check.Warning.ForbiddenModule do 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 @@ -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 diff --git a/lib/credo/check/warning/iex_pry.ex b/lib/credo/check/warning/iex_pry.ex index f908251ce..b2ede1f3d 100644 --- a/lib/credo/check/warning/iex_pry.ex +++ b/lib/credo/check/warning/iex_pry.ex @@ -24,8 +24,8 @@ defmodule Credo.Check.Warning.IExPry do defp traverse( { - {:., _, [{:__aliases__, _, [:IEx]}, :pry]}, - meta, + {:., _, [{:__aliases__, meta, [:IEx]}, :pry]}, + _, _arguments } = ast, issues, @@ -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] diff --git a/lib/credo/check/warning/lazy_logging.ex b/lib/credo/check/warning/lazy_logging.ex index a0240abc8..ec340453e 100644 --- a/lib/credo/check/warning/lazy_logging.ex +++ b/lib/credo/check/warning/lazy_logging.ex @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/lib/credo/check/warning/leaky_environment.ex b/lib/credo/check/warning/leaky_environment.ex index dda853845..1a56628f5 100644 --- a/lib/credo/check/warning/leaky_environment.ex +++ b/lib/credo/check/warning/leaky_environment.ex @@ -27,13 +27,22 @@ 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 @@ -41,14 +50,14 @@ defmodule Credo.Check.Warning.LeakyEnvironment 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 @@ -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 diff --git a/lib/credo/check/warning/map_get_unsafe_pass.ex b/lib/credo/check/warning/map_get_unsafe_pass.ex index e8d2d62ad..405dbc6c1 100644 --- a/lib/credo/check/warning/map_get_unsafe_pass.ex +++ b/lib/credo/check/warning/map_get_unsafe_pass.ex @@ -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 @@ -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 diff --git a/lib/credo/check/warning/mix_env.ex b/lib/credo/check/warning/mix_env.ex index eb2f542a3..6fddba684 100644 --- a/lib/credo/check/warning/mix_env.ex +++ b/lib/credo/check/warning/mix_env.ex @@ -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 @@ -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 diff --git a/lib/credo/check/warning/operation_on_same_values.ex b/lib/credo/check/warning/operation_on_same_values.ex index 0abda9148..7ec8f0b5f 100644 --- a/lib/credo/check/warning/operation_on_same_values.ex +++ b/lib/credo/check/warning/operation_on_same_values.ex @@ -62,13 +62,13 @@ defmodule Credo.Check.Warning.OperationOnSameValues do new_issue = issue_for( issue_meta, - meta[:line], + meta, unquote(op), unquote(operation_name), unquote(constant_result) ) - {ast, issues ++ [new_issue]} + {ast, [new_issue | issues]} else {ast, issues} end @@ -88,12 +88,13 @@ defmodule Credo.Check.Warning.OperationOnSameValues do {ast, issues} end - defp issue_for(issue_meta, line_no, trigger, operation, constant_result) do + defp issue_for(issue_meta, meta, trigger, operation, constant_result) do format_issue( issue_meta, message: "#{operation} will always return #{constant_result}.", trigger: trigger, - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/lib/credo/check/warning/operation_with_constant_result.ex b/lib/credo/check/warning/operation_with_constant_result.ex index 4da87fbbd..7c77b2ad4 100644 --- a/lib/credo/check/warning/operation_with_constant_result.ex +++ b/lib/credo/check/warning/operation_with_constant_result.ex @@ -49,12 +49,12 @@ defmodule Credo.Check.Warning.OperationWithConstantResult do new_issue = issue_for( issue_meta, - meta[:line], + meta, unquote(op), unquote(constant_result) ) - {ast, issues ++ [new_issue]} + {ast, [new_issue | issues]} end end @@ -62,12 +62,13 @@ defmodule Credo.Check.Warning.OperationWithConstantResult do {ast, issues} end - defp issue_for(issue_meta, line_no, trigger, constant_result) do + defp issue_for(issue_meta, meta, trigger, constant_result) do format_issue( issue_meta, message: "Operation will always return #{constant_result}.", trigger: trigger, - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/lib/credo/check/warning/raise_inside_rescue.ex b/lib/credo/check/warning/raise_inside_rescue.ex index 957ef4bec..86e182564 100644 --- a/lib/credo/check/warning/raise_inside_rescue.ex +++ b/lib/credo/check/warning/raise_inside_rescue.ex @@ -71,19 +71,20 @@ defmodule Credo.Check.Warning.RaiseInsideRescue do defp traverse(ast, issues, _issue_meta), do: {ast, issues} defp find_issues({:raise, meta, _arguments} = ast, issues, issue_meta) do - issue = issue_for(issue_meta, meta[:line]) + issue = issue_for(issue_meta, meta) - {ast, issues ++ [issue]} + {ast, [issue | issues]} end defp find_issues(ast, issues, _), do: {ast, issues} - defp issue_for(issue_meta, line_no) do + defp issue_for(issue_meta, meta) do format_issue( issue_meta, message: "Use `reraise` inside a rescue block to preserve the original stacktrace.", trigger: "raise", - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/lib/credo/check/warning/spec_with_struct.ex b/lib/credo/check/warning/spec_with_struct.ex index 021740abe..497054c53 100644 --- a/lib/credo/check/warning/spec_with_struct.ex +++ b/lib/credo/check/warning/spec_with_struct.ex @@ -29,15 +29,15 @@ defmodule Credo.Check.Warning.SpecWithStruct do Credo.Code.prewalk(source_file, &traverse(&1, &2, issue_meta)) end - defp traverse({:@, meta, [{:spec, _, args}]}, issues, issue_meta) do + defp traverse({:@, _, [{:spec, _, args}]}, issues, issue_meta) do case Macro.prewalk(args, [], &find_structs/2) do {ast, []} -> {ast, issues} {ast, structs} -> issues = - Enum.reduce(structs, issues, fn curr, acc -> - [issue_for(issue_meta, meta[:line], curr) | acc] + Enum.reduce(structs, issues, fn {curr, meta}, acc -> + [issue_for(issue_meta, meta, curr) | acc] end) {ast, issues} @@ -48,19 +48,20 @@ defmodule Credo.Check.Warning.SpecWithStruct do {ast, issues} end - defp find_structs({:%, _, [{:__aliases__, _, _} = aliases | _]} = ast, acc) do - {ast, [Name.full(aliases) | acc]} + defp find_structs({:%, meta, [{:__aliases__, _, _} = aliases | _]} = ast, acc) do + {ast, [{Name.full(aliases), meta} | acc]} end defp find_structs(ast, acc) do {ast, acc} end - defp issue_for(issue_meta, line_no, struct) do + defp issue_for(issue_meta, meta, struct) do format_issue(issue_meta, message: "Struct %#{struct}{} found in `@spec`.", trigger: "%#{struct}{", - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/lib/credo/check/warning/unsafe_exec.ex b/lib/credo/check/warning/unsafe_exec.ex index 1b8a18528..373b81074 100644 --- a/lib/credo/check/warning/unsafe_exec.ex +++ b/lib/credo/check/warning/unsafe_exec.ex @@ -37,7 +37,8 @@ defmodule Credo.Check.Warning.UnsafeExec do defp traverse({{:., _loc, call}, meta, args} = ast, issues, issue_meta) do case get_forbidden_call(call, args) do {bad, suggestion, trigger} -> - {ast, [issue_for(bad, suggestion, trigger, meta[:line], issue_meta) | issues]} + [module, _function] = call + {ast, [issue_for(bad, suggestion, trigger, meta, module, issue_meta) | issues]} nil -> {ast, issues} @@ -65,11 +66,16 @@ defmodule Credo.Check.Warning.UnsafeExec do nil end - defp issue_for(call, suggestion, trigger, line_no, issue_meta) do + @colon_and_dot_length 2 + defp issue_for(call, suggestion, trigger, meta, module, issue_meta) do + len = module |> Atom.to_string() |> String.length() + column = meta[:column] - len - @colon_and_dot_length + format_issue(issue_meta, message: "Prefer #{suggestion} over #{call} to prevent command injection.", trigger: trigger, - line_no: line_no + line_no: meta[:line], + column: column ) end end diff --git a/lib/credo/check/warning/unused_function_return_helper.ex b/lib/credo/check/warning/unused_function_return_helper.ex index bb4fda254..d0f05f8b3 100644 --- a/lib/credo/check/warning/unused_function_return_helper.ex +++ b/lib/credo/check/warning/unused_function_return_helper.ex @@ -40,7 +40,7 @@ defmodule Credo.Check.Warning.UnusedFunctionReturnHelper do nil ) do if mods == required_mod_list do - {ast, acc ++ [ast]} + {ast, [ast | acc]} else {ast, acc} end @@ -53,7 +53,7 @@ defmodule Credo.Check.Warning.UnusedFunctionReturnHelper do restrict_fun_names ) do if mods == required_mod_list and fun_name in restrict_fun_names do - {ast, acc ++ [ast]} + {ast, [ast | acc]} else {ast, acc} end diff --git a/lib/credo/check/warning/unused_operation.ex b/lib/credo/check/warning/unused_operation.ex index b82a466fd..125b280f9 100644 --- a/lib/credo/check/warning/unused_operation.ex +++ b/lib/credo/check/warning/unused_operation.ex @@ -26,7 +26,7 @@ defmodule Credo.Check.Warning.UnusedOperation do ) Enum.reduce(all_unused_calls, [], fn invalid_call, issues -> - {_, meta, _} = invalid_call + {{:., _, [{:__aliases__, meta, _}, _fun_name]}, _, _} = invalid_call trigger = invalid_call @@ -34,16 +34,17 @@ defmodule Credo.Check.Warning.UnusedOperation do |> String.split("(") |> List.first() - issues ++ [issue_for(format_issue_fun, issue_meta, meta[:line], trigger, checked_module)] + [issue_for(format_issue_fun, issue_meta, meta, trigger, checked_module) | issues] end) end - defp issue_for(format_issue_fun, issue_meta, line_no, trigger, checked_module) do + defp issue_for(format_issue_fun, issue_meta, meta, trigger, checked_module) do format_issue_fun.( issue_meta, message: "There should be no unused return values for #{checked_module} functions.", trigger: trigger, - line_no: line_no + line_no: meta[:line], + column: meta[:column] ) end end diff --git a/test/credo/check/refactor/io_puts_test.exs b/test/credo/check/refactor/io_puts_test.exs index 42046c80f..6fb147b29 100644 --- a/test/credo/check/refactor/io_puts_test.exs +++ b/test/credo/check/refactor/io_puts_test.exs @@ -37,6 +37,24 @@ defmodule Credo.Check.Refactor.IoPutsTest do |> assert_issue() end + test "it should report a violation with two on the same line" do + """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + IO.puts(parameter1); IO.puts(parameter2) + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issues(fn [two, one] -> + assert one.line_no == 3 + assert one.column == 5 + assert two.line_no == 3 + assert two.column == 26 + end) + end + test "it should report a violation /2" do """ defmodule CredoSampleModule do 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 b4b0e07b9..6759de6f0 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 @@ -67,42 +67,45 @@ defmodule Credo.Check.Warning.ApplicationConfigInModuleAttributeTest do assert_issues = [ { "Module attribute @config_1 makes use of unsafe Application configuration call Application.fetch_env/2", - 2, + {2, 13}, "Application.fetch_env" }, { "Module attribute @config_3 makes use of unsafe Application configuration call Application.fetch_env!/2", - 4, + {4, 13}, "Application.fetch_env" }, { "Module attribute @config_5 makes use of unsafe Application configuration call Application.get_all_env/1", - 6, + {6, 13}, "Application.get_all_env" }, { "Module attribute @config_7 makes use of unsafe Application configuration call Application.get_env/2", - 8, + {8, 13}, "Application.get_env" }, { "Module attribute @config_9 makes use of unsafe Application configuration call Application.get_env/3", - 10, + {10, 13}, "Application.get_env" } ] assert length(issues) == 5 - Enum.each(assert_issues, fn {error_message, line_no, trigger} -> - assert error_exists?(issues, error_message, line_no, trigger) + Enum.each(assert_issues, fn {error_message, pos, trigger} -> + assert error_exists?(issues, error_message, pos, trigger) end) end - defp error_exists?(errors, error_message, line_no, trigger) do + defp error_exists?(errors, error_message, {line_no, column}, trigger) do Enum.any?(errors, fn - %Credo.Issue{message: ^error_message, line_no: ^line_no, trigger: ^trigger} -> true - _ -> false + %Credo.Issue{message: ^error_message, line_no: ^line_no, column: ^column, trigger: ^trigger} -> + true + + _ -> + false end) 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 4bdc839e2..58085022f 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 @@ -98,6 +98,8 @@ defmodule Credo.Check.Warning.BoolOperationOnSameValuesTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.trigger == "and" + assert issue.line_no == 5 + assert issue.column == 7 end) end end diff --git a/test/credo/check/warning/dbg_test.exs b/test/credo/check/warning/dbg_test.exs index 8311a62d8..226f2207f 100644 --- a/test/credo/check/warning/dbg_test.exs +++ b/test/credo/check/warning/dbg_test.exs @@ -81,11 +81,11 @@ defmodule Credo.Check.Warning.DbgTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issues(fn [one, two] -> + |> assert_issues(fn [two, one] -> assert one.line_no == 3 - assert one.column == 23 + assert one.column == 5 assert two.line_no == 3 - assert two.column == 5 + assert two.column == 23 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 ec4f4336b..8d389f005 100644 --- a/test/credo/check/warning/expensive_empty_enum_check_test.exs +++ b/test/credo/check/warning/expensive_empty_enum_check_test.exs @@ -131,6 +131,8 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheckTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.trigger == "length" + assert issue.line_no == 3 + assert issue.column == 8 end) end @@ -148,7 +150,11 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheckTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.trigger == "length" + assert issue.line_no == 3 + assert issue.column == 13 + end) end test "it should report when checking if Enum.count/1 is 0" do @@ -168,6 +174,8 @@ defmodule Credo.Check.Warning.ExpensiveEmptyEnumCheckTest do |> assert_issue(fn issue -> assert issue.trigger == "Enum.count" assert issue.message =~ "Enum.empty" + assert issue.line_no == 3 + assert issue.column == 8 end) end @@ -185,7 +193,11 @@ 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.message =~ "Enum.empty" + assert issue.line_no == 3 + assert issue.column == 13 + end) end test "it should report when checking if Enum.count/2 is 0" do diff --git a/test/credo/check/warning/forbidden_module_test.exs b/test/credo/check/warning/forbidden_module_test.exs index ee713a68e..827007d6c 100644 --- a/test/credo/check/warning/forbidden_module_test.exs +++ b/test/credo/check/warning/forbidden_module_test.exs @@ -50,7 +50,10 @@ defmodule Credo.Check.Warning.ForbiddenModuleTest do """ |> to_source_file |> run_check(@described_check, modules: [CredoSampleModule.ForbiddenModule]) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 2 + assert issue.column == 26 + end) end test "it should report on aliases" do @@ -62,7 +65,10 @@ defmodule Credo.Check.Warning.ForbiddenModuleTest do """ |> to_source_file |> run_check(@described_check, modules: [CredoSampleModule.ForbiddenModule]) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 2 + assert issue.column == 9 + end) end test "it should report on grouped aliases" do @@ -76,7 +82,14 @@ defmodule Credo.Check.Warning.ForbiddenModuleTest do |> run_check(@described_check, modules: [CredoSampleModule.ForbiddenModule, CredoSampleModule.ForbiddenModule2] ) - |> assert_issues() + |> assert_issues(fn [two, one] -> + assert one.trigger == "ForbiddenModule" + assert one.line_no == 2 + assert one.column == 43 + assert two.trigger == "ForbiddenModule2" + assert two.line_no == 2 + assert two.column == 60 + end) end test "it should report on import" do @@ -114,6 +127,7 @@ defmodule Credo.Check.Warning.ForbiddenModuleTest do |> run_check(@described_check, modules: [{CredoSampleModule.ForbiddenModule, "my message"}]) |> assert_issue(fn issue -> assert issue.line_no == 3 + assert issue.column == 5 assert issue.trigger == "CredoSampleModule.ForbiddenModule" assert issue.message == "my message" end) diff --git a/test/credo/check/warning/iex_pry_test.exs b/test/credo/check/warning/iex_pry_test.exs index d9f0f1c07..6c5d2dbc0 100644 --- a/test/credo/check/warning/iex_pry_test.exs +++ b/test/credo/check/warning/iex_pry_test.exs @@ -40,4 +40,22 @@ defmodule Credo.Check.Warning.IExPryTest do assert issue.trigger == "IEx.pry" end) end + + test "it should report a violation with two on the same line" do + """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + IEx.pry(); IEx.pry() + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issues(fn [two, one] -> + assert one.line_no == 3 + assert one.column == 5 + assert two.line_no == 3 + assert two.column == 16 + end) + end end diff --git a/test/credo/check/warning/lazy_logging_test.exs b/test/credo/check/warning/lazy_logging_test.exs index 98a5caa79..5995ebbb0 100644 --- a/test/credo/check/warning/lazy_logging_test.exs +++ b/test/credo/check/warning/lazy_logging_test.exs @@ -120,7 +120,19 @@ defmodule Credo.Check.Warning.LazyLoggingTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issues() + |> assert_issues(fn [three, two, one] -> + assert one.trigger == "Logger.debug" + assert one.line_no == 5 + assert one.column == 5 + + assert two.trigger == "Logger.debug" + assert two.line_no == 6 + assert two.column == 5 + + assert three.trigger == "Logger.debug" + assert three.line_no == 7 + assert three.column == 5 + end) end test "it should report a violation with imported :debug from Logger" do @@ -137,6 +149,8 @@ defmodule Credo.Check.Warning.LazyLoggingTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.trigger == "debug" + assert issue.line_no == 5 + assert issue.column == 5 end) end end diff --git a/test/credo/check/warning/leaky_environment_test.exs b/test/credo/check/warning/leaky_environment_test.exs index 0838c584e..e1efedeb9 100644 --- a/test/credo/check/warning/leaky_environment_test.exs +++ b/test/credo/check/warning/leaky_environment_test.exs @@ -40,6 +40,7 @@ defmodule Credo.Check.Warning.LeakyEnvironmentTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 3 + assert issue.column == 5 assert issue.trigger == "System.cmd" end) end @@ -56,6 +57,7 @@ defmodule Credo.Check.Warning.LeakyEnvironmentTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 3 + assert issue.column == 5 assert issue.trigger == "System.cmd" end) end @@ -72,6 +74,7 @@ defmodule Credo.Check.Warning.LeakyEnvironmentTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 3 + assert issue.column == 5 assert issue.trigger == ":erlang.open_port" end) end 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 250f5f3a8..502bb964c 100644 --- a/test/credo/check/warning/map_get_unsafe_pass_test.exs +++ b/test/credo/check/warning/map_get_unsafe_pass_test.exs @@ -76,7 +76,10 @@ defmodule Credo.Check.Warning.MapGetUnsafePassTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 5 + assert issue.column == 8 + end) end test "it should report a violation /2" do @@ -93,7 +96,10 @@ defmodule Credo.Check.Warning.MapGetUnsafePassTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 5 + assert issue.column == 5 + end) end test "it should report a violation /3" do @@ -116,6 +122,7 @@ defmodule Credo.Check.Warning.MapGetUnsafePassTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 7 + assert issue.column == 22 assert issue.trigger == "Map.get" end) end diff --git a/test/credo/check/warning/missed_metadata_key_in_logger_config_test.exs b/test/credo/check/warning/missed_metadata_key_in_logger_config_test.exs index a8d242d62..139d983fe 100644 --- a/test/credo/check/warning/missed_metadata_key_in_logger_config_test.exs +++ b/test/credo/check/warning/missed_metadata_key_in_logger_config_test.exs @@ -136,7 +136,9 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfigTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 5 + end) end end @@ -153,7 +155,13 @@ defmodule Credo.Check.Warning.MissedMetadataKeyInLoggerConfigTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issues() + |> assert_issues(fn [two, one] -> + assert one.trigger == "user_id" + assert one.line_no == 4 + + assert two.trigger == "key" + assert two.line_no == 6 + end) end test "it should report a violation when Logger.log/3 is used with disallowed metadata" do diff --git a/test/credo/check/warning/mix_env_test.exs b/test/credo/check/warning/mix_env_test.exs index f0f6d3d16..32e8aa18a 100644 --- a/test/credo/check/warning/mix_env_test.exs +++ b/test/credo/check/warning/mix_env_test.exs @@ -128,6 +128,24 @@ defmodule Credo.Check.Warning.MixEnvTest do end) end + test "it should report a violation with two on the same line" do + """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + Mix.env(); Mix.env() + end + end + """ + |> to_source_file + |> run_check(@described_check) + |> assert_issues(fn [two, one] -> + assert one.line_no == 3 + assert one.column == 5 + assert two.line_no == 3 + assert two.column == 16 + end) + end + test "it should report violations from variables named like def operations" do """ defmodule CredoSampleModule do 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 6c5eae444..20d57b482 100644 --- a/test/credo/check/warning/operation_on_same_values_test.exs +++ b/test/credo/check/warning/operation_on_same_values_test.exs @@ -67,7 +67,10 @@ defmodule Credo.Check.Warning.OperationOnSameValuesTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 5 + assert issue.column == 14 + end) end test "it should report a violation for module attributes" do @@ -82,6 +85,8 @@ defmodule Credo.Check.Warning.OperationOnSameValuesTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.trigger == "-" + assert issue.line_no == 4 + assert issue.column == 29 end) end 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 7957793c3..dca81674a 100644 --- a/test/credo/check/warning/operation_with_constant_result_test.exs +++ b/test/credo/check/warning/operation_with_constant_result_test.exs @@ -74,8 +74,14 @@ defmodule Credo.Check.Warning.OperationWithConstantResultTest do """ |> to_source_file |> run_check(@described_check) - |> assert_issues(fn issues -> - assert 2 == Enum.count(issues) + |> assert_issues(fn [two, one] -> + assert one.trigger == "*" + assert one.line_no == 5 + assert one.column == 7 + + assert two.trigger == "*" + assert two.line_no == 6 + assert two.column == 7 end) end end diff --git a/test/credo/check/warning/raise_inside_rescue_test.exs b/test/credo/check/warning/raise_inside_rescue_test.exs index e83187569..bde932f29 100644 --- a/test/credo/check/warning/raise_inside_rescue_test.exs +++ b/test/credo/check/warning/raise_inside_rescue_test.exs @@ -71,6 +71,7 @@ defmodule Credo.Check.Warning.RaiseInsideRescueTest do |> assert_issue(fn issue -> assert "raise" == issue.trigger assert 10 == issue.line_no + assert 9 == issue.column end) end @@ -93,6 +94,7 @@ defmodule Credo.Check.Warning.RaiseInsideRescueTest do |> assert_issue(fn issue -> assert "raise" == issue.trigger assert 9 == issue.line_no + assert 7 == issue.column end) end @@ -115,6 +117,7 @@ defmodule Credo.Check.Warning.RaiseInsideRescueTest do |> assert_issue(fn issue -> assert "raise" == issue.trigger assert 8 == issue.line_no + assert 53 == issue.column 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 f0108c898..417af9607 100644 --- a/test/credo/check/warning/spec_with_struct_test.exs +++ b/test/credo/check/warning/spec_with_struct_test.exs @@ -99,6 +99,7 @@ defmodule Credo.Check.Warning.SpecWithStructTest do |> assert_issue(fn issue -> assert %{line_no: 2, message: "Struct %MyApp.MyStruct{} found in `@spec`."} = issue assert issue.trigger == "%MyApp.MyStruct{" + assert issue.column == 16 end) end @@ -118,6 +119,7 @@ defmodule Credo.Check.Warning.SpecWithStructTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 2 + assert issue.column == 16 assert issue.trigger == "%AStruct{" end) end @@ -137,7 +139,10 @@ defmodule Credo.Check.Warning.SpecWithStructTest do ] |> to_source_files() |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 3 + assert issue.column == 24 + end) end test "it should report an issue if a struct is used as an argument in a spec" do @@ -155,7 +160,10 @@ defmodule Credo.Check.Warning.SpecWithStructTest do ] |> to_source_files() |> run_check(@described_check) - |> assert_issue() + |> assert_issue(fn issue -> + assert issue.line_no == 3 + assert issue.column == 33 + end) end test "it should report an issue if a struct has an argument name in a spec" do @@ -172,7 +180,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.column == 24 + end) end test "it should report multiple issues in separate specs" do diff --git a/test/credo/check/warning/unsafe_exec_test.exs b/test/credo/check/warning/unsafe_exec_test.exs index 1c6fbb4d2..bb93d7a87 100644 --- a/test/credo/check/warning/unsafe_exec_test.exs +++ b/test/credo/check/warning/unsafe_exec_test.exs @@ -42,7 +42,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.column == 5 + end) end test "it should report a violation /2" do @@ -57,6 +60,7 @@ defmodule Credo.Check.Warning.UnsafeExecTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 3 + assert issue.column == 5 assert issue.trigger == ":os.cmd" end) end @@ -73,6 +77,7 @@ defmodule Credo.Check.Warning.UnsafeExecTest do |> run_check(@described_check) |> assert_issue(fn issue -> assert issue.line_no == 3 + assert issue.column == 5 assert issue.trigger == ":erlang.open_port" end) end diff --git a/test/credo/check/warning/unused_enum_operation_test.exs b/test/credo/check/warning/unused_enum_operation_test.exs index 81bc8f7c6..058d14c37 100644 --- a/test/credo/check/warning/unused_enum_operation_test.exs +++ b/test/credo/check/warning/unused_enum_operation_test.exs @@ -885,7 +885,9 @@ defmodule Credo.Check.Warning.UnusedEnumOperationTest do |> to_source_file |> run_check(@described_check) |> assert_issue(fn issue -> - assert "Enum.map" == issue.trigger + assert issue.trigger == "Enum.map" + assert issue.line_no == 5 + assert issue.column == 7 end) end end