refactor: replaced most abstractions provided by Variable

Author: gp-pereira

lib/refactorex/refactor/ast.ex

@@ -50,7 +50,7 @@ defmodule Refactorex.Refactor.AST do
     zipper
     |> Z.top()
     |> Z.traverse([], fn %{node: node} = zipper, nodes ->
-      if finder.(node),
+      if finder.(zipper),
         do: {zipper, [node | nodes]},
         else: {zipper, nodes}
     end)

lib/refactorex/refactor/constant/extract_constant.ex

@@ -25,7 +25,7 @@ defmodule Refactorex.Refactor.Constant.ExtractConstant do
       not Module.inside_one?(zipper) ->
         false
 
-      Enum.any?(Variable.list_variables(node)) ->
+      Enum.any?(AST.find(selection, &Variable.at_one?/1)) ->
         :skip
 
       match?(%{node: {:|>, _, [_, ^node]}}, Z.up(zipper)) ->
@@ -68,8 +68,10 @@ defmodule Refactorex.Refactor.Constant.ExtractConstant do
       {{:@, _, [{:behaviour, _, _}]}, i} ->
         i + 1
 
-      {{:@, _, [constant]}, i} ->
-        if Variable.member?(constants_used, constant), do: i + 1, else: 0
+      {{:@, _, [{name, _, _}]}, i} ->
+        if Enum.any?(constants_used, &match?({^name, _, _}, &1)),
+          do: i + 1,
+          else: 0
 
       _ ->
         0
@@ -79,7 +81,7 @@ defmodule Refactorex.Refactor.Constant.ExtractConstant do
 
   defp find_constants_used(to_be_constant) do
     to_be_constant
-    |> AST.find(&match?({:@, _, [{_, _, nil}]}, &1))
+    |> AST.find(&match?({:@, _, [{_, _, nil}]}, &1.node))
     |> Enum.map(fn {:@, _, [constant]} -> constant end)
   end
 end

lib/refactorex/refactor/constant/inline_constant.ex

@@ -27,7 +27,7 @@ defmodule Refactorex.Refactor.Constant.InlineConstant do
 
   defp find_definition(%{node: {:@, _, [{id, _, _}]}} = zipper) do
     zipper
-    |> AST.find(&match?({:@, _, [{^id, _, u}]} when not is_nil(u), &1))
+    |> AST.find(&match?({:@, _, [{^id, _, u}]} when not is_nil(u), &1.node))
     |> List.first()
   end
 end

lib/refactorex/refactor/dataflow.ex

@@ -1,12 +1,6 @@
 defmodule Refactorex.Refactor.Dataflow do
   alias Sourceror.Zipper, as: Z
-
-  defguard is_variable(node)
-           when is_tuple(node) and
-                  tuple_size(node) == 3 and
-                  is_atom(elem(node, 0)) and
-                  elem(node, 0) != :binary and
-                  is_nil(elem(node, 2))
+  import Refactorex.Refactor.Variable, only: [is_variable: 1]
 
   defstruct commands: [],
             variables: []

lib/refactorex/refactor/function/collapse_anonymous_function.ex

@@ -6,6 +6,7 @@ defmodule Refactorex.Refactor.Function.CollapseAnonymousFunction do
 
   alias Refactorex.Refactor.{
     Block,
+    Dataflow,
     Variable
   }
 
@@ -19,12 +20,14 @@ defmodule Refactorex.Refactor.Function.CollapseAnonymousFunction do
       Block.has_multiple_statements?(body) ->
         false
 
-      args != Variable.list_unpinned_variables(args) ->
+      not Variable.plain_variables?(args) ->
+        false
+
+      some_arg_not_used?(args, Dataflow.analyze(node)) ->
         false
 
       true ->
-        used_variables = Variable.list_unique_variables(body)
-        Enum.all?(args, &Variable.member?(used_variables, &1))
+        true
     end
   end
 
@@ -38,4 +41,7 @@ defmodule Refactorex.Refactor.Function.CollapseAnonymousFunction do
       {:&, [], [Variable.replace_variables_by_values(body, args, new_args, node)]}
     )
   end
+
+  defp some_arg_not_used?(args, dataflow),
+    do: not Enum.all?(args, &match?([_ | _], dataflow[&1]))
 end

lib/refactorex/refactor/function/inline_function.ex

@@ -104,7 +104,7 @@ defmodule Refactorex.Refactor.Function.InlineFunction do
             statement -> [statement]
           end
 
-        if all_simple_variables?(args) do
+        if Variable.plain_variables?(args) do
           Variable.replace_variables_by_values(
             statements,
             args,
@@ -157,7 +157,4 @@ defmodule Refactorex.Refactor.Function.InlineFunction do
         else: zipper
     )
   end
-
-  defp all_simple_variables?(args),
-    do: Enum.all?(args, &match?({_, _, nil}, &1))
 end

