From ed480662ed0c8185c1428341e7494207f04cddff Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 14 Nov 2024 14:20:24 -0800 Subject: [PATCH] Explicitly reject fields that are closed enums with implicit presence in Java generators. The normal case of this is already rejected by the .proto parser, this additional check covers legacy_closed_enum cases. Those cases will otherwise reach a CHECK-fail, this just makes for a clearer error message that this is simply a disallowed combination. PiperOrigin-RevId: 696648744 --- .../Reflection/FeatureSetDescriptor.g.cs | 17 -------------- src/google/protobuf/compiler/java/file.cc | 11 ++++++++++ .../compiler/java/generator_unittest.cc | 22 +++++++++++++++++++ 3 files changed, 33 insertions(+), 17 deletions(-) delete mode 100644 csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs diff --git a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs b/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs deleted file mode 100644 index 208ce1fcb6..0000000000 --- a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs +++ /dev/null @@ -1,17 +0,0 @@ -#region Copyright notice and license -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file or at -// https://developers.google.com/open-source/licenses/bsd -#endregion - -namespace Google.Protobuf.Reflection; - -internal sealed partial class FeatureSetDescriptor -{ - // Canonical serialized form of the edition defaults, generated by embed_edition_defaults. - private const string DefaultsBase64 = - "ChMYhAciACoMCAEQAhgCIAMoATACChMY5wciACoMCAIQARgBIAIoATABChMY6AciDAgBEAEYASACKAEwASoAIOYHKOgH"; -} diff --git a/src/google/protobuf/compiler/java/file.cc b/src/google/protobuf/compiler/java/file.cc index c4eb3752d2..34aad86670 100644 --- a/src/google/protobuf/compiler/java/file.cc +++ b/src/google/protobuf/compiler/java/file.cc @@ -240,6 +240,17 @@ bool FileGenerator::Validate(std::string* error) { << "name for the .proto file to be safe."; } + // Check that no field is a closed enum with implicit presence. For normal + // cases this will be rejected by protoc before the generator is invoked, but + // for cases like legacy_closed_enum it may reach the generator. + google::protobuf::internal::VisitDescriptors(*file_, [&](const FieldDescriptor& field) { + if (field.enum_type() != nullptr && !SupportUnknownEnumValue(&field) && + !field.has_presence() && !field.is_repeated()) { + absl::StrAppend(error, "Field ", field.full_name(), + " has a closed enum type with implicit presence.\n"); + } + }); + // Print a warning if optimize_for = LITE_RUNTIME is used. if (file_->options().optimize_for() == FileOptions::LITE_RUNTIME && !options_.enforce_lite) { diff --git a/src/google/protobuf/compiler/java/generator_unittest.cc b/src/google/protobuf/compiler/java/generator_unittest.cc index 8552840bee..55725d3b3d 100644 --- a/src/google/protobuf/compiler/java/generator_unittest.cc +++ b/src/google/protobuf/compiler/java/generator_unittest.cc @@ -65,6 +65,28 @@ TEST_F(JavaGeneratorTest, BasicError) { "foo.proto:4:7: Expected \"required\", \"optional\", or \"repeated\""); } +TEST_F(JavaGeneratorTest, ImplicitPresenceLegacyClosedEnumDisallowed) { + CreateTempFile("foo.proto", + R"schema( + edition = "2023"; + import "third_party/java/protobuf/java_features.proto"; + option features.field_presence = IMPLICIT; + enum Bar { + AAA = 0; + } + message Foo { + Bar bar = 1 [features.(pb.java).legacy_closed_enum = true]; + } + )schema"); + + RunProtoc( + "protocol_compiler --proto_path=$tmpdir --java_out=$tmpdir foo.proto"); + + ExpectErrorSubstring( + "foo.proto: Field Foo.bar has a closed enum type with implicit " + "presence."); +} + } // namespace } // namespace java