diff --git a/experimental/ir/ir_test.go b/experimental/ir/ir_test.go index efe8c856..9c554606 100644 --- a/experimental/ir/ir_test.go +++ b/experimental/ir/ir_test.go @@ -46,6 +46,7 @@ import ( "github.com/bufbuild/protocompile/internal/ext/slicesx" compilerpb "github.com/bufbuild/protocompile/internal/gen/buf/compiler/v1alpha1" "github.com/bufbuild/protocompile/internal/golden" + "github.com/bufbuild/protocompile/internal/messageset" "github.com/bufbuild/protocompile/internal/prototest" ) @@ -85,6 +86,12 @@ type Test struct { // Whether to output a symbol table. Useful for tests that build symbol // tables. Symtab bool `yaml:"symtab"` + + // If true, skip this test when the protobuf-go runtime supports message + // sets (e.g. when built with the `protolegacy` build tag). Used for tests + // that assert on diagnostics that are only emitted when message sets are + // unsupported. + RequiresNoMessageSetSupport bool `yaml:"requires_no_message_set_support"` } func (t *Test) Unmarshal(path string, text string) error { @@ -158,6 +165,10 @@ func TestIR(t *testing.T) { test := new(Test) require.NoError(t, test.Unmarshal(path, text)) + if test.RequiresNoMessageSetSupport && messageset.CanSupportMessageSets() { + t.Skip("skipping: test requires that the protobuf-go runtime does not support message sets") + } + var files source.Opener = source.NewMap(maps.Collect(iterx.Map1To2( slices.Values(test.Files), func(f File) (string, *source.File) { diff --git a/experimental/ir/lower_eval.go b/experimental/ir/lower_eval.go index f5743a82..6f617483 100644 --- a/experimental/ir/lower_eval.go +++ b/experimental/ir/lower_eval.go @@ -496,6 +496,8 @@ validate: mapFieldHelp(d) } + checkMessageSetFieldUsage(member, expr.Key(), e.Report) + return member } diff --git a/experimental/ir/lower_options.go b/experimental/ir/lower_options.go index 9f42450c..4b2347d7 100644 --- a/experimental/ir/lower_options.go +++ b/experimental/ir/lower_options.go @@ -27,8 +27,30 @@ import ( "github.com/bufbuild/protocompile/experimental/token/keyword" "github.com/bufbuild/protocompile/internal/ext/iterx" "github.com/bufbuild/protocompile/internal/intern" + "github.com/bufbuild/protocompile/internal/messageset" ) +// checkMessageSetFieldUsage reports an error if the given field belongs to a +// message set type. Message set wire format is a legacy proto1 feature that +// is not supported. +func checkMessageSetFieldUsage(field Member, span source.Spanner, r *report.Report) { + if field.IsZero() || !field.Container().IsMessageSet() { + return + } + if messageset.CanSupportMessageSets() { + return + } + extendee := field.Container() + r.Errorf( + "field `%s` may not be used in an option: it uses 'message set wire format' legacy proto1 feature which is not supported", + field.FullName(), + ).Apply( + report.Snippet(span), + report.PageBreak, + report.Snippetf(extendee.AST().Stem(), "`%s` declared as message set here", extendee.FullName()), + ) +} + // resolveEarlyOptions resolves options whose values must be discovered very // early during compilation. This does not create option values, nor does it // generate diagnostics; it simply records this information to special fields @@ -483,6 +505,8 @@ func (r optionRef) resolve() { } } + checkMessageSetFieldUsage(field, pc, r.Report) + if pc.IsFirst() { switch field.InternedFullName() { case ids.MapEntry: diff --git a/experimental/ir/testdata/options/values/message_set.proto b/experimental/ir/testdata/options/values/message_set.proto new file mode 100644 index 00000000..580b95fd --- /dev/null +++ b/experimental/ir/testdata/options/values/message_set.proto @@ -0,0 +1,47 @@ +// 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. + +//% requires_no_message_set_support: true +syntax = "proto2"; + +package test; + +import "google/protobuf/descriptor.proto"; + +message MessageSet { + option message_set_wire_format = true; + extensions 1 to max; +} + +message Foo { + extend MessageSet { + optional Foo message_set_field = 12345; + } + optional string name = 1; +} + +extend google.protobuf.FileOptions { + optional MessageSet m1 = 10101; + optional MessageSet m2 = 10102; +} + +// Using a message-set extension as an option path is rejected. +option (m1).(Foo.message_set_field).name = "abc"; + +// Using a message-set extension inside a message literal is also rejected. +option (m2) = { + [test.Foo.message_set_field] { + name: "abc" + } +}; diff --git a/experimental/ir/testdata/options/values/message_set.proto.stderr.txt b/experimental/ir/testdata/options/values/message_set.proto.stderr.txt new file mode 100644 index 00000000..4c7ccd8e --- /dev/null +++ b/experimental/ir/testdata/options/values/message_set.proto.stderr.txt @@ -0,0 +1,34 @@ +warning: message set types are deprecated + --> testdata/options/values/message_set.proto:23:12 + | +22 | message MessageSet { + | ------------------ +23 | option message_set_wire_format = true; + | ^^^^^^^^^^^^^^^^^^^^^^^ declared as message set here + | + = help: message set types are not implemented correctly in most Protobuf + implementations + +error: field `test.Foo.message_set_field` may not be used in an option: it uses + 'message set wire format' legacy proto1 feature which is not supported + --> testdata/options/values/message_set.proto:40:12 + | +40 | option (m1).(Foo.message_set_field).name = "abc"; + | ^^^^^^^^^^^^^^^^^^^^^^^^ + ::: testdata/options/values/message_set.proto:22:1 + | +22 | message MessageSet { + | ------------------ `test.MessageSet` declared as message set here + +error: field `test.Foo.message_set_field` may not be used in an option: it uses + 'message set wire format' legacy proto1 feature which is not supported + --> testdata/options/values/message_set.proto:44:5 + | +44 | [test.Foo.message_set_field] { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ::: testdata/options/values/message_set.proto:22:1 + | +22 | message MessageSet { + | ------------------ `test.MessageSet` declared as message set here + +encountered 2 errors and 1 warning