From 782fe1042b6ddd4589dbd2d21f6db86aeb38c76f Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Tue, 7 May 2024 22:16:47 +0100 Subject: [PATCH] Track method visibility in the index This commit supports visibility tracking for methods in the index by: - Adding a visibility field to the Member entry, which is a superclass of Accessor, Method, and Alias. - In the DeclarationListener, create a nested stack structure to track the visibility of the current scope. --- .../lib/ruby_indexer/declaration_listener.rb | 89 +++++++++++++++++-- lib/ruby_indexer/lib/ruby_indexer/entry.rb | 22 +++-- lib/ruby_indexer/test/method_test.rb | 37 ++++++++ lib/ruby_indexer/test/test_case.rb | 4 +- 4 files changed, 141 insertions(+), 11 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index 449712112a..dc72b84d38 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -11,6 +11,7 @@ class DeclarationListener def initialize(index, dispatcher, parse_result, file_path) @index = index @file_path = file_path + @visibility_stack = T.let([[:public]], T::Array[T::Array[Symbol]]) @comments_by_line = T.let( parse_result.comments.to_h do |c| [c.location.start_line, c] @@ -35,6 +36,7 @@ def initialize(index, dispatcher, parse_result, file_path) :on_def_node_enter, :on_def_node_leave, :on_call_node_enter, + :on_call_node_leave, :on_multi_write_node_enter, :on_constant_path_write_node_enter, :on_constant_path_or_write_node_enter, @@ -50,6 +52,7 @@ def initialize(index, dispatcher, parse_result, file_path) sig { params(node: Prism::ClassNode).void } def on_class_node_enter(node) + create_visibility_scope name = node.constant_path.location.slice comments = collect_comments(node) @@ -77,10 +80,12 @@ def on_class_node_enter(node) def on_class_node_leave(node) @stack.pop @owner_stack.pop + drop_visibility_scope end sig { params(node: Prism::ModuleNode).void } def on_module_node_enter(node) + create_visibility_scope name = node.constant_path.location.slice comments = collect_comments(node) @@ -95,6 +100,7 @@ def on_module_node_enter(node) def on_module_node_leave(node) @stack.pop @owner_stack.pop + drop_visibility_scope end sig { params(node: Prism::MultiWriteNode).void } @@ -198,6 +204,21 @@ def on_call_node_enter(node) handle_module_operation(node, :included_modules) when :prepend handle_module_operation(node, :prepended_modules) + when :public, :protected, :private + change_scope_visibility(message) + end + end + + sig { params(node: Prism::CallNode).void } + def on_call_node_leave(node) + message = node.name + case message + when :public, :protected, :private + # We want to restore the visibility stack when we leave a method definition with a visibility modifier + # e.g. `private def foo; end` + if node.arguments&.arguments&.first&.is_a?(Prism::DefNode) + restore_scope_visibility + end end end @@ -206,6 +227,7 @@ def on_def_node_enter(node) @inside_def = true method_name = node.name.to_s comments = collect_comments(node) + case node.receiver when nil @index << Entry::InstanceMethod.new( @@ -214,6 +236,7 @@ def on_def_node_enter(node) node.location, comments, node.parameters, + current_visibility, @owner_stack.last, ) when Prism::SelfNode @@ -223,6 +246,7 @@ def on_def_node_enter(node) node.location, comments, node.parameters, + current_visibility, @owner_stack.last, ) end @@ -284,17 +308,38 @@ def add_constant(node, name, value = nil) @index << case value when Prism::ConstantReadNode, Prism::ConstantPathNode - Entry::UnresolvedAlias.new(value.slice, @stack.dup, name, @file_path, node.location, comments) + Entry::UnresolvedAlias.new( + value.slice, + @stack.dup, + name, + @file_path, + node.location, + comments, + ) when Prism::ConstantWriteNode, Prism::ConstantAndWriteNode, Prism::ConstantOrWriteNode, Prism::ConstantOperatorWriteNode # If the right hand side is another constant assignment, we need to visit it because that constant has to be # indexed too - Entry::UnresolvedAlias.new(value.name.to_s, @stack.dup, name, @file_path, node.location, comments) + Entry::UnresolvedAlias.new( + value.name.to_s, + @stack.dup, + name, + @file_path, + node.location, + comments, + ) when Prism::ConstantPathWriteNode, Prism::ConstantPathOrWriteNode, Prism::ConstantPathOperatorWriteNode, Prism::ConstantPathAndWriteNode - Entry::UnresolvedAlias.new(value.target.slice, @stack.dup, name, @file_path, node.location, comments) + Entry::UnresolvedAlias.new( + value.target.slice, + @stack.dup, + name, + @file_path, + node.location, + comments, + ) else Entry::Constant.new(name, @file_path, node.location, comments) end @@ -354,8 +399,15 @@ def handle_attribute(node, reader:, writer:) next unless name && loc - @index << Entry::Accessor.new(name, @file_path, loc, comments, @owner_stack.last) if reader - @index << Entry::Accessor.new("#{name}=", @file_path, loc, comments, @owner_stack.last) if writer + @index << Entry::Accessor.new(name, @file_path, loc, comments, current_visibility, @owner_stack.last) if reader + @index << Entry::Accessor.new( + "#{name}=", + @file_path, + loc, + comments, + current_visibility, + @owner_stack.last, + ) if writer end end @@ -380,5 +432,32 @@ def handle_module_operation(node, operation) collection = operation == :included_modules ? owner.included_modules : owner.prepended_modules collection.concat(names) end + + sig { returns(Symbol) } + def current_visibility + T.must(@visibility_stack.last&.last) + end + + sig { params(visibility: Symbol).void } + def change_scope_visibility(visibility) + scope_visibility_stack = T.must(@visibility_stack.last) + scope_visibility_stack.push(visibility) + end + + sig { void } + def restore_scope_visibility + scope_visibility_stack = T.must(@visibility_stack.last) + scope_visibility_stack.pop + end + + sig { void } + def create_visibility_scope + @visibility_stack.push([:public]) + end + + sig { void } + def drop_visibility_scope + @visibility_stack.pop + end end end diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb index df1be9d081..5352fff193 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb @@ -182,17 +182,22 @@ class Member < Entry sig { returns(T.nilable(Entry::Namespace)) } attr_reader :owner + sig { returns(Symbol) } + attr_reader :visibility + sig do params( name: String, file_path: String, location: T.any(Prism::Location, RubyIndexer::Location), comments: T::Array[String], + visibility: Symbol, owner: T.nilable(Entry::Namespace), ).void end - def initialize(name, file_path, location, comments, owner) + def initialize(name, file_path, location, comments, visibility, owner) # rubocop:disable Metrics/ParameterLists super(name, file_path, location, comments) + @visibility = visibility @owner = owner end @@ -227,11 +232,12 @@ class Method < Member location: T.any(Prism::Location, RubyIndexer::Location), comments: T::Array[String], parameters_node: T.nilable(Prism::ParametersNode), + visibility: Symbol, owner: T.nilable(Entry::Namespace), ).void end - def initialize(name, file_path, location, comments, parameters_node, owner) # rubocop:disable Metrics/ParameterLists - super(name, file_path, location, comments, owner) + def initialize(name, file_path, location, comments, parameters_node, visibility, owner) # rubocop:disable Metrics/ParameterLists + super(name, file_path, location, comments, visibility, owner) @parameters = T.let(list_params(parameters_node), T::Array[Parameter]) end @@ -375,8 +381,14 @@ class Alias < Entry sig { params(target: String, unresolved_alias: UnresolvedAlias).void } def initialize(target, unresolved_alias) - super(unresolved_alias.name, unresolved_alias.file_path, unresolved_alias.location, unresolved_alias.comments) - + super( + unresolved_alias.name, + unresolved_alias.file_path, + unresolved_alias.location, + unresolved_alias.comments, + ) + + @visibility = unresolved_alias.visibility @target = target end end diff --git a/lib/ruby_indexer/test/method_test.rb b/lib/ruby_indexer/test/method_test.rb index ff1245fa6e..bf8c1e5f2e 100644 --- a/lib/ruby_indexer/test/method_test.rb +++ b/lib/ruby_indexer/test/method_test.rb @@ -71,6 +71,43 @@ def bar assert_equal("Bar", second_entry.owner.name) end + def test_visibility_tracking + index(<<~RUBY) + private def foo + end + + def bar; end + + protected + + def baz; end + RUBY + + assert_entry("foo", Entry::InstanceMethod, "/fake/path/foo.rb:0-8:1-3", visibility: :private) + assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:3-0:3-12", visibility: :public) + assert_entry("baz", Entry::InstanceMethod, "/fake/path/foo.rb:7-0:7-12", visibility: :protected) + end + + def test_visibility_tracking_with_nested_class_or_modules + index(<<~RUBY) + class Foo + private + + def foo; end + + class Bar + def bar; end + end + + def baz; end + end + RUBY + + assert_entry("foo", Entry::InstanceMethod, "/fake/path/foo.rb:3-2:3-14", visibility: :private) + assert_entry("bar", Entry::InstanceMethod, "/fake/path/foo.rb:6-4:6-16", visibility: :public) + assert_entry("baz", Entry::InstanceMethod, "/fake/path/foo.rb:9-2:9-14", visibility: :private) + end + def test_method_with_parameters index(<<~RUBY) class Foo diff --git a/lib/ruby_indexer/test/test_case.rb b/lib/ruby_indexer/test/test_case.rb index cf666cb4e8..61708495d1 100644 --- a/lib/ruby_indexer/test/test_case.rb +++ b/lib/ruby_indexer/test/test_case.rb @@ -15,7 +15,7 @@ def index(source) @index.index_single(IndexablePath.new(nil, "/fake/path/foo.rb"), source) end - def assert_entry(expected_name, type, expected_location) + def assert_entry(expected_name, type, expected_location, visibility: nil) entries = @index[expected_name] refute_empty(entries, "Expected #{expected_name} to be indexed") @@ -28,6 +28,8 @@ def assert_entry(expected_name, type, expected_location) ":#{location.end_line - 1}-#{location.end_column}" assert_equal(expected_location, location_string) + + assert_equal(visibility, entry.visibility) if visibility end def refute_entry(expected_name)