Skip to content

Commit

Permalink
Update 'syntax' and synthetic oneof to support protobuf 27.1 descript…
Browse files Browse the repository at this point in the history
…or API (#208)
  • Loading branch information
dibenede authored Jun 14, 2024
1 parent 45fbfcb commit 893b51d
Show file tree
Hide file tree
Showing 7 changed files with 533 additions and 79 deletions.
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

0 comments on commit 893b51d

Please sign in to comment.