rename knows about def/defp/test

Author: jeremylightsmith

CHANGELOG.md

@@ -1,6 +1,7 @@
 0.1.4
 * ability to run rename
 * better tests
+* rename knows about def/defp/test
 
 0.1.3
 * case insensitive sorting

README.md

@@ -63,14 +63,14 @@ alias Foo.Bar.{Baz, Boom}
 > mix refactor rename [FILE] [LINE] [COLUMN]
 ```
 
-Renames a variable (currently this is a very, very naive implementation)
+Renames a variable (currently this is a fairly naive implementation - issues welcome!)
 
 
 More to come...
 
 ----
 
-## Running against a codebase
+## Running against an entire codebase
 
 Find is your friend
 

lib/exacto_knife/refactorings/rename.ex

@@ -12,23 +12,35 @@ defmodule ExactoKnife.Refactorings.Rename do
         IO.puts("Couldn't find a variable at #{inspect(cursor)}.")
         zipper
 
-      {{current_name, _, _}, _} ->
-        rename_instances(zipper, current_name, String.to_atom(new_name))
+      {{current_name, _, _}, _} = pos_zipper ->
+        scope = Ze.find_up(pos_zipper, &lexical_scope?/1) || zipper
+        rename_instances(scope, current_name, String.to_atom(new_name))
     end
   end
 
   def rename_instances(zipper, current_name, new_name) do
     zipper
     |> Z.traverse(fn
-      {{marker, _, _} = node, _} = z when is_atom(marker) ->
-        if marker == current_name do
-          Z.replace(z, put_elem(node, 0, new_name))
-        else
-          z
+      {{marker, _, _} = node, _} = z ->
+        cond do
+          marker == current_name ->
+            Z.replace(z, put_elem(node, 0, new_name))
+
+          # skip renaming inside of other scopes
+          Ze.safe_node(zipper) != Ze.safe_node(z) && lexical_scope?(node) ->
+            Z.skip(z)
+
+          true ->
+            z
         end
 
       z ->
         z
     end)
   end
+
+  def lexical_scope?({:def, _, _}), do: true
+  def lexical_scope?({:defp, _, _}), do: true
+  def lexical_scope?({:test, _, _}), do: true
+  def lexical_scope?(_), do: false
 end

lib/exacto_knife/refactorings/zipper_extensions.ex

@@ -8,13 +8,13 @@ defmodule ExactoKnife.Refactorings.ZipperExtensions do
   defdelegate zip(quoted), to: Z
   defdelegate next(z), to: Z
 
-  def find_back(nil, _f), do: nil
+  def find_up(nil, _f), do: nil
 
-  def find_back({node, _} = zipper, f) do
+  def find_up({node, _} = zipper, f) do
     if f.(node) do
       zipper
     else
-      find_back(Z.prev(zipper), f)
+      find_up(Z.up(zipper), f)
     end
   end
 
@@ -38,4 +38,11 @@ defmodule ExactoKnife.Refactorings.ZipperExtensions do
   # returns node, or nil if there is no node/zipper
   def safe_node({n, _}), do: n
   def safe_node(nil), do: nil
+
+  def compact(zipper) do
+    Z.traverse(zipper, fn
+      {{marker, _, children}, _} = z -> Z.replace(z, {marker, [], children})
+      z -> z
+    end)
+  end
 end

test/exacto_knife/refactorings/rename_test.exs

@@ -2,6 +2,8 @@ defmodule ExactoKnife.Refactorings.RenameTest do
   @moduledoc false
   use ExactoKnife.TestCase, async: true
 
+  alias ExactoKnife.Refactorings.Rename
+
   import ExUnit.CaptureIO
 
   describe "rename/1" do
@@ -56,7 +58,6 @@ defmodule ExactoKnife.Refactorings.RenameTest do
              """ == refactor(:rename, source, [{8, 17}, "food"])
     end
 
-    @tag :skip
     test "should only change instances in a function" do
       source = ~S"""
       bar = 3
@@ -72,20 +73,6 @@ defmodule ExactoKnife.Refactorings.RenameTest do
       end
       """
 
-      assert ~S"""
-             bomb = 3
-
-             def foo() do
-               bar = 2
-               baz = bar * 2
-             end
-
-             defp food() do
-               bar = 2
-               baz = bar * 2
-             end
-             """ == refactor(:rename, source, [{1, 2}, "bomb"])
-
       assert ~S"""
              bar = 3
 
@@ -114,5 +101,38 @@ defmodule ExactoKnife.Refactorings.RenameTest do
              end
              """ == refactor(:rename, source, [{9, 3}, "bomb"])
     end
+
+    test "should not change instances in a named function when variable changes outside" do
+      source = ~S"""
+      bar = 3
+
+      def foo() do
+        bar = 2
+        baz = bar * 2
+      end
+      """
+
+      assert ~S"""
+             bomb = 3
+
+             def foo() do
+               bar = 2
+               baz = bar * 2
+             end
+             """ == refactor(:rename, source, [{1, 2}, "bomb"])
+    end
+  end
+
+  describe "#lexical_scope?/1" do
+    test "finds def, defp, test" do
+      assert Rename.lexical_scope?(build("def foo() do\nend"))
+      assert Rename.lexical_scope?(build("defp foo() do\nend"))
+      assert Rename.lexical_scope?(build("test \"bob\" do\nend"))
+    end
+
+    test "not other stuff" do
+      refute Rename.lexical_scope?(5)
+      refute Rename.lexical_scope?(build("foo"))
+    end
   end
 end

test/exacto_knife/refactorings/zipper_extensions_test.exs

@@ -5,7 +5,7 @@ defmodule ExactoKnife.Refactorings.ZipperExtensionsTest do
   alias ExactoKnife.Refactorings.ZipperExtensions, as: Ze
   alias Sourceror.Zipper, as: Z
 
-  describe "#find_back/2" do
+  describe "#find" do
     test "finds a thing" do
       z = Z.zip([1, [2, [3, 4]], 5, 6])
 
@@ -14,8 +14,28 @@ defmodule ExactoKnife.Refactorings.ZipperExtensionsTest do
       assert nil == z |> Z.find(&(&1 == 10))
       assert {3, %{l: nil, r: [4]}} = z |> Z.find(&(&1 == 3))
       assert {5, %{r: [6]}} = at_5
-      assert {2, %{r: [[3, 4]]}} = at_5 |> Ze.find_back(&(&1 == 2))
-      assert nil == at_5 |> Ze.find_back(&(&1 == 20))
+      assert {2, %{r: [[3, 4]]}} = at_5 |> Z.find(:prev, &(&1 == 2))
+      assert nil == at_5 |> Z.find(:prev, &(&1 == 20))
+    end
+  end
+
+  describe "#find_up/2" do
+    test "finds a thing" do
+      z = Z.zip([1, [2, [3, 4]], 5, 6])
+
+      at_3 = z |> Z.find(&(&1 == 3))
+
+      first_is? = fn i ->
+        fn
+          list when is_list(list) -> Enum.at(list, 0) == i
+          _ -> false
+        end
+      end
+
+      assert nil == at_3 |> Ze.find_up(first_is?.(4))
+      assert nil == at_3 |> Ze.find_up(first_is?.(5))
+      assert at_3 |> Ze.find_up(first_is?.(1))
+      assert at_3 |> Ze.find_up(first_is?.(2))
     end
   end