Add correct column metadata to all warning checks

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

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
     ]

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

lib/credo/check/warning/expensive_empty_enum_check.ex

@@ -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

lib/credo/check/warning/forbidden_module.ex

@@ -29,9 +29,12 @@ defmodule Credo.Check.Warning.ForbiddenModule do
 
     modules =
       if Keyword.keyword?(modules) do
-        Enum.map(modules, fn {key, value} -> {Name.full(key), value} end)
+        Map.new(modules, fn {key, value} -> {Name.full(key), value} end)
       else
-        Enum.map(modules, fn key -> {Name.full(key), nil} end)
+        Map.new(modules, fn module ->
+          full = Name.full(module)
+          {full, "The `#{Name.full(module)}` module is not allowed."}
+        end)
       end
 
     Credo.Code.prewalk(
@@ -43,9 +46,11 @@ 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)
-
-    {ast, issues}
+    if found_module?(module, forbidden_modules) do
+      {ast, [issue_for(issue_meta, meta, module, forbidden_modules[module]) | issues]}
+    else
+      {ast, issues}
+    end
   end
 
   defp traverse(
@@ -54,51 +59,36 @@ 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_name = Name.full([base_alias, module])
+
+        if found_module?(full_name, forbidden_modules) do
+          message = forbidden_modules[full_name]
+          trigger = Name.full(module)
+          [issue_for(issue_meta, meta, trigger, message) | issues]
+        else
+          issues
+        end
       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
-    forbidden_module_names = Enum.map(forbidden_modules, &elem(&1, 0))
+  defp found_module?(module, forbidden_modules) when is_map_key(forbidden_modules, module),
+    do: true
 
-    if found_module?(forbidden_module_names, module) do
-      [issue_for(issue_meta, line_no, module, forbidden_modules) | issues]
-    else
-      issues
-    end
-  end
-
-  defp found_module?(forbidden_module_names, module) do
-    Enum.member?(forbidden_module_names, module)
-  end
-
-  defp issue_for(issue_meta, line_no, module, forbidden_modules) do
-    trigger = Name.full(module)
-    message = message(forbidden_modules, module) || "The `#{trigger}` module is not allowed."
+  defp found_module?(_, _), do: false
 
+  defp issue_for(issue_meta, meta, trigger, message) do
     format_issue(
       issue_meta,
       message: message,
       trigger: trigger,
-      line_no: line_no
+      line_no: meta[:line],
+      column: meta[:column]
     )
   end
-
-  defp message(forbidden_modules, module) do
-    Enum.find_value(forbidden_modules, fn
-      {^module, message} -> message
-      _ -> nil
-    end)
-  end
 end

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]

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