Skip to content

Commit ac5a646

Browse files
committed
Fix definition_id_to_declaration_id for SelfReceiver methods
definition_to_declaration_id maps a definition to its owning declaration. For Method definitions, it used the enclosing namespace + member lookup, which works for instance methods but returns the wrong declaration for SelfReceiver methods (def self.foo). When both def self.run and def run exist, member("run") on the enclosing class finds the instance method declaration instead of the singleton method declaration. This caused pending_detachments during invalidation to detach from the wrong place, leaving the SelfReceiver definition attached to the singleton declaration. On re-add, add_definition panicked with "duplicate definition". Fix: check for SelfReceiver and look up through the singleton class, mirroring how resolution places these methods.
1 parent e93d443 commit ac5a646

2 files changed

Lines changed: 99 additions & 9 deletions

File tree

rust/rubydex/src/model/graph.rs

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::sync::LazyLock;
66
use crate::diagnostic::Diagnostic;
77
use crate::indexing::local_graph::LocalGraph;
88
use crate::model::declaration::{Ancestor, ClassDeclaration, Declaration, Namespace};
9-
use crate::model::definitions::Definition;
9+
use crate::model::definitions::{Definition, Receiver};
1010
use crate::model::document::Document;
1111
use crate::model::encoding::Encoding;
1212
use crate::model::identity_maps::{IdentityHashMap, IdentityHashSet};
@@ -376,14 +376,24 @@ impl Graph {
376376
self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
377377
it.str_id(),
378378
),
379-
Definition::Method(it) => (
380-
self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
381-
it.str_id(),
382-
),
383-
Definition::MethodAlias(it) => (
384-
self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
385-
it.new_name_str_id(),
386-
),
379+
Definition::Method(it) => {
380+
if let Some(Receiver::SelfReceiver(def_id)) = it.receiver() {
381+
return self.find_self_receiver_declaration(*def_id, *it.str_id());
382+
}
383+
(
384+
self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
385+
it.str_id(),
386+
)
387+
}
388+
Definition::MethodAlias(it) => {
389+
if let Some(Receiver::SelfReceiver(def_id)) = it.receiver() {
390+
return self.find_self_receiver_declaration(*def_id, *it.new_name_str_id());
391+
}
392+
(
393+
self.find_enclosing_namespace_name_id(it.lexical_nesting_id().as_ref()),
394+
it.new_name_str_id(),
395+
)
396+
}
387397
};
388398

389399
let nesting_declaration_id = match nesting_name_id {
@@ -414,6 +424,20 @@ impl Graph {
414424
None
415425
}
416426

427+
/// Looks up the declaration for a `SelfReceiver` method/alias through the singleton class.
428+
fn find_self_receiver_declaration(&self, def_id: DefinitionId, member_str_id: StringId) -> Option<&DeclarationId> {
429+
let owner_decl_id = self.definition_id_to_declaration_id(def_id)?;
430+
let singleton_id = self
431+
.declarations
432+
.get(owner_decl_id)?
433+
.as_namespace()?
434+
.singleton_class()?;
435+
self.declarations
436+
.get(singleton_id)?
437+
.as_namespace()?
438+
.member(&member_str_id)
439+
}
440+
417441
#[must_use]
418442
pub fn name_id_to_declaration_id(&self, name_id: NameId) -> Option<&DeclarationId> {
419443
let name = self.names.get(&name_id);
@@ -3686,4 +3710,28 @@ mod incremental_resolution_tests {
36863710
assert_declaration_exists!(context, "Parent::Target");
36873711
assert_declaration_exists!(context, "Parent::Target::<Target>");
36883712
}
3713+
3714+
#[test]
3715+
fn no_duplicate_definition_on_identical_file_delete_readd() {
3716+
let source = "class Foo; def self.run; end; def run; end; end";
3717+
3718+
let mut context = GraphTest::new();
3719+
context.index_uri("file:///a.rb", source);
3720+
context.index_uri("file:///b.rb", source);
3721+
context.resolve();
3722+
3723+
assert_declaration_exists!(context, "Foo");
3724+
assert_declaration_exists!(context, "Foo::<Foo>#run()");
3725+
assert_declaration_exists!(context, "Foo#run()");
3726+
3727+
context.delete_uri("file:///a.rb");
3728+
context.resolve();
3729+
3730+
context.index_uri("file:///a.rb", source);
3731+
context.resolve();
3732+
3733+
assert_declaration_exists!(context, "Foo");
3734+
assert_declaration_exists!(context, "Foo::<Foo>#run()");
3735+
assert_declaration_exists!(context, "Foo#run()");
3736+
}
36893737
} // mod incremental_resolution_tests

rust/rubydex/src/resolution.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2294,6 +2294,48 @@ mod tests {
22942294
assert_owner_eq!(context, "Bar::<Bar>", "Bar");
22952295
}
22962296

2297+
#[test]
2298+
fn resolution_for_self_method_with_same_name_instance_method() {
2299+
let mut context = GraphTest::new();
2300+
context.index_uri(
2301+
"file:///foo.rb",
2302+
r"
2303+
class Foo
2304+
def self.run; end
2305+
def run; end
2306+
end
2307+
",
2308+
);
2309+
context.resolve();
2310+
2311+
assert_no_diagnostics!(&context);
2312+
2313+
assert_members_eq!(context, "Foo", ["run()"]);
2314+
assert_members_eq!(context, "Foo::<Foo>", ["run()"]);
2315+
}
2316+
2317+
#[test]
2318+
fn resolution_for_self_method_alias_with_same_name_instance_method() {
2319+
let mut context = GraphTest::new();
2320+
context.index_rbs_uri(
2321+
"file:///foo.rbs",
2322+
r"
2323+
class Foo
2324+
def self.run: () -> void
2325+
def run: () -> void
2326+
alias self.execute self.run
2327+
alias execute run
2328+
end
2329+
",
2330+
);
2331+
context.resolve();
2332+
2333+
assert_no_diagnostics!(&context);
2334+
2335+
assert_members_eq!(context, "Foo::<Foo>", ["execute()", "run()"]);
2336+
assert_members_eq!(context, "Foo", ["execute()", "run()"]);
2337+
}
2338+
22972339
#[test]
22982340
fn linearizing_super_classes() {
22992341
let mut context = GraphTest::new();

0 commit comments

Comments
 (0)