lib/refactorex/refactor/guard/extract_guard.ex

@@ -7,9 +7,9 @@ defmodule Refactorex.Refactor.Guard.ExtractGuard do
   alias Refactorex.NameCache
 
   alias Refactorex.Refactor.{
+    Dataflow,
     Guard,
-    Module,
-    Variable
+    Module
   }
 
   @guard_name "extracted_guard"
@@ -32,7 +32,7 @@ defmodule Refactorex.Refactor.Guard.ExtractGuard do
 
   def refactor(%{node: node} = zipper, _) do
     name = next_available_guard_name(zipper)
-    args = Variable.list_unique_variables(node)
+    args = Dataflow.outer_variables(node)
 
     zipper
     |> Z.replace({name, [], args})

lib/refactorex/refactor/variable.ex

@@ -6,21 +6,46 @@ defmodule Refactorex.Refactor.Variable do
     Dataflow
   }
 
-  def at_one?(%{node: {name, _, nil}} = zipper) do
-    cond do
-      not is_atom(name) or name in ~w(binary _)a ->
-        false
+  defguard is_variable(node)
+           when is_tuple(node) and
+                  tuple_size(node) == 3 and
+                  is_atom(elem(node, 0)) and
+                  elem(node, 0) not in ~w(binary _)a and
+                  is_nil(elem(node, 2))
 
-      match?(%{node: {:@, _, _}}, Z.up(zipper)) ->
-        false
+  def at_one?(%{node: node} = zipper) when is_variable(node),
+    do: not match?(%{node: {:@, _, _}}, Z.up(zipper))
+
+  def at_one?(_zipper), do: false
+
+  def plain_variables?(nodes), do: Enum.all?(nodes, &is_variable(&1))
 
-      true ->
+  def inside_declaration?(%{node: node} = zipper) do
+    case parent = Z.up(zipper) do
+      %{node: {id, _, [^node, _]}} when id in ~w(def defp <- when =)a ->
         true
+
+      %{node: {:->, _, [^node, _]}} ->
+        cond do
+          match?(%{node: {:fn, _, _}}, Z.up(parent)) -> true
+          match?(%{node: {:case, _, _}}, AST.up(parent, 4)) -> true
+          true -> false
+        end
+
+      %{node: {_, _, [_ | _]}} ->
+        inside_declaration?(parent)
+
+      %{node: [_ | _]} ->
+        inside_declaration?(parent)
+
+      %{node: {_, _}} ->
+        inside_declaration?(parent)
+
+      _ ->
+        false
     end
   end
 
-  def at_one?(_zipper), do: false
-
   def find_all_references(zipper, {name, _, _} = variable) do
     zipper
     |> Z.topmost_root()
@@ -48,65 +73,6 @@ defmodule Refactorex.Refactor.Variable do
     |> Z.node()
   end
 
-  def list_unique_variables(node, filter_fn \\ fn _zipper -> true end) do
-    node
-    |> list_variables(filter_fn)
-    |> Enum.uniq_by(fn {name, _, _} -> name end)
-  end
-
-  def list_unpinned_variables(node),
-    do: list_variables(node, &(not match?(%{node: {:^, _, _}}, Z.up(&1))))
-
-  def list_variables(node, filter_fn \\ fn _zipper -> true end) do
-    node
-    |> Z.zip()
-    |> Z.traverse_while([], fn
-      %{node: node} = zipper, variables ->
-        cond do
-          not at_one?(zipper) ->
-            {:cont, zipper, variables}
-
-          not filter_fn.(zipper) ->
-            {:cont, zipper, variables}
-
-          true ->
-            {:cont, zipper, variables ++ [node]}
-        end
-    end)
-    |> elem(1)
-  end
-
-  def member?(variables, {name, _, _} = _variable),
-    do: Enum.any?(variables, &match?({^name, _, _}, &1))
-
-  def member?(_, _), do: false
-
-  def inside_declaration?(%{node: node} = zipper) do
-    case parent = Z.up(zipper) do
-      %{node: {id, _, [^node, _]}} when id in ~w(def defp <- when =)a ->
-        true
-
-      %{node: {:->, _, [^node, _]}} ->
-        cond do
-          match?(%{node: {:fn, _, _}}, Z.up(parent)) -> true
-          match?(%{node: {:case, _, _}}, AST.up(parent, 4)) -> true
-          true -> false
-        end
-
-      %{node: {_, _, [_ | _]}} ->
-        inside_declaration?(parent)
-
-      %{node: [_ | _]} ->
-        inside_declaration?(parent)
-
-      %{node: {_, _}} ->
-        inside_declaration?(parent)
-
-      _ ->
-        false
-    end
-  end
-
   # find &{i} usages and replace them with arg{i}
   def turn_captures_into_variables(node) do
     node