From 6fac261a6fa59a752c9243a2b5d873e8b05b5847 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 15 Jan 2026 16:57:32 -0300 Subject: [PATCH] Remember attributes as bitflags --- ext/rubydex/definition.c | 22 -- ext/rubydex/definition.h | 3 - rust/rubydex-sys/src/definition_api.rs | 16 +- rust/rubydex/src/indexing/ruby_indexer.rs | 355 ++++++++++++++++------ rust/rubydex/src/model/definitions.rs | 293 +++--------------- rust/rubydex/src/model/graph.rs | 3 - rust/rubydex/src/resolution.rs | 29 +- test/definition_test.rb | 26 +- 8 files changed, 315 insertions(+), 432 deletions(-) diff --git a/ext/rubydex/definition.c b/ext/rubydex/definition.c index 127acad4..5725bf62 100644 --- a/ext/rubydex/definition.c +++ b/ext/rubydex/definition.c @@ -2,7 +2,6 @@ #include "graph.h" #include "handle.h" #include "location.h" -#include "ruby/internal/scan_args.h" #include "rustbindings.h" static VALUE mRubydex; @@ -14,9 +13,6 @@ VALUE cModuleDefinition; VALUE cConstantDefinition; VALUE cConstantAliasDefinition; VALUE cMethodDefinition; -VALUE cAttrAccessorDefinition; -VALUE cAttrReaderDefinition; -VALUE cAttrWriterDefinition; VALUE cGlobalVariableDefinition; VALUE cInstanceVariableDefinition; VALUE cClassVariableDefinition; @@ -38,12 +34,6 @@ VALUE definition_class_for_kind(DefinitionKind kind) { return cConstantAliasDefinition; case DefinitionKind_Method: return cMethodDefinition; - case DefinitionKind_AttrAccessor: - return cAttrAccessorDefinition; - case DefinitionKind_AttrReader: - return cAttrReaderDefinition; - case DefinitionKind_AttrWriter: - return cAttrWriterDefinition; case DefinitionKind_GlobalVariable: return cGlobalVariableDefinition; case DefinitionKind_InstanceVariable: @@ -179,18 +169,6 @@ void initialize_definition(VALUE mod) { rb_define_alloc_func(cMethodDefinition, sr_handle_alloc); rb_define_method(cMethodDefinition, "initialize", sr_handle_initialize, 2); - cAttrAccessorDefinition = rb_define_class_under(mRubydex, "AttrAccessorDefinition", cDefinition); - rb_define_alloc_func(cAttrAccessorDefinition, sr_handle_alloc); - rb_define_method(cAttrAccessorDefinition, "initialize", sr_handle_initialize, 2); - - cAttrReaderDefinition = rb_define_class_under(mRubydex, "AttrReaderDefinition", cDefinition); - rb_define_alloc_func(cAttrReaderDefinition, sr_handle_alloc); - rb_define_method(cAttrReaderDefinition, "initialize", sr_handle_initialize, 2); - - cAttrWriterDefinition = rb_define_class_under(mRubydex, "AttrWriterDefinition", cDefinition); - rb_define_alloc_func(cAttrWriterDefinition, sr_handle_alloc); - rb_define_method(cAttrWriterDefinition, "initialize", sr_handle_initialize, 2); - cGlobalVariableDefinition = rb_define_class_under(mRubydex, "GlobalVariableDefinition", cDefinition); rb_define_alloc_func(cGlobalVariableDefinition, sr_handle_alloc); rb_define_method(cGlobalVariableDefinition, "initialize", sr_handle_initialize, 2); diff --git a/ext/rubydex/definition.h b/ext/rubydex/definition.h index e4318ff1..c0f31a03 100644 --- a/ext/rubydex/definition.h +++ b/ext/rubydex/definition.h @@ -11,9 +11,6 @@ extern VALUE cModuleDefinition; extern VALUE cConstantDefinition; extern VALUE cConstantAliasDefinition; extern VALUE cMethodDefinition; -extern VALUE cAttrAccessorDefinition; -extern VALUE cAttrReaderDefinition; -extern VALUE cAttrWriterDefinition; extern VALUE cGlobalVariableDefinition; extern VALUE cInstanceVariableDefinition; extern VALUE cClassVariableDefinition; diff --git a/rust/rubydex-sys/src/definition_api.rs b/rust/rubydex-sys/src/definition_api.rs index fd64b8f9..efafe75f 100644 --- a/rust/rubydex-sys/src/definition_api.rs +++ b/rust/rubydex-sys/src/definition_api.rs @@ -18,14 +18,11 @@ pub enum DefinitionKind { Constant = 3, ConstantAlias = 4, Method = 5, - AttrAccessor = 6, - AttrReader = 7, - AttrWriter = 8, - GlobalVariable = 9, - InstanceVariable = 10, - ClassVariable = 11, - MethodAlias = 12, - GlobalVariableAlias = 13, + GlobalVariable = 6, + InstanceVariable = 7, + ClassVariable = 8, + MethodAlias = 9, + GlobalVariableAlias = 10, } pub(crate) fn map_definition_to_kind(defn: &Definition) -> DefinitionKind { @@ -36,9 +33,6 @@ pub(crate) fn map_definition_to_kind(defn: &Definition) -> DefinitionKind { 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, diff --git a/rust/rubydex/src/indexing/ruby_indexer.rs b/rust/rubydex/src/indexing/ruby_indexer.rs index 4a4d5e74..fdd6d140 100644 --- a/rust/rubydex/src/indexing/ruby_indexer.rs +++ b/rust/rubydex/src/indexing/ruby_indexer.rs @@ -4,11 +4,10 @@ use crate::diagnostic::Rule; use crate::indexing::local_graph::LocalGraph; use crate::model::comment::Comment; use crate::model::definitions::{ - AttrAccessorDefinition, AttrReaderDefinition, AttrWriterDefinition, ClassDefinition, ClassVariableDefinition, - ConstantAliasDefinition, ConstantDefinition, Definition, DefinitionFlags, ExtendDefinition, - GlobalVariableAliasDefinition, GlobalVariableDefinition, IncludeDefinition, InstanceVariableDefinition, - MethodAliasDefinition, MethodDefinition, Mixin, ModuleDefinition, Parameter, ParameterStruct, PrependDefinition, - SingletonClassDefinition, + ClassDefinition, ClassVariableDefinition, ConstantAliasDefinition, ConstantDefinition, Definition, DefinitionFlags, + ExtendDefinition, GlobalVariableAliasDefinition, GlobalVariableDefinition, IncludeDefinition, + InstanceVariableDefinition, MethodAliasDefinition, MethodDefinition, MethodFlags, Mixin, ModuleDefinition, + Parameter, ParameterStruct, PrependDefinition, SingletonClassDefinition, }; use crate::model::document::Document; use crate::model::ids::{DefinitionId, NameId, StringId, UriId}; @@ -1218,6 +1217,7 @@ impl Visit<'_> for RubyIndexer<'_> { let str_id = self.local_graph.intern_string(format!("{name}()")); let offset = Offset::from_prism_location(&node.location()); let (comments, flags) = self.find_comments_for(offset.start()); + let flags = MethodFlags::from(flags); let parent_nesting_id = self.current_nesting_definition_id(); let parameters = self.collect_parameters(node); let is_singleton = node.receiver().is_some(); @@ -1324,7 +1324,6 @@ impl Visit<'_> for RubyIndexer<'_> { let mut index_attr = |kind: AttrKind, call: &ruby_prism::CallNode| { Self::each_string_or_symbol_arg(call, |name, location| { - let str_id = self.local_graph.intern_string(format!("{name}()")); let parent_nesting_id = self.parent_nesting_id(); let offset = Offset::from_prism_location(&location); let (comments, flags) = self.find_comments_for(offset.start()); @@ -1335,38 +1334,86 @@ impl Visit<'_> for RubyIndexer<'_> { v => *v, }; - let definition = match kind { - AttrKind::Accessor => Definition::AttrAccessor(Box::new(AttrAccessorDefinition::new( - str_id, - self.uri_id, - offset, - comments, - flags, - parent_nesting_id, - visibility, - ))), - AttrKind::Reader => Definition::AttrReader(Box::new(AttrReaderDefinition::new( - str_id, - self.uri_id, - offset, - comments, - flags, - parent_nesting_id, - visibility, - ))), - AttrKind::Writer => Definition::AttrWriter(Box::new(AttrWriterDefinition::new( - str_id, - self.uri_id, - offset, - comments, - flags, - parent_nesting_id, - visibility, - ))), - }; + match kind { + AttrKind::Accessor => { + let writer_name = format!("{name}=()"); + let reader_str_id = self.local_graph.intern_string(format!("{name}()")); + let writer_str_id = self.local_graph.intern_string(writer_name); - let definition_id = self.local_graph.add_definition(definition); - self.add_member_to_current_owner(definition_id); + let reader_definition = Definition::Method(Box::new(MethodDefinition::new( + reader_str_id, + self.uri_id, + offset.clone(), + comments.clone(), + MethodFlags::from(flags.clone()) + | MethodFlags::ATTRIBUTE_READER + | MethodFlags::ATTRIBUTE_WRITER, + parent_nesting_id, + Vec::new(), + visibility, + None, + ))); + + let reader_definition_id = self.local_graph.add_definition(reader_definition); + self.add_member_to_current_owner(reader_definition_id); + + let writer_definition = Definition::Method(Box::new(MethodDefinition::new( + writer_str_id, + self.uri_id, + offset, + comments, + MethodFlags::from(flags) | MethodFlags::ATTRIBUTE_READER | MethodFlags::ATTRIBUTE_WRITER, + parent_nesting_id, + Vec::new(), + visibility, + None, + ))); + + let writer_definition_id = self.local_graph.add_definition(writer_definition); + self.add_member_to_current_owner(writer_definition_id); + } + AttrKind::Reader => { + let str_id = self.local_graph.intern_string(format!("{name}()")); + + let definition = Definition::Method(Box::new(MethodDefinition::new( + str_id, + self.uri_id, + offset, + comments, + MethodFlags::from(flags) | MethodFlags::ATTRIBUTE_READER, + parent_nesting_id, + Vec::new(), + visibility, + None, + ))); + + let definition_id = self.local_graph.add_definition(definition); + self.add_member_to_current_owner(definition_id); + } + AttrKind::Writer => { + let writer_name = format!("{name}=()"); + let reader_str_id = self.local_graph.intern_string(format!("{name}()")); + let writer_str_id = self.local_graph.intern_string(writer_name); + + let definition = Definition::Method(Box::new(MethodDefinition::new( + writer_str_id, + self.uri_id, + offset.clone(), + comments, + MethodFlags::from(flags) | MethodFlags::ATTRIBUTE_WRITER, + parent_nesting_id, + vec![Parameter::RequiredPositional(ParameterStruct::new( + offset, + reader_str_id, + ))], + visibility, + None, + ))); + + let definition_id = self.local_graph.add_definition(definition); + self.add_member_to_current_owner(definition_id); + } + } }); }; @@ -2894,9 +2941,10 @@ mod tests { assert_name_eq!(&context, "baz()", singleton_method); assert_eq!(singleton_method.visibility(), &Visibility::Public); - assert_definition_at!(&context, "7:16-7:25", AttrReader, |def| { + assert_definition_at!(&context, "7:16-7:25", Method, |def| { assert_name_eq!(&context, "attribute()", def); assert_eq!(def.visibility(), &Visibility::Private); + assert!(def.flags().is_reader()); }); assert_definition_at!(&context, "11:3-11:15", Method, |def| { @@ -3067,7 +3115,7 @@ mod tests { attr_accessor :foo class Foo - attr_accessor :bar, :baz + attr_accessor :bar, :qux end foo.attr_accessor :not_indexed @@ -3075,29 +3123,67 @@ mod tests { }); assert_no_diagnostics!(&context); - assert_eq!(context.graph().definitions().len(), 4); + assert_eq!(context.graph().definitions().len(), 7); - assert_definition_at!(&context, "1:16-1:19", AttrAccessor, |def| { - assert_name_eq!(&context, "foo()", def); - assert!(def.lexical_nesting_id().is_none()); + let definitions = context.all_definitions_at("1:16-1:19"); + let Definition::Method(foo_reader) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "foo()", foo_reader); + assert!(foo_reader.lexical_nesting_id().is_none()); + assert!(foo_reader.flags().is_accessor()); + + let Definition::Method(foo_writer) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "foo=()", foo_writer); + assert!(foo_writer.lexical_nesting_id().is_none()); + assert!(foo_writer.flags().is_accessor()); + + let definitions = context.all_definitions_at("4:18-4:21"); + let Definition::Method(bar_reader) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "bar()", bar_reader); + assert!(bar_reader.flags().is_accessor()); + + assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { + assert_eq!(parent_nesting.id(), bar_reader.lexical_nesting_id().unwrap()); + assert_eq!(parent_nesting.members()[0], bar_reader.id()); }); - assert_definition_at!(&context, "4:18-4:21", AttrAccessor, |def| { - assert_name_eq!(&context, "bar()", def); + let Definition::Method(bar_writer) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "bar=()", bar_writer); + assert!(bar_writer.flags().is_accessor()); - assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { - assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); - assert_eq!(parent_nesting.members()[0], def.id()); - }); + assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { + assert_eq!(parent_nesting.id(), bar_writer.lexical_nesting_id().unwrap()); + assert_eq!(parent_nesting.members()[1], bar_writer.id()); }); - assert_definition_at!(&context, "4:24-4:27", AttrAccessor, |def| { - assert_name_eq!(&context, "baz()", def); + let definitions = context.all_definitions_at("4:24-4:27"); + let Definition::Method(qux_reader) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "qux()", qux_reader); + assert!(qux_reader.flags().is_accessor()); - assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { - assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); - assert_eq!(parent_nesting.members()[1], def.id()); - }); + assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { + assert_eq!(parent_nesting.id(), qux_reader.lexical_nesting_id().unwrap()); + assert_eq!(parent_nesting.members()[2], qux_reader.id()); + }); + + let Definition::Method(qux_writer) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "qux=()", qux_writer); + assert!(qux_writer.flags().is_accessor()); + + assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { + assert_eq!(parent_nesting.id(), qux_writer.lexical_nesting_id().unwrap()); + assert_eq!(parent_nesting.members()[3], qux_writer.id()); }); } @@ -3116,13 +3202,15 @@ mod tests { assert_no_diagnostics!(&context); assert_eq!(context.graph().definitions().len(), 4); - assert_definition_at!(&context, "1:14-1:17", AttrReader, |def| { + assert_definition_at!(&context, "1:14-1:17", Method, |def| { assert_name_eq!(&context, "foo()", def); assert!(def.lexical_nesting_id().is_none()); + assert!(def.flags().is_reader()); }); - assert_definition_at!(&context, "4:16-4:19", AttrReader, |def| { + assert_definition_at!(&context, "4:16-4:19", Method, |def| { assert_name_eq!(&context, "bar()", def); + assert!(def.flags().is_reader()); assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); @@ -3130,8 +3218,9 @@ mod tests { }); }); - assert_definition_at!(&context, "4:22-4:25", AttrReader, |def| { + assert_definition_at!(&context, "4:22-4:25", Method, |def| { assert_name_eq!(&context, "baz()", def); + assert!(def.flags().is_reader()); assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); @@ -3155,13 +3244,15 @@ mod tests { assert_no_diagnostics!(&context); assert_eq!(context.graph().definitions().len(), 4); - assert_definition_at!(&context, "1:14-1:17", AttrWriter, |def| { - assert_name_eq!(&context, "foo()", def); + assert_definition_at!(&context, "1:14-1:17", Method, |def| { + assert_name_eq!(&context, "foo=()", def); assert!(def.lexical_nesting_id().is_none()); + assert!(def.flags().is_writer()); }); - assert_definition_at!(&context, "4:16-4:19", AttrWriter, |def| { - assert_name_eq!(&context, "bar()", def); + assert_definition_at!(&context, "4:16-4:19", Method, |def| { + assert_name_eq!(&context, "bar=()", def); + assert!(def.flags().is_writer()); assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); @@ -3169,8 +3260,9 @@ mod tests { }); }); - assert_definition_at!(&context, "4:22-4:25", AttrWriter, |def| { - assert_name_eq!(&context, "baz()", def); + assert_definition_at!(&context, "4:22-4:25", Method, |def| { + assert_name_eq!(&context, "baz=()", def); + assert!(def.flags().is_writer()); assert_definition_at!(&context, "3:1-5:4", Class, |parent_nesting| { assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); @@ -3194,41 +3286,60 @@ mod tests { }); assert_no_diagnostics!(&context); - assert_eq!(context.graph().definitions().len(), 6); + assert_eq!(context.graph().definitions().len(), 7); - assert_definition_at!(&context, "1:6-1:10", AttrReader, |def| { + assert_definition_at!(&context, "1:6-1:10", Method, |def| { assert_name_eq!(&context, "a1()", def); assert!(def.lexical_nesting_id().is_none()); + assert!(def.flags().is_reader()); }); - assert_definition_at!(&context, "1:13-1:15", AttrReader, |def| { + assert_definition_at!(&context, "1:13-1:15", Method, |def| { assert_name_eq!(&context, "a2()", def); assert!(def.lexical_nesting_id().is_none()); + assert!(def.flags().is_reader()); }); - assert_definition_at!(&context, "4:8-4:12", AttrAccessor, |def| { - assert_name_eq!(&context, "a3()", def); + let definitions = context.all_definitions_at("4:8-4:12"); + let Definition::Method(a3_reader) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "a3()", a3_reader); + assert!(a3_reader.flags().is_accessor()); - assert_definition_at!(&context, "3:1-7:4", Class, |parent_nesting| { - assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); - assert_eq!(parent_nesting.members()[0], def.id()); - }); + assert_definition_at!(&context, "3:1-7:4", Class, |parent_nesting| { + assert_eq!(parent_nesting.id(), a3_reader.lexical_nesting_id().unwrap()); + assert_eq!(parent_nesting.members()[0], a3_reader.id()); }); - assert_definition_at!(&context, "5:9-5:11", AttrReader, |def| { + let Definition::Method(a3_writer) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "a3=()", a3_writer); + assert!(a3_writer.flags().is_accessor()); + + assert_definition_at!(&context, "3:1-7:4", Class, |parent_nesting| { + assert_eq!(parent_nesting.id(), a3_writer.lexical_nesting_id().unwrap()); + assert_eq!(parent_nesting.members()[1], a3_writer.id()); + }); + + assert_definition_at!(&context, "5:9-5:11", Method, |def| { assert_name_eq!(&context, "a4()", def); + assert!(def.flags().is_reader()); assert_definition_at!(&context, "3:1-7:4", Class, |parent_nesting| { assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); - assert_eq!(parent_nesting.members()[1], def.id()); + assert_eq!(parent_nesting.members()[2], def.id()); }); }); - assert_definition_at!(&context, "6:9-6:11", AttrReader, |def| { + assert_definition_at!(&context, "6:9-6:11", Method, |def| { assert_name_eq!(&context, "a5()", def); + assert!(def.flags().is_reader()); + assert_definition_at!(&context, "3:1-7:4", Class, |parent_nesting| { assert_eq!(parent_nesting.id(), def.lexical_nesting_id().unwrap()); - assert_eq!(parent_nesting.members()[2], def.id()); + assert_eq!(parent_nesting.members()[3], def.id()); }); }); } @@ -3249,19 +3360,31 @@ mod tests { assert_no_diagnostics!(&context); - assert_definition_at!(&context, "1:16-1:19", AttrAccessor, |def| { - assert_name_eq!(&context, "foo()", def); - assert_eq!(def.visibility(), &Visibility::Private); - }); + let definitions = context.all_definitions_at("1:16-1:19"); + let Definition::Method(foo_reader) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "foo()", foo_reader); + assert_eq!(foo_reader.visibility(), &Visibility::Private); + assert!(foo_reader.flags().is_accessor()); + + let Definition::Method(foo_writer) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "foo=()", foo_writer); + assert_eq!(foo_writer.visibility(), &Visibility::Private); + assert!(foo_writer.flags().is_accessor()); - assert_definition_at!(&context, "3:24-3:27", AttrReader, |def| { + assert_definition_at!(&context, "3:24-3:27", Method, |def| { assert_name_eq!(&context, "bar()", def); assert_eq!(def.visibility(), &Visibility::Protected); + assert!(def.flags().is_reader()); }); - assert_definition_at!(&context, "7:14-7:17", AttrWriter, |def| { - assert_name_eq!(&context, "baz()", def); + assert_definition_at!(&context, "7:14-7:17", Method, |def| { + assert_name_eq!(&context, "baz=()", def); assert_eq!(def.visibility(), &Visibility::Public); + assert!(def.flags().is_writer()); }); } @@ -3293,24 +3416,46 @@ mod tests { assert_no_diagnostics!(&context); - assert_definition_at!(&context, "4:18-4:21", AttrAccessor, |def| { - assert_name_eq!(&context, "foo()", def); - assert_eq!(def.visibility(), &Visibility::Public); - }); + let definitions = context.all_definitions_at("4:18-4:21"); + let Definition::Method(foo_reader) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "foo()", foo_reader); + assert_eq!(foo_reader.visibility(), &Visibility::Public); + assert!(foo_reader.flags().is_accessor()); - assert_definition_at!(&context, "9:20-9:23", AttrAccessor, |def| { - assert_name_eq!(&context, "bar()", def); - assert_eq!(def.visibility(), &Visibility::Public); - }); + let Definition::Method(foo_writer) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "foo=()", foo_writer); + assert_eq!(foo_writer.visibility(), &Visibility::Public); + assert!(foo_writer.flags().is_accessor()); + + let definitions = context.all_definitions_at("9:20-9:23"); + let Definition::Method(bar_reader) = definitions.first().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "bar()", bar_reader); + assert_eq!(bar_reader.visibility(), &Visibility::Public); + assert!(bar_reader.flags().is_accessor()); - assert_definition_at!(&context, "13:18-13:21", AttrReader, |def| { + let Definition::Method(bar_writer) = definitions.last().unwrap() else { + panic!("should be a method"); + }; + assert_name_eq!(&context, "bar=()", bar_writer); + assert_eq!(bar_writer.visibility(), &Visibility::Public); + assert!(bar_writer.flags().is_accessor()); + + assert_definition_at!(&context, "13:18-13:21", Method, |def| { assert_name_eq!(&context, "baz()", def); assert_eq!(def.visibility(), &Visibility::Private); + assert!(def.flags().is_reader()); }); - assert_definition_at!(&context, "18:16-18:19", AttrWriter, |def| { - assert_name_eq!(&context, "qux()", def); + assert_definition_at!(&context, "18:16-18:19", Method, |def| { + assert_name_eq!(&context, "qux=()", def); assert_eq!(def.visibility(), &Visibility::Private); + assert!(def.flags().is_writer()); }); } @@ -4661,8 +4806,10 @@ mod tests { assert_definition_at!(&context, "2:3-9:6", Module, |bar| { assert_definition_at!(&context, "5:5-7:8", Method, |qux| { assert_definition_at!(&context, "6:7-6:11", InstanceVariable, |var| { - assert_definition_at!(&context, "8:18-8:23", AttrReader, |hello| { + assert_definition_at!(&context, "8:18-8:23", Method, |hello| { assert_name_id_to_string_eq!(&context, "Bar", bar); + assert!(hello.flags().is_reader()); + assert_eq!(foo.id(), bar.lexical_nesting_id().unwrap()); assert_eq!(foo.members()[0], bar.id()); @@ -4720,8 +4867,10 @@ mod tests { assert_definition_at!(&context, "2:3-9:6", Module, |bar| { assert_definition_at!(&context, "5:5-7:8", Method, |qux| { assert_definition_at!(&context, "6:7-6:11", InstanceVariable, |var| { - assert_definition_at!(&context, "8:18-8:23", AttrReader, |hello| { + assert_definition_at!(&context, "8:18-8:23", Method, |hello| { assert_name_id_to_string_eq!(&context, "Zip::Bar", bar); + assert!(hello.flags().is_reader()); + assert_eq!(foo.id(), bar.lexical_nesting_id().unwrap()); assert_eq!(foo.members()[0], bar.id()); @@ -4779,8 +4928,10 @@ mod tests { assert_definition_at!(&context, "2:3-9:6", Class, |bar| { assert_definition_at!(&context, "5:5-7:8", Method, |qux| { assert_definition_at!(&context, "6:7-6:11", InstanceVariable, |var| { - assert_definition_at!(&context, "8:18-8:23", AttrReader, |hello| { + assert_definition_at!(&context, "8:18-8:23", Method, |hello| { assert_name_id_to_string_eq!(&context, "Bar", bar); + assert!(hello.flags().is_reader()); + assert_eq!(foo.id(), bar.lexical_nesting_id().unwrap()); assert_eq!(foo.members()[0], bar.id()); @@ -4840,8 +4991,10 @@ mod tests { assert_definition_at!(&context, "2:3-9:6", Class, |bar| { assert_definition_at!(&context, "5:5-7:8", Method, |qux| { assert_definition_at!(&context, "6:7-6:11", InstanceVariable, |var| { - assert_definition_at!(&context, "8:18-8:23", AttrReader, |hello| { + assert_definition_at!(&context, "8:18-8:23", Method, |hello| { assert_name_id_to_string_eq!(&context, "Bar", bar); + assert!(hello.flags().is_reader()); + assert_eq!(foo.id(), bar.lexical_nesting_id().unwrap()); assert_eq!(foo.members()[0], bar.id()); @@ -4899,8 +5052,10 @@ mod tests { assert_definition_at!(&context, "2:3-9:6", Class, |bar| { assert_definition_at!(&context, "5:5-7:8", Method, |qux| { assert_definition_at!(&context, "6:7-6:11", InstanceVariable, |var| { - assert_definition_at!(&context, "8:18-8:23", AttrReader, |hello| { + assert_definition_at!(&context, "8:18-8:23", Method, |hello| { assert_name_id_to_string_eq!(&context, "Zip::Bar", bar); + assert!(hello.flags().is_reader()); + assert_eq!(foo.id(), bar.lexical_nesting_id().unwrap()); assert_eq!(foo.members()[0], bar.id()); diff --git a/rust/rubydex/src/model/definitions.rs b/rust/rubydex/src/model/definitions.rs index a3f83140..0ae82445 100644 --- a/rust/rubydex/src/model/definitions.rs +++ b/rust/rubydex/src/model/definitions.rs @@ -38,7 +38,15 @@ use crate::{ bitflags! { #[derive(Debug, Clone)] pub struct DefinitionFlags: u8 { - const DEPRECATED = 0b0001; + const DEPRECATED = 0b0000_0001; + } + + #[derive(Debug, Clone)] + pub struct MethodFlags: u8 { + const DEPRECATED = 0b0000_0001; + // Flags to indicate if this method originated from an attribute. For attr_accessor, both bits will be set + const ATTRIBUTE_READER = 0b0000_0010; + const ATTRIBUTE_WRITER = 0b0000_0100; } } @@ -49,6 +57,34 @@ impl DefinitionFlags { } } +impl MethodFlags { + #[must_use] + pub fn is_deprecated(&self) -> bool { + self.contains(Self::DEPRECATED) + } + + #[must_use] + pub fn is_reader(&self) -> bool { + self.contains(Self::ATTRIBUTE_READER) && !self.contains(Self::ATTRIBUTE_WRITER) + } + + #[must_use] + pub fn is_writer(&self) -> bool { + self.contains(Self::ATTRIBUTE_WRITER) && !self.contains(Self::ATTRIBUTE_READER) + } + + #[must_use] + pub fn is_accessor(&self) -> bool { + self.contains(Self::ATTRIBUTE_READER) && self.contains(Self::ATTRIBUTE_WRITER) + } +} + +impl From for MethodFlags { + fn from(def_flags: DefinitionFlags) -> Self { + MethodFlags::from_bits_truncate(def_flags.bits()) + } +} + #[derive(Debug)] pub enum Definition { Class(Box), @@ -57,9 +93,6 @@ pub enum Definition { Constant(Box), ConstantAlias(Box), Method(Box), - AttrAccessor(Box), - AttrReader(Box), - AttrWriter(Box), GlobalVariable(Box), InstanceVariable(Box), ClassVariable(Box), @@ -78,9 +111,6 @@ macro_rules! all_definitions { Definition::GlobalVariable($var) => $expr, Definition::InstanceVariable($var) => $expr, Definition::ClassVariable($var) => $expr, - Definition::AttrAccessor($var) => $expr, - Definition::AttrReader($var) => $expr, - Definition::AttrWriter($var) => $expr, Definition::Method($var) => $expr, Definition::MethodAlias($var) => $expr, Definition::GlobalVariableAlias($var) => $expr, @@ -123,9 +153,6 @@ impl Definition { 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", @@ -657,7 +684,7 @@ pub struct MethodDefinition { str_id: StringId, uri_id: UriId, offset: Offset, - flags: DefinitionFlags, + flags: MethodFlags, comments: Vec, lexical_nesting_id: Option, parameters: Vec, @@ -673,7 +700,7 @@ impl MethodDefinition { uri_id: UriId, offset: Offset, comments: Vec, - flags: DefinitionFlags, + flags: MethodFlags, lexical_nesting_id: Option, parameters: Vec, visibility: Visibility, @@ -744,7 +771,7 @@ impl MethodDefinition { } #[must_use] - pub fn flags(&self) -> &DefinitionFlags { + pub fn flags(&self) -> &MethodFlags { &self.flags } } @@ -785,246 +812,6 @@ impl ParameterStruct { } } -/// An attr accessor definition -/// -/// # Example -/// ```ruby -/// attr_accessor :foo -/// ``` -#[derive(Debug)] -pub struct AttrAccessorDefinition { - str_id: StringId, - uri_id: UriId, - offset: Offset, - flags: DefinitionFlags, - comments: Vec, - lexical_nesting_id: Option, - visibility: Visibility, -} - -impl AttrAccessorDefinition { - #[must_use] - pub const fn new( - str_id: StringId, - uri_id: UriId, - offset: Offset, - comments: Vec, - flags: DefinitionFlags, - lexical_nesting_id: Option, - visibility: Visibility, - ) -> Self { - Self { - str_id, - uri_id, - offset, - flags, - comments, - lexical_nesting_id, - visibility, - } - } - - #[must_use] - pub fn id(&self) -> DefinitionId { - DefinitionId::from(&format!("{}{}{}", *self.uri_id, self.offset.start(), *self.str_id)) - } - - #[must_use] - pub fn str_id(&self) -> &StringId { - &self.str_id - } - - #[must_use] - pub fn uri_id(&self) -> &UriId { - &self.uri_id - } - - #[must_use] - pub fn offset(&self) -> &Offset { - &self.offset - } - - #[must_use] - pub fn comments(&self) -> &[Comment] { - &self.comments - } - - #[must_use] - pub fn lexical_nesting_id(&self) -> &Option { - &self.lexical_nesting_id - } - - #[must_use] - pub fn visibility(&self) -> &Visibility { - &self.visibility - } - - #[must_use] - pub fn flags(&self) -> &DefinitionFlags { - &self.flags - } -} - -/// An attr reader definition -/// -/// # Example -/// ```ruby -/// attr_reader :foo -/// ``` -#[derive(Debug)] -pub struct AttrReaderDefinition { - str_id: StringId, - uri_id: UriId, - offset: Offset, - flags: DefinitionFlags, - comments: Vec, - lexical_nesting_id: Option, - visibility: Visibility, -} - -impl AttrReaderDefinition { - #[must_use] - pub const fn new( - str_id: StringId, - uri_id: UriId, - offset: Offset, - comments: Vec, - flags: DefinitionFlags, - lexical_nesting_id: Option, - visibility: Visibility, - ) -> Self { - Self { - str_id, - uri_id, - offset, - flags, - comments, - lexical_nesting_id, - visibility, - } - } - - #[must_use] - pub fn id(&self) -> DefinitionId { - DefinitionId::from(&format!("{}{}{}", *self.uri_id, self.offset.start(), *self.str_id)) - } - - #[must_use] - pub fn str_id(&self) -> &StringId { - &self.str_id - } - - #[must_use] - pub fn uri_id(&self) -> &UriId { - &self.uri_id - } - - #[must_use] - pub fn offset(&self) -> &Offset { - &self.offset - } - - #[must_use] - pub fn comments(&self) -> &[Comment] { - &self.comments - } - - #[must_use] - pub fn lexical_nesting_id(&self) -> &Option { - &self.lexical_nesting_id - } - - #[must_use] - pub fn visibility(&self) -> &Visibility { - &self.visibility - } - - #[must_use] - pub fn flags(&self) -> &DefinitionFlags { - &self.flags - } -} - -/// An attr writer definition -/// -/// # Example -/// ```ruby -/// attr_writer :foo -/// ``` -#[derive(Debug)] -pub struct AttrWriterDefinition { - str_id: StringId, - uri_id: UriId, - offset: Offset, - flags: DefinitionFlags, - comments: Vec, - lexical_nesting_id: Option, - visibility: Visibility, -} - -impl AttrWriterDefinition { - #[must_use] - pub const fn new( - str_id: StringId, - uri_id: UriId, - offset: Offset, - comments: Vec, - flags: DefinitionFlags, - lexical_nesting_id: Option, - visibility: Visibility, - ) -> Self { - Self { - str_id, - uri_id, - offset, - flags, - comments, - lexical_nesting_id, - visibility, - } - } - - #[must_use] - pub fn id(&self) -> DefinitionId { - DefinitionId::from(&format!("{}{}{}", *self.uri_id, self.offset.start(), *self.str_id)) - } - - #[must_use] - pub fn str_id(&self) -> &StringId { - &self.str_id - } - - #[must_use] - pub fn comments(&self) -> &[Comment] { - &self.comments - } - - #[must_use] - pub fn uri_id(&self) -> &UriId { - &self.uri_id - } - - #[must_use] - pub fn offset(&self) -> &Offset { - &self.offset - } - - #[must_use] - pub fn lexical_nesting_id(&self) -> &Option { - &self.lexical_nesting_id - } - - #[must_use] - pub fn visibility(&self) -> &Visibility { - &self.visibility - } - - #[must_use] - pub fn flags(&self) -> &DefinitionFlags { - &self.flags - } -} - /// A global variable definition /// /// # Example diff --git a/rust/rubydex/src/model/graph.rs b/rust/rubydex/src/model/graph.rs index 894a9a08..94647878 100644 --- a/rust/rubydex/src/model/graph.rs +++ b/rust/rubydex/src/model/graph.rs @@ -131,9 +131,6 @@ impl Graph { Definition::GlobalVariable(it) => it.str_id(), Definition::InstanceVariable(it) => it.str_id(), Definition::ClassVariable(it) => it.str_id(), - Definition::AttrAccessor(it) => it.str_id(), - Definition::AttrReader(it) => it.str_id(), - Definition::AttrWriter(it) => it.str_id(), Definition::Method(it) => it.str_id(), Definition::MethodAlias(it) => it.new_name_str_id(), Definition::GlobalVariableAlias(it) => it.new_name_str_id(), diff --git a/rust/rubydex/src/resolution.rs b/rust/rubydex/src/resolution.rs index 2cfbaea6..1335b531 100644 --- a/rust/rubydex/src/resolution.rs +++ b/rust/rubydex/src/resolution.rs @@ -301,27 +301,6 @@ impl<'a> Resolver<'a> { Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id))) }); } - Definition::AttrAccessor(attr) => { - let owner_id = self.resolve_lexical_owner(*attr.lexical_nesting_id()); - - self.create_declaration(*attr.str_id(), id, owner_id, |name| { - Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id))) - }); - } - Definition::AttrReader(attr) => { - let owner_id = self.resolve_lexical_owner(*attr.lexical_nesting_id()); - - self.create_declaration(*attr.str_id(), id, owner_id, |name| { - Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id))) - }); - } - Definition::AttrWriter(attr) => { - let owner_id = self.resolve_lexical_owner(*attr.lexical_nesting_id()); - - self.create_declaration(*attr.str_id(), id, owner_id, |name| { - Declaration::Method(Box::new(MethodDeclaration::new(name, owner_id))) - }); - } Definition::GlobalVariable(var) => { let owner_id = *OBJECT_ID; let str_id = *var.str_id(); @@ -3660,7 +3639,7 @@ mod tests { assert_members_eq!( context, "Foo", - vec!["accessor_attr()", "reader_attr()", "writer_attr()"] + vec!["accessor_attr()", "accessor_attr=()", "reader_attr()", "writer_attr=()"] ); } @@ -4354,11 +4333,9 @@ mod tests { assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar::CONST"))); assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar::#@class_ivar"))); assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#baz()"))); - // TODO: needs the fix for attributes - // assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#qux=()"))); + assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#qux=()"))); assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#zip()"))); - // TODO: needs the fix for attributes - // assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#zip=()"))); + assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#zip=()"))); assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#instance_m()"))); assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar#@@class_var"))); assert!(declarations.contains_key(&DeclarationId::from("Foo::Bar::#singleton_m()"))); diff --git a/test/definition_test.rb b/test/definition_test.rb index 909831ca..37e77eca 100644 --- a/test/definition_test.rb +++ b/test/definition_test.rb @@ -18,9 +18,6 @@ def test_instantiating_a_definition_from_ruby_fails assert_raises(NoMethodError) { Rubydex::ConstantDefinition.new } assert_raises(NoMethodError) { Rubydex::ConstantAliasDefinition.new } assert_raises(NoMethodError) { Rubydex::MethodDefinition.new } - assert_raises(NoMethodError) { Rubydex::AttrAccessorDefinition.new } - assert_raises(NoMethodError) { Rubydex::AttrReaderDefinition.new } - assert_raises(NoMethodError) { Rubydex::AttrWriterDefinition.new } assert_raises(NoMethodError) { Rubydex::GlobalVariableDefinition.new } assert_raises(NoMethodError) { Rubydex::InstanceVariableDefinition.new } assert_raises(NoMethodError) { Rubydex::ClassVariableDefinition.new } @@ -57,17 +54,18 @@ def bar; end assert_instance_of(Rubydex::ClassDefinition, defs[0]) assert_instance_of(Rubydex::ClassVariableDefinition, defs[1]) - assert_instance_of(Rubydex::AttrAccessorDefinition, defs[2]) - assert_instance_of(Rubydex::AttrReaderDefinition, defs[3]) - assert_instance_of(Rubydex::AttrWriterDefinition, defs[4]) - assert_instance_of(Rubydex::ModuleDefinition, defs[5]) - assert_instance_of(Rubydex::ConstantAliasDefinition, defs[6]) - assert_instance_of(Rubydex::ConstantDefinition, defs[7]) - assert_instance_of(Rubydex::MethodDefinition, defs[8]) - assert_instance_of(Rubydex::GlobalVariableDefinition, defs[9]) - assert_instance_of(Rubydex::InstanceVariableDefinition, defs[10]) - assert_instance_of(Rubydex::MethodAliasDefinition, defs[11]) - assert_instance_of(Rubydex::GlobalVariableAliasDefinition, defs[12]) + assert_instance_of(Rubydex::MethodDefinition, defs[2]) + assert_instance_of(Rubydex::MethodDefinition, defs[3]) + assert_instance_of(Rubydex::MethodDefinition, defs[4]) + assert_instance_of(Rubydex::MethodDefinition, defs[5]) + assert_instance_of(Rubydex::ModuleDefinition, defs[6]) + assert_instance_of(Rubydex::ConstantAliasDefinition, defs[7]) + assert_instance_of(Rubydex::ConstantDefinition, defs[8]) + assert_instance_of(Rubydex::MethodDefinition, defs[9]) + assert_instance_of(Rubydex::GlobalVariableDefinition, defs[10]) + assert_instance_of(Rubydex::InstanceVariableDefinition, defs[11]) + assert_instance_of(Rubydex::MethodAliasDefinition, defs[12]) + assert_instance_of(Rubydex::GlobalVariableAliasDefinition, defs[13]) end end