Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push singleton context when inside singleton method #2189

Merged
merged 1 commit into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,21 +303,33 @@ def on_def_node_enter(node)
@owner_stack.last,
))
when Prism::SelfNode
singleton = singleton_klass

@index.add(Entry::Method.new(
method_name,
@file_path,
node.location,
comments,
list_params(node.parameters),
current_visibility,
singleton_klass,
singleton,
))

if singleton
@owner_stack << singleton
@stack << "<Class:#{@stack.last}>"
end
end
end

sig { params(node: Prism::DefNode).void }
def on_def_node_leave(node)
@inside_def = false

if node.receiver.is_a?(Prism::SelfNode)
@owner_stack.pop
@stack.pop
end
end

sig { params(node: Prism::InstanceVariableWriteNode).void }
Expand Down
32 changes: 32 additions & 0 deletions lib/ruby_indexer/test/instance_variables_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,5 +172,37 @@ def test_top_level_instance_variables
entry = T.must(@index["@a"]&.first)
assert_nil(entry.owner)
end

def test_class_instance_variables_inside_self_method
index(<<~RUBY)
class Foo
def self.bar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check that the receiver is self on the def?

Consider this:

class Bar
  def Foo.bar; end
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do check that when entering the singleton context. I added a test to demonstrate the current behaviour (which is to associate the instance variable with the owner owner since we can't process dynamic def something.bar declarations.

@a = 123
end
end
RUBY

entry = T.must(@index["@a"]&.first)
owner = T.must(entry.owner)
assert_instance_of(Entry::SingletonClass, owner)
assert_equal("Foo::<Class:Foo>", owner.name)
end

def test_instance_variable_inside_dynamic_method_declaration
index(<<~RUBY)
class Foo
def something.bar
@a = 123
end
end
RUBY

# If the surrounding method is beind defined on any dynamic value that isn't `self`, then we attribute the
# instance variable to the wrong owner since there's no way to understand that statically
entry = T.must(@index["@a"]&.first)
owner = T.must(entry.owner)
assert_instance_of(Entry::Class, owner)
assert_equal("Foo", owner.name)
end
end
end
Loading