From 230ee0d8605d2fb4fab44d362f80a69d5cdefde1 Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Fri, 31 Jul 2026 13:06:08 +0900 Subject: [PATCH 1/3] Index RBS attribute methods --- rust/rubydex/src/indexing/rbs_indexer.rs | 237 ++++++++++++++++++++++- rust/rubydex/src/resolution_tests.rs | 32 +++ 2 files changed, 267 insertions(+), 2 deletions(-) diff --git a/rust/rubydex/src/indexing/rbs_indexer.rs b/rust/rubydex/src/indexing/rbs_indexer.rs index 36fc84cbd..ea5577dd1 100644 --- a/rust/rubydex/src/indexing/rbs_indexer.rs +++ b/rust/rubydex/src/indexing/rbs_indexer.rs @@ -3,8 +3,9 @@ use core::panic; use ruby_rbs::node::{ - self, AliasKind, ClassNode, CommentNode, ConstantNode, ExtendNode, FunctionTypeNode, GlobalNode, IncludeNode, - ModuleNode, Node, NodeList, PrependNode, TypeNameNode, Visit, + self, AliasKind, AttrAccessorNode, AttrReaderNode, AttrWriterNode, AttributeKind, AttributeVisibility, ClassNode, + CommentNode, ConstantNode, ExtendNode, FunctionTypeNode, GlobalNode, IncludeNode, ModuleNode, Node, NodeList, + PrependNode, TypeNameNode, Visit, }; use crate::diagnostic::Rule; @@ -223,6 +224,111 @@ impl<'a> RBSIndexer<'a> { definition_id } + #[allow(clippy::too_many_arguments)] + fn register_attribute_methods( + &mut self, + name: &str, + offset: Offset, + name_offset: Offset, + comments: Box<[Comment]>, + flags: DefinitionFlags, + lexical_nesting_id: Option, + kind: AttributeKind, + attribute_visibility: AttributeVisibility, + reader: bool, + writer: bool, + ) { + let (visibility, receiver) = match kind { + AttributeKind::Instance => { + let visibility = match attribute_visibility { + AttributeVisibility::Public => Visibility::Public, + AttributeVisibility::Private => Visibility::Private, + AttributeVisibility::Unspecified => self.current_visibility, + }; + (visibility, None) + } + AttributeKind::Singleton => { + let visibility = match attribute_visibility { + AttributeVisibility::Private => Visibility::Private, + AttributeVisibility::Public | AttributeVisibility::Unspecified => Visibility::Public, + }; + ( + visibility, + Some(Receiver::SelfReceiver( + lexical_nesting_id.expect("Singleton attribute must have a lexical enclosing scope"), + )), + ) + } + }; + + if reader { + self.register_attribute_method( + name, + false, + offset.clone(), + name_offset.clone(), + comments.clone(), + flags.clone(), + lexical_nesting_id, + visibility, + receiver.clone(), + ); + } + + if writer { + self.register_attribute_method( + name, + true, + offset, + name_offset, + comments, + flags, + lexical_nesting_id, + visibility, + receiver, + ); + } + } + + #[allow(clippy::too_many_arguments)] + fn register_attribute_method( + &mut self, + name: &str, + writer: bool, + offset: Offset, + name_offset: Offset, + comments: Box<[Comment]>, + flags: DefinitionFlags, + lexical_nesting_id: Option, + visibility: Visibility, + receiver: Option, + ) { + let str_id = self + .local_graph + .intern_string(format!("{name}{}()", if writer { "=" } else { "" })); + let signatures = if writer { + let parameter_name = self.local_graph.intern_string("arg0".to_string()); + let parameter = Parameter::RequiredPositional(ParameterStruct::new(name_offset.clone(), parameter_name)); + Signatures::Simple(vec![parameter].into_boxed_slice()) + } else { + Signatures::Simple(Box::new([])) + }; + + let definition = Definition::Method(Box::new(MethodDefinition::new( + str_id, + self.uri_id, + offset, + name_offset, + comments, + flags, + lexical_nesting_id, + signatures, + visibility, + receiver, + ))); + self.register_definition(definition, lexical_nesting_id); + } + #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] fn source_at(&self, location: &node::RBSLocationRange) -> String { let start = location.start() as usize; @@ -574,6 +680,51 @@ impl Visit for RBSIndexer<'_> { self.register_definition(definition, lexical_nesting_id); } + fn visit_attr_reader_node(&mut self, attribute_node: &AttrReaderNode) { + self.register_attribute_methods( + attribute_node.name().as_str(), + Offset::from_rbs_location(&attribute_node.location()), + Offset::from_rbs_location(&attribute_node.name_location()), + self.collect_comments(attribute_node.comment()), + Self::flags(&attribute_node.annotations()), + self.parent_lexical_scope_id(), + attribute_node.kind(), + attribute_node.visibility(), + true, + false, + ); + } + + fn visit_attr_writer_node(&mut self, attribute_node: &AttrWriterNode) { + self.register_attribute_methods( + attribute_node.name().as_str(), + Offset::from_rbs_location(&attribute_node.location()), + Offset::from_rbs_location(&attribute_node.name_location()), + self.collect_comments(attribute_node.comment()), + Self::flags(&attribute_node.annotations()), + self.parent_lexical_scope_id(), + attribute_node.kind(), + attribute_node.visibility(), + false, + true, + ); + } + + fn visit_attr_accessor_node(&mut self, attribute_node: &AttrAccessorNode) { + self.register_attribute_methods( + attribute_node.name().as_str(), + Offset::from_rbs_location(&attribute_node.location()), + Offset::from_rbs_location(&attribute_node.name_location()), + self.collect_comments(attribute_node.comment()), + Self::flags(&attribute_node.annotations()), + self.parent_lexical_scope_id(), + attribute_node.kind(), + attribute_node.visibility(), + true, + true, + ); + } + fn visit_method_definition_node(&mut self, def_node: &node::MethodDefinitionNode) { let str_id = self.local_graph.intern_string(format!("{}()", def_node.name())); let offset = Offset::from_rbs_location(&def_node.location()); @@ -1051,6 +1202,88 @@ mod tests { }); } + #[test] + fn indexes_attribute_members_as_methods_without_retaining_types_or_instance_variables() { + let context = index_source({ + " + class Foo + # Reader documentation + %a{deprecated} + attr_reader inferred: Integer + attr_reader absent(): Symbol + attr_writer explicit (@writer): String + attr_accessor accessor: bool + private + attr_reader inherited_visibility: Float + public + private attr_accessor self.class_value (@class_value): bool + end + " + }); + + assert_no_local_diagnostics!(&context); + assert_eq!(context.graph().definitions().len(), 9); + + let method = |name: &str| { + context + .graph() + .definitions() + .values() + .find_map(|definition| match definition { + Definition::Method(method) + if context + .graph() + .strings() + .get(method.str_id()) + .is_some_and(|string| string.as_str() == name) => + { + Some(method) + } + _ => None, + }) + .unwrap_or_else(|| panic!("expected `{name}` method definition")) + }; + + for name in [ + "inferred()", + "absent()", + "explicit=()", + "accessor()", + "accessor=()", + "inherited_visibility()", + "class_value()", + "class_value=()", + ] { + assert_eq!(method(name).signatures().as_slice().len(), 1); + } + + for name in [ + "inferred()", + "absent()", + "accessor()", + "inherited_visibility()", + "class_value()", + ] { + assert!(method(name).signatures().as_slice()[0].is_empty()); + } + + for name in ["explicit=()", "accessor=()", "class_value=()"] { + let signature = &method(name).signatures().as_slice()[0]; + let [Parameter::RequiredPositional(parameter)] = signature.as_ref() else { + panic!("expected `{name}` to have one required positional parameter"); + }; + assert_string_eq!(&context, parameter.str(), "arg0"); + } + + assert_eq!(method("class_value()").visibility(), &Visibility::Private); + assert_eq!(method("class_value=()").visibility(), &Visibility::Private); + assert_eq!(method("inherited_visibility()").visibility(), &Visibility::Private); + assert_method_has_receiver!(&context, method("class_value()"), "Foo"); + assert_method_has_receiver!(&context, method("class_value=()"), "Foo"); + assert_def_comments_eq!(&context, method("inferred()"), ["# Reader documentation"]); + assert!(method("inferred()").flags().contains(DefinitionFlags::DEPRECATED)); + } + #[test] fn index_alias_node() { let context = index_source({ diff --git a/rust/rubydex/src/resolution_tests.rs b/rust/rubydex/src/resolution_tests.rs index 8030d299d..aa28e817b 100644 --- a/rust/rubydex/src/resolution_tests.rs +++ b/rust/rubydex/src/resolution_tests.rs @@ -5024,6 +5024,38 @@ mod rbs_tests { ); } + #[test] + fn rbs_attributes_create_method_declarations_without_instance_variable_declarations() { + let mut context = graph_test(); + context.index_rbs_uri("file:///attributes.rbs", { + r" + class Foo + attr_reader reader: String + attr_writer writer (@writer): Integer + attr_accessor accessor(): bool + private attr_accessor self.class_value (@class_value): Symbol + end + " + }); + context.resolve(); + + assert_no_diagnostics!(&context); + for method in [ + "Foo#reader()", + "Foo#writer=()", + "Foo#accessor()", + "Foo#accessor=()", + "Foo::#class_value()", + "Foo::#class_value=()", + ] { + assert_declaration_exists!(context, method); + assert_declaration_kind_eq!(context, method, "Method"); + } + for instance_variable in ["Foo#@reader", "Foo#@writer", "Foo::#@class_value"] { + assert_declaration_does_not_exist!(context, instance_variable); + } + } + #[test] fn rbs_mixin_resolution() { let mut context = graph_test(); From a9f8b3af362236513bf59d58653abfd46d2d0552 Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 3 Aug 2026 11:39:36 +0900 Subject: [PATCH 2/3] Use attribute names for RBS writer parameters --- rust/rubydex/src/indexing/rbs_indexer.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/rust/rubydex/src/indexing/rbs_indexer.rs b/rust/rubydex/src/indexing/rbs_indexer.rs index ea5577dd1..6c1e997de 100644 --- a/rust/rubydex/src/indexing/rbs_indexer.rs +++ b/rust/rubydex/src/indexing/rbs_indexer.rs @@ -307,7 +307,7 @@ impl<'a> RBSIndexer<'a> { .local_graph .intern_string(format!("{name}{}()", if writer { "=" } else { "" })); let signatures = if writer { - let parameter_name = self.local_graph.intern_string("arg0".to_string()); + let parameter_name = self.local_graph.intern_string(name.to_owned()); let parameter = Parameter::RequiredPositional(ParameterStruct::new(name_offset.clone(), parameter_name)); Signatures::Simple(vec![parameter].into_boxed_slice()) } else { @@ -1267,12 +1267,17 @@ mod tests { assert!(method(name).signatures().as_slice()[0].is_empty()); } - for name in ["explicit=()", "accessor=()", "class_value=()"] { + for (name, parameter_name) in [ + ("explicit=()", "explicit"), + ("accessor=()", "accessor"), + ("class_value=()", "class_value"), + ] { let signature = &method(name).signatures().as_slice()[0]; let [Parameter::RequiredPositional(parameter)] = signature.as_ref() else { panic!("expected `{name}` to have one required positional parameter"); }; - assert_string_eq!(&context, parameter.str(), "arg0"); + assert_string_eq!(&context, parameter.str(), parameter_name); + assert_offset_string!(&context, parameter.offset(), parameter_name); } assert_eq!(method("class_value()").visibility(), &Visibility::Private); From cf43b950e71e112960801514bc1c74654ce367cd Mon Sep 17 00:00:00 2001 From: Soutaro Matsumoto Date: Mon, 3 Aug 2026 11:40:13 +0900 Subject: [PATCH 3/3] Avoid cloning single RBS attribute definitions --- rust/rubydex/src/indexing/rbs_indexer.rs | 48 +++++++++++++++++------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/rust/rubydex/src/indexing/rbs_indexer.rs b/rust/rubydex/src/indexing/rbs_indexer.rs index 6c1e997de..c5319d70f 100644 --- a/rust/rubydex/src/indexing/rbs_indexer.rs +++ b/rust/rubydex/src/indexing/rbs_indexer.rs @@ -261,22 +261,43 @@ impl<'a> RBSIndexer<'a> { } }; - if reader { - self.register_attribute_method( + match (reader, writer) { + (true, true) => { + self.register_attribute_method( + name, + false, + offset.clone(), + name_offset.clone(), + comments.clone(), + flags.clone(), + lexical_nesting_id, + visibility, + receiver.clone(), + ); + self.register_attribute_method( + name, + true, + offset, + name_offset, + comments, + flags, + lexical_nesting_id, + visibility, + receiver, + ); + } + (true, false) => self.register_attribute_method( name, false, - offset.clone(), - name_offset.clone(), - comments.clone(), - flags.clone(), + offset, + name_offset, + comments, + flags, lexical_nesting_id, visibility, - receiver.clone(), - ); - } - - if writer { - self.register_attribute_method( + receiver, + ), + (false, true) => self.register_attribute_method( name, true, offset, @@ -286,7 +307,8 @@ impl<'a> RBSIndexer<'a> { lexical_nesting_id, visibility, receiver, - ); + ), + (false, false) => unreachable!("attribute must have a reader or writer"), } }