Merge branch 'add-correct-column-reporting-to-warning-checks' of gith…

…ub.com:NJichev/credo into NJichev-add-correct-column-reporting-to-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

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

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

lib/credo/check/warning/leaky_environment.ex

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