diff --git a/rust/rubydex/src/indexing/local_graph.rs b/rust/rubydex/src/indexing/local_graph.rs index 559270a5..6f5bbcf9 100644 --- a/rust/rubydex/src/indexing/local_graph.rs +++ b/rust/rubydex/src/indexing/local_graph.rs @@ -3,6 +3,7 @@ use std::collections::hash_map::Entry; use crate::diagnostic::{Diagnostic, Rule}; use crate::model::definitions::Definition; use crate::model::document::Document; +use crate::model::graph::NameDependent; use crate::model::identity_maps::IdentityHashMap; use crate::model::ids::{DefinitionId, NameId, ReferenceId, StringId, UriId}; use crate::model::name::{Name, NameRef}; @@ -18,6 +19,7 @@ type LocalGraphParts = ( IdentityHashMap, IdentityHashMap, IdentityHashMap, + IdentityHashMap>, ); #[derive(Debug)] @@ -29,6 +31,7 @@ pub struct LocalGraph { names: IdentityHashMap, constant_references: IdentityHashMap, method_references: IdentityHashMap, + name_dependents: IdentityHashMap>, } impl LocalGraph { @@ -42,6 +45,7 @@ impl LocalGraph { names: IdentityHashMap::default(), constant_references: IdentityHashMap::default(), method_references: IdentityHashMap::default(), + name_dependents: IdentityHashMap::default(), } } @@ -70,6 +74,13 @@ impl LocalGraph { pub fn add_definition(&mut self, definition: Definition) -> DefinitionId { let definition_id = definition.id(); + if let Some(name_id) = definition.name_id() { + self.name_dependents + .entry(*name_id) + .or_default() + .push(NameDependent::Definition(definition_id)); + } + if self.definitions.insert(definition_id, definition).is_some() { debug_assert!(false, "DefinitionId collision in local graph"); } @@ -117,6 +128,18 @@ impl LocalGraph { entry.get_mut().increment_ref_count(1); } Entry::Vacant(entry) => { + if let Some(&parent_scope) = name.parent_scope().as_ref() { + self.name_dependents + .entry(parent_scope) + .or_default() + .push(NameDependent::ChildName(name_id)); + } + if let Some(&nesting_id) = name.nesting().as_ref() { + self.name_dependents + .entry(nesting_id) + .or_default() + .push(NameDependent::NestedName(name_id)); + } entry.insert(NameRef::Unresolved(Box::new(name))); } } @@ -133,6 +156,10 @@ impl LocalGraph { pub fn add_constant_reference(&mut self, reference: ConstantReference) -> ReferenceId { let reference_id = reference.id(); + self.name_dependents + .entry(*reference.name_id()) + .or_default() + .push(NameDependent::Reference(reference_id)); if self.constant_references.insert(reference_id, reference).is_some() { debug_assert!(false, "ReferenceId collision in local graph"); @@ -172,6 +199,13 @@ impl LocalGraph { self.document.add_diagnostic(diagnostic); } + // Name dependents + + #[must_use] + pub fn name_dependents(&self) -> &IdentityHashMap> { + &self.name_dependents + } + // Into parts #[must_use] @@ -184,6 +218,7 @@ impl LocalGraph { self.names, self.constant_references, self.method_references, + self.name_dependents, ) } } diff --git a/rust/rubydex/src/indexing/ruby_indexer.rs b/rust/rubydex/src/indexing/ruby_indexer.rs index 66a1f1d5..98883326 100644 --- a/rust/rubydex/src/indexing/ruby_indexer.rs +++ b/rust/rubydex/src/indexing/ruby_indexer.rs @@ -5917,3 +5917,82 @@ mod tests { }); } } + +#[cfg(test)] +mod name_dependent_tests { + use crate::assert_dependents; + use crate::test_utils::LocalGraphTest; + + fn index_source(source: &str) -> LocalGraphTest { + LocalGraphTest::new("file:///foo.rb", source) + } + + #[test] + fn track_dependency_chain() { + let context = index_source( + " + module Bar; end + CONST = 1 + CONST2 = CONST + + module Foo + class Bar::Baz + CONST + end + + CONST2 + end + ", + ); + + assert_dependents!(&context, "Bar", [ChildName("Baz")]); + assert_dependents!(&context, "Foo", [NestedName("Baz"), NestedName("CONST2")]); + assert_dependents!(&context, "Bar::Baz", [Definition("Baz"), NestedName("CONST")]); + } + + #[test] + fn multi_level_chain() { + let context = index_source( + " + module Foo + module Bar + module Baz + end + end + end + ", + ); + + assert_dependents!(&context, "Foo", [NestedName("Bar")]); + assert_dependents!(&context, "Bar", [NestedName("Baz")]); + } + + #[test] + fn singleton_class() { + let context = index_source( + " + class Foo + class << self + def bar; end + end + end + ", + ); + + assert_dependents!(&context, "Foo", [ChildName("")]); + } + + #[test] + fn nested_vs_compact() { + let context = index_source( + " + module Foo + class Bar; end + class Foo::Baz; end + end + ", + ); + + assert_dependents!(&context, "Foo", [NestedName("Bar"), ChildName("Baz")]); + } +} diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 3436ec72..9a85d7f7 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -14,6 +14,18 @@ use crate::model::references::{ConstantReference, MethodRef}; use crate::model::string_ref::StringRef; use crate::stats; +/// An entity whose validity depends on a particular `NameId`. +/// Used as the value type in the `name_dependents` reverse index. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NameDependent { + Definition(DefinitionId), + Reference(ReferenceId), + /// This name's `parent_scope` is the key name — structural dependency. + ChildName(NameId), + /// This name's `nesting` is the key name — reference-only dependency. + NestedName(NameId), +} + pub static BASIC_OBJECT_ID: LazyLock = LazyLock::new(|| DeclarationId::from("BasicObject")); pub static OBJECT_ID: LazyLock = LazyLock::new(|| DeclarationId::from("Object")); pub static MODULE_ID: LazyLock = LazyLock::new(|| DeclarationId::from("Module")); @@ -41,6 +53,10 @@ pub struct Graph { /// The position encoding used for LSP line/column locations. Not related to the actual encoding of the file position_encoding: Encoding, + + /// Reverse index: for each `NameId`, which definitions, references, and child/nested names depend on it. + /// Used during invalidation to efficiently find affected entities without scanning the full graph. + name_dependents: IdentityHashMap>, } impl Graph { @@ -55,6 +71,7 @@ impl Graph { constant_references: IdentityHashMap::default(), method_references: IdentityHashMap::default(), position_encoding: Encoding::default(), + name_dependents: IdentityHashMap::default(), } } @@ -501,6 +518,11 @@ impl Graph { &self.names } + #[must_use] + pub fn name_dependents(&self) -> &IdentityHashMap> { + &self.name_dependents + } + /// Converts a `Resolved` `NameRef` back to `Unresolved`, preserving the original `Name` data. /// Returns the `DeclarationId` it was previously resolved to, if any. fn unresolve_name(&mut self, name_id: NameId) -> Option { @@ -533,11 +555,34 @@ impl Graph { } } - /// Removes a name from the graph entirely. + /// Removes a name from the graph and cleans up its name-to-name edges from parent names. fn remove_name(&mut self, name_id: NameId) { + if let Some(name_ref) = self.names.get(&name_id) { + let parent_scope = name_ref.parent_scope().as_ref().copied(); + let nesting = name_ref.nesting().as_ref().copied(); + + if let Some(ps_id) = parent_scope { + self.remove_name_dependent(ps_id, NameDependent::ChildName(name_id)); + } + if let Some(nesting_id) = nesting { + self.remove_name_dependent(nesting_id, NameDependent::NestedName(name_id)); + } + } + self.name_dependents.remove(&name_id); self.names.remove(&name_id); } + /// Removes a specific dependent from the `name_dependents` entry for `name_id`, + /// cleaning up the entry if no dependents remain. + fn remove_name_dependent(&mut self, name_id: NameId, dependent: NameDependent) { + if let Some(deps) = self.name_dependents.get_mut(&name_id) { + deps.retain(|d| *d != dependent); + if deps.is_empty() { + self.name_dependents.remove(&name_id); + } + } + } + /// Decrements the ref count for a name and removes it if the count reaches zero. /// /// This does not recursively untrack `parent_scope` or `nesting` names. @@ -687,7 +732,7 @@ impl Graph { /// Merges everything in `other` into this Graph. This method is meant to merge all graph representations from /// different threads, but not meant to handle updates to the existing global representation pub fn extend(&mut self, local_graph: LocalGraph) { - let (uri_id, document, definitions, strings, names, constant_references, method_references) = + let (uri_id, document, definitions, strings, names, constant_references, method_references, name_dependents) = local_graph.into_parts(); if self.documents.insert(uri_id, document).is_some() { @@ -735,6 +780,15 @@ impl Graph { debug_assert!(false, "Method ReferenceId collision in global graph"); } } + + for (name_id, deps) in name_dependents { + let global_deps = self.name_dependents.entry(name_id).or_default(); + for dep in deps { + if !global_deps.contains(&dep) { + global_deps.push(dep); + } + } + } } /// Updates the global representation with the information contained in `other`, handling deletions, insertions and @@ -765,6 +819,7 @@ impl Graph { self.unresolve_reference(*ref_id); if let Some(constant_ref) = self.constant_references.remove(ref_id) { + self.remove_name_dependent(*constant_ref.name_id(), NameDependent::Reference(*ref_id)); self.untrack_name(*constant_ref.name_id()); } } @@ -800,8 +855,9 @@ impl Graph { } } - if let Some(name_id) = self.definitions.get(def_id).unwrap().name_id() { - self.untrack_name(*name_id); + if let Some(name_id) = self.definitions.get(def_id).unwrap().name_id().copied() { + self.remove_name_dependent(name_id, NameDependent::Definition(*def_id)); + self.untrack_name(name_id); } } @@ -999,7 +1055,7 @@ mod tests { use crate::model::comment::Comment; use crate::model::declaration::Ancestors; use crate::test_utils::GraphTest; - use crate::{assert_descendants, assert_members_eq, assert_no_diagnostics, assert_no_members}; + use crate::{assert_dependents, assert_descendants, assert_members_eq, assert_no_diagnostics, assert_no_members}; #[test] fn deleting_a_uri() { @@ -1021,6 +1077,55 @@ mod tests { ); } + #[test] + fn deleting_file_triggers_name_dependent_cleanup() { + let mut context = GraphTest::new(); + + context.index_uri( + "file:///foo.rb", + " + module Foo + CONST + end + ", + ); + context.index_uri( + "file:///bar.rb", + " + module Foo + class Bar; end + end + ", + ); + context.resolve(); + + assert_dependents!( + &context, + "Foo", + [ + Definition("Foo"), + Definition("Foo"), + NestedName("Bar"), + NestedName("CONST"), + ] + ); + + // Deleting bar.rb removes Bar's name (and its NestedName edge from Foo) + // and one Definition dependent (bar.rb's `module Foo` definition). + context.delete_uri("file:///bar.rb"); + assert_dependents!(&context, "Foo", [Definition("Foo"), NestedName("CONST")]); + + // Deleting foo.rb cleans up everything + context.delete_uri("file:///foo.rb"); + let foo_ids = context + .graph() + .names() + .iter() + .filter(|(_, n)| *n.str() == StringId::from("Foo")) + .count(); + assert_eq!(foo_ids, 0, "Foo name should be removed after deleting both files"); + } + #[test] fn updating_index_with_deleted_definitions() { let mut context = GraphTest::new(); diff --git a/rust/rubydex/src/test_utils/graph_test.rs b/rust/rubydex/src/test_utils/graph_test.rs index 534822cd..a3e731bb 100644 --- a/rust/rubydex/src/test_utils/graph_test.rs +++ b/rust/rubydex/src/test_utils/graph_test.rs @@ -2,7 +2,8 @@ use super::normalize_indentation; #[cfg(test)] use crate::diagnostic::Rule; use crate::indexing::{self, LanguageId}; -use crate::model::graph::Graph; +use crate::model::graph::{Graph, NameDependent}; +use crate::model::ids::{NameId, StringId}; use crate::resolution::Resolver; #[derive(Default)] @@ -42,6 +43,79 @@ impl GraphTest { resolver.resolve_all(); } + // Name dependents helpers (shared with LocalGraphTest for assert_dependents! macro) + + /// # Panics + /// + /// Panics if no names match the given path. + #[must_use] + pub fn find_name_ids(&self, path: &str) -> Vec { + let (parent, name) = match path.rsplit_once("::") { + Some((p, n)) => (Some(p), n), + None => (None, path), + }; + let target_str_id = StringId::from(name); + let ids: Vec = self + .graph() + .names() + .iter() + .filter(|(_, name_ref)| { + if *name_ref.str() != target_str_id { + return false; + } + match parent { + None => name_ref.parent_scope().as_ref().is_none(), + Some(p) => name_ref.parent_scope().as_ref().is_some_and(|ps_id| { + let ps = self.graph().names().get(ps_id).unwrap(); + *ps.str() == StringId::from(p) + }), + } + }) + .map(|(id, _)| *id) + .collect(); + assert!(!ids.is_empty(), "could not find name `{path}`"); + ids + } + + #[must_use] + pub fn name_dependents_for(&self, name_id: NameId) -> Vec { + self.graph() + .name_dependents() + .get(&name_id) + .cloned() + .unwrap_or_default() + } + + /// # Panics + /// + /// Panics if the name's string is not in the strings map. + #[must_use] + pub fn name_str(&self, name_id: &NameId) -> Option<&str> { + self.graph() + .names() + .get(name_id) + .map(|n| self.graph().strings().get(n.str()).unwrap().as_str()) + } + + /// Returns the unqualified name string for a `NameDependent`, if available. + #[must_use] + pub fn dependent_name_str(&self, dep: &NameDependent) -> Option<&str> { + match dep { + NameDependent::ChildName(id) | NameDependent::NestedName(id) => self.name_str(id), + NameDependent::Definition(id) => self + .graph() + .definitions() + .get(id) + .and_then(|d| d.name_id()) + .and_then(|name_id| self.name_str(name_id)), + NameDependent::Reference(id) => self + .graph() + .constant_references() + .get(id) + .and_then(|r| self.name_str(r.name_id())), + } + } + /// # Panics /// /// Panics if a diagnostic points to an invalid document diff --git a/rust/rubydex/src/test_utils/local_graph_test.rs b/rust/rubydex/src/test_utils/local_graph_test.rs index c4bbd165..2dfb6634 100644 --- a/rust/rubydex/src/test_utils/local_graph_test.rs +++ b/rust/rubydex/src/test_utils/local_graph_test.rs @@ -3,7 +3,8 @@ use crate::indexing::local_graph::LocalGraph; use crate::indexing::rbs_indexer::RBSIndexer; use crate::indexing::ruby_indexer::RubyIndexer; use crate::model::definitions::Definition; -use crate::model::ids::UriId; +use crate::model::graph::NameDependent; +use crate::model::ids::{NameId, StringId, UriId}; use crate::offset::Offset; use crate::position::Position; @@ -161,6 +162,83 @@ impl LocalGraphTest { .parse() .unwrap_or_else(|_| panic!("Invalid {field} '{value}' in location {location}")) } + + // Name dependents helpers + + /// Finds all `NameId`s matching a path. `"Foo"` matches names with str="Foo" and no + /// `parent_scope`. `"Bar::Baz"` matches names with str="Baz" and `parent_scope` str="Bar". + /// Multiple matches are possible when the same constant appears at different nestings. + /// + /// # Panics + /// + /// Panics if no names match the given path. + #[must_use] + pub fn find_name_ids(&self, path: &str) -> Vec { + let (parent, name) = match path.rsplit_once("::") { + Some((p, n)) => (Some(p), n), + None => (None, path), + }; + let target_str_id = StringId::from(name); + let ids: Vec = self + .graph() + .names() + .iter() + .filter(|(_, name_ref)| { + if *name_ref.str() != target_str_id { + return false; + } + match parent { + None => name_ref.parent_scope().as_ref().is_none(), + Some(p) => name_ref.parent_scope().as_ref().is_some_and(|ps_id| { + let ps = self.graph().names().get(ps_id).unwrap(); + *ps.str() == StringId::from(p) + }), + } + }) + .map(|(id, _)| *id) + .collect(); + assert!(!ids.is_empty(), "could not find name `{path}`"); + ids + } + + #[must_use] + pub fn name_dependents_for(&self, name_id: NameId) -> Vec { + self.graph() + .name_dependents() + .get(&name_id) + .cloned() + .unwrap_or_default() + } + + /// # Panics + /// + /// Panics if the name's string is not in the strings map. + #[must_use] + pub fn name_str(&self, name_id: &NameId) -> Option<&str> { + self.graph() + .names() + .get(name_id) + .map(|n| self.graph().strings().get(n.str()).unwrap().as_str()) + } + + /// Returns the unqualified name string for a `NameDependent`, if available. + #[must_use] + pub fn dependent_name_str(&self, dep: &NameDependent) -> Option<&str> { + match dep { + NameDependent::ChildName(id) | NameDependent::NestedName(id) => self.name_str(id), + NameDependent::Definition(id) => self + .graph() + .definitions() + .get(id) + .and_then(|d| d.name_id()) + .and_then(|name_id| self.name_str(name_id)), + NameDependent::Reference(id) => self + .graph() + .constant_references() + .get(id) + .and_then(|r| self.name_str(r.name_id())), + } + } } // Primitive assertions @@ -367,6 +445,40 @@ macro_rules! assert_def_mixins_eq { }}; } +// Name dependent assertions + +/// Asserts that `owner` has dependents matching the given list. +/// Each entry uses `Variant("name")` syntax. When multiple names match the owner path +/// (different nestings), any match suffices for each expected dependent. +/// +/// Usage: +/// ```ignore +/// assert_dependents!(ctx, "Bar", [ChildName("Baz"), Definition("Bar")]); +/// assert_dependents!(ctx, "Bar::Baz", [NestedName("CONST"), Definition("Baz")]); +/// ``` +#[cfg(test)] +#[macro_export] +macro_rules! assert_dependents { + ($ctx:expr, $owner:expr, [$($variant:ident($dep:expr)),* $(,)?]) => {{ + let owner_ids = $ctx.find_name_ids($owner); + $( + let found = owner_ids.iter().any(|owner_id| { + $ctx.name_dependents_for(*owner_id).iter().any(|d| { + matches!(d, $crate::model::graph::NameDependent::$variant(_)) + && $ctx.dependent_name_str(d) == Some($dep) + }) + }); + assert!( + found, + "expected {}({}) in {}'s dependents", + stringify!($variant), + $dep, + $owner + ); + )* + }}; +} + // Diagnostic assertions #[cfg(test)]