diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index a39139ca..c23688aa 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -124,6 +124,7 @@ rules! { InvalidMethodVisibility; // Resolution + SuperclassMismatch; UndefinedMethodVisibilityTarget; UndefinedConstantVisibilityTarget; } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 9ee152af..2ec5a360 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -15,6 +15,7 @@ use crate::model::{ name::{Name, NameRef, ParentScope}, visibility::Visibility, }; +use crate::offset::Offset; enum Outcome { /// The constant was successfully resolved to the given declaration ID. @@ -2004,7 +2005,7 @@ impl<'a> Resolver<'a> { } Declaration::Namespace(Namespace::Class(_)) => { // For classes (the regular case), we need to return the singleton class of its superclass - let Some((superclass_id, unresolved_superclass)) = self.get_superclass(attached_id) else { + let Some((superclass_id, unresolved_superclass, _)) = self.get_superclass(attached_id) else { // BasicObject has no superclass, but its singleton class inherits from Class return (*CLASS_ID, false); }; @@ -2024,13 +2025,17 @@ impl<'a> Resolver<'a> { /// Returns the selected superclass and any unresolved explicit superclass. The top-level `BasicObject` declaration /// is Ruby's root class and is the only class without a superclass. - fn get_superclass(&self, declaration_id: DeclarationId) -> Option<(DeclarationId, Option)> { + fn get_superclass( + &self, + declaration_id: DeclarationId, + ) -> Option<(DeclarationId, Option, Vec<(UriId, Offset)>)> { if declaration_id == *BASIC_OBJECT_ID { return None; } let declaration = self.graph.declarations().get(&declaration_id).unwrap(); - let mut explicit_superclasses = Vec::new(); + let mut explicit_superclass = None; + let mut mismatches = Vec::new(); let mut unresolved_superclass = None; for definition_id in declaration.definitions() { @@ -2045,7 +2050,13 @@ impl<'a> Resolver<'a> { match name { NameRef::Resolved(resolved) => { if let Some(superclass_id) = self.resolve_to_namespace(*resolved.declaration_id()) { - explicit_superclasses.push(superclass_id); + match explicit_superclass { + Some(expected_id) if expected_id != superclass_id => { + mismatches.push((constant_reference.uri_id(), constant_reference.offset().clone())); + } + None => explicit_superclass = Some(superclass_id), + _ => {} + } } } NameRef::Unresolved(_) => { @@ -2055,16 +2066,36 @@ impl<'a> Resolver<'a> { } } - // If there's more than one superclass that isn't `Object` and they are different, then there's a superclass - // mismatch error. TODO: We should add a diagnostic here Some(( - explicit_superclasses.first().copied().unwrap_or(*OBJECT_ID), + explicit_superclass.unwrap_or(*OBJECT_ID), unresolved_superclass, + mismatches, )) } fn linearize_superclass(&mut self, declaration_id: DeclarationId, state: &mut ChainState) -> Option { - let (superclass_id, unresolved_superclass) = self.get_superclass(declaration_id)?; + let (superclass_id, unresolved_superclass, mismatches) = self.get_superclass(declaration_id)?; + + if !mismatches.is_empty() { + let class_name = self + .graph + .declarations() + .get(&declaration_id) + .unwrap() + .name() + .to_string(); + for (uri_id, offset) in mismatches { + let diagnostic = Diagnostic::new( + Rule::SuperclassMismatch, + Severity::Error, + uri_id, + offset, + format!("superclass mismatch for class `{class_name}`"), + ); + self.graph.add_document_diagnostic(uri_id, diagnostic); + } + } + let mut result = self.linearize_ancestors(superclass_id); if let Some(name_id) = unresolved_superclass { diff --git a/rust/rubydex/src/resolution_tests.rs b/rust/rubydex/src/resolution_tests.rs index cc9c87ef..7633af95 100644 --- a/rust/rubydex/src/resolution_tests.rs +++ b/rust/rubydex/src/resolution_tests.rs @@ -981,6 +981,57 @@ mod constant_alias_tests { mod superclass_tests { use super::*; + #[test] + fn contradictory_superclasses_emit_diagnostic() { + let mut context = graph_test(); + context.index_uri("file:///foo.rb", { + r" + class Bar; end + class NotBar; end + class Foo < Bar; end + class Foo < NotBar; end + " + }); + context.resolve(); + + assert_diagnostics_eq!( + context, + &["superclass-mismatch: superclass mismatch for class `Foo` (4:13-4:19)"], + severity: Severity::Error + ); + assert_ancestors_eq!(context, "Foo", ["Foo", "Bar", "Object", "Kernel", "BasicObject"]); + } + + #[test] + fn repeated_superclass_does_not_emit_diagnostic() { + let mut context = graph_test(); + context.index_uri("file:///foo.rb", { + r" + class Bar; end + class Foo < Bar; end + class Foo < Bar; end + " + }); + context.resolve(); + + assert_no_diagnostics!(&context); + } + + #[test] + fn superclassless_reopening_does_not_emit_diagnostic() { + let mut context = graph_test(); + context.index_uri("file:///foo.rb", { + r" + class Bar; end + class Foo < Bar; end + class Foo; end + " + }); + context.resolve(); + + assert_no_diagnostics!(&context); + } + #[test] fn linearizing_super_classes() { let mut context = graph_test();