Skip to content

Commit f39ef05

Browse files
committed
Replace has_unresolved_dependency inference with explicit invalidation variants
Split InvalidationItem::Name into UnresolveName and ReevaluateReferences so callers explicitly communicate the invalidation mode instead of having invalidate_name infer it by inspecting nesting/parent_scope state. - Split invalidate_name into unresolve_dependent_name and reevaluate_references - Add scope_broken parameter to queue_dependent_names for per-dependent parent_scope check (dependents qualified by the changed name need unresolve) - Remove has_unresolved_dependency entirely
1 parent 3445fb9 commit f39ef05

2 files changed

Lines changed: 123 additions & 84 deletions

File tree

rust/rubydex/src/model/graph.rs

Lines changed: 122 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ pub enum NameDependent {
3030
enum InvalidationItem {
3131
/// A declaration whose ancestor chain is stale, or that has become empty and needs removal.
3232
Declaration(DeclarationId),
33-
/// A name whose dependencies may have changed, needing cascade or reference re-evaluation.
34-
Name(NameId),
33+
/// Structural dependency broken — unresolve the name and cascade to all dependents.
34+
UnresolveName(NameId),
35+
/// Ancestor context changed — unresolve references under this name but keep the name resolved.
36+
UnresolveReferences(NameId),
3537
}
3638

3739
pub static OBJECT_ID: LazyLock<DeclarationId> = LazyLock::new(|| DeclarationId::from("Object"));
@@ -987,8 +989,11 @@ impl Graph {
987989
InvalidationItem::Declaration(decl_id) => {
988990
self.invalidate_declaration(decl_id, &mut queue, &mut visited_declarations);
989991
}
990-
InvalidationItem::Name(name_id) => {
991-
self.invalidate_name(name_id, &mut queue, &mut visited_names);
992+
InvalidationItem::UnresolveName(name_id) => {
993+
self.unresolve_dependent_name(name_id, &mut queue, &mut visited_names);
994+
}
995+
InvalidationItem::UnresolveReferences(name_id) => {
996+
self.unresolve_dependent_references(name_id, &mut queue, &mut visited_names);
992997
}
993998
}
994999
}
@@ -1028,7 +1033,7 @@ impl Graph {
10281033
// Unresolve names resolved to this declaration, cascade to dependents
10291034
for name_id in self.names_for_declaration(decl_id) {
10301035
self.unresolve_name(name_id);
1031-
self.queue_dependent_names(name_id, queue);
1036+
self.queue_dependent_names(name_id, queue, true);
10321037
}
10331038

10341039
// Clean up owner membership and queue remaining definitions for re-resolution
@@ -1082,17 +1087,14 @@ impl Graph {
10821087

10831088
// Queue dependent names of resolved names (skipping seeds themselves)
10841089
for seed_name_id in self.names_for_declaration(decl_id) {
1085-
self.queue_dependent_names(seed_name_id, queue);
1090+
self.queue_dependent_names(seed_name_id, queue, false);
10861091
}
10871092
}
10881093
}
10891094

1090-
/// Processes a name in the invalidation worklist.
1091-
///
1092-
/// Always propagates to `name_dependents`. Then checks whether the name needs full
1093-
/// structural cascade (nesting or parent scope dependency broken) or just reference
1094-
/// re-evaluation (ancestor context changed).
1095-
fn invalidate_name(
1095+
/// The name's structural dependency is broken (its nesting or parent scope was removed).
1096+
/// Unresolves the name and cascades to all dependents — both references and definitions.
1097+
fn unresolve_dependent_name(
10961098
&mut self,
10971099
name_id: NameId,
10981100
queue: &mut Vec<InvalidationItem>,
@@ -1103,58 +1105,69 @@ impl Graph {
11031105
}
11041106

11051107
let dependents: Vec<NameDependent> = self.name_dependents.get(&name_id).cloned().unwrap_or_default();
1108+
self.queue_dependent_names(name_id, queue, true);
11061109

1107-
// Propagate to names of dependents that are nested/scoped under this name
1108-
self.queue_dependent_names(name_id, queue);
1109-
1110-
if self.has_unresolved_dependency(name_id) {
1111-
// Structural cascade: the name's resolution is invalid
1112-
if let Some(old_decl_id) = self.unresolve_name(name_id) {
1113-
for dep in &dependents {
1114-
match dep {
1115-
NameDependent::Reference(ref_id) => {
1116-
if let Some(decl) = self.declarations.get_mut(&old_decl_id) {
1117-
decl.remove_reference(ref_id);
1118-
}
1119-
self.push_work(Unit::ConstantRef(*ref_id));
1110+
if let Some(old_decl_id) = self.unresolve_name(name_id) {
1111+
for dep in &dependents {
1112+
match dep {
1113+
NameDependent::Reference(ref_id) => {
1114+
if let Some(decl) = self.declarations.get_mut(&old_decl_id) {
1115+
decl.remove_reference(ref_id);
11201116
}
1121-
NameDependent::Definition(def_id) => {
1122-
self.push_work(Unit::Definition(*def_id));
1123-
1124-
if let Some(decl) = self.declarations.get_mut(&old_decl_id) {
1125-
decl.remove_definition(def_id);
1126-
}
1127-
1128-
if self
1129-
.declarations
1130-
.get(&old_decl_id)
1131-
.is_some_and(Declaration::has_no_definitions)
1132-
{
1133-
queue.push(InvalidationItem::Declaration(old_decl_id));
1134-
}
1117+
self.push_work(Unit::ConstantRef(*ref_id));
1118+
}
1119+
NameDependent::Definition(def_id) => {
1120+
self.push_work(Unit::Definition(*def_id));
1121+
1122+
if let Some(decl) = self.declarations.get_mut(&old_decl_id) {
1123+
decl.remove_definition(def_id);
1124+
}
1125+
1126+
if self
1127+
.declarations
1128+
.get(&old_decl_id)
1129+
.is_some_and(Declaration::has_no_definitions)
1130+
{
1131+
queue.push(InvalidationItem::Declaration(old_decl_id));
11351132
}
11361133
}
11371134
}
11381135
}
1139-
} else {
1140-
// Ancestor change only: re-evaluate constant references under this name
1141-
let is_resolved = matches!(self.names.get(&name_id), Some(NameRef::Resolved(_)));
1136+
}
1137+
}
11421138

1143-
for dep in &dependents {
1144-
if let NameDependent::Reference(ref_id) = dep {
1145-
if is_resolved {
1146-
self.unresolve_reference(*ref_id);
1147-
}
1148-
self.push_work(Unit::ConstantRef(*ref_id));
1139+
/// Ancestor context changed but the name itself is still valid.
1140+
/// Unresolves constant references under this name without unresolving the name itself.
1141+
fn unresolve_dependent_references(
1142+
&mut self,
1143+
name_id: NameId,
1144+
queue: &mut Vec<InvalidationItem>,
1145+
visited_names: &mut IdentityHashSet<NameId>,
1146+
) {
1147+
if !visited_names.insert(name_id) {
1148+
return;
1149+
}
1150+
1151+
let dependents: Vec<NameDependent> = self.name_dependents.get(&name_id).cloned().unwrap_or_default();
1152+
self.queue_dependent_names(name_id, queue, false);
1153+
1154+
let is_resolved = matches!(self.names.get(&name_id), Some(NameRef::Resolved(_)));
1155+
1156+
for dep in &dependents {
1157+
if let NameDependent::Reference(ref_id) = dep {
1158+
if is_resolved {
1159+
self.unresolve_reference(*ref_id);
11491160
}
1161+
self.push_work(Unit::ConstantRef(*ref_id));
11501162
}
11511163
}
11521164
}
11531165

1154-
/// Looks at `name_dependents[name_id]` and queues the `name_id` of each dependent
1155-
/// whose own name differs from `name_id` (i.e., dependents registered here via
1156-
/// `nesting`/`parent_scope` expansion).
1157-
fn queue_dependent_names(&self, name_id: NameId, queue: &mut Vec<InvalidationItem>) {
1166+
/// Queues dependent names for invalidation. For each dependent registered under `name_id`
1167+
/// whose own name differs, decides the invalidation mode:
1168+
/// - `UnresolveName` if the scope is broken or the dependent's `parent_scope` is this name
1169+
/// - `UnresolveReferences` otherwise (only ancestor context changed)
1170+
fn queue_dependent_names(&self, name_id: NameId, queue: &mut Vec<InvalidationItem>, scope_broken: bool) {
11581171
if let Some(deps) = self.name_dependents.get(&name_id) {
11591172
for dep in deps {
11601173
let dep_name_id = match dep {
@@ -1166,7 +1179,20 @@ impl Graph {
11661179
if let Some(dep_name_id) = dep_name_id
11671180
&& dep_name_id != name_id
11681181
{
1169-
queue.push(InvalidationItem::Name(dep_name_id));
1182+
// A dependent qualified by this name (e.g. name_id=Foo, dep=Foo::Bar)
1183+
// resolves through it, so ancestor changes invalidate its resolution.
1184+
let needs_unresolve = scope_broken
1185+
|| self
1186+
.names
1187+
.get(&dep_name_id)
1188+
.and_then(|n| n.parent_scope().as_ref().copied())
1189+
== Some(name_id);
1190+
1191+
if needs_unresolve {
1192+
queue.push(InvalidationItem::UnresolveName(dep_name_id));
1193+
} else {
1194+
queue.push(InvalidationItem::UnresolveReferences(dep_name_id));
1195+
}
11701196
}
11711197
}
11721198
}
@@ -1201,31 +1227,6 @@ impl Graph {
12011227
names
12021228
}
12031229

1204-
/// Returns true if the name's nesting or parent scope dependency has been broken,
1205-
/// meaning the name needs full structural cascade rather than just reference re-evaluation.
1206-
fn has_unresolved_dependency(&self, name_id: NameId) -> bool {
1207-
let Some(name_ref) = self.names.get(&name_id) else {
1208-
return false;
1209-
};
1210-
1211-
// Nesting is unresolved or removed: the lexical scope this name lives in is invalid
1212-
if let Some(nesting_id) = name_ref.nesting()
1213-
&& matches!(self.names.get(nesting_id), Some(NameRef::Unresolved(_)) | None)
1214-
{
1215-
return true;
1216-
}
1217-
1218-
// Parent scope is still resolved (pointing to a now-stale declaration) or was removed:
1219-
// the qualifier (e.g. `Foo` in `Foo::Bar`) may resolve differently
1220-
if let Some(parent_id) = name_ref.parent_scope().as_ref()
1221-
&& matches!(self.names.get(parent_id), Some(NameRef::Resolved(_)) | None)
1222-
{
1223-
return true;
1224-
}
1225-
1226-
false
1227-
}
1228-
12291230
/// Sets the encoding that should be used for transforming byte offsets into LSP code unit line/column positions
12301231
pub fn set_encoding(&mut self, encoding: Encoding) {
12311232
self.position_encoding = encoding;
@@ -2894,4 +2895,47 @@ mod tests {
28942895
let empty_ancestors: [&str; 0] = [];
28952896
assert_ancestors_eq!(context, "Foo", empty_ancestors);
28962897
}
2898+
2899+
#[test]
2900+
fn deleting_module_invalidates_multiple_includers() {
2901+
let mut context = GraphTest::new();
2902+
2903+
context.index_uri(
2904+
"file:///m.rb",
2905+
r"
2906+
module M
2907+
CONST = 1
2908+
end
2909+
",
2910+
);
2911+
context.index_uri(
2912+
"file:///a.rb",
2913+
r"
2914+
class A
2915+
include M
2916+
CONST
2917+
end
2918+
",
2919+
);
2920+
context.index_uri(
2921+
"file:///b.rb",
2922+
r"
2923+
class B
2924+
include M
2925+
CONST
2926+
end
2927+
",
2928+
);
2929+
context.resolve();
2930+
2931+
assert_constant_reference_to!(context, "M::CONST", "file:///a.rb:3:3-3:8");
2932+
assert_ancestors_eq!(context, "A", ["A", "M", "Object"]);
2933+
assert_ancestors_eq!(context, "B", ["B", "M", "Object"]);
2934+
2935+
context.delete_uri("file:///m.rb");
2936+
2937+
let empty_ancestors: [&str; 0] = [];
2938+
assert_ancestors_eq!(context, "A", empty_ancestors);
2939+
assert_ancestors_eq!(context, "B", empty_ancestors);
2940+
}
28972941
}

rust/rubydex/src/test_utils/graph_test.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -299,12 +299,7 @@ macro_rules! assert_constant_reference_unresolved {
299299
None
300300
}
301301
})
302-
.unwrap_or_else(|| {
303-
panic!(
304-
"No constant reference with unqualified name `{}`",
305-
$unqualified_name
306-
)
307-
});
302+
.unwrap_or_else(|| panic!("No constant reference with unqualified name `{}`", $unqualified_name));
308303

309304
assert!(
310305
matches!(reference_name, $crate::model::name::NameRef::Unresolved(_)),

0 commit comments

Comments
 (0)