Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions experimental/ir/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type builtins struct {
OptionTargets Member
CType, JSType Member
Lazy, UnverifiedLazy Member
AllowAlias Member
MessageSet Member

JSONName Member
Expand Down
18 changes: 10 additions & 8 deletions experimental/ir/ir_member.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ type rawMember struct {
options arena.Pointer[rawValue]
oneof int32
optionTargets uint32
jsonName intern.ID

jsonName intern.ID

isGroup bool
isGroup bool
numberOk bool // An error occurred while computing the field number.
}

// IsMessageField returns whether this is a non-extension message field.
Expand Down Expand Up @@ -584,12 +584,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.
Expand Down
20 changes: 13 additions & 7 deletions experimental/ir/lower_numbers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -100,6 +99,7 @@ func evaluateFieldNumbers(f File, r *report.Report) {

tags.raw.first = n
tags.raw.last = n
tags.raw.rangeOk = ok
}
}
}
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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}
Expand Down
24 changes: 24 additions & 0 deletions experimental/ir/lower_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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.
Expand Down Expand Up @@ -98,6 +99,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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
Expand All @@ -35,7 +35,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -51,7 +51,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -70,7 +70,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -94,7 +94,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -119,7 +119,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -145,7 +145,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -169,7 +169,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand Down Expand Up @@ -201,7 +201,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -227,7 +227,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -241,7 +241,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -255,7 +255,7 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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"
Expand All @@ -273,6 +273,6 @@ tables:
- { extn: "buf.test.f", name: "d", value: "false" }
- { extn: "buf.test.f", name: "e", value: "<invalid type>" }
- { 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: {} } } }
33 changes: 33 additions & 0 deletions experimental/ir/testdata/tags/allow_alias.proto
Original file line number Diff line number Diff line change
@@ -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;
}
15 changes: 15 additions & 0 deletions experimental/ir/testdata/tags/allow_alias.proto.stderr.txt
Original file line number Diff line number Diff line change
@@ -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
11 changes: 10 additions & 1 deletion experimental/ir/testdata/tags/fields.proto.stderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,15 @@ error: field number out of range
= note: the range for field numbers is `0x1 to 0x1fffffff`,
minus `0x4a38 to 0x4e1f`, which is reserved for internal use

warning: message fields have the same JSON name
--> testdata/tags/fields.proto:33:20
|
31 | optional int32 z6 = 0xffffffffffffffffffffffffffffffff;
| -- this implies JSON name `z6`
32 |
33 | optional int32 z6 = -1;
| ^^ this also implies that name

error: field number out of range
--> testdata/tags/fields.proto:33:25
|
Expand Down Expand Up @@ -302,4 +311,4 @@ error: cannot find `a` in this scope
|
= help: the full name of this scope is `test.M`

encountered 34 errors
encountered 34 errors and 1 warning
5 changes: 5 additions & 0 deletions experimental/ir/testdata/tags/reserved.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,9 @@ enum E {

reserved = 100;
reserved "reserved", "unused";
}

enum Z {
K = 0;
reserved 0;
}
10 changes: 9 additions & 1 deletion experimental/ir/testdata/tags/reserved.proto.stderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,12 @@ error: use of reserved enum value `100`
45 | reserved = 100;
| ^^^ used here

encountered 16 errors and 2 warnings
error: use of reserved enum value `0`
--> testdata/tags/reserved.proto:50:9
|
50 | K = 0;
| ^ used here
51 | reserved 0;
| - enum value reserved here

encountered 17 errors and 2 warnings
Loading