Skip to content

Commit dc37cbf

Browse files
committed
Collect singleton classes after reference deletion
1 parent 76224ea commit dc37cbf

1 file changed

Lines changed: 50 additions & 4 deletions

File tree

rust/rubydex/src/model/graph.rs

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,21 +1099,34 @@ impl Graph {
10991099
}
11001100
}
11011101

1102+
// Detach references and queue any declaration that becomes removable as a
1103+
// result. `invalidate()` only seeds from removed definitions, so a singleton
1104+
// kept alive purely by a reference (e.g. `Foo::<Foo>` materialized by
1105+
// `Foo.new`) would otherwise linger after the reference's file is deleted.
1106+
let mut emptied_by_ref_removal: Vec<InvalidationItem> = Vec::new();
11021107
for ref_id in document.constant_references() {
11031108
if let Some(constant_ref) = self.constant_references.remove(ref_id) {
11041109
// Detach from target declaration. References unresolved during invalidation
11051110
// were already detached; this catches the rest.
1106-
if let Some(NameRef::Resolved(resolved)) = self.names.get(constant_ref.name_id())
1107-
&& let Some(declaration) = self.declarations.get_mut(resolved.declaration_id())
1108-
{
1109-
declaration.remove_constant_reference(ref_id);
1111+
if let Some(NameRef::Resolved(resolved)) = self.names.get(constant_ref.name_id()) {
1112+
let target_id = *resolved.declaration_id();
1113+
if let Some(declaration) = self.declarations.get_mut(&target_id) {
1114+
declaration.remove_constant_reference(ref_id);
1115+
if declaration.is_removable() {
1116+
emptied_by_ref_removal.push(InvalidationItem::Declaration(target_id));
1117+
}
1118+
}
11101119
}
11111120

11121121
self.remove_name_dependent(*constant_ref.name_id(), NameDependent::Reference(*ref_id));
11131122
self.untrack_name(*constant_ref.name_id());
11141123
}
11151124
}
11161125

1126+
if !emptied_by_ref_removal.is_empty() {
1127+
self.invalidate_graph(emptied_by_ref_removal, IdentityHashMap::default());
1128+
}
1129+
11171130
// Detach removed definitions from their declarations.
11181131
// Most definitions were already detached by invalidate_declaration via
11191132
// pending_detachments. Definitions not handled by pending_detachments are
@@ -4520,4 +4533,37 @@ mod incremental_resolution_tests {
45204533

45214534
assert_declaration_ids_match(&incremental, &fresh);
45224535
}
4536+
4537+
/// A singleton class materialized solely by a constant reference (e.g.
4538+
/// `Foo.new` creating `Foo::<Foo>` even though no `class << Foo` block
4539+
/// exists) must be collected when its sole supporting reference is
4540+
/// removed. `invalidate()` only seeds from removed definitions, so the
4541+
/// reference detachment in `remove_document_data` queues now-removable
4542+
/// declarations for cleanup.
4543+
#[test]
4544+
fn singleton_kept_only_by_reference_collected_on_ref_delete() {
4545+
let mut context = GraphTest::new();
4546+
context.index_uri("file:///foo.rb", "class Foo; end");
4547+
context.index_uri("file:///user.rb", "Foo.new");
4548+
context.resolve();
4549+
assert_declaration_exists!(context, "Foo::<Foo>");
4550+
4551+
context.delete_uri("file:///user.rb");
4552+
context.resolve();
4553+
4554+
// The reference that materialized the singleton is gone, so the
4555+
// singleton itself must not linger and `Foo.singleton_class_id` must
4556+
// be cleared.
4557+
assert_declaration_does_not_exist!(context, "Foo::<Foo>");
4558+
let foo = context
4559+
.graph()
4560+
.declarations()
4561+
.get(&crate::model::ids::DeclarationId::from("Foo"))
4562+
.expect("Foo should still exist");
4563+
let foo_ns = foo.as_namespace().expect("Foo is a namespace");
4564+
assert!(
4565+
foo_ns.singleton_class().is_none(),
4566+
"Foo.singleton_class_id should be cleared after the singleton is removed"
4567+
);
4568+
}
45234569
} // mod incremental_resolution_tests

0 commit comments

Comments
 (0)