Skip to content

Commit

Permalink
Explicitly reject fields that are closed enums with implicit presence…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Nov 14, 2024
1 parent 251f5ee commit ed48066
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 17 deletions.
17 changes: 0 additions & 17 deletions csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs

This file was deleted.

11 changes: 11 additions & 0 deletions src/google/protobuf/compiler/java/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 22 additions & 0 deletions src/google/protobuf/compiler/java/generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ed48066

Please sign in to comment.