Diagnose contradictory superclasses

rust/rubydex/src/diagnostic.rs

@@ -124,6 +124,7 @@ rules! {
     InvalidMethodVisibility;
 
     // Resolution
+    SuperclassMismatch;
     UndefinedMethodVisibilityTarget;
     UndefinedConstantVisibilityTarget;
 }

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<NameId>)> {
+    fn get_superclass(
+        &self,
+        declaration_id: DeclarationId,
+    ) -> Option<(DeclarationId, Option<NameId>, 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<Ancestors> {
-        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 {

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();