diff --git a/rust/rubydex/src/diagnostic.rs b/rust/rubydex/src/diagnostic.rs index bdd3fe8c8..b5ab412bf 100644 --- a/rust/rubydex/src/diagnostic.rs +++ b/rust/rubydex/src/diagnostic.rs @@ -105,4 +105,10 @@ rules! { TopLevelMixinSelf; // Resolution + KindRedefinition; + ParentRedefinition; + NonClassSuperclass; + CircularDependency; + NonModuleMixin; + UnresolvedConstantReference; } diff --git a/rust/rubydex/src/model/declaration.rs b/rust/rubydex/src/model/declaration.rs index f608d57f4..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}, }; @@ -51,6 +52,56 @@ 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 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 { + 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 +298,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 +401,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/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..948b06e93 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -3,8 +3,8 @@ 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::declaration::{Ancestor, Declaration, DeclarationKind, Namespace}; +use crate::model::definitions::{Definition, DefinitionKind}; use crate::model::document::Document; use crate::model::encoding::Encoding; use crate::model::identity_maps::{IdentityHashMap, IdentityHashSet}; @@ -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 { @@ -608,8 +615,8 @@ 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 declarations_types: HashMap = HashMap::new(); + let mut definition_types: HashMap = HashMap::new(); let mut multi_definition_count = 0; for declaration in self.declarations.values() { @@ -664,7 +671,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!(); @@ -672,7 +679,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/resolution.rs b/rust/rubydex/src/resolution.rs index bcc627eee..774acca77 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -1,19 +1,23 @@ use std::{ collections::{HashSet, VecDeque}, hash::BuildHasher, + ops::Deref, }; -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 +150,9 @@ impl<'a> Resolver<'a> { } self.handle_remaining_definitions(other_ids); + + self.validate_constant_references(); + self.validate_declarations(); } /// Resolves a single constant against the graph. This method is not meant to be used by the resolution phase, but by @@ -1455,8 +1462,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) } @@ -1480,6 +1486,238 @@ 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)); + all_diagnostics.extend(self.validate_superclass(declaration)); + all_diagnostics.extend(self.validate_mixins(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 + } + + 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::>(); + + 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()); + + 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]; + + 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 + } + + 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() + .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 + .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 + } + + 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)] @@ -1910,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(); @@ -2041,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() @@ -2062,8 +2311,6 @@ mod tests { .get(&DeclarationId::from("Foo::Bar::Baz")) .is_none() ); - - assert_no_diagnostics!(&context); } #[test] @@ -2402,7 +2649,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"]); } @@ -2440,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!( @@ -2776,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` @@ -2819,7 +3080,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"]); } @@ -3069,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` @@ -3090,7 +3354,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"]); } @@ -3798,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"); @@ -3815,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"); @@ -4034,7 +4311,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 +4701,132 @@ 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)", + ] + ); + } + + #[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![ + // 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)", + ] + ); + } + + #[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)", + ] + ); + } + + #[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)", + ] + ); + } } 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";