From e53d16339d53105768029dd02eb5bfc08d04dae7 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 11:35:58 -0500 Subject: [PATCH 1/9] Extract `DefinitionKind` Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/model/definitions.rs | 69 +++++++++++++++++++++------ rust/rubydex/src/model/graph.rs | 6 +-- rust/rubydex/src/visualization/dot.rs | 4 +- rust/rubydex/tests/cli.rs | 2 +- 4 files changed, 60 insertions(+), 21 deletions(-) diff --git a/rust/rubydex/src/model/definitions.rs b/rust/rubydex/src/model/definitions.rs index a3f83140b..7d563d9f9 100644 --- a/rust/rubydex/src/model/definitions.rs +++ b/rust/rubydex/src/model/definitions.rs @@ -49,6 +49,45 @@ impl DefinitionFlags { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum DefinitionKind { + Class, + SingletonClass, + Module, + Constant, + ConstantAlias, + GlobalVariable, + InstanceVariable, + ClassVariable, + AttrAccessor, + AttrReader, + AttrWriter, + Method, + MethodAlias, + GlobalVariableAlias, +} + +impl std::fmt::Display for DefinitionKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DefinitionKind::Class => write!(f, "class"), + DefinitionKind::SingletonClass => write!(f, "singleton class"), + DefinitionKind::Module => write!(f, "module"), + DefinitionKind::Constant => write!(f, "constant"), + DefinitionKind::ConstantAlias => write!(f, "constant alias"), + DefinitionKind::Method => write!(f, "method"), + DefinitionKind::AttrAccessor => write!(f, "attr_accessor"), + DefinitionKind::AttrReader => write!(f, "attr_reader"), + DefinitionKind::AttrWriter => write!(f, "attr_writer"), + DefinitionKind::GlobalVariable => write!(f, "global variable"), + DefinitionKind::InstanceVariable => write!(f, "instance variable"), + DefinitionKind::ClassVariable => write!(f, "class variable"), + DefinitionKind::MethodAlias => write!(f, "method alias"), + DefinitionKind::GlobalVariableAlias => write!(f, "global variable alias"), + } + } +} + #[derive(Debug)] pub enum Definition { Class(Box), @@ -115,22 +154,22 @@ impl Definition { } #[must_use] - pub fn kind(&self) -> &'static str { + pub fn kind(&self) -> DefinitionKind { match self { - Definition::Class(_) => "Class", - Definition::SingletonClass(_) => "SingletonClass", - Definition::Module(_) => "Module", - Definition::Constant(_) => "Constant", - Definition::ConstantAlias(_) => "ConstantAlias", - Definition::Method(_) => "Method", - Definition::AttrAccessor(_) => "AttrAccessor", - Definition::AttrReader(_) => "AttrReader", - Definition::AttrWriter(_) => "AttrWriter", - Definition::GlobalVariable(_) => "GlobalVariable", - Definition::InstanceVariable(_) => "InstanceVariable", - Definition::ClassVariable(_) => "ClassVariable", - Definition::MethodAlias(_) => "AliasMethod", - Definition::GlobalVariableAlias(_) => "GlobalVariableAlias", + Definition::Class(_) => DefinitionKind::Class, + Definition::SingletonClass(_) => DefinitionKind::SingletonClass, + Definition::Module(_) => DefinitionKind::Module, + Definition::Constant(_) => DefinitionKind::Constant, + Definition::ConstantAlias(_) => DefinitionKind::ConstantAlias, + Definition::Method(_) => DefinitionKind::Method, + Definition::AttrAccessor(_) => DefinitionKind::AttrAccessor, + Definition::AttrReader(_) => DefinitionKind::AttrReader, + Definition::AttrWriter(_) => DefinitionKind::AttrWriter, + Definition::GlobalVariable(_) => DefinitionKind::GlobalVariable, + Definition::InstanceVariable(_) => DefinitionKind::InstanceVariable, + Definition::ClassVariable(_) => DefinitionKind::ClassVariable, + Definition::MethodAlias(_) => DefinitionKind::MethodAlias, + Definition::GlobalVariableAlias(_) => DefinitionKind::GlobalVariableAlias, } } diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index bf94cdf04..7e32a1104 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -4,7 +4,7 @@ use std::sync::LazyLock; use crate::diagnostic::Diagnostic; use crate::indexing::local_graph::LocalGraph; use crate::model::declaration::{Ancestor, Declaration, Namespace}; -use crate::model::definitions::Definition; +use crate::model::definitions::{Definition, DefinitionKind}; use crate::model::document::Document; use crate::model::encoding::Encoding; use crate::model::identity_maps::{IdentityHashMap, IdentityHashSet}; @@ -609,7 +609,7 @@ impl Graph { let mut declarations_with_docs = 0; let mut total_doc_size = 0; let mut declarations_types: HashMap<&str, usize> = HashMap::new(); - let mut definition_types: HashMap<&str, usize> = HashMap::new(); + let mut definition_types: HashMap = HashMap::new(); let mut multi_definition_count = 0; for declaration in self.declarations.values() { @@ -672,7 +672,7 @@ impl Graph { let mut types: Vec<_> = definition_types.iter().collect(); types.sort_by_key(|(_, count)| std::cmp::Reverse(**count)); for (kind, count) in types { - println!(" {kind:20} {count:6}"); + println!(" {kind:20} {count:6}", kind = kind.to_string()); } } } diff --git a/rust/rubydex/src/visualization/dot.rs b/rust/rubydex/src/visualization/dot.rs index 83923b061..57894cb2b 100644 --- a/rust/rubydex/src/visualization/dot.rs +++ b/rust/rubydex/src/visualization/dot.rs @@ -160,8 +160,8 @@ mod tests { "Name:TestModule" [label="TestModule",shape=hexagon]; "Name:TestModule" -> "def_{module_def_id}" [dir=both]; - "def_{class_def_id}" [label="Class(TestClass)",shape=ellipse]; - "def_{module_def_id}" [label="Module(TestModule)",shape=ellipse]; + "def_{class_def_id}" [label="class(TestClass)",shape=ellipse]; + "def_{module_def_id}" [label="module(TestModule)",shape=ellipse]; "file:///test.rb" [label="test.rb",shape=box]; "def_{class_def_id}" -> "file:///test.rb"; diff --git a/rust/rubydex/tests/cli.rs b/rust/rubydex/tests/cli.rs index f77ca5fd8..b3bf268cd 100644 --- a/rust/rubydex/tests/cli.rs +++ b/rust/rubydex/tests/cli.rs @@ -101,7 +101,7 @@ fn visualize_simple_class() { "Name:SimpleClass" [label="SimpleClass",shape=hexagon]; "Name:SimpleClass" -> "def_" [dir=both]; - "def_" [label="Class(SimpleClass)",shape=ellipse]; + "def_" [label="class(SimpleClass)",shape=ellipse]; "file:///simple.rb" [label="simple.rb",shape=box]; "def_" -> "file:///simple.rb"; From 67f8568d5d7667c2e537edd2242a9a2722df8fb1 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 11:52:55 -0500 Subject: [PATCH 2/9] Extract `DeclarationKind` Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/model/declaration.rs | 51 +++++++++++++++++++++------ rust/rubydex/src/model/graph.rs | 6 ++-- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index f608d57f4..bad5c5e83 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -51,6 +51,35 @@ impl<'a> IntoIterator for &'a Ancestors { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum DeclarationKind { + Class, + SingletonClass, + Module, + Constant, + ConstantAlias, + Method, + GlobalVariable, + InstanceVariable, + ClassVariable, +} + +impl std::fmt::Display for DeclarationKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DeclarationKind::Class => write!(f, "class"), + DeclarationKind::SingletonClass => write!(f, "singleton class"), + DeclarationKind::Module => write!(f, "module"), + DeclarationKind::Constant => write!(f, "constant"), + DeclarationKind::ConstantAlias => write!(f, "constant alias"), + DeclarationKind::Method => write!(f, "method"), + DeclarationKind::GlobalVariable => write!(f, "global variable"), + DeclarationKind::InstanceVariable => write!(f, "instance variable"), + DeclarationKind::ClassVariable => write!(f, "class variable"), + } + } +} + macro_rules! all_declarations { ($value:expr, $var:ident => $expr:expr) => { match $value { @@ -247,15 +276,15 @@ impl Declaration { } #[must_use] - pub fn kind(&self) -> &'static str { + pub fn kind(&self) -> DeclarationKind { match self { Declaration::Namespace(namespace) => namespace.kind(), - Declaration::Constant(_) => "Constant", - Declaration::ConstantAlias(_) => "ConstantAlias", - Declaration::Method(_) => "Method", - Declaration::GlobalVariable(_) => "GlobalVariable", - Declaration::InstanceVariable(_) => "InstanceVariable", - Declaration::ClassVariable(_) => "ClassVariable", + Declaration::Constant(_) => DeclarationKind::Constant, + Declaration::ConstantAlias(_) => DeclarationKind::ConstantAlias, + Declaration::Method(_) => DeclarationKind::Method, + Declaration::GlobalVariable(_) => DeclarationKind::GlobalVariable, + Declaration::InstanceVariable(_) => DeclarationKind::InstanceVariable, + Declaration::ClassVariable(_) => DeclarationKind::ClassVariable, } } @@ -350,11 +379,11 @@ pub enum Namespace { impl Namespace { #[must_use] - pub fn kind(&self) -> &'static str { + pub fn kind(&self) -> DeclarationKind { match self { - Namespace::Class(_) => "Class", - Namespace::SingletonClass(_) => "SingletonClass", - Namespace::Module(_) => "Module", + Namespace::Class(_) => DeclarationKind::Class, + Namespace::SingletonClass(_) => DeclarationKind::SingletonClass, + Namespace::Module(_) => DeclarationKind::Module, } } diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 7e32a1104..ceec1cc77 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -3,7 +3,7 @@ use std::sync::LazyLock; use crate::diagnostic::Diagnostic; use crate::indexing::local_graph::LocalGraph; -use crate::model::declaration::{Ancestor, Declaration, Namespace}; +use crate::model::declaration::{Ancestor, Declaration, DeclarationKind, Namespace}; use crate::model::definitions::{Definition, DefinitionKind}; use crate::model::document::Document; use crate::model::encoding::Encoding; @@ -608,7 +608,7 @@ impl Graph { let mut declarations_with_docs = 0; let mut total_doc_size = 0; - let mut declarations_types: HashMap<&str, usize> = HashMap::new(); + let mut declarations_types: HashMap = HashMap::new(); let mut definition_types: HashMap = HashMap::new(); let mut multi_definition_count = 0; @@ -664,7 +664,7 @@ impl Graph { let mut types: Vec<_> = declarations_types.iter().collect(); types.sort_by_key(|(_, count)| std::cmp::Reverse(**count)); for (kind, count) in types { - println!(" {kind:20} {count:6}"); + println!(" {kind:20} {count:6}", kind = kind.to_string()); } println!(); From 6a78e5bebf6c29f31f239e18bdc58d415d384879 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Wed, 14 Jan 2026 12:34:21 -0500 Subject: [PATCH 3/9] Add resolution diagnostic for kind mismatch Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/diagnostic.rs | 1 + rust/rubydex/src/model/declaration.rs | 22 +++++ rust/rubydex/src/model/graph.rs | 7 ++ rust/rubydex/src/resolution.rs | 113 +++++++++++++++++++++++--- 4 files changed, 132 insertions(+), 11 deletions(-) diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index bdd3fe8c8..90d5b8548 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -105,4 +105,5 @@ rules! { TopLevelMixinSelf; // Resolution + KindRedefinition; } diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index bad5c5e83..28fa74340 100644 --- a/rust/rubydex/src/model/declaration.rs +++ b/rust/rubydex/src/model/declaration.rs @@ -1,4 +1,5 @@ use crate::model::{ + definitions::DefinitionKind, identity_maps::{IdentityHashMap, IdentityHashSet}, ids::{DeclarationId, DefinitionId, NameId, ReferenceId, StringId}, }; @@ -64,6 +65,27 @@ pub enum DeclarationKind { ClassVariable, } +impl DeclarationKind { + #[must_use] + pub fn from_definition_kind(definition_kind: DefinitionKind) -> Self { + match definition_kind { + DefinitionKind::Class => DeclarationKind::Class, + DefinitionKind::SingletonClass => DeclarationKind::SingletonClass, + DefinitionKind::Module => DeclarationKind::Module, + DefinitionKind::Constant => DeclarationKind::Constant, + DefinitionKind::ConstantAlias => DeclarationKind::ConstantAlias, + DefinitionKind::Method + | DefinitionKind::MethodAlias + | DefinitionKind::AttrAccessor + | DefinitionKind::AttrReader + | DefinitionKind::AttrWriter => DeclarationKind::Method, + DefinitionKind::InstanceVariable => DeclarationKind::InstanceVariable, + DefinitionKind::ClassVariable => DeclarationKind::ClassVariable, + DefinitionKind::GlobalVariable | DefinitionKind::GlobalVariableAlias => DeclarationKind::GlobalVariable, + } + } +} + impl std::fmt::Display for DeclarationKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index ceec1cc77..948b06e93 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -172,11 +172,18 @@ impl Graph { &self.method_references } + // Diagnostics + #[must_use] pub fn diagnostics(&self) -> &Vec { &self.diagnostics } + #[must_use] + pub fn diagnostics_mut(&mut self) -> &mut Vec { + &mut self.diagnostics + } + /// Interns a string in the graph unless already interned. This method is only used to back the /// `Graph#resolve_constant` Ruby API because every string must be interned in the graph to properly resolve. pub fn intern_string(&mut self, string: String) -> StringId { diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index bcc627eee..d58e125b8 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -3,17 +3,20 @@ use std::{ hash::BuildHasher, }; -use crate::model::{ - declaration::{ - Ancestor, Ancestors, ClassDeclaration, ClassVariableDeclaration, ConstantAliasDeclaration, ConstantDeclaration, - Declaration, GlobalVariableDeclaration, InstanceVariableDeclaration, MethodDeclaration, ModuleDeclaration, - Namespace, SingletonClassDeclaration, +use crate::{ + diagnostic::{Diagnostic, Rule}, + model::{ + declaration::{ + Ancestor, Ancestors, ClassDeclaration, ClassVariableDeclaration, ConstantAliasDeclaration, + ConstantDeclaration, Declaration, DeclarationKind, GlobalVariableDeclaration, InstanceVariableDeclaration, + MethodDeclaration, ModuleDeclaration, Namespace, SingletonClassDeclaration, + }, + definitions::{Definition, Mixin}, + graph::{CLASS_ID, Graph, MODULE_ID, OBJECT_ID}, + identity_maps::{IdentityHashMap, IdentityHashSet}, + ids::{DeclarationId, DefinitionId, NameId, ReferenceId, StringId}, + name::{Name, NameRef}, }, - definitions::{Definition, Mixin}, - graph::{CLASS_ID, Graph, MODULE_ID, OBJECT_ID}, - identity_maps::{IdentityHashMap, IdentityHashSet}, - ids::{DeclarationId, DefinitionId, NameId, ReferenceId, StringId}, - name::{Name, NameRef}, }; pub enum Unit { @@ -146,6 +149,8 @@ impl<'a> Resolver<'a> { } self.handle_remaining_definitions(other_ids); + + self.validate_declarations(); } /// Resolves a single constant against the graph. This method is not meant to be used by the resolution phase, but by @@ -1480,6 +1485,45 @@ impl<'a> Resolver<'a> { _ => None, } } + + fn validate_declarations(&mut self) { + let mut all_diagnostics = Vec::new(); + + for declaration in self.graph.declarations().values() { + all_diagnostics.extend(self.validate_kind(declaration)); + } + + self.graph.diagnostics_mut().extend(all_diagnostics); + } + + fn validate_kind(&self, declaration: &Declaration) -> Vec { + let mut diagnostics = Vec::new(); + + if declaration.definitions().len() > 1 { + let first_definition = self.graph.definitions().get(&declaration.definitions()[0]).unwrap(); + let first_definition_declaration_kind = DeclarationKind::from_definition_kind(first_definition.kind()); + + for definition in declaration.definitions().iter().skip(1) { + let definition = self.graph.definitions().get(definition).unwrap(); + let definition_declaration_kind = DeclarationKind::from_definition_kind(definition.kind()); + if definition_declaration_kind != first_definition_declaration_kind { + diagnostics.push(Diagnostic::new( + Rule::KindRedefinition, + *definition.uri_id(), + definition.offset().clone(), + format!( + "Redefining `{}` as `{}`, previously defined as `{}`", + declaration.name(), + definition_declaration_kind, + first_definition_declaration_kind + ), + )); + } + } + } + + diagnostics + } } #[cfg(test)] @@ -4034,7 +4078,13 @@ mod tests { " }); context.resolve(); - assert_no_diagnostics!(&context, &[Rule::ParseWarning]); + assert_diagnostics_eq!( + &context, + vec![ + "kind-redefinition: Redefining `Outer::NESTED` as `class`, previously defined as `constant alias` (9:1-11:4)" + ], + &[Rule::ParseWarning] + ); assert_constant_alias_target_eq!(context, "ALIAS", "Outer"); assert_constant_alias_target_eq!(context, "Outer::NESTED", "Outer::Inner"); @@ -4418,4 +4468,45 @@ mod tests { assert_no_members!(context, "Foo"); assert_members_eq!(context, "Bar::Foo", vec!["FOO"]); } + + // Diagnostics tests + + #[test] + fn resolution_diagnostics_for_kind_redefinition() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + module Foo; end + class Foo; end + + class Bar; end + module Bar; end + + class Baz; end + Baz = 123 + + module Qux; end + module Qux; end + + def foo; end + attr_reader :foo + + class Qaz + class Array; end + def Array; end + end + " + }); + + context.resolve(); + + assert_diagnostics_eq!( + &context, + vec![ + "kind-redefinition: Redefining `Foo` as `class`, previously defined as `module` (2:1-2:15)", + "kind-redefinition: Redefining `Bar` as `module`, previously defined as `class` (5:1-5:16)", + "kind-redefinition: Redefining `Baz` as `constant`, previously defined as `class` (8:1-8:4)", + ] + ); + } } From 410f67978236f41d5d137e463e8292dc1133978f Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 11:58:09 -0500 Subject: [PATCH 4/9] Add resolution diagnostic for redefinition of parent Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/diagnostic.rs | 1 + rust/rubydex/src/resolution.rs | 97 +++++++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index 90d5b8548..04483250b 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -106,4 +106,5 @@ rules! { // Resolution KindRedefinition; + ParentRedefinition; } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index d58e125b8..02ff61d47 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1,6 +1,7 @@ use std::{ collections::{HashSet, VecDeque}, hash::BuildHasher, + ops::Deref, }; use crate::{ @@ -1460,8 +1461,7 @@ impl<'a> Resolver<'a> { } } - // If there's more than one parent class that isn't `Object` and they are different, then there's a superclass - // mismatch error. TODO: We should add a diagnostic here + // If there's more than one parent class that isn't `Object` and they are different, then there's a superclass mismatch error. (explicit_parents.first().copied().unwrap_or(*OBJECT_ID), partial) } @@ -1491,6 +1491,7 @@ impl<'a> Resolver<'a> { for declaration in self.graph.declarations().values() { all_diagnostics.extend(self.validate_kind(declaration)); + all_diagnostics.extend(self.validate_superclass(declaration)); } self.graph.diagnostics_mut().extend(all_diagnostics); @@ -1524,6 +1525,67 @@ impl<'a> Resolver<'a> { diagnostics } + + fn validate_superclass(&self, declaration: &Declaration) -> Vec { + let mut diagnostics = Vec::new(); + + let mut explicit_parents = declaration + .definitions() + .iter() + .filter_map(|definition_id| { + let definition = self.graph.definitions().get(definition_id).unwrap(); + if let Definition::Class(class) = definition { + if let Some(superclass) = class.superclass_ref() { + let name = self + .graph + .names() + .get(self.graph.constant_references().get(superclass).unwrap().name_id()) + .unwrap(); + match name { + NameRef::Resolved(resolved) => Some((resolved, superclass, definition)), + NameRef::Unresolved(_) => None, + } + } else { + None + } + } else { + None + } + }) + .collect::>(); + + // Dedup explicit parents that resolve to the same declaration + explicit_parents.dedup_by(|(name_a, _, _), (name_b, _, _)| name_a.declaration_id() == name_b.declaration_id()); + + if explicit_parents.len() > 1 { + let (first_parent_name, _first_superclass, _first_definition) = explicit_parents[0]; + + for (parent_name, superclass, definition) in explicit_parents.iter().skip(1) { + diagnostics.push(Diagnostic::new( + Rule::ParentRedefinition, + *definition.uri_id(), + self.graph + .constant_references() + .get(superclass) + .unwrap() + .offset() + .clone(), + format!( + "Parent of class `{}` redefined from `{}` to `{}`", + declaration.name(), + self.graph + .strings() + .get(first_parent_name.name().str()) + .unwrap() + .deref(), + self.graph.strings().get(parent_name.name().str()).unwrap().deref() + ), + )); + } + } + + diagnostics + } } #[cfg(test)] @@ -4509,4 +4571,35 @@ mod tests { ] ); } + + #[test] + fn resolution_diagnostics_for_parent_redefinition() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + class Parent1; end + class Parent2; end + + class Child1; end + class Child1; end + class Child1 < Object; end + class Child1 < ::Object; end + + class Child2; end + class Child2 < Parent1; end + class ::Child2 < ::Parent1; end + + class Child3; end + class Child3 < Parent1; end + class Child3 < Parent2; end + " + }); + + context.resolve(); + + assert_diagnostics_eq!( + &context, + vec!["parent-redefinition: Parent of class `Child3` redefined from `Parent1` to `Parent2` (15:16-15:23)",] + ); + } } From 2e0a591eb4a650623c51010c9c141b1e30b6c54c Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 12:31:06 -0500 Subject: [PATCH 5/9] Add resolution diagnostic for non-class parents Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/diagnostic.rs | 1 + rust/rubydex/src/resolution.rs | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index 04483250b..fb8005c19 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -107,4 +107,5 @@ rules! { // Resolution KindRedefinition; ParentRedefinition; + NonClassSuperclass; } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 02ff61d47..58bc403b5 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1557,6 +1557,28 @@ impl<'a> Resolver<'a> { // Dedup explicit parents that resolve to the same declaration explicit_parents.dedup_by(|(name_a, _, _), (name_b, _, _)| name_a.declaration_id() == name_b.declaration_id()); + for (parent_name, superclass, definition) in &explicit_parents { + let parent_declaration = self.graph.declarations().get(parent_name.declaration_id()).unwrap(); + if parent_declaration.kind() != DeclarationKind::Class { + diagnostics.push(Diagnostic::new( + Rule::NonClassSuperclass, + *definition.uri_id(), + self.graph + .constant_references() + .get(superclass) + .unwrap() + .offset() + .clone(), + format!( + "Superclass `{}` of `{}` is not a class (found `{}`)", + parent_declaration.name(), + declaration.name(), + parent_declaration.kind() + ), + )); + } + } + if explicit_parents.len() > 1 { let (first_parent_name, _first_superclass, _first_definition) = explicit_parents[0]; @@ -4602,4 +4624,28 @@ mod tests { vec!["parent-redefinition: Parent of class `Child3` redefined from `Parent1` to `Parent2` (15:16-15:23)",] ); } + + #[test] + fn resolution_diagnostics_for_non_class_superclass() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + module Parent1; end + Parent2 = 42 + + class Child1 < Parent1; end + class Child2 < Parent2; end + " + }); + + context.resolve(); + + assert_diagnostics_eq!( + &context, + vec![ + "non-class-superclass: Superclass `Parent1` of `Child1` is not a class (found `module`) (4:16-4:23)", + "non-class-superclass: Superclass `Parent2` of `Child2` is not a class (found `constant`) (5:16-5:23)", + ] + ); + } } From 7f8cfb2b2c5206b83e3bf17e2fa6473eac6123b5 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 14:26:28 -0500 Subject: [PATCH 6/9] Add resolution diagnostic for circular superclasses Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/diagnostic.rs | 1 + rust/rubydex/src/resolution.rs | 27 ++++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index fb8005c19..aa8e8e076 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -108,4 +108,5 @@ rules! { KindRedefinition; ParentRedefinition; NonClassSuperclass; + CircularDependency; } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 58bc403b5..140828e5c 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1554,6 +1554,24 @@ impl<'a> Resolver<'a> { }) .collect::>(); + if !explicit_parents.is_empty() + && matches!(declaration.as_namespace().unwrap().ancestors(), Ancestors::Cyclic(_)) + { + for (_parent_name, superclass, definition) in &explicit_parents { + diagnostics.push(Diagnostic::new( + Rule::CircularDependency, + *definition.uri_id(), + self.graph + .constant_references() + .get(superclass) + .unwrap() + .offset() + .clone(), + format!("Circular dependency: `{}` is parent of itself", declaration.name()), + )); + } + } + // Dedup explicit parents that resolve to the same declaration explicit_parents.dedup_by(|(name_a, _, _), (name_b, _, _)| name_a.declaration_id() == name_b.declaration_id()); @@ -2530,7 +2548,14 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context); + assert_diagnostics_eq!( + &context, + vec![ + "circular-dependency: Circular dependency: `Foo` is parent of itself (1:13-1:16)", + "circular-dependency: Circular dependency: `Bar` is parent of itself (2:13-2:16)", + "circular-dependency: Circular dependency: `Baz` is parent of itself (3:13-3:16)" + ] + ); assert_ancestors_eq!(context, "Foo", ["Foo", "Bar", "Baz", "Object"]); } From 77fa76522bafaa6aefa21a569a00fc541632574c Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 16:49:34 -0500 Subject: [PATCH 7/9] Add resolution diagnostic for non module mixins Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/diagnostic.rs | 1 + rust/rubydex/src/resolution.rs | 76 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index aa8e8e076..2179b4363 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -109,4 +109,5 @@ rules! { ParentRedefinition; NonClassSuperclass; CircularDependency; + NonModuleMixin; } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 140828e5c..7a24e6689 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1492,6 +1492,7 @@ impl<'a> Resolver<'a> { for declaration in self.graph.declarations().values() { all_diagnostics.extend(self.validate_kind(declaration)); all_diagnostics.extend(self.validate_superclass(declaration)); + all_diagnostics.extend(self.validate_mixins(declaration)); } self.graph.diagnostics_mut().extend(all_diagnostics); @@ -1626,6 +1627,54 @@ impl<'a> Resolver<'a> { diagnostics } + + fn validate_mixins(&self, declaration: &Declaration) -> Vec { + let mut diagnostics = Vec::new(); + + let mixins = declaration + .definitions() + .iter() + .filter_map(|definition_id| { + self.mixins_of(*definition_id).map(|mixins| { + mixins + .into_iter() + .map(|mixin| (mixin, *definition_id)) + .collect::>() + }) + }) + .flatten(); + + for (mixin, definition_id) in mixins { + let constant_reference = self + .graph + .constant_references() + .get(mixin.constant_reference_id()) + .unwrap(); + + if let NameRef::Resolved(resolved) = self.graph.names().get(constant_reference.name_id()).unwrap() { + let definition = self.graph.definitions().get(&definition_id).unwrap(); + let mixin_declaration = self.graph.declarations().get(resolved.declaration_id()).unwrap(); + if mixin_declaration.kind() != DeclarationKind::Module { + diagnostics.push(Diagnostic::new( + Rule::NonModuleMixin, + *definition.uri_id(), + self.graph + .constant_references() + .get(mixin.constant_reference_id()) + .unwrap() + .offset() + .clone(), + format!( + "Mixin `{}` is not a module", + self.graph.strings().get(resolved.name().str()).unwrap().deref() + ), + )); + } + } + } + + diagnostics + } } #[cfg(test)] @@ -4673,4 +4722,31 @@ mod tests { ] ); } + + #[test] + fn resolution_diagnostics_for_non_module_mixin() { + let mut context = GraphTest::new(); + context.index_uri("file:///foo.rb", { + r" + class Mixin; end + + class Child + include Mixin; + prepend Mixin; + extend Mixin; + end + " + }); + + context.resolve(); + + assert_diagnostics_eq!( + &context, + vec![ + "non-module-mixin: Mixin `Mixin` is not a module (4:11-4:16)", + "non-module-mixin: Mixin `Mixin` is not a module (5:11-5:16)", + "non-module-mixin: Mixin `Mixin` is not a module (6:10-6:15)", + ] + ); + } } From 2c3e79afb8c9f0642112b1adac9cd4272c8caf62 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 17:45:05 -0500 Subject: [PATCH 8/9] Add resolution diagnostic for circular mixins Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/resolution.rs | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 7a24e6689..fcbd57be2 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1642,7 +1642,28 @@ impl<'a> Resolver<'a> { .collect::>() }) }) - .flatten(); + .flatten() + .collect::>(); + + if !mixins.is_empty() + && let Some(namespace) = declaration.as_namespace() + && matches!(namespace.ancestors(), Ancestors::Cyclic(_)) + { + for (mixin, definition_id) in &mixins { + let definition = self.graph.definitions().get(definition_id).unwrap(); + diagnostics.push(Diagnostic::new( + Rule::CircularDependency, + *definition.uri_id(), + self.graph + .constant_references() + .get(mixin.constant_reference_id()) + .unwrap() + .offset() + .clone(), + format!("Circular dependency: `{}` is parent of itself", declaration.name()), + )); + } + } for (mixin, definition_id) in mixins { let constant_reference = self @@ -3021,7 +3042,10 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context); + assert_diagnostics_eq!( + &context, + vec!["circular-dependency: Circular dependency: `Foo` is parent of itself (2:11-2:14)"] + ); assert_ancestors_eq!(context, "Foo", ["Foo"]); } @@ -3292,7 +3316,10 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context); + assert_diagnostics_eq!( + &context, + vec!["circular-dependency: Circular dependency: `Foo` is parent of itself (2:11-2:14)"] + ); assert_ancestors_eq!(context, "Foo", ["Foo"]); } From 38517964faefbdddb246e3267231d39862d0ee2e Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 15 Jan 2026 17:56:59 -0500 Subject: [PATCH 9/9] Add resolution diagnostic for unresolved constant reference Signed-off-by: Alexandre Terrasa --- rust/rubydex/src/diagnostic.rs | 1 + rust/rubydex/src/resolution.rs | 71 +++++++++++++++++++++++++++++----- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index 2179b4363..b5ab412bf 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -110,4 +110,5 @@ rules! { NonClassSuperclass; CircularDependency; NonModuleMixin; + UnresolvedConstantReference; } diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index fcbd57be2..774acca77 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -151,6 +151,7 @@ impl<'a> Resolver<'a> { self.handle_remaining_definitions(other_ids); + self.validate_constant_references(); self.validate_declarations(); } @@ -1696,6 +1697,27 @@ impl<'a> Resolver<'a> { diagnostics } + + fn validate_constant_references(&mut self) { + let mut diagnostics = Vec::new(); + + for reference in self.graph.constant_references().values() { + let name = self.graph.names().get(reference.name_id()).unwrap(); + if let NameRef::Unresolved(_) = name { + diagnostics.push(Diagnostic::new( + Rule::UnresolvedConstantReference, + reference.uri_id(), + reference.offset().clone(), + format!( + "Unresolved constant reference: `{}`", + self.graph.strings().get(name.str()).unwrap().deref() + ), + )); + } + } + + self.graph.diagnostics_mut().extend(diagnostics); + } } #[cfg(test)] @@ -2126,7 +2148,11 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context, &[Rule::ParseWarning]); + assert_diagnostics_eq!( + &context, + vec!["unresolved-constant-reference: Unresolved constant reference: `Foo` (1:1-1:4)"], + &[Rule::ParseWarning] + ); let reference = context.graph().constant_references().values().next().unwrap(); @@ -2257,6 +2283,13 @@ mod tests { " }); context.resolve(); + + assert_diagnostics_eq!( + &context, + vec!["unresolved-constant-reference: Unresolved constant reference: `Foo` (1:7-1:10)"], + &[Rule::ParseWarning] + ); + assert!( context .graph() @@ -2278,8 +2311,6 @@ mod tests { .get(&DeclarationId::from("Foo::Bar::Baz")) .is_none() ); - - assert_no_diagnostics!(&context); } #[test] @@ -2663,7 +2694,14 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context); + assert_diagnostics_eq!( + &context, + vec![ + "unresolved-constant-reference: Unresolved constant reference: `Foo` (1:13-1:16)", + "unresolved-constant-reference: Unresolved constant reference: `CONST` (2:3-2:8)", + ], + &[Rule::ParseWarning] + ); let declaration = context.graph().declarations().get(&DeclarationId::from("Bar")).unwrap(); assert!(matches!( @@ -2999,7 +3037,7 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context); + assert_no_diagnostics!(&context, &[Rule::UnresolvedConstantReference]); assert_ancestors_eq!(context, "B", ["B"]); // TODO: this is a temporary hack to avoid crashing on `Struct.new`, `Class.new` and `Module.new` @@ -3295,7 +3333,7 @@ mod tests { }); context.resolve(); - assert_no_diagnostics!(&context); + assert_no_diagnostics!(&context, &[Rule::UnresolvedConstantReference]); assert_ancestors_eq!(context, "B", ["B"]); // TODO: this is a temporary hack to avoid crashing on `Struct.new`, `Class.new` and `Module.new` @@ -4027,7 +4065,12 @@ mod tests { " }); context.resolve(); - assert_no_diagnostics!(&context); + + assert_diagnostics_eq!( + &context, + vec!["unresolved-constant-reference: Unresolved constant reference: `NonExistent` (1:11-1:22)"], + &[Rule::ParseWarning] + ); assert_constant_alias_target_eq!(context, "ALIAS_2", "ALIAS_1"); assert_no_constant_alias_target!(context, "ALIAS_1"); @@ -4044,7 +4087,12 @@ mod tests { " }); context.resolve(); - assert_no_diagnostics!(&context, &[Rule::ParseWarning]); + + assert_diagnostics_eq!( + &context, + vec!["unresolved-constant-reference: Unresolved constant reference: `NOPE` (3:8-3:12)"], + &[Rule::ParseWarning] + ); assert_constant_alias_target_eq!(context, "ALIAS", "VALUE"); @@ -4722,7 +4770,12 @@ mod tests { assert_diagnostics_eq!( &context, - vec!["parent-redefinition: Parent of class `Child3` redefined from `Parent1` to `Parent2` (15:16-15:23)",] + vec![ + // FIXME: Object should resolve + "unresolved-constant-reference: Unresolved constant reference: `Object` (6:16-6:22)", + "unresolved-constant-reference: Unresolved constant reference: `Object` (7:16-7:24)", + "parent-redefinition: Parent of class `Child3` redefined from `Parent1` to `Parent2` (15:16-15:23)", + ] ); }