From 08088bc547fd50e53f1f9c10efa74cefae2e6339 Mon Sep 17 00:00:00 2001 From: Jonathan Baker Date: Tue, 7 Jul 2026 14:48:27 -0400 Subject: [PATCH 1/6] Add FieldDescriptor wrapper and MessageDescriptor#fields Introduce a FieldDescriptor wrapper around FieldDescriptorProto with type-resolution (#type_descriptor via the context), type predicates (#message?/#enum?/#group?/#scalar?), and cardinality helpers (#repeated?/#required?/#optional?/#proto3_optional?). MessageDescriptor now exposes #fields and threads the Context through its initializer so fields can resolve their referenced types. --- lib/proto_plugin.rb | 1 + lib/proto_plugin/field_descriptor.rb | 107 +++++++++++++++++++ lib/proto_plugin/file_descriptor.rb | 2 +- lib/proto_plugin/message_descriptor.rb | 18 +++- test/proto_plugin/field_descriptor_test.rb | 96 +++++++++++++++++ test/proto_plugin/message_descriptor_test.rb | 14 +++ 6 files changed, 235 insertions(+), 3 deletions(-) create mode 100644 lib/proto_plugin/field_descriptor.rb create mode 100644 test/proto_plugin/field_descriptor_test.rb diff --git a/lib/proto_plugin.rb b/lib/proto_plugin.rb index 67f2134..2881bb2 100644 --- a/lib/proto_plugin.rb +++ b/lib/proto_plugin.rb @@ -8,6 +8,7 @@ module ProtoPlugin require_relative "proto_plugin/context" require_relative "proto_plugin/file_descriptor" require_relative "proto_plugin/enum_descriptor" +require_relative "proto_plugin/field_descriptor" require_relative "proto_plugin/message_descriptor" require_relative "proto_plugin/service_descriptor" require_relative "proto_plugin/method_descriptor" diff --git a/lib/proto_plugin/field_descriptor.rb b/lib/proto_plugin/field_descriptor.rb new file mode 100644 index 0000000..3eccd0d --- /dev/null +++ b/lib/proto_plugin/field_descriptor.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require "delegate" + +module ProtoPlugin + # A wrapper class around `Google::Protobuf::FieldDescriptorProto` + # which provides helpers and more idiomatic Ruby access patterns. + # + # Any method not defined directly is delegated to the descriptor the wrapper was initialized with. + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L242 + # Google::Protobuf::FieldDescriptorProto + class FieldDescriptor < SimpleDelegator + # @return [Google::Protobuf::FieldDescriptorProto] + attr_reader :descriptor + + # The message descriptor this field was defined within. + # + # @return [MessageDescriptor] + attr_reader :message + + # @param descriptor [Google::Protobuf::FieldDescriptorProto] + # @param message [MessageDescriptor] The message this field was defined within. + # @param context [Context] + def initialize(descriptor, message, context) + super(descriptor) + @descriptor = descriptor + @message = message + @context = context + end + + # Resolves the message or enum descriptor referenced by this field. + # + # Only message, enum, and group fields reference another type. For scalar + # fields (or when the referenced type was not included in the request), + # `nil` is returned. + # + # @return [MessageDescriptor] if the field is a message or group type + # @return [EnumDescriptor] if the field is an enum type + # @return [nil] if the field is a scalar type or the type was not found + def type_descriptor + return if scalar? + + @context.type_by_proto_name(type_name) + end + + # Returns true if the field is a message type. + # + # @return [Boolean] + def message? + type == :TYPE_MESSAGE + end + + # Returns true if the field is an enum type. + # + # @return [Boolean] + def enum? + type == :TYPE_ENUM + end + + # Returns true if the field is a group type. + # + # @return [Boolean] + def group? + type == :TYPE_GROUP + end + + # Returns true if the field is a scalar type (i.e. not a message, enum, or group). + # + # @return [Boolean] + def scalar? + !message? && !enum? && !group? + end + + # Returns true if the field has the `repeated` label. + # + # @return [Boolean] + def repeated? + label == :LABEL_REPEATED + end + + # Returns true if the field has the `required` label (proto2 only). + # + # @return [Boolean] + def required? + label == :LABEL_REQUIRED + end + + # Returns true if the field has the `optional` label. + # + # @note In proto3 all singular fields carry the `optional` label internally. + # Use {#proto3_optional?} to detect fields with explicit presence tracking. + # + # @return [Boolean] + def optional? + label == :LABEL_OPTIONAL + end + + # Returns true if the field was declared with proto3 explicit presence, + # i.e. an `optional` keyword in a proto3 file. + # + # @return [Boolean] + def proto3_optional? + descriptor.proto3_optional + end + end +end diff --git a/lib/proto_plugin/file_descriptor.rb b/lib/proto_plugin/file_descriptor.rb index 6515547..1579076 100644 --- a/lib/proto_plugin/file_descriptor.rb +++ b/lib/proto_plugin/file_descriptor.rb @@ -42,7 +42,7 @@ def enums # Google::Protobuf::DescriptorProto#message_type def messages @messages ||= @descriptor.message_type.map do |m| - MessageDescriptor.new(m, self) + MessageDescriptor.new(m, self, @context) end end diff --git a/lib/proto_plugin/message_descriptor.rb b/lib/proto_plugin/message_descriptor.rb index 88770f1..bf16940 100644 --- a/lib/proto_plugin/message_descriptor.rb +++ b/lib/proto_plugin/message_descriptor.rb @@ -23,10 +23,24 @@ class MessageDescriptor < SimpleDelegator # @param descriptor [Google::Protobuf::DescriptorProto] # @param parent [FileDescriptorFileDescriptorProto, MessageDescriptor] # The file or message descriptor this message was defined within. - def initialize(descriptor, parent) + # @param context [Context] + def initialize(descriptor, parent, context) super(descriptor) @descriptor = descriptor @parent = parent + @context = context + end + + # The fields defined on this message. + # + # @return [Array] + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L138 + # Google::Protobuf::DescriptorProto#field + def fields + @fields ||= @descriptor.field.map do |f| + FieldDescriptor.new(f, self, @context) + end end # The enums defined as children of this message. @@ -49,7 +63,7 @@ def enums # Google::Protobuf::DescriptorProto#nested_type def messages @nested_messages ||= @descriptor.nested_type.map do |m| - MessageDescriptor.new(m, self) + MessageDescriptor.new(m, self, @context) end end diff --git a/test/proto_plugin/field_descriptor_test.rb b/test/proto_plugin/field_descriptor_test.rb new file mode 100644 index 0000000..fead32c --- /dev/null +++ b/test/proto_plugin/field_descriptor_test.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require "test_helper" + +module ProtoPlugin + class FieldDescriptorTest < Minitest::Test + def setup + @context = Context.new(request: load_request_fixture) + @article = @context.type_by_proto_name(".proto_plugin.fixtures.Article") + @fields = @article.fields.each_with_object({}) do |field, hash| + hash[field.name] = field + end + end + + def test_message + assert_equal(@article, @fields["title"].message) + end + + def test_scalar_type_predicates + title = @fields["title"] + + assert(title.scalar?) + refute(title.message?) + refute(title.enum?) + refute(title.group?) + end + + def test_message_type_predicates + author = @fields["author"] + + assert(author.message?) + refute(author.scalar?) + refute(author.enum?) + end + + def test_type_descriptor_for_scalar + assert_nil(@fields["title"].type_descriptor) + end + + def test_type_descriptor_for_message + author = @fields["author"].type_descriptor + + assert_instance_of(MessageDescriptor, author) + assert_equal("ProtoPlugin::Fixtures::Article::Author", author.full_name) + end + + def test_type_descriptor_for_repeated_message + comment = @fields["comments"].type_descriptor + + assert_instance_of(MessageDescriptor, comment) + assert_equal("ProtoPlugin::Fixtures::Comment", comment.full_name) + end + + def test_type_descriptor_for_imported_message + timestamp = @fields["published_at"].type_descriptor + + assert_instance_of(MessageDescriptor, timestamp) + assert_equal("Timestamp", timestamp.name) + end + + def test_type_descriptor_returns_nil_for_unindexed_type + field = FieldDescriptor.new( + Google::Protobuf::FieldDescriptorProto.new( + name: "mystery", + type: :TYPE_MESSAGE, + type_name: ".does.not.Exist", + ), + @article, + @context, + ) + + assert(field.message?) + assert_nil(field.type_descriptor) + end + + def test_cardinality + assert(@fields["comments"].repeated?) + refute(@fields["comments"].optional?) + + assert(@fields["title"].optional?) + refute(@fields["title"].repeated?) + refute(@fields["title"].required?) + end + + def test_proto3_optional + refute(@fields["title"].proto3_optional?) + end + + def test_delegates_to_descriptor + title = @fields["title"] + + assert_equal("title", title.name) + assert_equal(2, title.number) + end + end +end diff --git a/test/proto_plugin/message_descriptor_test.rb b/test/proto_plugin/message_descriptor_test.rb index 4c566bd..762c6cc 100644 --- a/test/proto_plugin/message_descriptor_test.rb +++ b/test/proto_plugin/message_descriptor_test.rb @@ -31,6 +31,20 @@ def test_messages assert_equal(@message, child_two.parent) end + def test_fields + assert_equal(6, @message.fields.count) + + @message.fields.each do |f| + assert_instance_of(FieldDescriptor, f) + assert_equal(@message, f.message) + end + + assert_equal( + ["id", "title", "author", "content", "published_at", "comments"], + @message.fields.map(&:name), + ) + end + def test_full_name child_one = @message.messages[0] child_two = @message.messages[1] From d99c4bd02aac7c4a9ba40d36ded48f902a9b72ba Mon Sep 17 00:00:00 2001 From: Jonathan Baker Date: Tue, 7 Jul 2026 14:58:29 -0400 Subject: [PATCH 2/6] Add EnumValueDescriptor and OneofDescriptor wrappers Introduce EnumValueDescriptor (name/number/#full_name) exposed via EnumDescriptor#values, and OneofDescriptor (#name/#index/#fields) exposed via MessageDescriptor#oneofs. FieldDescriptor gains #oneof? and #oneof to correlate a field with its containing oneof, excluding proto3 synthetic optional oneofs. Add a CommentEvent message with a oneof to the blog fixtures (and regenerate blog.cgr/blog.fds) to exercise oneof handling. --- lib/proto_plugin.rb | 2 + lib/proto_plugin/enum_descriptor.rb | 12 +++++ lib/proto_plugin/enum_value_descriptor.rb | 40 +++++++++++++++ lib/proto_plugin/field_descriptor.rb | 20 ++++++++ lib/proto_plugin/message_descriptor.rb | 12 +++++ lib/proto_plugin/oneof_descriptor.rb | 46 ++++++++++++++++++ test/fixtures/blog.cgr | Bin 15108 -> 16081 bytes test/fixtures/blog.fds | Bin 2110 -> 2251 bytes test/fixtures/blog/comment.proto | 10 ++++ test/proto_plugin/enum_descriptor_test.rb | 11 +++++ .../enum_value_descriptor_test.rb | 37 ++++++++++++++ test/proto_plugin/field_descriptor_test.rb | 16 ++++++ test/proto_plugin/message_descriptor_test.rb | 11 +++++ test/proto_plugin/oneof_descriptor_test.rb | 38 +++++++++++++++ 14 files changed, 255 insertions(+) create mode 100644 lib/proto_plugin/enum_value_descriptor.rb create mode 100644 lib/proto_plugin/oneof_descriptor.rb create mode 100644 test/proto_plugin/enum_value_descriptor_test.rb create mode 100644 test/proto_plugin/oneof_descriptor_test.rb diff --git a/lib/proto_plugin.rb b/lib/proto_plugin.rb index 2881bb2..ea0beb0 100644 --- a/lib/proto_plugin.rb +++ b/lib/proto_plugin.rb @@ -8,7 +8,9 @@ module ProtoPlugin require_relative "proto_plugin/context" require_relative "proto_plugin/file_descriptor" require_relative "proto_plugin/enum_descriptor" +require_relative "proto_plugin/enum_value_descriptor" require_relative "proto_plugin/field_descriptor" +require_relative "proto_plugin/oneof_descriptor" require_relative "proto_plugin/message_descriptor" require_relative "proto_plugin/service_descriptor" require_relative "proto_plugin/method_descriptor" diff --git a/lib/proto_plugin/enum_descriptor.rb b/lib/proto_plugin/enum_descriptor.rb index 6a016cc..4d6d03e 100644 --- a/lib/proto_plugin/enum_descriptor.rb +++ b/lib/proto_plugin/enum_descriptor.rb @@ -28,6 +28,18 @@ def initialize(descriptor, parent) @parent = parent end + # The values defined for this enum. + # + # @return [Array] + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L343 + # Google::Protobuf::EnumDescriptorProto#value + def values + @values ||= @descriptor.value.map do |v| + EnumValueDescriptor.new(v, self) + end + end + # The full name of the enum, including parent namespace. # # @example diff --git a/lib/proto_plugin/enum_value_descriptor.rb b/lib/proto_plugin/enum_value_descriptor.rb new file mode 100644 index 0000000..d408843 --- /dev/null +++ b/lib/proto_plugin/enum_value_descriptor.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require "delegate" + +module ProtoPlugin + # A wrapper class around `Google::Protobuf::EnumValueDescriptorProto` + # which provides helpers and more idiomatic Ruby access patterns. + # + # Any method not defined directly is delegated to the descriptor the wrapper was initialized with. + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L356 + # Google::Protobuf::EnumValueDescriptorProto + class EnumValueDescriptor < SimpleDelegator + # @return [Google::Protobuf::EnumValueDescriptorProto] + attr_reader :descriptor + + # The enum descriptor this value was defined within. + # + # @return [EnumDescriptor] + attr_reader :enum + + # @param descriptor [Google::Protobuf::EnumValueDescriptorProto] + # @param enum [EnumDescriptor] The enum this value was defined within. + def initialize(descriptor, enum) + super(descriptor) + @descriptor = descriptor + @enum = enum + end + + # The full name of the enum value, including parent namespace. + # + # @example + # "My::Ruby::Package::EnumName::VALUE_NAME" + # + # @return [String] + def full_name + @full_name ||= "#{enum.full_name}::#{name}" + end + end +end diff --git a/lib/proto_plugin/field_descriptor.rb b/lib/proto_plugin/field_descriptor.rb index 3eccd0d..8826725 100644 --- a/lib/proto_plugin/field_descriptor.rb +++ b/lib/proto_plugin/field_descriptor.rb @@ -103,5 +103,25 @@ def optional? def proto3_optional? descriptor.proto3_optional end + + # Returns true if the field is a member of a oneof. + # + # @note Fields declared with the proto3 `optional` keyword are backed by a + # synthetic oneof. Those are not considered oneof members here. + # + # @return [Boolean] + def oneof? + descriptor.has_oneof_index? && !proto3_optional? + end + + # The oneof this field is a member of, if any. + # + # @return [OneofDescriptor] if the field is a member of a oneof + # @return [nil] otherwise + def oneof + return unless oneof? + + message.oneofs[descriptor.oneof_index] + end end end diff --git a/lib/proto_plugin/message_descriptor.rb b/lib/proto_plugin/message_descriptor.rb index bf16940..d98ec3f 100644 --- a/lib/proto_plugin/message_descriptor.rb +++ b/lib/proto_plugin/message_descriptor.rb @@ -43,6 +43,18 @@ def fields end end + # The oneofs defined on this message. + # + # @return [Array] + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L145 + # Google::Protobuf::DescriptorProto#oneof_decl + def oneofs + @oneofs ||= @descriptor.oneof_decl.each_with_index.map do |o, i| + OneofDescriptor.new(o, self, i) + end + end + # The enums defined as children of this message. # # @return [Array] diff --git a/lib/proto_plugin/oneof_descriptor.rb b/lib/proto_plugin/oneof_descriptor.rb new file mode 100644 index 0000000..524385e --- /dev/null +++ b/lib/proto_plugin/oneof_descriptor.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "delegate" + +module ProtoPlugin + # A wrapper class around `Google::Protobuf::OneofDescriptorProto` + # which provides helpers and more idiomatic Ruby access patterns. + # + # Any method not defined directly is delegated to the descriptor the wrapper was initialized with. + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L349 + # Google::Protobuf::OneofDescriptorProto + class OneofDescriptor < SimpleDelegator + # @return [Google::Protobuf::OneofDescriptorProto] + attr_reader :descriptor + + # The message descriptor this oneof was defined within. + # + # @return [MessageDescriptor] + attr_reader :message + + # The index of this oneof within its message's `oneof_decl` list. + # + # @return [Integer] + attr_reader :index + + # @param descriptor [Google::Protobuf::OneofDescriptorProto] + # @param message [MessageDescriptor] The message this oneof was defined within. + # @param index [Integer] The index of this oneof within the message. + def initialize(descriptor, message, index) + super(descriptor) + @descriptor = descriptor + @message = message + @index = index + end + + # The fields that are members of this oneof. + # + # @return [Array] + def fields + @fields ||= message.fields.select do |field| + field.oneof? && field.oneof_index == index + end + end + end +end diff --git a/test/fixtures/blog.cgr b/test/fixtures/blog.cgr index 32d5e697b5aa8827e44af0156028d5a083fac522..485b32309cac00fc4e49675c31f5497a4810dd93 100644 GIT binary patch delta 1247 zcmeH_Uu)A)7{+r>(l&kDZflrpnz{CD_-D#0on?9_LlIO^*w~dIBTY_iFv}@P+TlQU z=E5tFyVab^BI(dNDh2*sG!TIsu~B6F!n^i zL~p9N!_bzz!2%Iu8fN=34+E@Wf>^MbE4l&f34cDVCOvao2R5?(A&-*x=H*H@SzAt% zd^0b#Firs~ki#t@({y?zLsf=dsCM&%P{g8h^c^OxdY2rpy&+J(Ck_sH5Z@o+!&m`u zX1wpYCShb<+byP*hpu5k!3nt?bJwJ(6(1zlr)rd>5_#i zR_+#|FtA_x!ge41$R{tZUYUHfI`kL-sD@)$${N9URl}K7OJ9T%Za=|8SC=&l$~Z1U zP7-8<3zth5GE$MuIuhD+{8v`A)!aUr}XG9Fd%3_9cY0aGcytq-J Q|Bp^?*uQpkXRTJ{H?;BF+5i9m delta 280 zcmcau+fp_mmYY>TR)SH9q3W<9*W>~ng^3;4IS=Y{u?w-vG4yZFVEo0#*uOcM;~t;D zTt+Sy76v8;A(jqCCK0K=$${dAj9ily_{2CAd`pW<6p~UW_i@Prg%~A7xfC+?K80vz6k_RPWD=6P#yB}b!j97~H9fHesDx|sR|y9Wg|ft)($r$E$+nWJ zoM}b*xe5jfCHa%{B~^5Rn%KE`xmbbrGlI?F;DvFSK}HG)Cb1P1<(K3eR~=*Dd_XdZ mk<05Q3m2OZ3j>41WCaVc&F7_Um^Mqx>+mw+Q?SHPmlpsEFGTPF diff --git a/test/fixtures/blog.fds b/test/fixtures/blog.fds index 844cefc16a0a237d7c063c4645d1dae5b2860f55..8fcdff266c65e10796cdbec0f110607cbaedda5f 100644 GIT binary patch delta 176 zcmdlda9VH!Gb8tRW-i|3{M_8sypqWcEV7%m8EqKrmAV+Yc$^`ku4O1QSq>(}N)hCLkoi#g>|q36f%lNtxKCgMGb7V0rp>I3NsODnFkNQ_07&r$?f?J) diff --git a/test/fixtures/blog/comment.proto b/test/fixtures/blog/comment.proto index b98ff0e..0fbe1d4 100644 --- a/test/fixtures/blog/comment.proto +++ b/test/fixtures/blog/comment.proto @@ -23,3 +23,13 @@ message Comment { DELETED = 4; } } + +message CommentEvent { + uint64 comment_id = 1; + + oneof payload { + string created = 2; + string edited = 3; + bool deleted = 4; + } +} diff --git a/test/proto_plugin/enum_descriptor_test.rb b/test/proto_plugin/enum_descriptor_test.rb index e097158..7d799da 100644 --- a/test/proto_plugin/enum_descriptor_test.rb +++ b/test/proto_plugin/enum_descriptor_test.rb @@ -18,5 +18,16 @@ def test_full_name_of_message_enum enum = @article.enums.first assert_equal("ProtoPlugin::Fixtures::Article::Status", enum.full_name) end + + def test_values + assert_equal( + ["CATEGORY_UNSPECIFIED", "CATEGORY_ANNOUNCEMENT", "CATEGORY_PRODUCT_RELEASE"], + @category.values.map(&:name), + ) + + @category.values.each do |v| + assert_instance_of(EnumValueDescriptor, v) + end + end end end diff --git a/test/proto_plugin/enum_value_descriptor_test.rb b/test/proto_plugin/enum_value_descriptor_test.rb new file mode 100644 index 0000000..87f1c5b --- /dev/null +++ b/test/proto_plugin/enum_value_descriptor_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "test_helper" + +module ProtoPlugin + class EnumValueDescriptorTest < Minitest::Test + def setup + @context = Context.new(request: load_request_fixture) + @category = @context.type_by_proto_name(".proto_plugin.fixtures.Category") + @article = @context.type_by_proto_name(".proto_plugin.fixtures.Article") + end + + def test_name_and_number + value = @category.values.first + + assert_instance_of(EnumValueDescriptor, value) + assert_equal("CATEGORY_UNSPECIFIED", value.name) + assert_equal(0, value.number) + end + + def test_enum + value = @category.values.first + assert_equal(@category, value.enum) + end + + def test_full_name_of_file_enum_value + value = @category.values.last + assert_equal("ProtoPlugin::Fixtures::Category::CATEGORY_PRODUCT_RELEASE", value.full_name) + end + + def test_full_name_of_message_enum_value + status = @article.enums.first + value = status.values.first + assert_equal("ProtoPlugin::Fixtures::Article::Status::DRAFT", value.full_name) + end + end +end diff --git a/test/proto_plugin/field_descriptor_test.rb b/test/proto_plugin/field_descriptor_test.rb index fead32c..fa3326d 100644 --- a/test/proto_plugin/field_descriptor_test.rb +++ b/test/proto_plugin/field_descriptor_test.rb @@ -86,6 +86,22 @@ def test_proto3_optional refute(@fields["title"].proto3_optional?) end + def test_oneof_membership + event = @context.type_by_proto_name(".proto_plugin.fixtures.CommentEvent") + fields = event.fields.each_with_object({}) do |field, hash| + hash[field.name] = field + end + + created = fields["created"] + assert(created.oneof?) + assert_instance_of(OneofDescriptor, created.oneof) + assert_equal("payload", created.oneof.name) + + comment_id = fields["comment_id"] + refute(comment_id.oneof?) + assert_nil(comment_id.oneof) + end + def test_delegates_to_descriptor title = @fields["title"] diff --git a/test/proto_plugin/message_descriptor_test.rb b/test/proto_plugin/message_descriptor_test.rb index 762c6cc..c4e6759 100644 --- a/test/proto_plugin/message_descriptor_test.rb +++ b/test/proto_plugin/message_descriptor_test.rb @@ -45,6 +45,17 @@ def test_fields ) end + def test_oneofs + event = @context.type_by_proto_name(".proto_plugin.fixtures.CommentEvent") + + assert_equal(1, event.oneofs.count) + + oneof = event.oneofs.first + assert_instance_of(OneofDescriptor, oneof) + assert_equal("payload", oneof.name) + assert_equal(0, oneof.index) + end + def test_full_name child_one = @message.messages[0] child_two = @message.messages[1] diff --git a/test/proto_plugin/oneof_descriptor_test.rb b/test/proto_plugin/oneof_descriptor_test.rb new file mode 100644 index 0000000..d92c46c --- /dev/null +++ b/test/proto_plugin/oneof_descriptor_test.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "test_helper" + +module ProtoPlugin + class OneofDescriptorTest < Minitest::Test + def setup + @context = Context.new(request: load_request_fixture) + @message = @context.type_by_proto_name(".proto_plugin.fixtures.CommentEvent") + @oneof = @message.oneofs.first + end + + def test_name_and_index + assert_instance_of(OneofDescriptor, @oneof) + assert_equal("payload", @oneof.name) + assert_equal(0, @oneof.index) + end + + def test_message + assert_equal(@message, @oneof.message) + end + + def test_fields + assert_equal( + ["created", "edited", "deleted"], + @oneof.fields.map(&:name), + ) + + @oneof.fields.each do |f| + assert_instance_of(FieldDescriptor, f) + end + end + + def test_fields_excludes_non_members + refute_includes(@oneof.fields.map(&:name), "comment_id") + end + end +end From 26831f08874217aba95c4056413341b0439416c0 Mon Sep 17 00:00:00 2001 From: Jonathan Baker Date: Tue, 7 Jul 2026 15:06:08 -0400 Subject: [PATCH 3/6] Add comment access via SourceCodeInfo (Commentable) Add a Commentable mixin exposing #leading_comments, #trailing_comments, and #leading_detached_comments on every descriptor wrapper (file, message, field, enum, enum value, oneof, service, method). FileDescriptor builds an identity-keyed index from each raw descriptor proto to its SourceCodeInfo::Location by reconstructing the numeric paths protoc emits; wrappers reach their file via a uniform #file accessor. Empty comments normalize to nil. Add comments to the blog fixtures (and regenerate) to exercise comment resolution for each descriptor type. --- lib/proto_plugin.rb | 1 + lib/proto_plugin/commentable.rb | 54 +++++++++++++ lib/proto_plugin/enum_descriptor.rb | 9 +++ lib/proto_plugin/enum_value_descriptor.rb | 9 +++ lib/proto_plugin/field_descriptor.rb | 9 +++ lib/proto_plugin/file_descriptor.rb | 92 ++++++++++++++++++++++ lib/proto_plugin/message_descriptor.rb | 9 +++ lib/proto_plugin/method_descriptor.rb | 9 +++ lib/proto_plugin/oneof_descriptor.rb | 9 +++ lib/proto_plugin/service_descriptor.rb | 9 +++ test/fixtures/blog.cgr | Bin 16081 -> 16733 bytes test/fixtures/blog/article.proto | 8 +- test/fixtures/blog/category.proto | 1 + test/fixtures/blog/comment.proto | 1 + test/fixtures/blog/service.proto | 2 + test/proto_plugin/commentable_test.rb | 74 +++++++++++++++++ 16 files changed, 294 insertions(+), 2 deletions(-) create mode 100644 lib/proto_plugin/commentable.rb create mode 100644 test/proto_plugin/commentable_test.rb diff --git a/lib/proto_plugin.rb b/lib/proto_plugin.rb index ea0beb0..f07f206 100644 --- a/lib/proto_plugin.rb +++ b/lib/proto_plugin.rb @@ -6,6 +6,7 @@ module ProtoPlugin require_relative "proto_plugin/utils" require_relative "proto_plugin/context" +require_relative "proto_plugin/commentable" require_relative "proto_plugin/file_descriptor" require_relative "proto_plugin/enum_descriptor" require_relative "proto_plugin/enum_value_descriptor" diff --git a/lib/proto_plugin/commentable.rb b/lib/proto_plugin/commentable.rb new file mode 100644 index 0000000..6f8bda8 --- /dev/null +++ b/lib/proto_plugin/commentable.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module ProtoPlugin + # A mixin providing access to the comments associated with a descriptor via + # its file's `SourceCodeInfo`. + # + # Including classes must respond to `#file` (returning the {FileDescriptor} + # the element belongs to) and `#descriptor` (returning the raw descriptor + # proto the comments are keyed against). + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L1213 + # Google::Protobuf::SourceCodeInfo::Location + module Commentable + # The `SourceCodeInfo::Location` associated with this element, if source + # info was included in the request. + # + # @return [Google::Protobuf::SourceCodeInfo::Location] + # @return [nil] if no location is available + def source_location + file&.location_for(descriptor) + end + + # The comment block appearing directly above this element. + # + # @return [String] the leading comment, as provided by `protoc` + # @return [nil] if there is no leading comment + def leading_comments + presence(source_location&.leading_comments) + end + + # The comment appearing directly after this element on the same or + # following line. + # + # @return [String] the trailing comment, as provided by `protoc` + # @return [nil] if there is no trailing comment + def trailing_comments + presence(source_location&.trailing_comments) + end + + # Any comment blocks that were detached from this element by one or more + # blank lines. + # + # @return [Array] + def leading_detached_comments + source_location&.leading_detached_comments&.to_a || [] + end + + private + + def presence(value) + value unless value.nil? || value.empty? + end + end +end diff --git a/lib/proto_plugin/enum_descriptor.rb b/lib/proto_plugin/enum_descriptor.rb index 4d6d03e..b09d3d3 100644 --- a/lib/proto_plugin/enum_descriptor.rb +++ b/lib/proto_plugin/enum_descriptor.rb @@ -10,6 +10,8 @@ module ProtoPlugin # # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L336 class EnumDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::EnumDescriptorProto] attr_reader :descriptor @@ -28,6 +30,13 @@ def initialize(descriptor, parent) @parent = parent end + # The file descriptor this enum belongs to. + # + # @return [FileDescriptor] + def file + parent.file + end + # The values defined for this enum. # # @return [Array] diff --git a/lib/proto_plugin/enum_value_descriptor.rb b/lib/proto_plugin/enum_value_descriptor.rb index d408843..e2aa264 100644 --- a/lib/proto_plugin/enum_value_descriptor.rb +++ b/lib/proto_plugin/enum_value_descriptor.rb @@ -11,6 +11,8 @@ module ProtoPlugin # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L356 # Google::Protobuf::EnumValueDescriptorProto class EnumValueDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::EnumValueDescriptorProto] attr_reader :descriptor @@ -27,6 +29,13 @@ def initialize(descriptor, enum) @enum = enum end + # The file descriptor this enum value belongs to. + # + # @return [FileDescriptor] + def file + enum.file + end + # The full name of the enum value, including parent namespace. # # @example diff --git a/lib/proto_plugin/field_descriptor.rb b/lib/proto_plugin/field_descriptor.rb index 8826725..20159ca 100644 --- a/lib/proto_plugin/field_descriptor.rb +++ b/lib/proto_plugin/field_descriptor.rb @@ -11,6 +11,8 @@ module ProtoPlugin # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L242 # Google::Protobuf::FieldDescriptorProto class FieldDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::FieldDescriptorProto] attr_reader :descriptor @@ -29,6 +31,13 @@ def initialize(descriptor, message, context) @context = context end + # The file descriptor this field belongs to. + # + # @return [FileDescriptor] + def file + message.file + end + # Resolves the message or enum descriptor referenced by this field. # # Only message, enum, and group fields reference another type. For scalar diff --git a/lib/proto_plugin/file_descriptor.rb b/lib/proto_plugin/file_descriptor.rb index 1579076..74d3eab 100644 --- a/lib/proto_plugin/file_descriptor.rb +++ b/lib/proto_plugin/file_descriptor.rb @@ -11,6 +11,8 @@ module ProtoPlugin # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L97 # Google::Protobuf::FileDescriptorProto class FileDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::FileDescriptorProto] attr_reader :descriptor @@ -22,6 +24,26 @@ def initialize(context, descriptor) @descriptor = descriptor end + # The file descriptor this element belongs to. + # + # For a `FileDescriptor` this is the descriptor itself. Defined so that + # {Commentable} can resolve comments uniformly across all descriptor types. + # + # @return [FileDescriptor] + def file + self + end + + # Returns the `SourceCodeInfo::Location` for a given raw descriptor proto + # defined within this file, if source info was included in the request. + # + # @param proto [Object] a raw descriptor proto contained in this file + # @return [Google::Protobuf::SourceCodeInfo::Location] + # @return [nil] if no matching location is available + def location_for(proto) + source_locations[proto] + end + # The enums defined as children of this file. # # @return [Array] @@ -85,5 +107,75 @@ def services ServiceDescriptor.new(s, self, @context) end end + + private + + # Field numbers of the relevant repeated fields within their parent + # descriptor proto, used to construct `SourceCodeInfo` paths. + # + # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto + FILE_MESSAGE = 4 + FILE_ENUM = 5 + FILE_SERVICE = 6 + MESSAGE_FIELD = 2 + MESSAGE_NESTED = 3 + MESSAGE_ENUM = 4 + MESSAGE_ONEOF = 8 + ENUM_VALUE = 2 + SERVICE_METHOD = 2 + private_constant :FILE_MESSAGE, + :FILE_ENUM, + :FILE_SERVICE, + :MESSAGE_FIELD, + :MESSAGE_NESTED, + :MESSAGE_ENUM, + :MESSAGE_ONEOF, + :ENUM_VALUE, + :SERVICE_METHOD + + # Builds a map from each raw descriptor proto within this file to its + # `SourceCodeInfo::Location`, keyed by object identity. + # + # The `SourceCodeInfo` locations are addressed by a numeric path into the + # `FileDescriptorProto` tree. This walks that tree in the same order, + # reconstructing each path and associating it with the descriptor found + # there. + # + # @return [Hash] + def source_locations + @source_locations ||= begin + by_path = (@descriptor.source_code_info&.location || []).each_with_object({}) do |loc, hash| + hash[loc.path.to_a] = loc + end + + index = {}.compare_by_identity + assign = ->(proto, path) { (loc = by_path[path]) && index[proto] = loc } + + assign.call(@descriptor, []) + + visit_enum = ->(enum, path) { + assign.call(enum, path) + enum.value.each_with_index { |v, i| assign.call(v, path + [ENUM_VALUE, i]) } + } + + visit_message = ->(message, path) { + assign.call(message, path) + message.field.each_with_index { |f, i| assign.call(f, path + [MESSAGE_FIELD, i]) } + message.oneof_decl.each_with_index { |o, i| assign.call(o, path + [MESSAGE_ONEOF, i]) } + message.enum_type.each_with_index { |e, i| visit_enum.call(e, path + [MESSAGE_ENUM, i]) } + message.nested_type.each_with_index { |n, i| visit_message.call(n, path + [MESSAGE_NESTED, i]) } + } + + @descriptor.message_type.each_with_index { |m, i| visit_message.call(m, [FILE_MESSAGE, i]) } + @descriptor.enum_type.each_with_index { |e, i| visit_enum.call(e, [FILE_ENUM, i]) } + @descriptor.service.each_with_index do |s, i| + sp = [FILE_SERVICE, i] + assign.call(s, sp) + s["method"].each_with_index { |m, j| assign.call(m, sp + [SERVICE_METHOD, j]) } + end + + index + end + end end end diff --git a/lib/proto_plugin/message_descriptor.rb b/lib/proto_plugin/message_descriptor.rb index d98ec3f..23c0814 100644 --- a/lib/proto_plugin/message_descriptor.rb +++ b/lib/proto_plugin/message_descriptor.rb @@ -11,6 +11,8 @@ module ProtoPlugin # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L134 # Google::Protobuf::DescriptorProto class MessageDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::DescriptorProto] attr_reader :descriptor @@ -31,6 +33,13 @@ def initialize(descriptor, parent, context) @context = context end + # The file descriptor this message belongs to. + # + # @return [FileDescriptor] + def file + parent.file + end + # The fields defined on this message. # # @return [Array] diff --git a/lib/proto_plugin/method_descriptor.rb b/lib/proto_plugin/method_descriptor.rb index 199e85f..ddef1ea 100644 --- a/lib/proto_plugin/method_descriptor.rb +++ b/lib/proto_plugin/method_descriptor.rb @@ -10,6 +10,8 @@ module ProtoPlugin # # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L381 class MethodDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::MethodDescriptorProto] attr_reader :descriptor @@ -28,6 +30,13 @@ def initialize(descriptor, service, context) @context = context end + # The file descriptor this method belongs to. + # + # @return [FileDescriptor] + def file + service.file + end + # Returns the `MessageDescriptor` of the method's input type. # # @return [MessageDescriptor] diff --git a/lib/proto_plugin/oneof_descriptor.rb b/lib/proto_plugin/oneof_descriptor.rb index 524385e..474b072 100644 --- a/lib/proto_plugin/oneof_descriptor.rb +++ b/lib/proto_plugin/oneof_descriptor.rb @@ -11,6 +11,8 @@ module ProtoPlugin # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L349 # Google::Protobuf::OneofDescriptorProto class OneofDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::OneofDescriptorProto] attr_reader :descriptor @@ -34,6 +36,13 @@ def initialize(descriptor, message, index) @index = index end + # The file descriptor this oneof belongs to. + # + # @return [FileDescriptor] + def file + message.file + end + # The fields that are members of this oneof. # # @return [Array] diff --git a/lib/proto_plugin/service_descriptor.rb b/lib/proto_plugin/service_descriptor.rb index b4a53a6..c7f9dab 100644 --- a/lib/proto_plugin/service_descriptor.rb +++ b/lib/proto_plugin/service_descriptor.rb @@ -10,6 +10,8 @@ module ProtoPlugin # # @see https://github.com/protocolbuffers/protobuf/blob/v28.2/src/google/protobuf/descriptor.proto#L373 class ServiceDescriptor < SimpleDelegator + include Commentable + # @return [Google::Protobuf::ServiceDescriptorProto] attr_reader :descriptor @@ -28,6 +30,13 @@ def initialize(descriptor, parent, context) @context = context end + # The file descriptor this service belongs to. + # + # @return [FileDescriptor] + def file + parent.file + end + # The full name of the service, including parent namespace. # # @example diff --git a/test/fixtures/blog.cgr b/test/fixtures/blog.cgr index 485b32309cac00fc4e49675c31f5497a4810dd93..a591532ca22f8726e056dac1031c83e888671680 100644 GIT binary patch delta 4873 zcmeHJJC7Sx6rRT&dmf(kdhGFI{JMT*ce5xPq9_6hBoq-+A(VnY;B{tqEgXBXy@?hH zjYPQx&8FusfVKc3gs3Re1R6R@BuWH-fbZOUXS`CH6ciLLKb`Y^_nvd^Irsef`R`xo zU;o%JzVxnsYW$@;&+~3BV460#Z`rz`bH$O<;av9TYEnpqix1yW;LA)F< zCWrBoKRX$%_{r$>G@iwe`D)HrCovz-pFD|Ys{_|9xE2xh!0fms*CQDNpldg!$)pvLP)}5binVo&g&gFZ(YI55(7?R>>`+79s_h($X z$Nco{a5`C@AS3gc$jISzevEXuuE{7Fj$L%`At=qz0@Gtbw8uY2-p*!|PtRgLd5q*u zjwbPfAI%qnuY^@bK|>r~!eoL0q3L8?D+$%Ziw_vCr9!4d_0Z$ay42dXG=am;-jqYX)Z;L z>rN3GIntmiM|LF|UatY4Ecu8(n=d}$lNraGJ6^=gB_y`oyd>dTk?pTB@*V|c*H@x? z5|~Z9n%SvjO}n(l@NXjdsXt)rnmbjhg?E@9M`dlny+CoBh`Y;fSTcJiy{~Q|kW!|I zhuif;atR~X)Qv)mF#LJ3yvE2Wuhud?%zgA^>P}Cue~nI&^oy0X9rS*!p3!mBAtphR z@@W_hukk6gA;v*w2Y-lxAVbpww##m&q0w2pg^JSFsZ@J*Pf-E!Zuac$HO5hUrJBGy znh6K+_v)>+1J2s(@@$GcXO}+=e{sGyCed?AOKU2)u~d1keghyr5s_(vyL)Rq-!yhf? zPcb&pQR6YXh(01U>ZqfT!5pP2Z`a%dm*qqT9rk|I;}7H2_=I?TIhh@!vC|QLc#b(N z8=CyVc5E@92qn0uQyx;Z1W|J5_I-*E2%a8$s7xY9b~#0$d6k_Mf#%(LplB31d!$Sx z;@jdsgCcdNUll`+5N226-+L!@g^K^~M^qgeR@+)2sv}0wHhPTyToG@H$-ff-Z+Ja= zDYTL&|4tc839UkMD0W4wN;wLof}WFKc!Dko<(tEYuF*7Q9$knKJi~$rnGmiQqVq}5 zz%11wsYemkm7YOO*I@D=2Eg?i>Sq9gW5X*7aUjSwf^s7C%z(ADj?|+Mq=l`5?q`x- zTaA7Iv~XLEejr@7twui(Xl*t6fuP>T=yz~gkhs>xIVN@%nr{cE#- x@y-5)eR8vZsr_sBHTJIyeY1eQx`6G!<^p!1Z#FR6@HQLR|7`>N@W0%^{smh`)f)f+ delta 4247 zcmeHJJ&)u>5N+Glo_5<_&)75e%#PRg>! zHX)dZlwW|f(Ito!GJXIN31moQM7-(`&xHg?oWx{)s(P=ws=Lbd%a_5=UwrrdCj3?3 z{IK~WNPdrXT#^u%H;=a;gu&&Z?%o_TO%y0m(gp3L&a`9{l+lG==#*_CK&Q6W6b6#f znMZ6ZYL6nI&AFq6Wz?&4X9_{rIeCZK; zHtT!wrw{Hwy#Mm&hxF$C{;weUdS=tSIy~O)#zcn^Xt8Lt)*&HLC9VQ|N5p9=RhYOU z6lIi9X|w?lHzDm29FsKn2yv6St&ZSSQZlI!0BGq%3t$`5bnbWocxknC;ZUBOY~@54 z#*BP8*38p|qj7ce<;u|j3Tm8*t5ZxgPP0gT1`&I%>e2D2j23j;@d)7-bm0+ri|%=k zz+1fVqN5=|MVHPbAySdO3=v9*RPHi>AX2Toq=ZN{?7$m|1Q&fp8~eI|;JQi*o$Huw zT=-fo^_iYo2y?CSBSxj_vYOmsZd5&)xyX%L95LJ*l*_4)A@KSMe=hDCm+r$EJN&s1 zM1+%WeIEe9cbk<~D>5f-)!I@SqB<=ewUJ^B_qM2x7*!bCx|#T}cX-;~-}lml*d$n_1 zKwR}+&5szd_KW6-LEo?2BSx%!KTN&&XQanWgP{Pxl#&^c5(olm*E>g; zGJ4i~vPd?0@|m)(pK5lv`ReX(Ve+kXQySx|AI;Q?QJq+1<|HwPi&et`VpE+Nz0N0L z0MksF3BZU`srKH68=BpI&C%eP&|A*L$4XS?5opQs%A(Oc!sMkl9FLQu_naqJE>&js zAsjbSTAMM#2-6{b<*xTc@buMpxvD%2&cysUfzQwp{G&0s6lDJx0C;f)jeaI_YDMbj ztbksDm(6;(rBW=d9%oXbMeL6YZ@RK?1wR?V;-~elNLrD2zm`h7VAYNor~7$@q|9duau96myhA%W-9<1 zZtG?%5DvTL**fC1fZ%Q2a0P<;7Q+>PHUg^?Jpx7eQ}c9bO;B0VATDiCp3FQpIH=^H zJ696`gxR^80KswRY665PyE3aYG diff --git a/test/fixtures/blog/article.proto b/test/fixtures/blog/article.proto index bb9521b..d936a63 100644 --- a/test/fixtures/blog/article.proto +++ b/test/fixtures/blog/article.proto @@ -6,10 +6,12 @@ import "google/protobuf/timestamp.proto"; import "comment.proto"; +// An article published on the blog. message Article { + // The unique identifier for the article. uint64 id = 1; - - string title = 2; + + string title = 2; // The article's headline. Author author = 3; @@ -19,7 +21,9 @@ message Article { repeated Comment comments = 6; + // The lifecycle status of an article. enum Status { + // The article is a work in progress. DRAFT = 0; PUBLISHED = 1; DELETED = 2; diff --git a/test/fixtures/blog/category.proto b/test/fixtures/blog/category.proto index 5e8e236..bebebfb 100644 --- a/test/fixtures/blog/category.proto +++ b/test/fixtures/blog/category.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package proto_plugin.fixtures; +// A top-level content category. enum Category { CATEGORY_UNSPECIFIED = 0; CATEGORY_ANNOUNCEMENT = 1; diff --git a/test/fixtures/blog/comment.proto b/test/fixtures/blog/comment.proto index 0fbe1d4..7f2dd5b 100644 --- a/test/fixtures/blog/comment.proto +++ b/test/fixtures/blog/comment.proto @@ -27,6 +27,7 @@ message Comment { message CommentEvent { uint64 comment_id = 1; + // Describes what happened to the comment. oneof payload { string created = 2; string edited = 3; diff --git a/test/fixtures/blog/service.proto b/test/fixtures/blog/service.proto index 82cbcce..11c29a0 100644 --- a/test/fixtures/blog/service.proto +++ b/test/fixtures/blog/service.proto @@ -4,7 +4,9 @@ package proto_plugin.fixtures; import "article.proto"; +// Provides access to articles. service ArticlesService { + // Fetches a single article by id. rpc GetArticle(GetArticleRequest) returns (GetArticleResponse); rpc GetArticles(GetArticlesRequest) returns (GetArticlesResponse); diff --git a/test/proto_plugin/commentable_test.rb b/test/proto_plugin/commentable_test.rb new file mode 100644 index 0000000..afb14a8 --- /dev/null +++ b/test/proto_plugin/commentable_test.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require "test_helper" + +module ProtoPlugin + # Exercises comment resolution (via SourceCodeInfo) across every descriptor type. + class CommentableTest < Minitest::Test + def setup + @context = Context.new(request: load_request_fixture) + @article = @context.type_by_proto_name(".proto_plugin.fixtures.Article") + end + + def test_message_comment + assert_equal(" An article published on the blog.\n", @article.leading_comments) + end + + def test_field_leading_comment + id = @article.fields.find { |f| f.name == "id" } + assert_equal(" The unique identifier for the article.\n", id.leading_comments) + end + + def test_field_trailing_comment + title = @article.fields.find { |f| f.name == "title" } + assert_equal(" The article's headline.\n", title.trailing_comments) + end + + def test_nested_enum_comment + status = @article.enums.first + assert_equal(" The lifecycle status of an article.\n", status.leading_comments) + end + + def test_enum_value_comment + draft = @article.enums.first.values.first + assert_equal(" The article is a work in progress.\n", draft.leading_comments) + end + + def test_file_level_enum_comment + category = @context.type_by_proto_name(".proto_plugin.fixtures.Category") + assert_equal(" A top-level content category.\n", category.leading_comments) + end + + def test_service_comment + service = @context.file_by_filename("service.proto").services.first + assert_equal(" Provides access to articles.\n", service.leading_comments) + end + + def test_method_comment + method = @context.file_by_filename("service.proto").services.first.rpc_methods.first + assert_equal(" Fetches a single article by id.\n", method.leading_comments) + end + + def test_oneof_comment + event = @context.type_by_proto_name(".proto_plugin.fixtures.CommentEvent") + assert_equal(" Describes what happened to the comment.\n", event.oneofs.first.leading_comments) + end + + def test_absent_comments_are_nil + author = @article.fields.find { |f| f.name == "author" } + + assert_nil(author.leading_comments) + assert_nil(author.trailing_comments) + assert_empty(author.leading_detached_comments) + end + + def test_file_resolves_to_owning_descriptor + status = @article.enums.first + + assert_instance_of(FileDescriptor, @article.file) + assert_equal("article.proto", @article.file.name) + assert_equal(@article.file, status.file) + assert_equal(@article.file, status.values.first.file) + end + end +end From 1a65f23acb464ebe43edb275f95b627d9d22b876 Mon Sep 17 00:00:00 2001 From: Jonathan Baker Date: Tue, 7 Jul 2026 15:09:09 -0400 Subject: [PATCH 4/6] Add #comments helper aggregating all comments for an element Returns detached blocks, the leading comment, and the trailing comment as a single array in source order, omitting empties. --- lib/proto_plugin/commentable.rb | 14 ++++++++++++++ test/proto_plugin/commentable_test.rb | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/lib/proto_plugin/commentable.rb b/lib/proto_plugin/commentable.rb index 6f8bda8..a3dd9f3 100644 --- a/lib/proto_plugin/commentable.rb +++ b/lib/proto_plugin/commentable.rb @@ -45,6 +45,20 @@ def leading_detached_comments source_location&.leading_detached_comments&.to_a || [] end + # All comments associated with this element, in source order: any detached + # blocks first, then the leading comment, then the trailing comment. Blocks + # that are absent or empty are omitted. + # + # @example + # field.comments.join("\n").strip + # + # @return [Array] + def comments + [*leading_detached_comments, leading_comments, trailing_comments] + .compact + .reject(&:empty?) + end + private def presence(value) diff --git a/test/proto_plugin/commentable_test.rb b/test/proto_plugin/commentable_test.rb index afb14a8..a265bc9 100644 --- a/test/proto_plugin/commentable_test.rb +++ b/test/proto_plugin/commentable_test.rb @@ -54,6 +54,19 @@ def test_oneof_comment assert_equal(" Describes what happened to the comment.\n", event.oneofs.first.leading_comments) end + def test_comments_aggregates_leading_and_trailing + title = @article.fields.find { |f| f.name == "title" } + assert_equal([" The article's headline.\n"], title.comments) + + id = @article.fields.find { |f| f.name == "id" } + assert_equal([" The unique identifier for the article.\n"], id.comments) + end + + def test_comments_empty_when_absent + author = @article.fields.find { |f| f.name == "author" } + assert_empty(author.comments) + end + def test_absent_comments_are_nil author = @article.fields.find { |f| f.name == "author" } From 01e0a8d69f29948e31b0d7408bf6219603b9093a Mon Sep 17 00:00:00 2001 From: Jonathan Baker Date: Tue, 7 Jul 2026 15:12:09 -0400 Subject: [PATCH 5/6] Exclude detached comments from #comments Per protoc, leading_detached_comments appear before but are "not connected to" the element (separated by a blank line), so they are section headers or notes rather than documentation of the element. #comments now returns only the attached leading and trailing comments; detached blocks remain available via #leading_detached_comments. Add a detached comment to the fixtures to cover the distinction. --- lib/proto_plugin/commentable.rb | 15 +++++++++------ test/fixtures/blog.cgr | Bin 16733 -> 16847 bytes test/fixtures/blog/article.proto | 2 ++ test/proto_plugin/commentable_test.rb | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/proto_plugin/commentable.rb b/lib/proto_plugin/commentable.rb index a3dd9f3..24c80a7 100644 --- a/lib/proto_plugin/commentable.rb +++ b/lib/proto_plugin/commentable.rb @@ -45,18 +45,21 @@ def leading_detached_comments source_location&.leading_detached_comments&.to_a || [] end - # All comments associated with this element, in source order: any detached - # blocks first, then the leading comment, then the trailing comment. Blocks - # that are absent or empty are omitted. + # The comments attached to this element, in source order: the leading + # comment followed by the trailing comment. Absent blocks are omitted. + # + # Detached comments are intentionally excluded. Per protoc, they appear + # before "but [are] not connected to" the element (the author separated + # them with a blank line), so they are section headers or notes rather + # than documentation of the element. Access them via + # {#leading_detached_comments} if needed. # # @example # field.comments.join("\n").strip # # @return [Array] def comments - [*leading_detached_comments, leading_comments, trailing_comments] - .compact - .reject(&:empty?) + [leading_comments, trailing_comments].compact end private diff --git a/test/fixtures/blog.cgr b/test/fixtures/blog.cgr index a591532ca22f8726e056dac1031c83e888671680..7317ad78a0929562a791bac5950b6245d967463b 100644 GIT binary patch delta 411 zcmccH#CX1$af81+Q#0@8K>1&coc}qv*o0UZ7<4A@lvbQvswl#!v$;!gK6AY*7Yhpm zGouiTCW|(!k-37SLP}~$Vsb`mib7s~Nvc9gMq-IVN`7)_ZfaghF-R;UGcR2sGf$x) zv8W_7xilxSNRNw)hl>?t45JXUCXWagFBcn#$HXASti_?o#mB`C;j)1E0th~sc5RXxaS)E6Oi Date: Tue, 7 Jul 2026 15:13:40 -0400 Subject: [PATCH 6/6] Drop leading_detached_comments entirely Detached comments are proto file organization (section headers, notes) rather than documentation attached to any element, and their placement is awkward to reason about. Remove the accessor and the fixture that exercised it; #comments already surfaces only attached leading/trailing comments. --- lib/proto_plugin/commentable.rb | 17 ++++------------- test/fixtures/blog.cgr | Bin 16847 -> 16733 bytes test/fixtures/blog/article.proto | 2 -- test/proto_plugin/commentable_test.rb | 15 --------------- 4 files changed, 4 insertions(+), 30 deletions(-) diff --git a/lib/proto_plugin/commentable.rb b/lib/proto_plugin/commentable.rb index 24c80a7..6229444 100644 --- a/lib/proto_plugin/commentable.rb +++ b/lib/proto_plugin/commentable.rb @@ -37,22 +37,13 @@ def trailing_comments presence(source_location&.trailing_comments) end - # Any comment blocks that were detached from this element by one or more - # blank lines. - # - # @return [Array] - def leading_detached_comments - source_location&.leading_detached_comments&.to_a || [] - end - # The comments attached to this element, in source order: the leading # comment followed by the trailing comment. Absent blocks are omitted. # - # Detached comments are intentionally excluded. Per protoc, they appear - # before "but [are] not connected to" the element (the author separated - # them with a blank line), so they are section headers or notes rather - # than documentation of the element. Access them via - # {#leading_detached_comments} if needed. + # Detached comments (blocks the author separated from the element with a + # blank line) are not exposed. Per protoc they appear before "but [are] + # not connected to" the element, so they are file organization rather than + # documentation of any element. # # @example # field.comments.join("\n").strip diff --git a/test/fixtures/blog.cgr b/test/fixtures/blog.cgr index 7317ad78a0929562a791bac5950b6245d967463b..a591532ca22f8726e056dac1031c83e888671680 100644 GIT binary patch delta 318 zcmX@#%y_qnaf81+(sc5RXxaS)E6Oi1&coc}qv*o0UZ7<4A@lvbQvswl#!v$;!gK6AY*7Yhpm zGouiTCW|(!k-37SLP}~$Vsb`mib7s~Nvc9gMq-IVN`7)_ZfaghF-R;UGcR2sGf$x) zv8W_7xilxSNRNw)hl>?t45JXUCXWagFBcn#$HXASti_?o#mB`C;j)1E0th~