From 60b8df13099954de650395f01f31a57a800b7cc7 Mon Sep 17 00:00:00 2001 From: Alex Rocha Date: Fri, 27 Feb 2026 14:51:31 -0800 Subject: [PATCH] Index constants with RBS indexer This adds visit_constant_node so top-level, qualified, and class-nested constants are indexed. Also extracts collect_comments to deduplicate comment collection across the class, module, and constant visitors. --- rust/rubydex/src/indexing/rbs_indexer.rs | 124 +++++++++++++++--- rust/rubydex/src/indexing/ruby_indexer.rs | 23 +--- rust/rubydex/src/resolution.rs | 31 +++++ .../src/test_utils/local_graph_test.rs | 21 +++ 4 files changed, 159 insertions(+), 40 deletions(-) diff --git a/rust/rubydex/src/indexing/rbs_indexer.rs b/rust/rubydex/src/indexing/rbs_indexer.rs index 05c83640..308b531b 100644 --- a/rust/rubydex/src/indexing/rbs_indexer.rs +++ b/rust/rubydex/src/indexing/rbs_indexer.rs @@ -1,11 +1,11 @@ //! Visit the RBS AST and create type definitions. -use ruby_rbs::node::{self, ClassNode, ModuleNode, Node, TypeNameNode, Visit}; +use ruby_rbs::node::{self, ClassNode, CommentNode, ConstantNode, ModuleNode, Node, TypeNameNode, Visit}; use crate::diagnostic::Rule; use crate::indexing::local_graph::LocalGraph; use crate::model::comment::Comment; -use crate::model::definitions::{ClassDefinition, Definition, DefinitionFlags, ModuleDefinition}; +use crate::model::definitions::{ClassDefinition, ConstantDefinition, Definition, DefinitionFlags, ModuleDefinition}; use crate::model::document::Document; use crate::model::ids::{DefinitionId, NameId, UriId}; use crate::model::name::{Name, ParentScope}; @@ -112,6 +112,16 @@ impl<'a> RBSIndexer<'a> { } } + fn collect_comments(comment_node: Option) -> Vec { + comment_node + .into_iter() + .map(|comment| { + let text = Self::bytes_to_string(comment.string().as_bytes()); + Comment::new(Offset::from_rbs_location(&comment.location()), text) + }) + .collect() + } + fn register_definition( &mut self, definition: Definition, @@ -135,14 +145,7 @@ impl Visit for RBSIndexer<'_> { let offset = Offset::from_rbs_location(&class_node.location()); let name_offset = Offset::from_rbs_location(&type_name.name().location()); - let comments: Vec<_> = class_node - .comment() - .into_iter() - .map(|comment| { - let text = Self::bytes_to_string(comment.string().as_bytes()); - Comment::new(Offset::from_rbs_location(&comment.location()), text) - }) - .collect(); + let comments = Self::collect_comments(class_node.comment()); let superclass_ref = class_node.super_class().as_ref().map(|super_node| { let type_name = super_node.name(); @@ -182,14 +185,7 @@ impl Visit for RBSIndexer<'_> { let offset = Offset::from_rbs_location(&module_node.location()); let name_offset = Offset::from_rbs_location(&type_name.name().location()); - let comments: Vec<_> = module_node - .comment() - .into_iter() - .map(|comment| { - let text = Self::bytes_to_string(comment.string().as_bytes()); - Comment::new(Offset::from_rbs_location(&comment.location()), text) - }) - .collect(); + let comments = Self::collect_comments(module_node.comment()); let definition = Definition::Module(Box::new(ModuleDefinition::new( name_id, @@ -210,14 +206,36 @@ impl Visit for RBSIndexer<'_> { self.nesting_stack.pop(); } + + fn visit_constant_node(&mut self, constant_node: &ConstantNode) { + let lexical_nesting_id = self.parent_lexical_scope_id(); + let nesting_name_id = self.nesting_name_id(lexical_nesting_id); + + let type_name = constant_node.name(); + let name_id = self.index_type_name(&type_name, nesting_name_id); + let offset = Offset::from_rbs_location(&constant_node.location()); + + let comments = Self::collect_comments(constant_node.comment()); + + let definition = Definition::Constant(Box::new(ConstantDefinition::new( + name_id, + self.uri_id, + offset, + comments, + DefinitionFlags::empty(), + lexical_nesting_id, + ))); + + self.register_definition(definition, lexical_nesting_id); + } } #[cfg(test)] mod tests { use crate::test_utils::LocalGraphTest; use crate::{ - assert_def_name_eq, assert_def_name_offset_eq, assert_def_superclass_ref_eq, assert_definition_at, - assert_local_diagnostics_eq, assert_no_local_diagnostics, + assert_def_comments_eq, assert_def_name_eq, assert_def_name_offset_eq, assert_def_superclass_ref_eq, + assert_definition_at, assert_local_diagnostics_eq, assert_no_local_diagnostics, }; fn index_source(source: &str) -> LocalGraphTest { @@ -346,6 +364,72 @@ mod tests { }); } + #[test] + fn index_constant_node() { + let context = index_source("FOO: String"); + + assert_no_local_diagnostics!(&context); + assert_eq!(context.graph().definitions().len(), 1); + + assert_definition_at!(&context, "1:1-1:12", Constant, |def| { + assert_def_name_eq!(&context, def, "FOO"); + assert!(def.lexical_nesting_id().is_none()); + }); + } + + #[test] + fn index_qualified_constant_node() { + let context = index_source("Foo::BAR: String"); + + assert_no_local_diagnostics!(&context); + assert_eq!(context.graph().definitions().len(), 1); + + assert_definition_at!(&context, "1:1-1:17", Constant, |def| { + assert_def_name_eq!(&context, def, "Foo::BAR"); + }); + } + + #[test] + fn index_constant_inside_class() { + let context = index_source({ + " + class Foo + FOO: Integer + end + " + }); + + assert_no_local_diagnostics!(&context); + + assert_definition_at!(&context, "1:1-3:4", Class, |class_def| { + assert_def_name_eq!(&context, class_def, "Foo"); + assert_eq!(1, class_def.members().len()); + + assert_definition_at!(&context, "2:3-2:15", Constant, |def| { + assert_def_name_eq!(&context, def, "FOO"); + assert_eq!(class_def.id(), def.lexical_nesting_id().unwrap()); + assert_eq!(class_def.members()[0], def.id()); + }); + }); + } + + #[test] + fn index_constant_node_with_comment() { + let context = index_source({ + " + # Some documentation + FOO: String + " + }); + + assert_no_local_diagnostics!(&context); + + assert_definition_at!(&context, "2:1-2:12", Constant, |def| { + assert_def_name_eq!(&context, def, "FOO"); + assert_def_comments_eq!(&context, def, ["Some documentation\n"]); + }); + } + #[test] fn index_class_and_module_nesting() { let context = index_source({ diff --git a/rust/rubydex/src/indexing/ruby_indexer.rs b/rust/rubydex/src/indexing/ruby_indexer.rs index 62350d2f..0adc4641 100644 --- a/rust/rubydex/src/indexing/ruby_indexer.rs +++ b/rust/rubydex/src/indexing/ruby_indexer.rs @@ -2035,9 +2035,9 @@ impl Visit<'_> for RubyIndexer<'_> { #[cfg(test)] mod tests { use crate::{ - assert_def_name_eq, assert_def_name_offset_eq, assert_def_str_eq, assert_def_superclass_ref_eq, - assert_definition_at, assert_local_diagnostics_eq, assert_name_path_eq, assert_no_local_diagnostics, - assert_string_eq, + assert_def_comments_eq, assert_def_name_eq, assert_def_name_offset_eq, assert_def_str_eq, + assert_def_superclass_ref_eq, assert_definition_at, assert_local_diagnostics_eq, assert_name_path_eq, + assert_no_local_diagnostics, assert_string_eq, model::{ definitions::{Definition, Mixin, Parameter, Receiver}, ids::{StringId, UriId}, @@ -2046,23 +2046,6 @@ mod tests { test_utils::LocalGraphTest, }; - /// Asserts that a definition's comments matches the expected comments. - /// - /// Usage: - /// - `assert_def_comments_eq!(ctx, def, ["# Comment 1", "# Comment 2"])` - macro_rules! assert_def_comments_eq { - ($context:expr, $def:expr, $expected_comments:expr) => {{ - let actual_comments: Vec = $def.comments().iter().map(|c| c.string().to_string()).collect(); - assert_eq!( - $expected_comments, - actual_comments.as_slice(), - "comments mismatch: expected `{:?}`, got `{:?}`", - $expected_comments, - actual_comments - ); - }}; - } - /// Asserts that a definition's mixins matches the expected mixins. /// /// Usage: diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 2877c6cf..4504ad8c 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -4484,6 +4484,37 @@ mod tests { assert_ancestors_eq!(context, "Baz::Child", ["Baz::Child", "Baz::Base", "Object"]); } + #[test] + fn rbs_constant_declarations() { + let mut context = GraphTest::new(); + context.index_rbs_uri("file:///test.rbs", { + r" + FOO: String + + class Bar + BAZ: Integer + end + + Bar::QUX: ::String + " + }); + context.resolve(); + + assert_no_diagnostics!(&context); + + assert_declaration_exists!(context, "FOO"); + assert_declaration_kind_eq!(context, "FOO", "Constant"); + assert_owner_eq!(context, "FOO", "Object"); + + assert_declaration_exists!(context, "Bar::BAZ"); + assert_declaration_kind_eq!(context, "Bar::BAZ", "Constant"); + assert_owner_eq!(context, "Bar::BAZ", "Bar"); + + assert_declaration_exists!(context, "Bar::QUX"); + assert_declaration_kind_eq!(context, "Bar::QUX", "Constant"); + assert_owner_eq!(context, "Bar::QUX", "Bar"); + } + #[test] fn resolving_meta_programming_class_reopened() { // It's often not possible to provide first-class support to meta-programming constructs, but we have to prevent diff --git a/rust/rubydex/src/test_utils/local_graph_test.rs b/rust/rubydex/src/test_utils/local_graph_test.rs index 6af53a44..4aded989 100644 --- a/rust/rubydex/src/test_utils/local_graph_test.rs +++ b/rust/rubydex/src/test_utils/local_graph_test.rs @@ -300,6 +300,27 @@ macro_rules! assert_def_str_eq { }}; } +// Comment assertions + +#[cfg(test)] +#[macro_export] +/// Asserts that a definition's comments matches the expected comments. +/// +/// Usage: +/// - `assert_def_comments_eq!(ctx, def, ["# Comment 1", "# Comment 2"])` +macro_rules! assert_def_comments_eq { + ($context:expr, $def:expr, $expected_comments:expr) => {{ + let actual_comments: Vec = $def.comments().iter().map(|c| c.string().to_string()).collect(); + assert_eq!( + $expected_comments, + actual_comments.as_slice(), + "comments mismatch: expected `{:?}`, got `{:?}`", + $expected_comments, + actual_comments + ); + }}; +} + // Diagnostic assertions #[cfg(test)]