Merge pull request #177 from vinistock/vs/fix_other_environment_cases

Fix other environment cases

Author: kddnewton

lib/syntax_tree/visitor/with_environment.rb

@@ -44,8 +44,12 @@ def visit_module(node)
       with_new_environment { super }
     end
 
+    # When we find a method invocation with a block, only the code that happens
+    # inside of the block needs a fresh environment. The method invocation
+    # itself happens in the same environment
     def visit_method_add_block(node)
-      with_new_environment { super }
+      visit(node.call)
+      with_new_environment { visit(node.block) }
     end
 
     def visit_def(node)
@@ -63,9 +67,7 @@ def visit_def_endless(node)
     # Visit for keeping track of local arguments, such as method and block
     # arguments
     def visit_params(node)
-      node.requireds.each do |param|
-        current_environment.add_local_definition(param, :argument)
-      end
+      add_argument_definitions(node.requireds)
 
       node.posts.each do |param|
         current_environment.add_local_definition(param, :argument)
@@ -117,13 +119,6 @@ def visit_var_field(node)
     alias visit_pinned_var_ref visit_var_field
 
     # Visits for keeping track of variable and argument usages
-    def visit_aref_field(node)
-      name = node.collection.value
-      current_environment.add_local_usage(name, :variable) if name
-
-      super
-    end
-
     def visit_var_ref(node)
       value = node.value
 
@@ -137,5 +132,17 @@ def visit_var_ref(node)
 
       super
     end
+
+    private
+
+    def add_argument_definitions(list)
+      list.each do |param|
+        if param.is_a?(SyntaxTree::MLHSParen)
+          add_argument_definitions(param.contents.parts)
+        else
+          current_environment.add_local_definition(param, :argument)
+        end
+      end
+    end
   end
 end

test/visitor_with_environment_test.rb

@@ -426,5 +426,194 @@ def test_variables_in_the_top_level
       assert_equal(1, variable.definitions[0].start_line)
       assert_equal(2, variable.usages[0].start_line)
     end
+
+    def test_aref_field
+      tree = SyntaxTree.parse(<<~RUBY)
+        object = {}
+        object["name"] = "something"
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(0, visitor.arguments.length)
+      assert_equal(1, visitor.variables.length)
+
+      variable = visitor.variables["object"]
+      assert_equal(1, variable.definitions.length)
+      assert_equal(1, variable.usages.length)
+
+      assert_equal(1, variable.definitions[0].start_line)
+      assert_equal(2, variable.usages[0].start_line)
+    end
+
+    def test_aref_on_a_method_call
+      tree = SyntaxTree.parse(<<~RUBY)
+        object = MyObject.new
+        object.attributes["name"] = "something"
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(0, visitor.arguments.length)
+      assert_equal(1, visitor.variables.length)
+
+      variable = visitor.variables["object"]
+      assert_equal(1, variable.definitions.length)
+      assert_equal(1, variable.usages.length)
+
+      assert_equal(1, variable.definitions[0].start_line)
+      assert_equal(2, variable.usages[0].start_line)
+    end
+
+    def test_aref_with_two_accesses
+      tree = SyntaxTree.parse(<<~RUBY)
+        object = MyObject.new
+        object["first"]["second"] ||= []
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(0, visitor.arguments.length)
+      assert_equal(1, visitor.variables.length)
+
+      variable = visitor.variables["object"]
+      assert_equal(1, variable.definitions.length)
+      assert_equal(1, variable.usages.length)
+
+      assert_equal(1, variable.definitions[0].start_line)
+      assert_equal(2, variable.usages[0].start_line)
+    end
+
+    def test_aref_on_a_method_call_with_arguments
+      tree = SyntaxTree.parse(<<~RUBY)
+        object = MyObject.new
+        object.instance_variable_get(:@attributes)[:something] = :other_thing
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(0, visitor.arguments.length)
+      assert_equal(1, visitor.variables.length)
+
+      variable = visitor.variables["object"]
+      assert_equal(1, variable.definitions.length)
+      assert_equal(1, variable.usages.length)
+
+      assert_equal(1, variable.definitions[0].start_line)
+      assert_equal(2, variable.usages[0].start_line)
+    end
+
+    def test_double_aref_on_method_call
+      tree = SyntaxTree.parse(<<~RUBY)
+        object = MyObject.new
+        object["attributes"].find { |a| a["field"] == "expected" }["value"] = "changed"
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(1, visitor.arguments.length)
+      assert_equal(1, visitor.variables.length)
+
+      variable = visitor.variables["object"]
+      assert_equal(1, variable.definitions.length)
+      assert_equal(1, variable.usages.length)
+
+      assert_equal(1, variable.definitions[0].start_line)
+      assert_equal(2, variable.usages[0].start_line)
+
+      argument = visitor.arguments["a"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(2, argument.definitions[0].start_line)
+      assert_equal(2, argument.usages[0].start_line)
+    end
+
+    def test_nested_arguments
+      tree = SyntaxTree.parse(<<~RUBY)
+        [[1, [2, 3]]].each do |one, (two, three)|
+          one
+          two
+          three
+        end
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(3, visitor.arguments.length)
+      assert_equal(0, visitor.variables.length)
+
+      argument = visitor.arguments["one"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(2, argument.usages[0].start_line)
+
+      argument = visitor.arguments["two"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(3, argument.usages[0].start_line)
+
+      argument = visitor.arguments["three"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(4, argument.usages[0].start_line)
+    end
+
+    def test_double_nested_arguments
+      tree = SyntaxTree.parse(<<~RUBY)
+        [[1, [2, 3]]].each do |one, (two, (three, four))|
+          one
+          two
+          three
+          four
+        end
+      RUBY
+
+      visitor = Collector.new
+      visitor.visit(tree)
+
+      assert_equal(4, visitor.arguments.length)
+      assert_equal(0, visitor.variables.length)
+
+      argument = visitor.arguments["one"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(2, argument.usages[0].start_line)
+
+      argument = visitor.arguments["two"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(3, argument.usages[0].start_line)
+
+      argument = visitor.arguments["three"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(4, argument.usages[0].start_line)
+
+      argument = visitor.arguments["four"]
+      assert_equal(1, argument.definitions.length)
+      assert_equal(1, argument.usages.length)
+
+      assert_equal(1, argument.definitions[0].start_line)
+      assert_equal(5, argument.usages[0].start_line)
+    end
   end
 end