Merge pull request #897 from hrzndhrn/fix-apply

Fix Credo.Check.Refactor.Apply

lib/credo/check/refactor/apply.ex

@@ -35,23 +35,33 @@ defmodule Credo.Check.Refactor.Apply do
     end
   end
 
-  defp issue({:apply, meta, [_fun, args]}, issue_meta) when is_list(args),
-    do: issue_for(issue_meta, meta[:line])
-
-  defp issue({:apply, _meta, [_module, {atom, _meta2, nil}, args]}, _issue_meta)
-       when is_atom(atom) and is_list(args),
-       do: nil
+  defp issue({:apply, meta, [fun, args]}, issue_meta) do
+    do_issue(:apply2, fun, args, meta, issue_meta)
+  end
 
-  defp issue({:apply, meta, [_module, _fun, args]}, issue_meta) when is_list(args),
-    do: issue_for(issue_meta, meta[:line])
+  defp issue({:apply, meta, [_module, fun, args]}, issue_meta) do
+    do_issue(:apply3, fun, args, meta, issue_meta)
+  end
 
   defp issue(_ast, _issue_meta), do: nil
 
-  defp issue_for(issue_meta, line_no) do
+  defp do_issue(:apply2, {name, _meta, nil}, args, meta, issue_meta)
+       when is_atom(name) and is_list(args) do
+    issue_for(meta, issue_meta)
+  end
+
+  defp do_issue(:apply3, fun, args, meta, issue_meta)
+       when is_atom(fun) and is_list(args) do
+    issue_for(meta, issue_meta)
+  end
+
+  defp do_issue(_apply, _fun, _args, _meta, _issue_meta), do: nil
+
+  defp issue_for(meta, issue_meta) do
     format_issue(
       issue_meta,
       message: "Avoid `apply/2` and `apply/3` when the number of arguments is known",
-      line_no: line_no
+      line_no: meta[:line]
     )
   end
 end

test/credo/check/refactor/apply_test.exs

@@ -29,7 +29,7 @@ defmodule Credo.Check.Refactor.ApplyTest do
     |> refute_issues()
   end
 
-  test "it should NOT report violation for apply/4" do
+  test "it should NOT report violation for apply/3 when fun is a var" do
     """
     defmodule Test do
       def some_function(module, fun, arg1, arg2) do
@@ -42,7 +42,33 @@ defmodule Credo.Check.Refactor.ApplyTest do
     |> refute_issues()
   end
 
-  test "it should report a violation for apply/2" do
+  test "it should NOT report violation for apply/3 when fun is a function" do
+    ~S"""
+    defmodule Test do
+      def some_function(module, fun, arg1, arg2) do
+        apply(module, String.to_exisiting_atom("pre_#{fun}"), [arg1, arg2])
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  test "it should NOT report violation for apply/3 when args is a var" do
+    ~S"""
+    defmodule Test do
+      def some_function(args) when is_list(args) do
+        apply(Module, :fun, args)
+      end
+    end
+    """
+    |> to_source_file
+    |> run_check(@described_check)
+    |> refute_issues()
+  end
+
+  test "it should report violation for apply/2" do
     """
     defmodule Test do
       def some_function(fun, arg1, arg2) do
@@ -58,7 +84,7 @@ defmodule Credo.Check.Refactor.ApplyTest do
   test "it should report a violation for apply/3" do
     """
     defmodule Test do
-      def some_function(module, :fun_name, arg1, arg2) do
+      def some_function(module, arg1, arg2) do
         apply(module, :fun_name, [arg1, arg2])
       end
     end