From a2e950c397a8a18c9871b0e0fb4bbb6e5b4a59fb Mon Sep 17 00:00:00 2001 From: Miguel Young de la Sota Date: Wed, 15 Oct 2025 16:20:04 -0700 Subject: [PATCH] implement allow_alias --- experimental/ir/builtins.go | 1 + experimental/ir/ir_member.go | 16 +++++---- experimental/ir/lower_numbers.go | 20 +++++++---- experimental/ir/lower_validate.go | 24 ++++++++++++++ .../incomplete_feature.proto.symtab.yaml | 26 +++++++-------- .../ir/testdata/tags/allow_alias.proto | 33 +++++++++++++++++++ .../tags/allow_alias.proto.stderr.txt | 15 +++++++++ experimental/ir/testdata/tags/reserved.proto | 5 +++ .../testdata/tags/reserved.proto.stderr.txt | 10 +++++- 9 files changed, 123 insertions(+), 27 deletions(-) create mode 100644 experimental/ir/testdata/tags/allow_alias.proto create mode 100644 experimental/ir/testdata/tags/allow_alias.proto.stderr.txt diff --git a/experimental/ir/builtins.go b/experimental/ir/builtins.go index b27a4215e..2a38a9844 100644 --- a/experimental/ir/builtins.go +++ b/experimental/ir/builtins.go @@ -48,6 +48,7 @@ type builtins struct { OptionTargets Member CType, JSType Member Lazy, UnverifiedLazy Member + AllowAlias Member MessageSet Member FileDeprecated Member diff --git a/experimental/ir/ir_member.go b/experimental/ir/ir_member.go index fd1cf126f..765cb70c9 100644 --- a/experimental/ir/ir_member.go +++ b/experimental/ir/ir_member.go @@ -60,7 +60,9 @@ type rawMember struct { options arena.Pointer[rawValue] oneof int32 optionTargets uint32 - isGroup bool + + isGroup bool + numberOk bool // An error occurred while computing the field number. } // IsMessageField returns whether this is a non-extension message field. @@ -559,12 +561,14 @@ type ReservedRange struct { } type rawReservedRange struct { - decl ast.DeclRange - value ast.ExprAny - first, last int32 - options arena.Pointer[rawValue] - features arena.Pointer[rawFeatureSet] + decl ast.DeclRange + value ast.ExprAny + first, last int32 + options arena.Pointer[rawValue] + features arena.Pointer[rawFeatureSet] + forExtensions bool + rangeOk bool } // AST returns the expression that this range was evaluated from, if known. diff --git a/experimental/ir/lower_numbers.go b/experimental/ir/lower_numbers.go index 0c897b12e..e7a657bd1 100644 --- a/experimental/ir/lower_numbers.go +++ b/experimental/ir/lower_numbers.go @@ -46,10 +46,8 @@ func evaluateFieldNumbers(f File, r *report.Report) { } for member := range seq.Values(ty.Members()) { - n, ok := evaluateMemberNumber(f.Context(), scope, member.AST().Value(), kind, false, r) - if ok { - member.raw.number = n - } + member.raw.number, member.raw.numberOk = evaluateMemberNumber( + f.Context(), scope, member.AST().Value(), kind, false, r) } for _, raw := range ty.raw.ranges { @@ -91,6 +89,7 @@ func evaluateFieldNumbers(f File, r *report.Report) { tags.raw.first = start tags.raw.last = end + tags.raw.rangeOk = startOk && endOk default: n, ok := evaluateMemberNumber(f.Context(), scope, tags.AST(), kind, false, r) @@ -100,6 +99,7 @@ func evaluateFieldNumbers(f File, r *report.Report) { tags.raw.first = n tags.raw.last = n + tags.raw.rangeOk = ok } } } @@ -118,7 +118,8 @@ func evaluateFieldNumbers(f File, r *report.Report) { scope = ty.FullName() } - extn.raw.number, _ = evaluateMemberNumber(f.Context(), scope, extn.AST().Value(), kind, false, r) + extn.raw.number, extn.raw.numberOk = evaluateMemberNumber( + f.Context(), scope, extn.AST().Value(), kind, false, r) } } @@ -177,7 +178,7 @@ func buildFieldNumberRanges(f File, r *report.Report) { seq.Values(ty.ExtensionRanges()), ) { lo, hi := tagRange.Range() - if lo == 0 || hi == 0 { + if !tagRange.raw.rangeOk { continue // Diagnosed already. } disjoint := ty.raw.rangesByNumber.Insert(lo, hi, rawTagRange{ @@ -198,7 +199,7 @@ func buildFieldNumberRanges(f File, r *report.Report) { // first value is a member. for member := range seq.Values(ty.Members()) { n := member.Number() - if n == 0 { + if !member.raw.numberOk { continue // Diagnosed already. } ty.raw.rangesByNumber.Insert(n, n, rawTagRange{ @@ -215,6 +216,11 @@ func buildFieldNumberRanges(f File, r *report.Report) { } first := TagRange{ty.withContext, entry.Value[0]} + if ty.AllowsAlias() && first.raw.isMember { + // If all of the members of the intersections are members, + // we don't diagnose. + continue + } for _, tags := range entry.Value[1:] { tags := TagRange{ty.withContext, tags} diff --git a/experimental/ir/lower_validate.go b/experimental/ir/lower_validate.go index 5fb3ef0f9..0876dde62 100644 --- a/experimental/ir/lower_validate.go +++ b/experimental/ir/lower_validate.go @@ -25,6 +25,7 @@ import ( "github.com/bufbuild/protocompile/experimental/seq" "github.com/bufbuild/protocompile/experimental/token/keyword" "github.com/bufbuild/protocompile/internal/ext/iterx" + "github.com/bufbuild/protocompile/internal/ext/mapsx" ) // diagnoseUnusedImports generates diagnostics for each unused import. @@ -94,6 +95,29 @@ func validateEnum(ty Type, r *report.Report) { return } + // Check if allow_alias is actually used. This does not happen in + // lower_numbers.go because we want to be able to include the allow_alias + // option span in the diagnostic. + if ty.AllowsAlias() { + // Check to see if there are at least two enum values with the same + // number. + var hasAlias bool + numbers := make(map[int32]struct{}) + for member := range seq.Values(ty.Members()) { + if !mapsx.AddZero(numbers, member.Number()) { + hasAlias = true + break + } + } + + if !hasAlias { + option := ty.Options().Field(builtins.AllowAlias) + r.Errorf("`%s` requires at least one aliasing %s", option.Field().Name(), taxa.EnumValue).Apply( + report.Snippet(option.OptionSpan()), + ) + } + } + first := ty.Members().At(0) if first.Number() != 0 && !ty.IsClosedEnum() { // Figure out why this enum is open. diff --git a/experimental/ir/testdata/editions/incomplete_feature.proto.symtab.yaml b/experimental/ir/testdata/editions/incomplete_feature.proto.symtab.yaml index 2a34acddd..f932253bf 100644 --- a/experimental/ir/testdata/editions/incomplete_feature.proto.symtab.yaml +++ b/experimental/ir/testdata/editions/incomplete_feature.proto.symtab.yaml @@ -16,7 +16,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } symbols: - fqn: "buf.test" kind: KIND_PACKAGE @@ -35,7 +35,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Enum" kind: KIND_ENUM file: "testdata/editions/incomplete_feature.proto" @@ -51,7 +51,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.a" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -70,7 +70,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.b" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -94,7 +94,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.c" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -119,7 +119,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.d" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -145,7 +145,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.e" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -169,7 +169,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.f" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -201,7 +201,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.Features.g" kind: KIND_FIELD file: "testdata/editions/incomplete_feature.proto" @@ -227,7 +227,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.A" kind: KIND_ENUM_VALUE file: "testdata/editions/incomplete_feature.proto" @@ -241,7 +241,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.B" kind: KIND_ENUM_VALUE file: "testdata/editions/incomplete_feature.proto" @@ -255,7 +255,7 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } - fqn: "buf.test.f" kind: KIND_EXTENSION file: "testdata/editions/incomplete_feature.proto" @@ -273,6 +273,6 @@ tables: - { extn: "buf.test.f", name: "d", value: "false" } - { extn: "buf.test.f", name: "e", value: "" } - { extn: "buf.test.f", name: "f", value: "true" } - - { extn: "buf.test.f", name: "g", value: "0" } + - { extn: "buf.test.f", name: "g", value: "A" } options.message.fields: "features": { message.extns: { "buf.test.f": { message: {} } } } diff --git a/experimental/ir/testdata/tags/allow_alias.proto b/experimental/ir/testdata/tags/allow_alias.proto new file mode 100644 index 000000000..88020e4fe --- /dev/null +++ b/experimental/ir/testdata/tags/allow_alias.proto @@ -0,0 +1,33 @@ +// 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. + +edition = "2023"; + +package buf.test; + +enum NoAlias { + A1 = 0; + B1 = 0; +} + +enum AllowAlias { + option allow_alias = true; + A2 = 0; + B2 = 0; +} + +enum NeedsAlias { + option allow_alias = true; + A3 = 0; +} diff --git a/experimental/ir/testdata/tags/allow_alias.proto.stderr.txt b/experimental/ir/testdata/tags/allow_alias.proto.stderr.txt new file mode 100644 index 000000000..59f3e8197 --- /dev/null +++ b/experimental/ir/testdata/tags/allow_alias.proto.stderr.txt @@ -0,0 +1,15 @@ +error: enum value `0` used more than once + --> testdata/tags/allow_alias.proto:21:10 + | +20 | A1 = 0; + | - previously used here +21 | B1 = 0; + | ^ used here + +error: `allow_alias` requires at least one aliasing enum value + --> testdata/tags/allow_alias.proto:31:12 + | +31 | option allow_alias = true; + | ^^^^^^^^^^^^^^^^^^ + +encountered 2 errors diff --git a/experimental/ir/testdata/tags/reserved.proto b/experimental/ir/testdata/tags/reserved.proto index 288e61f1e..4821b61c2 100644 --- a/experimental/ir/testdata/tags/reserved.proto +++ b/experimental/ir/testdata/tags/reserved.proto @@ -38,4 +38,9 @@ enum E { reserved 2, 3 to 3, 5 to 10; reserved 7, 8 to max; reserved max to max; +} + +enum Z { + K = 0; + reserved 0; } \ No newline at end of file diff --git a/experimental/ir/testdata/tags/reserved.proto.stderr.txt b/experimental/ir/testdata/tags/reserved.proto.stderr.txt index ceb0d2a5e..79a3cf98f 100644 --- a/experimental/ir/testdata/tags/reserved.proto.stderr.txt +++ b/experimental/ir/testdata/tags/reserved.proto.stderr.txt @@ -130,4 +130,12 @@ error: `max` outside of range end | ^^^ = note: the special `max` expression can only be used at the end of a range -encountered 12 errors and 2 warnings +error: use of reserved enum value `0` + --> testdata/tags/reserved.proto:44:9 + | +44 | K = 0; + | ^ used here +45 | reserved 0; + | - enum value reserved here + +encountered 13 errors and 2 warnings