From 96b6d1e48ea3c16bced84bea2396498220b65042 Mon Sep 17 00:00:00 2001 From: Doria Keung Date: Mon, 27 Apr 2026 18:48:06 -0400 Subject: [PATCH] Support older vesrion of descriptor.proto --- experimental/ir/builtins.go | 2 +- experimental/ir/lower_options.go | 92 +++++-- .../broken_missing_required.proto.yaml | 7 + ...ken_missing_required.proto.yaml.stderr.txt | 24 +- .../vendored_no_range_options.proto.yaml | 229 ++++++++++++++++++ ...dored_no_range_options.proto.yaml.fds.yaml | 4 + ...red_no_range_options.proto.yaml.stderr.txt | 15 ++ ...ed_no_range_options.proto.yaml.symtab.yaml | 9 + 8 files changed, 349 insertions(+), 33 deletions(-) create mode 100644 experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml create mode 100644 experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.fds.yaml create mode 100644 experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.stderr.txt create mode 100644 experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.symtab.yaml diff --git a/experimental/ir/builtins.go b/experimental/ir/builtins.go index 79002eb2..2d135e03 100644 --- a/experimental/ir/builtins.go +++ b/experimental/ir/builtins.go @@ -42,7 +42,7 @@ type builtins struct { MessageOptions Member FieldOptions Member OneofOptions Member - RangeOptions Member + RangeOptions Member `builtin:"optional"` EnumOptions Member EnumValueOptions Member ServiceOptions Member diff --git a/experimental/ir/lower_options.go b/experimental/ir/lower_options.go index e995e0cd..c6fb7d03 100644 --- a/experimental/ir/lower_options.go +++ b/experimental/ir/lower_options.go @@ -108,6 +108,7 @@ func resolveEarlyOptions(file *File) { // resolveOptions resolves all of the options in a file. func resolveOptions(file *File, r *report.Report) { builtins := file.builtins() + ids := &file.session.builtins bodyOptions := func(decls seq.Inserter[ast.DeclAny]) iter.Seq[ast.Option] { return iterx.FilterMap(seq.Values(decls), func(d ast.DeclAny) (ast.Option, bool) { def := d.AsDef() @@ -126,8 +127,9 @@ func resolveOptions(file *File, r *report.Report) { scope: file.Package(), def: def, - field: builtins.FileOptions, - raw: &file.options, + field: builtins.FileOptions, + fieldFQN: ids.FileOptions, + raw: &file.options, }.resolve() } @@ -141,8 +143,10 @@ func resolveOptions(file *File, r *report.Report) { for def := range bodyOptions(ty.AST().Body().Decls()) { options := builtins.MessageOptions + optionsFQN := ids.MessageOptions if ty.IsEnum() { options = builtins.EnumOptions + optionsFQN = ids.EnumOptions } optionRef{ File: file, @@ -151,16 +155,19 @@ func resolveOptions(file *File, r *report.Report) { scope: ty.Scope(), def: def, - field: options, - raw: &ty.Raw().options, + field: options, + fieldFQN: optionsFQN, + raw: &ty.Raw().options, }.resolve() } for field := range seq.Values(ty.Members()) { for def := range seq.Values(field.AST().Options().Entries()) { options := builtins.FieldOptions + optionsFQN := ids.FieldOptions if ty.IsEnum() { options = builtins.EnumValueOptions + optionsFQN = ids.EnumValueOptions } optionRef{ File: file, @@ -169,9 +176,10 @@ func resolveOptions(file *File, r *report.Report) { scope: field.Scope(), def: def, - field: options, - raw: &field.Raw().options, - target: field, + field: options, + fieldFQN: optionsFQN, + raw: &field.Raw().options, + target: field, }.resolve() } } @@ -184,8 +192,9 @@ func resolveOptions(file *File, r *report.Report) { scope: ty.Scope(), def: def, - field: builtins.OneofOptions, - raw: &oneof.Raw().options, + field: builtins.OneofOptions, + fieldFQN: ids.OneofOptions, + raw: &oneof.Raw().options, }.resolve() } } @@ -206,8 +215,9 @@ func resolveOptions(file *File, r *report.Report) { scope: ty.Scope(), def: def, - field: builtins.RangeOptions, - raw: &extns.Raw().options, + field: builtins.RangeOptions, + fieldFQN: ids.RangeOptions, + raw: &extns.Raw().options, }.resolve() } @@ -223,9 +233,10 @@ func resolveOptions(file *File, r *report.Report) { scope: field.Scope(), def: def, - field: builtins.FieldOptions, - raw: &field.Raw().options, - target: field, + field: builtins.FieldOptions, + fieldFQN: ids.FieldOptions, + raw: &field.Raw().options, + target: field, }.resolve() } } @@ -238,8 +249,9 @@ func resolveOptions(file *File, r *report.Report) { scope: service.FullName(), def: def, - field: builtins.ServiceOptions, - raw: &service.Raw().options, + field: builtins.ServiceOptions, + fieldFQN: ids.ServiceOptions, + raw: &service.Raw().options, }.resolve() } @@ -252,8 +264,9 @@ func resolveOptions(file *File, r *report.Report) { scope: service.FullName(), def: def, - field: builtins.MethodOptions, - raw: &method.Raw().options, + field: builtins.MethodOptions, + fieldFQN: ids.MethodOptions, + raw: &method.Raw().options, }.resolve() } } @@ -398,12 +411,55 @@ type optionRef struct { field Member raw *id.ID[Value] + // fieldFQN is the interned FQN of the *Options builtin field that field + // resolves to. It is set even when field is zero (because the vendored + // descriptor.proto does not declare it), so the diagnostic emitted on the + // zero-field path can name the missing symbol. + fieldFQN intern.ID + // A member being annotated. This is used for pseudo-option resolution. target Member } // resolve performs symbol resolution. func (r optionRef) resolve() { + // If the *Options builtin we are resolving into is unresolved, we cannot + // resolve the option ref. This is the case when an invalid descriptor.proto + // has been vendored (missing a required builtin) or when the user wrote an + // option targeting an optional descriptor.proto field that the vendored + // copy predates. In both cases we bail out before the resolver attempts to + // dereference the zero builtin, which would otherwise panic in + // [Member.toRef] and [newMessage]. + // + // Note that `protoc` does not respect vendored descriptor.protos for option + // resolution; it always uses its own internally-bundled copy. This compiler + // and the legacy compiler both honor the vendored descriptor.proto so that + // users can depend on their own descriptor.proto files, and so downstream + // tooling built on the compiler can provide descriptor.proto (and the + // well-known types as a whole). + // + // We emit a diagnostic at the user's option site so they have a pointer + // from their source to the underlying descriptor.proto problem. + // [resolveBuiltins] also emits a "missing required symbol" diagnostic on + // descriptor.proto for required builtins that is complementary to this error. + if r.field.IsZero() { + fqn := r.session.intern.Value(r.fieldFQN) + d := r.Errorf("cannot resolve %s", taxa.Option).Apply( + report.Snippet(r.def), + ) + if dpFile := r.imports.DescriptorProto(); dpFile != nil { + d.Apply(report.Snippetf(dpFile.AST().Syntax(), + "resolved against this descriptor.proto")) + } + if _, optional := r.session.optionalBuiltins[r.fieldFQN]; optional { + d.Apply(report.Helpf( + "`%s` is not declared by this descriptor.proto; "+ + "use a newer descriptor.proto or remove this option", + fqn)) + } + return + } + ids := &r.session.builtins root := r.field.Element() diff --git a/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml b/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml index 30ad06e7..e17c6914 100644 --- a/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml +++ b/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml @@ -16,6 +16,11 @@ # like FileOptions, MessageOptions, FieldOptions) is diagnosed by the # compiler with one error per missing required symbol, attached to # descriptor.proto itself rather than to the user's proto file. +# +# The user file additionally sets `option go_package = ...` so we exercise +# the optionRef.resolve guard against a zero `field` builtin: with FileOptions +# unresolved, lowering would otherwise panic when constructing an options +# message rooted at the unresolved builtin. exclude_wkt_sources: true descriptor: true symtab: true @@ -43,6 +48,8 @@ files: syntax = "proto3"; package test; + option go_package = "example.com"; + message Simple { string name = 1; } diff --git a/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml.stderr.txt b/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml.stderr.txt index 38715e9e..dd02e39f 100644 --- a/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml.stderr.txt +++ b/experimental/ir/testdata/builtins/broken_missing_required.proto.yaml.stderr.txt @@ -1,17 +1,3 @@ -error: `google/protobuf/descriptor.proto` is missing required symbol - `google.protobuf.DescriptorProto.ExtensionRange.options` - --> google/protobuf/descriptor.proto:1:1 - | - 1 | syntax = "proto2"; - | ^ - 2 | / package google.protobuf; -... | -14 | | } - | \_^ - = help: the descriptor.proto supplied to the compiler does not declare this - message field; it may be vendored from a version that predates this - symbol, or may be genuinely corrupt - error: `google/protobuf/descriptor.proto` is missing required symbol `google.protobuf.DescriptorProto.options` --> google/protobuf/descriptor.proto:1:1 @@ -376,4 +362,14 @@ error: `google/protobuf/descriptor.proto` is missing required symbol message field; it may be vendored from a version that predates this symbol, or may be genuinely corrupt +error: cannot resolve option setting + --> test.proto:4:8 + | + 4 | option go_package = "example.com"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + ::: google/protobuf/descriptor.proto:1:1 + | + 1 | syntax = "proto2"; + | ------------------ resolved against this descriptor.proto + encountered 27 errors diff --git a/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml new file mode 100644 index 00000000..0fa87fe2 --- /dev/null +++ b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml @@ -0,0 +1,229 @@ +# Copyright 2020-2025 Buf Technologies, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http:#www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Tests that a vendored pre-v3.4.0 descriptor.proto, which lacks +# `DescriptorProto.ExtensionRange.options` and `ExtensionRangeOptions`, does +# not crash the compiler when a user file declares an extension range bearing +# options. RangeOptions is an optional builtin (introduced in protoc v3.4.0), +# so its absence is silent; the optionRef.resolve guard prevents a nil-context +# panic when the resolver tries to root an options message at the unresolved +# builtin. +exclude_wkt_sources: true +descriptor: true +symtab: true +files: +- path: "google/protobuf/descriptor.proto" + import: true + text: | + syntax = "proto2"; + package google.protobuf; + option go_package = "google.golang.org/protobuf/types/descriptorpb"; + + message FileDescriptorSet { + repeated FileDescriptorProto file = 1; + } + + message FileDescriptorProto { + optional string name = 1; + optional string package = 2; + repeated string dependency = 3; + repeated DescriptorProto message_type = 4; + repeated EnumDescriptorProto enum_type = 5; + repeated ServiceDescriptorProto service = 6; + repeated FieldDescriptorProto extension = 7; + optional FileOptions options = 8; + optional string syntax = 12; + } + + message DescriptorProto { + optional string name = 1; + repeated FieldDescriptorProto field = 2; + repeated FieldDescriptorProto extension = 6; + repeated DescriptorProto nested_type = 3; + repeated EnumDescriptorProto enum_type = 4; + message ExtensionRange { + optional int32 start = 1; + optional int32 end = 2; + } + repeated ExtensionRange extension_range = 5; + repeated OneofDescriptorProto oneof_decl = 8; + optional MessageOptions options = 7; + } + + message FieldDescriptorProto { + enum Type { + TYPE_DOUBLE = 1; + TYPE_FLOAT = 2; + TYPE_INT64 = 3; + TYPE_UINT64 = 4; + TYPE_INT32 = 5; + TYPE_FIXED64 = 6; + TYPE_FIXED32 = 7; + TYPE_BOOL = 8; + TYPE_STRING = 9; + TYPE_GROUP = 10; + TYPE_MESSAGE = 11; + TYPE_BYTES = 12; + TYPE_UINT32 = 13; + TYPE_ENUM = 14; + TYPE_SFIXED32 = 15; + TYPE_SFIXED64 = 16; + TYPE_SINT32 = 17; + TYPE_SINT64 = 18; + } + enum Label { + LABEL_OPTIONAL = 1; + LABEL_REPEATED = 3; + LABEL_REQUIRED = 2; + } + optional string name = 1; + optional int32 number = 3; + optional Label label = 4; + optional Type type = 5; + optional string type_name = 6; + optional string extendee = 2; + optional string default_value = 7; + optional int32 oneof_index = 9; + optional string json_name = 10; + optional FieldOptions options = 8; + } + + message OneofDescriptorProto { + optional string name = 1; + optional OneofOptions options = 2; + } + + message EnumDescriptorProto { + optional string name = 1; + repeated EnumValueDescriptorProto value = 2; + optional EnumOptions options = 3; + } + + message EnumValueDescriptorProto { + optional string name = 1; + optional int32 number = 2; + optional EnumValueOptions options = 3; + } + + message ServiceDescriptorProto { + optional string name = 1; + repeated MethodDescriptorProto method = 2; + optional ServiceOptions options = 3; + } + + message MethodDescriptorProto { + optional string name = 1; + optional string input_type = 2; + optional string output_type = 3; + optional MethodOptions options = 4; + } + + message FileOptions { + optional string java_package = 1; + optional string java_outer_classname = 8; + optional bool java_multiple_files = 10 [default = false]; + optional bool java_string_check_utf8 = 27 [default = false]; + enum OptimizeMode { + SPEED = 1; + CODE_SIZE = 2; + LITE_RUNTIME = 3; + } + optional OptimizeMode optimize_for = 9 [default = SPEED]; + optional string go_package = 11; + optional bool deprecated = 23 [default = false]; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message MessageOptions { + optional bool message_set_wire_format = 1 [default = false]; + optional bool no_standard_descriptor_accessor = 2 [default = false]; + optional bool deprecated = 3 [default = false]; + optional bool map_entry = 7; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message FieldOptions { + optional CType ctype = 1 [default = STRING]; + enum CType { + STRING = 0; + CORD = 1; + STRING_PIECE = 2; + } + optional bool packed = 2; + optional JSType jstype = 6 [default = JS_NORMAL]; + enum JSType { + JS_NORMAL = 0; + JS_STRING = 1; + JS_NUMBER = 2; + } + optional bool lazy = 5 [default = false]; + optional bool deprecated = 3 [default = false]; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message OneofOptions { + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message EnumOptions { + optional bool allow_alias = 2; + optional bool deprecated = 3 [default = false]; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message EnumValueOptions { + optional bool deprecated = 1 [default = false]; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message ServiceOptions { + optional bool deprecated = 33 [default = false]; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message MethodOptions { + optional bool deprecated = 33 [default = false]; + repeated UninterpretedOption uninterpreted_option = 999; + extensions 1000 to max; + } + + message UninterpretedOption { + message NamePart { + required string name_part = 1; + required bool is_extension = 2; + } + repeated NamePart name = 2; + optional string identifier_value = 3; + optional uint64 positive_int_value = 4; + optional int64 negative_int_value = 5; + optional double double_value = 6; + optional bytes string_value = 7; + optional string aggregate_value = 8; + } + +- path: "test.proto" + text: | + syntax = "proto2"; + package test; + + message Foo { + extensions 100 to 200 [verification = DECLARATION]; + } diff --git a/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.fds.yaml b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.fds.yaml new file mode 100644 index 00000000..827f3302 --- /dev/null +++ b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.fds.yaml @@ -0,0 +1,4 @@ +file: +- name: "test.proto" + package: "test" + message_type: [{ name: "Foo", extension_range: [{ start: 100, end: 201 }] }] diff --git a/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.stderr.txt b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.stderr.txt new file mode 100644 index 00000000..432ad658 --- /dev/null +++ b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.stderr.txt @@ -0,0 +1,15 @@ +error: cannot resolve option setting + --> test.proto:5:26 + | + 5 | extensions 100 to 200 [verification = DECLARATION]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + ::: google/protobuf/descriptor.proto:1:1 + | + 1 | syntax = "proto2"; + | ------------------ resolved against this descriptor.proto + | + = help: `google.protobuf.DescriptorProto.ExtensionRange.options` is not + declared by this descriptor.proto; use a newer descriptor.proto or + remove this option + +encountered 1 error diff --git a/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.symtab.yaml b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.symtab.yaml new file mode 100644 index 00000000..9549eabe --- /dev/null +++ b/experimental/ir/testdata/builtins/vendored_no_range_options.proto.yaml.symtab.yaml @@ -0,0 +1,9 @@ +tables: + "test.proto": + symbols: + - { fqn: "test", kind: KIND_PACKAGE, file: "test.proto" } + - fqn: "test.Foo" + kind: KIND_MESSAGE + file: "test.proto" + index: 1 + visible: true