diff --git a/rust/rubydex-sys/src/diagnostic_api.rs b/rust/rubydex-sys/src/diagnostic_api.rs index 38b6cb4b9..39f9cbc27 100644 --- a/rust/rubydex-sys/src/diagnostic_api.rs +++ b/rust/rubydex-sys/src/diagnostic_api.rs @@ -43,7 +43,7 @@ impl DiagnosticArray { pub unsafe extern "C" fn rdx_graph_diagnostics(pointer: GraphPointer) -> *mut DiagnosticArray { with_graph(pointer, |graph| { let entries = graph - .diagnostics() + .all_diagnostics() .iter() .map(|diagnostic| { let document = graph.documents().get(diagnostic.uri_id()).unwrap(); diff --git a/rust/rubydex/src/indexing/local_graph.rs b/rust/rubydex/src/indexing/local_graph.rs index 2f23b52cf..dfa62eb84 100644 --- a/rust/rubydex/src/indexing/local_graph.rs +++ b/rust/rubydex/src/indexing/local_graph.rs @@ -18,7 +18,6 @@ type LocalGraphParts = ( IdentityHashMap, IdentityHashMap, IdentityHashMap, - Vec, ); #[derive(Debug)] @@ -30,7 +29,6 @@ pub struct LocalGraph { names: IdentityHashMap, constant_references: IdentityHashMap, method_references: IdentityHashMap, - diagnostics: Vec, } impl LocalGraph { @@ -44,7 +42,6 @@ impl LocalGraph { names: IdentityHashMap::default(), constant_references: IdentityHashMap::default(), method_references: IdentityHashMap::default(), - diagnostics: Vec::new(), } } @@ -149,13 +146,13 @@ impl LocalGraph { // Diagnostics #[must_use] - pub fn diagnostics(&self) -> &Vec { - &self.diagnostics + pub fn diagnostics(&self) -> &[Diagnostic] { + self.document.diagnostics() } pub fn add_diagnostic(&mut self, rule: Rule, offset: Offset, message: String) { let diagnostic = Diagnostic::new(rule, self.uri_id, offset, message); - self.diagnostics.push(diagnostic); + self.document.add_diagnostic(diagnostic); } // Into parts @@ -170,7 +167,6 @@ impl LocalGraph { self.names, self.constant_references, self.method_references, - self.diagnostics, ) } } diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index f608d57f4..3ab975ded 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -1,3 +1,4 @@ +use crate::diagnostic::Diagnostic; use crate::model::{ identity_maps::{IdentityHashMap, IdentityHashSet}, ids::{DeclarationId, DefinitionId, NameId, ReferenceId, StringId}, @@ -102,6 +103,8 @@ macro_rules! namespace_declaration { descendants: IdentityHashSet, /// The singleton class associated with this declaration singleton_class_id: Option, + /// Diagnostics associated with this declaration + diagnostics: Vec, } impl $name { @@ -116,13 +119,15 @@ macro_rules! namespace_declaration { ancestors: Ancestors::Partial(Vec::new()), descendants: IdentityHashSet::default(), singleton_class_id: None, + diagnostics: Vec::new(), } } - pub fn extend(&mut self, other: Namespace) { + pub fn extend(&mut self, mut other: Namespace) { self.definition_ids.extend(other.definitions()); self.references.extend(other.references()); self.members.extend(other.members()); + self.diagnostics.extend(other.take_diagnostics()); } pub fn set_singleton_class_id(&mut self, declaration_id: DeclarationId) { @@ -205,6 +210,8 @@ macro_rules! simple_declaration { references: IdentityHashSet, /// The ID of the owner of this declaration owner_id: DeclarationId, + /// Diagnostics associated with this declaration + diagnostics: Vec, } impl $name { @@ -215,12 +222,14 @@ macro_rules! simple_declaration { definition_ids: Vec::new(), references: IdentityHashSet::default(), owner_id, + diagnostics: Vec::new(), } } - pub fn extend(&mut self, other: Declaration) { + pub fn extend(&mut self, mut other: Declaration) { self.definition_ids.extend(other.definitions()); self.references.extend(other.references()); + self.diagnostics.extend(other.take_diagnostics()); } } }; @@ -339,6 +348,23 @@ impl Declaration { after_colons.rsplit('#').next().unwrap_or(after_colons).to_string() }) } + + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + all_declarations!(self, it => &it.diagnostics) + } + + pub fn take_diagnostics(&mut self) -> Vec { + all_declarations!(self, it => std::mem::take(&mut it.diagnostics)) + } + + pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) { + all_declarations!(self, it => it.diagnostics.push(diagnostic)); + } + + pub fn clear_diagnostics(&mut self) { + all_declarations!(self, it => it.diagnostics.clear()); + } } #[derive(Debug)] @@ -373,6 +399,15 @@ impl Namespace { all_namespaces!(self, it => &it.members) } + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + all_namespaces!(self, it => &it.diagnostics) + } + + pub fn take_diagnostics(&mut self) -> Vec { + all_namespaces!(self, it => std::mem::take(&mut it.diagnostics)) + } + /// # Panics /// /// Panics if the declaration is not a namespace or a constant diff --git a/rust/rubydex/src/model/definitions.rs b/rust/rubydex/src/model/definitions.rs index a3f83140b..3e38434f1 100644 --- a/rust/rubydex/src/model/definitions.rs +++ b/rust/rubydex/src/model/definitions.rs @@ -26,6 +26,7 @@ use bitflags::bitflags; use crate::{ + diagnostic::Diagnostic, model::{ comment::Comment, ids::ReferenceId, @@ -149,6 +150,15 @@ impl Definition { pub fn is_deprecated(&self) -> bool { all_definitions!(self, it => it.flags().is_deprecated()) } + + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + all_definitions!(self, it => &it.diagnostics) + } + + pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) { + all_definitions!(self, it => it.diagnostics.push(diagnostic)); + } } /// Represents a mixin: include, prepend, or extend. @@ -216,6 +226,7 @@ pub struct ClassDefinition { members: Vec, superclass_ref: Option, mixins: Vec, + diagnostics: Vec, } impl ClassDefinition { @@ -239,6 +250,7 @@ impl ClassDefinition { superclass_ref, members: Vec::new(), mixins: Vec::new(), + diagnostics: Vec::new(), } } @@ -331,6 +343,7 @@ pub struct SingletonClassDefinition { members: Vec, /// Mixins declared in this singleton class mixins: Vec, + diagnostics: Vec, } impl SingletonClassDefinition { @@ -352,6 +365,7 @@ impl SingletonClassDefinition { lexical_nesting_id, members: Vec::new(), mixins: Vec::new(), + diagnostics: Vec::new(), } } @@ -426,6 +440,7 @@ pub struct ModuleDefinition { lexical_nesting_id: Option, members: Vec, mixins: Vec, + diagnostics: Vec, } impl ModuleDefinition { @@ -447,6 +462,7 @@ impl ModuleDefinition { lexical_nesting_id, members: Vec::new(), mixins: Vec::new(), + diagnostics: Vec::new(), } } @@ -518,6 +534,7 @@ pub struct ConstantDefinition { flags: DefinitionFlags, comments: Vec, lexical_nesting_id: Option, + diagnostics: Vec, } impl ConstantDefinition { @@ -537,6 +554,7 @@ impl ConstantDefinition { flags, comments, lexical_nesting_id, + diagnostics: Vec::new(), } } @@ -587,6 +605,7 @@ impl ConstantDefinition { pub struct ConstantAliasDefinition { alias_constant: ConstantDefinition, target_name_id: NameId, + diagnostics: Vec, } impl ConstantAliasDefinition { @@ -595,6 +614,7 @@ impl ConstantAliasDefinition { Self { alias_constant, target_name_id, + diagnostics: Vec::new(), } } @@ -663,6 +683,7 @@ pub struct MethodDefinition { parameters: Vec, visibility: Visibility, receiver: Option, + diagnostics: Vec, } impl MethodDefinition { @@ -689,6 +710,7 @@ impl MethodDefinition { parameters, visibility, receiver, + diagnostics: Vec::new(), } } @@ -800,6 +822,7 @@ pub struct AttrAccessorDefinition { comments: Vec, lexical_nesting_id: Option, visibility: Visibility, + diagnostics: Vec, } impl AttrAccessorDefinition { @@ -821,6 +844,7 @@ impl AttrAccessorDefinition { comments, lexical_nesting_id, visibility, + diagnostics: Vec::new(), } } @@ -880,6 +904,7 @@ pub struct AttrReaderDefinition { comments: Vec, lexical_nesting_id: Option, visibility: Visibility, + diagnostics: Vec, } impl AttrReaderDefinition { @@ -901,6 +926,7 @@ impl AttrReaderDefinition { comments, lexical_nesting_id, visibility, + diagnostics: Vec::new(), } } @@ -960,6 +986,7 @@ pub struct AttrWriterDefinition { comments: Vec, lexical_nesting_id: Option, visibility: Visibility, + diagnostics: Vec, } impl AttrWriterDefinition { @@ -981,6 +1008,7 @@ impl AttrWriterDefinition { comments, lexical_nesting_id, visibility, + diagnostics: Vec::new(), } } @@ -1039,6 +1067,7 @@ pub struct GlobalVariableDefinition { flags: DefinitionFlags, comments: Vec, lexical_nesting_id: Option, + diagnostics: Vec, } impl GlobalVariableDefinition { @@ -1058,6 +1087,7 @@ impl GlobalVariableDefinition { flags, comments, lexical_nesting_id, + diagnostics: Vec::new(), } } @@ -1111,6 +1141,7 @@ pub struct InstanceVariableDefinition { flags: DefinitionFlags, comments: Vec, lexical_nesting_id: Option, + diagnostics: Vec, } impl InstanceVariableDefinition { @@ -1130,6 +1161,7 @@ impl InstanceVariableDefinition { flags, comments, lexical_nesting_id, + diagnostics: Vec::new(), } } @@ -1183,6 +1215,7 @@ pub struct ClassVariableDefinition { flags: DefinitionFlags, comments: Vec, lexical_nesting_id: Option, + diagnostics: Vec, } impl ClassVariableDefinition { @@ -1202,6 +1235,7 @@ impl ClassVariableDefinition { flags, comments, lexical_nesting_id, + diagnostics: Vec::new(), } } @@ -1250,6 +1284,7 @@ pub struct MethodAliasDefinition { flags: DefinitionFlags, comments: Vec, lexical_nesting_id: Option, + diagnostics: Vec, } impl MethodAliasDefinition { @@ -1271,6 +1306,7 @@ impl MethodAliasDefinition { flags, comments, lexical_nesting_id, + diagnostics: Vec::new(), } } @@ -1330,6 +1366,7 @@ pub struct GlobalVariableAliasDefinition { flags: DefinitionFlags, comments: Vec, lexical_nesting_id: Option, + diagnostics: Vec, } impl GlobalVariableAliasDefinition { @@ -1351,6 +1388,7 @@ impl GlobalVariableAliasDefinition { flags, comments, lexical_nesting_id, + diagnostics: Vec::new(), } } diff --git a/rust/rubydex/src/model/document.rs b/rust/rubydex/src/model/document.rs index eaa347894..b439575d0 100644 --- a/rust/rubydex/src/model/document.rs +++ b/rust/rubydex/src/model/document.rs @@ -1,5 +1,6 @@ use line_index::LineIndex; +use crate::diagnostic::Diagnostic; use crate::model::ids::{DefinitionId, ReferenceId}; // Represents a document currently loaded into memory. Identified by its unique URI, it holds the edges to all @@ -11,6 +12,7 @@ pub struct Document { definition_ids: Vec, method_reference_ids: Vec, constant_reference_ids: Vec, + diagnostics: Vec, } impl Document { @@ -22,6 +24,7 @@ impl Document { definition_ids: Vec::new(), method_reference_ids: Vec::new(), constant_reference_ids: Vec::new(), + diagnostics: Vec::new(), } } @@ -66,6 +69,15 @@ impl Document { pub fn add_constant_reference(&mut self, reference_id: ReferenceId) { self.constant_reference_ids.push(reference_id); } + + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics + } + + pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) { + self.diagnostics.push(diagnostic); + } } #[cfg(test)] diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index bf94cdf04..5d2c21fb7 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -40,7 +40,6 @@ pub struct Graph { // Map of method references that still need to be resolved method_references: IdentityHashMap, - diagnostics: Vec, /// The position encoding used for LSP line/column locations. Not related to the actual encoding of the file position_encoding: Encoding, } @@ -57,7 +56,6 @@ impl Graph { names: IdentityHashMap::default(), constant_references: IdentityHashMap::default(), method_references: IdentityHashMap::default(), - diagnostics: Vec::new(), position_encoding: Encoding::default(), } } @@ -173,8 +171,22 @@ impl Graph { } #[must_use] - pub fn diagnostics(&self) -> &Vec { - &self.diagnostics + pub fn all_diagnostics(&self) -> Vec<&Diagnostic> { + let document_diagnostics = self.documents.values().flat_map(Document::diagnostics); + let declaration_diagnostics = self.declarations.values().flat_map(Declaration::diagnostics); + let definition_diagnostics = self.definitions.values().flat_map(Definition::diagnostics); + let constant_reference_diagnostics = self + .constant_references + .values() + .flat_map(ConstantReference::diagnostics); + let method_reference_diagnostics = self.method_references.values().flat_map(MethodRef::diagnostics); + + document_diagnostics + .chain(declaration_diagnostics) + .chain(definition_diagnostics) + .chain(constant_reference_diagnostics) + .chain(method_reference_diagnostics) + .collect() } /// Interns a string in the graph unless already interned. This method is only used to back the @@ -418,7 +430,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, diagnostics) = + let (uri_id, document, definitions, strings, names, constant_references, method_references) = local_graph.into_parts(); self.documents.insert(uri_id, document); @@ -445,7 +457,6 @@ impl Graph { } self.constant_references.extend(constant_references); self.method_references.extend(method_references); - self.diagnostics.extend(diagnostics); } /// Updates the global representation with the information contained in `other`, handling deletions, insertions and @@ -495,6 +506,7 @@ impl Graph { && let Some(declaration) = self.declarations.get_mut(&declaration_id) && declaration.remove_definition(def_id) { + declaration.clear_diagnostics(); if declaration.as_namespace().is_some() { declarations_to_invalidate_ancestor_chains.push(declaration_id); } @@ -1154,6 +1166,18 @@ mod tests { } } + #[test] + fn updating_index_with_deleted_diagnostics() { + let mut context = GraphTest::new(); + + // TODO: Add resolution error to test diagnostics attached to declarations + context.index_uri("file:///foo.rb", "class Foo"); + assert!(!context.graph().all_diagnostics().is_empty()); + + context.index_uri("file:///foo.rb", "class Foo; end"); + assert!(context.graph().all_diagnostics().is_empty()); + } + #[test] fn diagnostics_are_collected() { let mut context = GraphTest::new(); @@ -1170,9 +1194,9 @@ mod tests { " }); - let mut diagnostics = context + let mut diagnostics: Vec = context .graph() - .diagnostics() + .all_diagnostics() .iter() .map(|d| { format!( @@ -1182,7 +1206,7 @@ mod tests { context.graph().documents().get(d.uri_id()).unwrap().uri() ) }) - .collect::>(); + .collect(); diagnostics.sort(); diff --git a/rust/rubydex/src/model/references.rs b/rust/rubydex/src/model/references.rs index 5c1e4c0fe..c038444d6 100644 --- a/rust/rubydex/src/model/references.rs +++ b/rust/rubydex/src/model/references.rs @@ -1,4 +1,5 @@ use crate::{ + diagnostic::Diagnostic, model::ids::{NameId, ReferenceId, StringId, UriId}, offset::Offset, }; @@ -12,6 +13,8 @@ pub struct ConstantReference { uri_id: UriId, /// The offsets inside of the document where we found the reference offset: Offset, + /// Diagnostics associated with this reference + diagnostics: Vec, } impl ConstantReference { @@ -21,6 +24,7 @@ impl ConstantReference { name_id, uri_id, offset, + diagnostics: Vec::new(), } } @@ -51,6 +55,15 @@ impl ConstantReference { ); ReferenceId::from(&key) } + + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics + } + + pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) { + self.diagnostics.push(diagnostic); + } } /// A reference to a method @@ -62,12 +75,19 @@ pub struct MethodRef { uri_id: UriId, /// The offsets inside of the document where we found the reference offset: Offset, + /// Diagnostics associated with this reference + diagnostics: Vec, } impl MethodRef { #[must_use] pub fn new(str: StringId, uri_id: UriId, offset: Offset) -> Self { - Self { str, uri_id, offset } + Self { + str, + uri_id, + offset, + diagnostics: Vec::new(), + } } #[must_use] @@ -97,4 +117,13 @@ impl MethodRef { ); ReferenceId::from(&key) } + + #[must_use] + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics + } + + pub fn add_diagnostic(&mut self, diagnostic: Diagnostic) { + self.diagnostics.push(diagnostic); + } } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index bcc627eee..4e623a656 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1771,12 +1771,12 @@ mod tests { } fn format_diagnostics(context: &GraphTest, ignore_rules: &[Rule]) -> Vec { - let mut diagnostics = context + let mut diagnostics: Vec<_> = context .graph() - .diagnostics() - .iter() + .all_diagnostics() + .into_iter() .filter(|d| !ignore_rules.contains(d.rule())) - .collect::>(); + .collect(); diagnostics.sort_by_key(|d| { let uri = context.graph().documents().get(d.uri_id()).unwrap().uri();