Skip to content

Commit

Permalink
Add java to reserved names to escape extensions to java_. This ma…
Browse files Browse the repository at this point in the history
…y break existing references to gencode for extensions named `java`.

This prevents shadowing of `java.lang` package commonly used in protobuf gencode. Existing extensions named `java` may or may not previously fail to compile depending on if the contents of their .proto result in gencode using `java.lang`. This is needed to fix `java_features.proto` lite gencode since enum gencode uses `java.lang`. Fields named `java` should already be escaped.

*Warning: This may break user code for existing protos with extensions named `java`. References to the extension should be renamed to use `java_` e.g. registry.add(GeneratedClassName.java_)*

PiperOrigin-RevId: 632508249
  • Loading branch information
zhangskz authored and Copybara-Service committed May 10, 2024
1 parent 91b7cf3 commit c99cf4b
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 28 deletions.
2 changes: 1 addition & 1 deletion editions/golden/editions_transform_proto2.proto
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ edition = "2023";
package protobuf_editions_test;

import "net/proto/proto1_features.proto";
import "third_party/java/protobuf/java_features.proto";
import "google/protobuf/java_features.proto";
import "google/protobuf/cpp_features.proto";
import "google/protobuf/editions/proto/editions_transform_proto3.proto";

Expand Down
1 change: 1 addition & 0 deletions java/core/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ proto_library(
deps = [
"//:any_proto",
"//:descriptor_proto",
"//:java_features_proto",
"//:lite_test_protos",
"//:wrappers_proto",
"//src/google/protobuf:generic_test_protos",
Expand Down
8 changes: 4 additions & 4 deletions java/core/src/main/java/com/google/protobuf/Descriptors.java
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static FeatureSetDefaults getJavaEditionDefaults() {
if (javaEditionDefaults == null) {
try {
ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(JavaFeaturesProto.java);
registry.add(JavaFeaturesProto.java_);
setTestJavaEditionDefaults(
FeatureSetDefaults.parseFrom(
JavaEditionDefaults.PROTOBUF_INTERNAL_JAVA_EDITION_DEFAULTS.getBytes(
Expand Down Expand Up @@ -679,7 +679,7 @@ FeatureSet inferLegacyProtoFeatures() {
if (getEdition() == Edition.EDITION_PROTO2) {
if (proto.getOptions().getJavaStringCheckUtf8()) {
features.setExtension(
JavaFeaturesProto.java,
JavaFeaturesProto.java_,
JavaFeatures.newBuilder()
.setUtf8Validation(JavaFeatures.Utf8Validation.VERIFY)
.build());
Expand Down Expand Up @@ -1320,7 +1320,7 @@ public boolean needsUtf8Check() {
return true;
}
if (this.features
.getExtension(JavaFeaturesProto.java)
.getExtension(JavaFeaturesProto.java_)
.getUtf8Validation()
.equals(JavaFeatures.Utf8Validation.VERIFY)) {
return true;
Expand Down Expand Up @@ -1577,7 +1577,7 @@ public boolean legacyEnumFieldTreatedAsClosed() {
}

return getType() == Type.ENUM
&& (this.features.getExtension(JavaFeaturesProto.java).getLegacyClosedEnum()
&& (this.features.getExtension(JavaFeaturesProto.java_).getLegacyClosedEnum()
|| enumType.isClosed());
}

Expand Down
10 changes: 5 additions & 5 deletions java/core/src/test/java/com/google/protobuf/DescriptorsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ public void testLegacyInferProto2Utf8Validation() throws Exception {
.setOptions(FileOptions.newBuilder().setJavaStringCheckUtf8(true))
.build(),
new FileDescriptor[0]);
assertThat(file.features.getExtension(JavaFeaturesProto.java).getUtf8Validation())
assertThat(file.features.getExtension(JavaFeaturesProto.java_).getUtf8Validation())
.isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.VERIFY);
}

Expand All @@ -1178,8 +1178,8 @@ public void testProto2Defaults() {
assertThat(features.getJsonFormat())
.isEqualTo(DescriptorProtos.FeatureSet.JsonFormat.LEGACY_BEST_EFFORT);

assertThat(features.getExtension(JavaFeaturesProto.java).getLegacyClosedEnum()).isTrue();
assertThat(features.getExtension(JavaFeaturesProto.java).getUtf8Validation())
assertThat(features.getExtension(JavaFeaturesProto.java_).getLegacyClosedEnum()).isTrue();
assertThat(features.getExtension(JavaFeaturesProto.java_).getUtf8Validation())
.isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.DEFAULT);
}

Expand All @@ -1198,8 +1198,8 @@ public void testProto3Defaults() {
assertThat(features.getMessageEncoding())
.isEqualTo(DescriptorProtos.FeatureSet.MessageEncoding.LENGTH_PREFIXED);

assertThat(features.getExtension(JavaFeaturesProto.java).getLegacyClosedEnum()).isFalse();
assertThat(features.getExtension(JavaFeaturesProto.java).getUtf8Validation())
assertThat(features.getExtension(JavaFeaturesProto.java_).getLegacyClosedEnum()).isFalse();
assertThat(features.getExtension(JavaFeaturesProto.java_).getUtf8Validation())
.isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.DEFAULT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ edition = "2023";

package proto2_test_check_utf8;

option features.utf8_validation = VERIFY;
import "google/protobuf/java_features.proto";

option features.utf8_validation = NONE;
option features.(pb.java).utf8_validation = VERIFY;
option java_outer_classname = "TestCheckUtf8";

message StringWrapper {
Expand Down
4 changes: 2 additions & 2 deletions objectivec/Tests/unittest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1109,11 +1109,11 @@ message TestNestedGroupExtensionOuter {
repeated group Layer2RepeatedGroup = 2 {
extensions 3
// NOTE: extension metadata is not supported due to targets such as
// `//third_party/protobuf_legacy_opensource/src:shell_scripts_test`,
// `//google/protobuf_legacy_opensource/src:shell_scripts_test`,
// eee https://screenshot.googleplex.com/Axz2QD8nxjdpyFF
//[metadata = {
// NOTE: can't write type there due to some clever build gen code at
// http://google3/net/proto2/internal/BUILD;l=1247;rcl=411090862
// http://google3/google/protobuf/BUILD;l=1247;rcl=411090862
// type: "objc.protobuf.tests.TestNestedGroupExtensionInnerExtension",
// name: "inner",
// }]
Expand Down
22 changes: 11 additions & 11 deletions src/google/protobuf/compiler/java/names.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ const char* DefaultPackage(Options options) {
bool IsReservedName(absl::string_view name) {
static const auto& kReservedNames =
*new absl::flat_hash_set<absl::string_view>({
"abstract", "assert", "boolean", "break", "byte",
"case", "catch", "char", "class", "const",
"continue", "default", "do", "double", "else",
"enum", "extends", "false", "final", "finally",
"float", "for", "goto", "if", "implements",
"import", "instanceof", "int", "interface", "long",
"native", "new", "null", "package", "private",
"protected", "public", "return", "short", "static",
"strictfp", "super", "switch", "synchronized", "this",
"throw", "throws", "transient", "true", "try",
"void", "volatile", "while",
"abstract", "assert", "boolean", "break", "byte",
"case", "catch", "char", "class", "const",
"continue", "default", "do", "double", "else",
"enum", "extends", "false", "final", "finally",
"float", "for", "goto", "if", "implements",
"import", "instanceof", "int", "interface", "java",
"long", "native", "new", "null", "package",
"private", "protected", "public", "return", "short",
"static", "strictfp", "super", "switch", "synchronized",
"this", "throw", "throws", "transient", "true",
"try", "void", "volatile", "while",
});
return kReservedNames.contains(name);
}
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/edition_unittest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1254,11 +1254,11 @@ message TestNestedGroupExtensionOuter {
message Layer2RepeatedGroup {
extensions 3
// NOTE: extension metadata is not supported due to targets such as
// `//third_party/protobuf_legacy_opensource/src:shell_scripts_test`,
// `//google/protobuf_legacy_opensource/src:shell_scripts_test`,
// eee https://screenshot.googleplex.com/Axz2QD8nxjdpyFF
//[metadata = {
// NOTE: can't write type there due to some clever build gen code at
// http://google3/net/proto2/internal/BUILD;l=1247;rcl=411090862
// http://google3/google/protobuf/BUILD;l=1247;rcl=411090862
// type: "edition_unittest.TestNestedGroupExtensionInnerExtension",
// name: "inner",
// }]
Expand Down
4 changes: 2 additions & 2 deletions src/google/protobuf/unittest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1226,11 +1226,11 @@ message TestNestedGroupExtensionOuter {
repeated group Layer2RepeatedGroup = 2 {
extensions 3
// NOTE: extension metadata is not supported due to targets such as
// `//third_party/protobuf_legacy_opensource/src:shell_scripts_test`,
// `//google/protobuf_legacy_opensource/src:shell_scripts_test`,
// eee https://screenshot.googleplex.com/Axz2QD8nxjdpyFF
//[metadata = {
// NOTE: can't write type there due to some clever build gen code at
// http://google3/net/proto2/internal/BUILD;l=1247;rcl=411090862
// http://google3/google/protobuf/BUILD;l=1247;rcl=411090862
// type: "protobuf_unittest.TestNestedGroupExtensionInnerExtension",
// name: "inner",
// }]
Expand Down

0 comments on commit c99cf4b

Please sign in to comment.