Skip to content

Commit e0f5147

Browse files
committed
Create Todo declarations for multi-level compact namespaces
Previously, `class A::B::C` where A is unknown would only create a Todo for A (the first unresolvable parent), leaving B and C stuck in the retry loop indefinitely. This caused a panic when the class contained `def self.foo` with instance variables, because the SelfReceiver lookup expected C's definition to have a declaration. The root cause was in `name_owner_id`: `resolve_constant_internal(B)` returned `Retry(None)` (because A's name was unresolved), which was passed through unchanged. The `Unresolved(None)` branch that creates Todos was never reached for intermediate names in the constant path. Fix: merge `Retry(None)` with `Unresolved(None)` in `name_owner_id`. The recursive `name_owner_id(parent_scope)` call naturally walks the entire chain, creating Todos at each level. If real definitions appear later, the existing promotion mechanism upgrades Todos in place.
1 parent 0f06d12 commit e0f5147

File tree

1 file changed

+169
-6
lines changed

1 file changed

+169
-6
lines changed

rust/rubydex/src/resolution.rs

Lines changed: 169 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,15 +1060,16 @@ impl<'a> Resolver<'a> {
10601060
if let Some(&parent_scope) = name_ref.parent_scope().as_ref() {
10611061
// If we have `A::B`, the owner of `B` is whatever `A` resolves to.
10621062
// If `A` is an alias, resolve through to get the actual namespace.
1063-
// On `Retry`, we don't create a Todo: the parent may still resolve through inheritance once ancestors are
1064-
// linearized. We only create Todos for `Unresolved` outcomes where the parent is genuinely unknown.
10651063
match self.resolve_constant_internal(parent_scope) {
10661064
Outcome::Resolved(id, linearization) => self.resolve_to_primary_namespace(id, linearization),
1067-
// Retry or Unresolved(Some(_)) means we might find it later through ancestor linearization
1068-
Outcome::Retry(id) => Outcome::Retry(id),
1065+
// Retry(Some) or Unresolved(Some) means we might find it later through ancestor linearization
1066+
Outcome::Retry(Some(id)) => Outcome::Retry(Some(id)),
10691067
Outcome::Unresolved(Some(id)) => Outcome::Unresolved(Some(id)),
1070-
// Only create a Todo when genuinely unresolvable (no pending linearizations)
1071-
Outcome::Unresolved(None) => {
1068+
// Retry(None) means the parent's own parent scope is unresolved (e.g., B in `A::B::C`
1069+
// where A is unknown). Rather than retrying indefinitely, treat it the same as
1070+
// Unresolved(None) and eagerly create Todos for the entire parent chain. If a real
1071+
// definition appears later, the Todo gets promoted via the existing promotion mechanism.
1072+
Outcome::Retry(None) | Outcome::Unresolved(None) => {
10721073
let parent_name = self.graph.names().get(&parent_scope).unwrap();
10731074
let parent_str_id = *parent_name.str();
10741075
let parent_has_explicit_prefix = parent_name.parent_scope().as_ref().is_some();
@@ -5463,4 +5464,166 @@ mod tests {
54635464
assert_members_eq!(context, "Bar::Baz", vec!["qux()"]);
54645465
assert_declaration_does_not_exist!(context, "Foo::Bar");
54655466
}
5467+
5468+
#[test]
5469+
fn todo_chain_two_levels_unknown() {
5470+
// class A::B::C — neither A nor B exist. Both should become Todos, C is a Class.
5471+
let mut context = GraphTest::new();
5472+
context.index_uri("file:///a.rb", {
5473+
r"
5474+
class A::B::C
5475+
def foo; end
5476+
end
5477+
"
5478+
});
5479+
context.resolve();
5480+
5481+
assert_declaration_kind_eq!(context, "A", "<TODO>");
5482+
assert_declaration_kind_eq!(context, "A::B", "<TODO>");
5483+
assert_declaration_kind_eq!(context, "A::B::C", "Class");
5484+
assert_members_eq!(context, "Object", vec!["A"]);
5485+
assert_members_eq!(context, "A", vec!["B"]);
5486+
assert_members_eq!(context, "A::B", vec!["C"]);
5487+
assert_members_eq!(context, "A::B::C", vec!["foo()"]);
5488+
}
5489+
5490+
#[test]
5491+
fn todo_chain_three_levels_unknown() {
5492+
// class A::B::C::D — A, B, C are all unknown. Tests recursion beyond depth 2.
5493+
let mut context = GraphTest::new();
5494+
context.index_uri("file:///a.rb", {
5495+
r"
5496+
class A::B::C::D
5497+
def foo; end
5498+
end
5499+
"
5500+
});
5501+
context.resolve();
5502+
5503+
assert_declaration_kind_eq!(context, "A", "<TODO>");
5504+
assert_declaration_kind_eq!(context, "A::B", "<TODO>");
5505+
assert_declaration_kind_eq!(context, "A::B::C", "<TODO>");
5506+
assert_declaration_kind_eq!(context, "A::B::C::D", "Class");
5507+
assert_members_eq!(context, "Object", vec!["A"]);
5508+
assert_members_eq!(context, "A", vec!["B"]);
5509+
assert_members_eq!(context, "A::B", vec!["C"]);
5510+
assert_members_eq!(context, "A::B::C", vec!["D"]);
5511+
assert_members_eq!(context, "A::B::C::D", vec!["foo()"]);
5512+
}
5513+
5514+
#[test]
5515+
fn todo_chain_partially_unresolvable() {
5516+
// A exists but B doesn't — A resolves to a real Module, B becomes a Todo under A.
5517+
let mut context = GraphTest::new();
5518+
context.index_uri("file:///a.rb", {
5519+
r"
5520+
module A; end
5521+
class A::B::C
5522+
def foo; end
5523+
end
5524+
"
5525+
});
5526+
context.resolve();
5527+
5528+
assert_declaration_kind_eq!(context, "A", "Module");
5529+
assert_declaration_kind_eq!(context, "A::B", "<TODO>");
5530+
assert_declaration_kind_eq!(context, "A::B::C", "Class");
5531+
assert_members_eq!(context, "A", vec!["B"]);
5532+
assert_members_eq!(context, "A::B", vec!["C"]);
5533+
assert_members_eq!(context, "A::B::C", vec!["foo()"]);
5534+
}
5535+
5536+
#[test]
5537+
fn todo_chain_shared_by_sibling_classes() {
5538+
// Two classes share the same unknown parent chain. The Todos for A and B should
5539+
// be created once and reused, with both C and D as members of B.
5540+
let mut context = GraphTest::new();
5541+
context.index_uri("file:///a.rb", {
5542+
r"
5543+
class A::B::C
5544+
def c_method; end
5545+
end
5546+
5547+
class A::B::D
5548+
def d_method; end
5549+
end
5550+
"
5551+
});
5552+
context.resolve();
5553+
5554+
assert_declaration_kind_eq!(context, "A", "<TODO>");
5555+
assert_declaration_kind_eq!(context, "A::B", "<TODO>");
5556+
assert_declaration_kind_eq!(context, "A::B::C", "Class");
5557+
assert_declaration_kind_eq!(context, "A::B::D", "Class");
5558+
assert_members_eq!(context, "Object", vec!["A"]);
5559+
assert_members_eq!(context, "A", vec!["B"]);
5560+
assert_members_eq!(context, "A::B", vec!["C", "D"]);
5561+
assert_members_eq!(context, "A::B::C", vec!["c_method()"]);
5562+
assert_members_eq!(context, "A::B::D", vec!["d_method()"]);
5563+
}
5564+
5565+
#[test]
5566+
fn todo_chain_promoted_incrementally() {
5567+
// Index class A::B::C first (creates Todos), then provide real definitions.
5568+
// All Todos should be promoted to real namespaces.
5569+
//
5570+
// Note: we don't have true incremental resolution yet — each resolve() call
5571+
// clears all declarations and re-resolves from scratch. This test verifies that
5572+
// the promotion works when both files are present during the second resolution pass,
5573+
// not that Todos are surgically updated in place.
5574+
let mut context = GraphTest::new();
5575+
context.index_uri("file:///c.rb", {
5576+
r"
5577+
class A::B::C
5578+
def foo; end
5579+
end
5580+
"
5581+
});
5582+
context.resolve();
5583+
5584+
assert_declaration_kind_eq!(context, "A", "<TODO>");
5585+
assert_declaration_kind_eq!(context, "A::B", "<TODO>");
5586+
assert_declaration_kind_eq!(context, "A::B::C", "Class");
5587+
5588+
context.index_uri("file:///a.rb", {
5589+
r"
5590+
module A
5591+
module B
5592+
end
5593+
end
5594+
"
5595+
});
5596+
context.resolve();
5597+
5598+
// Todos should be promoted
5599+
assert_declaration_kind_eq!(context, "A", "Module");
5600+
assert_declaration_kind_eq!(context, "A::B", "Module");
5601+
assert_declaration_kind_eq!(context, "A::B::C", "Class");
5602+
assert_members_eq!(context, "A", vec!["B"]);
5603+
assert_members_eq!(context, "A::B", vec!["C"]);
5604+
assert_members_eq!(context, "A::B::C", vec!["foo()"]);
5605+
}
5606+
5607+
#[test]
5608+
fn todo_chain_with_self_method_and_ivar() {
5609+
// Regression test: def self.foo with @x inside a multi-level compact class
5610+
// used to panic because the SelfReceiver's definition had no declaration.
5611+
let mut context = GraphTest::new();
5612+
context.index_uri("file:///a.rb", {
5613+
r"
5614+
class A::B::C
5615+
def self.foo
5616+
@x = 1
5617+
end
5618+
end
5619+
"
5620+
});
5621+
context.resolve();
5622+
5623+
assert_declaration_kind_eq!(context, "A", "<TODO>");
5624+
assert_declaration_kind_eq!(context, "A::B", "<TODO>");
5625+
assert_declaration_kind_eq!(context, "A::B::C", "Class");
5626+
assert_declaration_exists!(context, "A::B::C::<C>#foo()");
5627+
assert_declaration_exists!(context, "A::B::C::<C>#@x");
5628+
}
54665629
}

0 commit comments

Comments
 (0)