From 07ac277f819ec1400994c7240c97d6039acda901 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 18 Oct 2022 13:40:21 -0400 Subject: [PATCH 1/3] Remove aref_field specific visit from WithEnvironment ARefField nodes end up with regular VarRef nodes inside, so we were actually counting them twice --- lib/syntax_tree/visitor/with_environment.rb | 7 -- test/visitor_with_environment_test.rb | 80 +++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/lib/syntax_tree/visitor/with_environment.rb b/lib/syntax_tree/visitor/with_environment.rb index ad101e0a..6fd30188 100644 --- a/lib/syntax_tree/visitor/with_environment.rb +++ b/lib/syntax_tree/visitor/with_environment.rb @@ -117,13 +117,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 diff --git a/test/visitor_with_environment_test.rb b/test/visitor_with_environment_test.rb index 302dbfbe..ea547811 100644 --- a/test/visitor_with_environment_test.rb +++ b/test/visitor_with_environment_test.rb @@ -426,5 +426,85 @@ 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 end end From 329ce7dc4e5f1af09bd83c0d6345998d0f7b4bb6 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 18 Oct 2022 13:58:07 -0400 Subject: [PATCH 2/3] Only create fresh environment for a MethodAddBlock block The call part of a MethodAddBlock node occurs in the same environment. Only the block portion of it occurs in a fresh environment. --- lib/syntax_tree/visitor/with_environment.rb | 6 ++++- test/visitor_with_environment_test.rb | 27 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/syntax_tree/visitor/with_environment.rb b/lib/syntax_tree/visitor/with_environment.rb index 6fd30188..cc27a671 100644 --- a/lib/syntax_tree/visitor/with_environment.rb +++ b/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) diff --git a/test/visitor_with_environment_test.rb b/test/visitor_with_environment_test.rb index ea547811..07a73848 100644 --- a/test/visitor_with_environment_test.rb +++ b/test/visitor_with_environment_test.rb @@ -506,5 +506,32 @@ def test_aref_on_a_method_call_with_arguments 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 end end From bd42046385d79c31fdbe32562bd483eaaef18915 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 18 Oct 2022 14:08:37 -0400 Subject: [PATCH 3/3] Handle nested required arguments Handle arguments of type [].each do |one, (two, three)| end when declaring arguments --- lib/syntax_tree/visitor/with_environment.rb | 16 +++- test/visitor_with_environment_test.rb | 82 +++++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/lib/syntax_tree/visitor/with_environment.rb b/lib/syntax_tree/visitor/with_environment.rb index cc27a671..043cbd4c 100644 --- a/lib/syntax_tree/visitor/with_environment.rb +++ b/lib/syntax_tree/visitor/with_environment.rb @@ -67,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) @@ -134,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 diff --git a/test/visitor_with_environment_test.rb b/test/visitor_with_environment_test.rb index 07a73848..b37bad16 100644 --- a/test/visitor_with_environment_test.rb +++ b/test/visitor_with_environment_test.rb @@ -533,5 +533,87 @@ def test_double_aref_on_method_call 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