Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update 'syntax' and synthetic oneof to support protobuf 27.1 #208

Merged
merged 1 commit into from
Jun 14, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ permissions: read-all

# update in build.yml and codeql.yml at same time
env:
PROTOC_VERSION: 23.1
PROTOC_VERSION: 27.1

jobs:
build:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ permissions: read-all

# update in build.yml and codeql.yml at same time
env:
PROTOC_VERSION: 23.1
PROTOC_VERSION: 27.1

on:
push:
Expand Down
2 changes: 1 addition & 1 deletion MODULE.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module(name = "protobuf_javascript", version = "3.21.2")

bazel_dep(name = "protobuf", version = "23.1", repo_name = "com_google_protobuf")
bazel_dep(name = "protobuf", version = "27.1", repo_name = "com_google_protobuf")
bazel_dep(name = "rules_pkg", version = "0.7.0")
122 changes: 122 additions & 0 deletions MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
name = "com_google_protobuf",
urls = ["https://github.com/protocolbuffers/protobuf/archive/refs/tags/v23.1.zip"],
sha256 = "c0ea9f4d75b37ea8e9d78ce4c670d066bcb7cebdba190fa5fc8c57b1f00c0c2c",
strip_prefix = "protobuf-23.1",
urls = ["https://github.com/protocolbuffers/protobuf/archive/refs/tags/v27.1.zip"],
sha256 = "7d7f2ddccc37e3c1c5dfe65ad69d99923d8fe84beac68ed9cdec489909c4d8d3",
strip_prefix = "protobuf-27.1",
)

load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
Expand Down
36 changes: 17 additions & 19 deletions generator/js_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,14 @@ bool IgnoreField(const FieldDescriptor* field) {
// Do we ignore this message type?
bool IgnoreMessage(const Descriptor* d) { return d->options().map_entry(); }

bool IsSyntheticOneof(const OneofDescriptor* oneof) {
return oneof->field_count() == 1 &&
oneof->field(0)->real_containing_oneof() == nullptr;
}

// Does JSPB ignore this entire oneof? True only if all fields are ignored.
bool IgnoreOneof(const OneofDescriptor* oneof) {
if (OneofDescriptorLegacy(oneof).is_synthetic()) return true;
if (IsSyntheticOneof(oneof)) { return true; }
for (int i = 0; i < oneof->field_count(); i++) {
if (!IgnoreField(oneof->field(i))) {
return false;
Expand Down Expand Up @@ -563,7 +568,7 @@ std::string JSOneofIndex(const OneofDescriptor* oneof) {
int index = -1;
for (int i = 0; i < oneof->containing_type()->oneof_decl_count(); i++) {
const OneofDescriptor* o = oneof->containing_type()->oneof_decl(i);
if (OneofDescriptorLegacy(o).is_synthetic()) continue;
if (IsSyntheticOneof(o)) { continue; }
// If at least one field in this oneof is not JSPB-ignored, count the oneof.
for (int j = 0; j < o->field_count(); j++) {
const FieldDescriptor* f = o->field(j);
Expand Down Expand Up @@ -783,8 +788,7 @@ std::string DoubleToString(double value) {
}

bool InRealOneof(const FieldDescriptor* field) {
return field->containing_oneof() &&
!OneofDescriptorLegacy(field->containing_oneof()).is_synthetic();
return field->real_containing_oneof() != nullptr;
}

// Return true if this is an integral field that should be represented as string
Expand Down Expand Up @@ -984,7 +988,7 @@ bool DeclaredReturnTypeIsNullable(const GeneratorOptions& options,
return false;
}

if (FileDescriptorLegacy(field->file()).syntax() == FileDescriptorLegacy::Syntax::SYNTAX_PROTO3 &&
if (!field->has_presence() && !field->is_repeated() &&
field->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) {
return false;
}
Expand Down Expand Up @@ -2345,17 +2349,13 @@ void Generator::GenerateClassFieldToObject(const GeneratorOptions& options,
printer->Print("msg.get$getter$()", "getter",
JSGetterName(options, field, BYTES_B64));
} else {
bool use_default = field->has_default_value();

if (FileDescriptorLegacy(field->file()).syntax() == FileDescriptorLegacy::Syntax::SYNTAX_PROTO3 &&
// Repeated fields get initialized to their default in the constructor
// (why?), so we emit a plain getField() call for them.
!field->is_repeated()) {
// Proto3 puts all defaults (including implicit defaults) in toObject().
// But for proto2 we leave the existing semantics unchanged: unset fields
// without default are unset.
use_default = true;
}
// We rely on the default field value if it is explicit in the .proto file
// or if the field in question doesn't have presence semantics (consider
// proto3 fields without optional, repeated fields)
// Repeated fields get initialized to their default in the constructor
// (why?), so we emit a plain getField() call for them.
const bool use_default = !field->is_repeated() &&
(field->has_default_value() || !field->has_presence());

// We don't implement this by calling the accessors, because the semantics
// of the accessors are changing independently of the toObject() semantics.
Expand Down Expand Up @@ -2755,9 +2755,7 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
/* force_present = */ false,
/* singular_if_not_packed = */ false));

if (FileDescriptorLegacy(field->file()).syntax() == FileDescriptorLegacy::Syntax::SYNTAX_PROTO3 &&
!field->is_repeated() && !field->is_map() &&
!HasFieldPresence(options, field)) {
if (!field->is_repeated() && !field->is_map() && !field->has_presence()) {
// Proto3 non-repeated and non-map fields without presence use the
// setProto3*Field function.
printer->Print(
Expand Down
Loading