From 3836058feec68d3c85f47fed65f082ac8e024cc6 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Tue, 21 Nov 2023 20:04:02 +0100 Subject: [PATCH 1/6] Expose editions features in descriptor sets --- .../protobuf-test/src/descriptor-set.test.ts | 210 ++++++++++++++++- packages/protobuf/package.json | 2 +- .../protobuf/src/create-descriptor-set.ts | 211 ++++++++++++++---- packages/protobuf/src/descriptor-set.ts | 51 ++++- .../src/private/feature-set-defaults.ts | 22 -- packages/protobuf/src/private/feature-set.ts | 122 ++++++++++ packages/protobuf/tsconfig.json | 3 +- .../protoplugin-test/src/transpile.test.ts | 5 +- 8 files changed, 548 insertions(+), 78 deletions(-) delete mode 100644 packages/protobuf/src/private/feature-set-defaults.ts create mode 100644 packages/protobuf/src/private/feature-set.ts diff --git a/packages/protobuf-test/src/descriptor-set.test.ts b/packages/protobuf-test/src/descriptor-set.test.ts index 717d16f5e..585cb1e98 100644 --- a/packages/protobuf-test/src/descriptor-set.test.ts +++ b/packages/protobuf-test/src/descriptor-set.test.ts @@ -14,9 +14,17 @@ import { describe, expect, test } from "@jest/globals"; import { readFileSync } from "fs"; +import type { AnyDesc } from "@bufbuild/protobuf"; import { createDescriptorSet, Edition, + FeatureSet, + FeatureSet_EnumType, + FeatureSet_JsonFormat, + FeatureSet_MessageEncoding, + FeatureSet_RepeatedFieldEncoding, + FeatureSet_Utf8Validation, + FeatureSet_FieldPresence, proto3, ScalarType, } from "@bufbuild/protobuf"; @@ -40,12 +48,32 @@ describe("DescriptorSet", () => { expect(descFile).toBeDefined(); expect(descFile?.syntax).toBe("proto2"); expect(descFile?.edition).toBe(Edition.EDITION_PROTO2); + expect(descFile?.getFeatures()).toStrictEqual( + new FeatureSet({ + fieldPresence: FeatureSet_FieldPresence.EXPLICIT, + enumType: FeatureSet_EnumType.CLOSED, + repeatedFieldEncoding: FeatureSet_RepeatedFieldEncoding.EXPANDED, + utf8Validation: FeatureSet_Utf8Validation.NONE, + messageEncoding: FeatureSet_MessageEncoding.LENGTH_PREFIXED, + jsonFormat: FeatureSet_JsonFormat.LEGACY_BEST_EFFORT, + }), + ); }); test("proto3 syntax", () => { const descFile = set.files.find((f) => f.name == "extra/proto3"); expect(descFile).toBeDefined(); expect(descFile?.syntax).toBe("proto3"); expect(descFile?.edition).toBe(Edition.EDITION_PROTO3); + expect(descFile?.getFeatures()).toStrictEqual( + new FeatureSet({ + fieldPresence: FeatureSet_FieldPresence.IMPLICIT, + enumType: FeatureSet_EnumType.OPEN, + repeatedFieldEncoding: FeatureSet_RepeatedFieldEncoding.PACKED, + utf8Validation: FeatureSet_Utf8Validation.VERIFY, + messageEncoding: FeatureSet_MessageEncoding.LENGTH_PREFIXED, + jsonFormat: FeatureSet_JsonFormat.ALLOW, + }), + ); }); test("edition 2023", () => { const descFile = set.files.find( @@ -54,6 +82,132 @@ describe("DescriptorSet", () => { expect(descFile).toBeDefined(); expect(descFile?.syntax).toBe("editions"); expect(descFile?.edition).toBe(Edition.EDITION_2023); + expect(descFile?.getFeatures()).toStrictEqual( + new FeatureSet({ + fieldPresence: FeatureSet_FieldPresence.EXPLICIT, + enumType: FeatureSet_EnumType.OPEN, + repeatedFieldEncoding: FeatureSet_RepeatedFieldEncoding.PACKED, + utf8Validation: FeatureSet_Utf8Validation.VERIFY, + messageEncoding: FeatureSet_MessageEncoding.LENGTH_PREFIXED, + jsonFormat: FeatureSet_JsonFormat.ALLOW, + }), + ); + }); + describe("edition feature options", () => { + test("file options should apply to all elements", async () => { + const bin = await new UpstreamProtobuf().compileToDescriptorSet(` + edition = "2023"; + option features.field_presence = IMPLICIT; + option features.enum_type = CLOSED; + option features.repeated_field_encoding = EXPANDED; + option features.utf8_validation = NONE; + option features.message_encoding = DELIMITED; + option features.json_format = LEGACY_BEST_EFFORT; + message M { + message NestedMessage { + message DeepNestedMessage {} + enum DeepNestedEnum { A = 0; } + } + enum NestedEnum { A = 0; } + } + enum E { A = 0; } + service S { + rpc A(M) returns (M); + }`); + const { files, messages, enums, services } = createDescriptorSet(bin); + const wantFeatures = new FeatureSet({ + fieldPresence: FeatureSet_FieldPresence.IMPLICIT, + enumType: FeatureSet_EnumType.CLOSED, + repeatedFieldEncoding: FeatureSet_RepeatedFieldEncoding.EXPANDED, + utf8Validation: FeatureSet_Utf8Validation.NONE, + messageEncoding: FeatureSet_MessageEncoding.DELIMITED, + jsonFormat: FeatureSet_JsonFormat.LEGACY_BEST_EFFORT, + }); + expect(files[0].getFeatures()).toStrictEqual(wantFeatures); + expect(messages.get("M")?.getFeatures()).toStrictEqual(wantFeatures); + expect(messages.get("M.NestedMessage")?.getFeatures()).toStrictEqual( + wantFeatures, + ); + expect( + messages.get("M.NestedMessage.DeepNestedMessage")?.getFeatures(), + ).toStrictEqual(wantFeatures); + expect(enums.get("E")?.getFeatures()).toStrictEqual(wantFeatures); + expect(enums.get("M.NestedEnum")?.getFeatures()).toStrictEqual( + wantFeatures, + ); + expect( + enums.get("M.NestedMessage.DeepNestedEnum")?.getFeatures(), + ).toStrictEqual(wantFeatures); + expect(services.get("S")?.getFeatures()).toStrictEqual(wantFeatures); + expect(services.get("S")?.methods[0].getFeatures()).toStrictEqual( + wantFeatures, + ); + }); + test("message options should apply to nested elements", async () => { + const bin = await new UpstreamProtobuf().compileToDescriptorSet(` + edition="2023"; + enum EnumJsonAllow { A = 0; } + message MessageJsonAllow { + int32 f = 1; + enum EnumJsonAllow { A = 0; } + message MessageJsonAllow { + int32 f = 1; + } + message MessageJsonLegacy { + option features.json_format = LEGACY_BEST_EFFORT; + int32 f = 1; + enum EnumJsonLegacy { A = 0; } + message MessageJsonLegacy { + int32 f = 1; + } + } + }`); + const { messages, enums } = createDescriptorSet(bin); + function expectJsonFormat( + jsonFormat: FeatureSet_JsonFormat, + desc: AnyDesc | undefined, + ) { + expect(desc?.getFeatures().jsonFormat).toBe(jsonFormat); + if (desc?.kind == "message") { + for (const field of desc.fields) { + expect(field.getFeatures().jsonFormat).toBe(jsonFormat); + } + for (const oneof of desc.oneofs) { + expect(oneof.getFeatures().jsonFormat).toBe(jsonFormat); + } + } + if (desc?.kind == "enum") { + for (const value of desc.values) { + expect(value.getFeatures().jsonFormat).toBe(jsonFormat); + } + } + } + expectJsonFormat(FeatureSet_JsonFormat.ALLOW, enums.get("EnumJsonAllow")); + expectJsonFormat( + FeatureSet_JsonFormat.ALLOW, + messages.get("MessageJsonAllow"), + ); + expectJsonFormat( + FeatureSet_JsonFormat.ALLOW, + enums.get("MessageJsonAllow.EnumJsonAllow"), + ); + expectJsonFormat( + FeatureSet_JsonFormat.ALLOW, + messages.get("MessageJsonAllow.MessageJsonAllow"), + ); + expectJsonFormat( + FeatureSet_JsonFormat.LEGACY_BEST_EFFORT, + messages.get("MessageJsonAllow.MessageJsonLegacy"), + ); + expectJsonFormat( + FeatureSet_JsonFormat.LEGACY_BEST_EFFORT, + enums.get("MessageJsonAllow.MessageJsonLegacy.EnumJsonLegacy"), + ); + expectJsonFormat( + FeatureSet_JsonFormat.LEGACY_BEST_EFFORT, + messages.get("MessageJsonAllow.MessageJsonLegacy.MessageJsonLegacy"), + ); + }); }); describe("repeated field packing", () => { test("proto2 is unpacked by default", async () => { @@ -98,6 +252,47 @@ describe("DescriptorSet", () => { expect(fields?.[3].packed).toBe(true); expect(fields?.[4].packed).toBe(false); }); + test("edition2023 is packed by default", async () => { + const bin = await new UpstreamProtobuf().compileToDescriptorSet(` + edition="2023"; + message M { + int32 not_packable = 1; + repeated bytes also_not_packable = 2; + repeated int32 default = 3; + repeated int32 explicitly_packed = 4 [features.repeated_field_encoding = PACKED]; + repeated int32 explicitly_expanded = 5 [features.repeated_field_encoding = EXPANDED]; + } + `); + const fields = createDescriptorSet(bin).messages.get("M")?.fields; + expect(fields?.[0].packedByDefault).toBe(true); + expect(fields?.[1].packedByDefault).toBe(false); + expect(fields?.[2].packedByDefault).toBe(true); + expect(fields?.[0].packed).toBe(true); + expect(fields?.[1].packed).toBe(false); + expect(fields?.[2].packed).toBe(true); + expect(fields?.[3].packed).toBe(true); + expect(fields?.[4].packed).toBe(false); + }); + test("edition2023 with repeated_field_encoding file option", async () => { + const bin = await new UpstreamProtobuf().compileToDescriptorSet(` + edition="2023"; + option features.repeated_field_encoding = EXPANDED; + message M { + int32 not_packable = 1; + repeated bytes also_not_packable = 2; + repeated int32 default = 3; + repeated int32 explicitly_packed = 4 [features.repeated_field_encoding = PACKED]; + } + `); + const fields = createDescriptorSet(bin).messages.get("M")?.fields; + expect(fields?.[0].packedByDefault).toBe(true); + expect(fields?.[1].packedByDefault).toBe(false); + expect(fields?.[2].packedByDefault).toBe(true); + expect(fields?.[0].packed).toBe(false); + expect(fields?.[1].packed).toBe(false); + expect(fields?.[2].packed).toBe(false); + expect(fields?.[3].packed).toBe(true); + }); }); test("knows extension", () => { const ext = set.extensions.get( @@ -204,10 +399,8 @@ describe("DescriptorSet", () => { } }); }); - }); - describe("for message", () => { - const message = set.messages.get(MessageWithComments.typeName); - test("itself", () => { + test("for message", () => { + const message = set.messages.get(MessageWithComments.typeName); const comments = message?.getComments(); expect(comments).toBeDefined(); if (comments) { @@ -219,10 +412,11 @@ describe("DescriptorSet", () => { expect(comments.trailing).toBeUndefined(); } }); - test("field", () => { - const comments = message?.fields - .find((field) => field.name === "foo") - ?.getComments(); + test("for field", () => { + const field = set.messages + .get(MessageWithComments.typeName) + ?.fields.find((field) => field.name === "foo"); + const comments = field?.getComments(); expect(comments).toBeDefined(); if (comments) { expect(comments.leadingDetached).toStrictEqual([ diff --git a/packages/protobuf/package.json b/packages/protobuf/package.json index ec9a2726e..01ab903e7 100644 --- a/packages/protobuf/package.json +++ b/packages/protobuf/package.json @@ -14,7 +14,7 @@ "build:cjs": "../../node_modules/typescript/bin/tsc --project tsconfig.json --module commonjs --outDir ./dist/cjs --declaration --declarationDir ./dist/cjs && echo >./dist/cjs/package.json '{\"type\":\"commonjs\"}'", "build:esm": "../../node_modules/typescript/bin/tsc --project tsconfig.json --module ES2015 --verbatimModuleSyntax --outDir ./dist/esm --declaration --declarationDir ./dist/esm", "build:proxy": "node ../../scripts/gen-esm-proxy.mjs .", - "bootstrap:featureset-defaults": "upstream-inject-feature-defaults src/private/feature-set-defaults.ts", + "bootstrap:featureset-defaults": "upstream-inject-feature-defaults src/private/feature-set.ts", "prebootstrap:wkt": "rm -rf .tmp && mkdir -p .tmp/google/protobuf && cp -rp src/google/protobuf/* .tmp/google/protobuf", "bootstrap:wkt": "protoc --es_out=src --es_opt=bootstrap_wkt=true,ts_nocheck=false,target=ts --proto_path $(upstream-include wkt) $(upstream-files wkt) && license-header src/google/protobuf", "postbootstrap:wkt": "diff >/dev/null -r .tmp/google/protobuf src/google/protobuf && cp -rp .tmp/google/protobuf/* src/google/protobuf || true", diff --git a/packages/protobuf/src/create-descriptor-set.ts b/packages/protobuf/src/create-descriptor-set.ts index 84f9ff63b..5e65cb328 100644 --- a/packages/protobuf/src/create-descriptor-set.ts +++ b/packages/protobuf/src/create-descriptor-set.ts @@ -20,6 +20,8 @@ import type { } from "./google/protobuf/descriptor_pb.js"; import { Edition, + FeatureSet_RepeatedFieldEncoding, + FeatureSetDefaults, FieldDescriptorProto_Label, FieldDescriptorProto_Type, FieldOptions_JSType, @@ -50,6 +52,11 @@ import { parseTextFormatEnumValue, parseTextFormatScalarValue, } from "./private/text-format.js"; +import type { FeatureResolverFn } from "./private/feature-set.js"; +import { + createFeatureResolver, + featureSetDefaults, +} from "./private/feature-set.js"; /** * Create a DescriptorSet, a convenient interface for working with a set of @@ -61,6 +68,7 @@ import { */ export function createDescriptorSet( input: FileDescriptorProto[] | FileDescriptorSet | Uint8Array, + options?: CreateDescriptorSetOptions, ): DescriptorSet { const cart = { enums: new Map(), @@ -68,6 +76,9 @@ export function createDescriptorSet( services: new Map(), extensions: new Map(), mapEntries: new Map(), + resolveFeatures: createFeatureResolver( + options?.featureSetDefaults ?? featureSetDefaults, + ), }; const fileDescriptors = input instanceof FileDescriptorSet @@ -79,6 +90,25 @@ export function createDescriptorSet( return { files, ...cart }; } +/** + * Options to createDescriptorSet() + */ +interface CreateDescriptorSetOptions { + /** + * Editions support language-specific features with extensions to + * google.protobuf.FeatureSet. They can define defaults, and specify on + * which targets the features can be set. + * + * To create a DescriptorSet that provides your language-specific features, + * you have to provide a google.protobuf.FeatureSetDefaults message in this + * option. + * + * The defaults can be generated with `protoc` - see the flag + * `--experimental_edition_defaults_out`. + */ + featureSetDefaults?: FeatureSetDefaults; +} + /** * Cart is an implementation detail. It captures a few variables we * use to resolve reference when creating descriptors. @@ -89,6 +119,7 @@ interface Cart { services: Map; extensions: Map; mapEntries: Map; + resolveFeatures: FeatureResolverFn; } /** @@ -120,6 +151,9 @@ function newFile(proto: FileDescriptorProto, cart: Cart): DescFile { FieldNumber.FileDescriptorProto_Package, ]); }, + getFeatures() { + return cart.resolveFeatures(this.edition, this.proto.options?.features); + }, }; cart.mapEntries.clear(); // map entries are local to the file, we can safely discard for (const enumProto of proto.enumType) { @@ -176,7 +210,7 @@ function addExtensions(desc: DescFile | DescMessage, cart: Cart): void { */ function addFields(message: DescMessage, cart: Cart): void { const allOneofs = message.proto.oneofDecl.map((proto) => - newOneof(proto, message), + newOneof(proto, message, cart), ); const oneofsSeen = new Set(); for (const proto of message.proto.field) { @@ -241,6 +275,15 @@ function addEnum( ]; return findComments(file.proto.sourceCodeInfo, path); }, + getFeatures() { + const parentFeatures = + this.parent?.getFeatures() ?? this.file.getFeatures(); + return cart.resolveFeatures( + this.file.edition, + parentFeatures, + this.proto.options?.features, + ); + }, }; cart.enums.set(desc.typeName, desc); proto.value.forEach((proto) => { @@ -274,6 +317,13 @@ function addEnum( ]; return findComments(file.proto.sourceCodeInfo, path); }, + getFeatures() { + return cart.resolveFeatures( + this.parent.file.edition, + this.parent.getFeatures(), + this.proto.options?.features, + ); + }, }); }); (parent?.nestedEnums ?? file.enums).push(desc); @@ -320,6 +370,15 @@ function addMessage( ]; return findComments(file.proto.sourceCodeInfo, path); }, + getFeatures() { + const parentFeatures = + this.parent?.getFeatures() ?? this.file.getFeatures(); + return cart.resolveFeatures( + this.file.edition, + parentFeatures, + this.proto.options?.features, + ); + }, }; if (proto.options?.mapEntry === true) { cart.mapEntries.set(desc.typeName, desc); @@ -363,6 +422,13 @@ function addService( ]; return findComments(file.proto.sourceCodeInfo, path); }, + getFeatures() { + return cart.resolveFeatures( + this.file.edition, + this.file.getFeatures(), + this.proto.options?.features, + ); + }, }; file.services.push(desc); cart.services.set(desc.typeName, desc); @@ -440,13 +506,24 @@ function newMethod( ]; return findComments(parent.file.proto.sourceCodeInfo, path); }, + getFeatures() { + return cart.resolveFeatures( + this.parent.file.edition, + this.parent.getFeatures(), + this.proto.options?.features, + ); + }, }; } /** * Create a descriptor for a oneof group. */ -function newOneof(proto: OneofDescriptorProto, parent: DescMessage): DescOneof { +function newOneof( + proto: OneofDescriptorProto, + parent: DescMessage, + cart: Cart, +): DescOneof { assert(proto.name, `invalid OneofDescriptorProto: missing name`); return { kind: "oneof", @@ -466,6 +543,13 @@ function newOneof(proto: OneofDescriptorProto, parent: DescMessage): DescOneof { ]; return findComments(parent.file.proto.sourceCodeInfo, path); }, + getFeatures() { + return cart.resolveFeatures( + this.parent.file.edition, + this.parent.getFeatures(), + this.proto.options?.features, + ); + }, }; } @@ -482,7 +566,6 @@ function newField( assert(proto.name, `invalid FieldDescriptorProto: missing name`); assert(proto.number, `invalid FieldDescriptorProto: missing number`); assert(proto.type, `invalid FieldDescriptorProto: missing type`); - const packedByDefault = isPackedFieldByDefault(proto, file.syntax); const common = { proto, deprecated: proto.options?.deprecated ?? false, @@ -491,8 +574,8 @@ function newField( parent, oneof, optional: isOptionalField(proto, file.syntax), - packed: proto.options?.packed ?? packedByDefault, - packedByDefault, + packedByDefault: isPackedFieldByDefault(file, proto, cart.resolveFeatures), + packed: isPackedField(file, parent, proto, cart.resolveFeatures), jsonName: proto.jsonName === fieldJsonName(proto.name) ? undefined : proto.jsonName, scalar: undefined, @@ -501,11 +584,11 @@ function newField( enum: undefined, mapKey: undefined, mapValue: undefined, + declarationString, + // toString, getComments, getFeatures are overridden in newExtension toString(): string { - // note that newExtension() calls us with parent = null return `field ${this.parent.typeName}.${this.name}`; }, - declarationString, getComments() { const path = [ ...this.parent.getComments().sourcePath, @@ -514,6 +597,13 @@ function newField( ]; return findComments(file.proto.sourceCodeInfo, path); }, + getFeatures() { + return cart.resolveFeatures( + file.edition, + this.parent.getFeatures(), + this.proto.options?.features, + ); + }, }; const repeated = proto.label === FieldDescriptorProto_Label.REPEATED; switch (proto.type) { @@ -614,6 +704,8 @@ function newExtension( parent, file, extendee, + // Must override toString, getComments, getFeatures from newField, because we + // call newField with parent undefined. toString(): string { return `extension ${this.typeName}`; }, @@ -630,6 +722,13 @@ function newExtension( ]; return findComments(file.proto.sourceCodeInfo, path); }, + getFeatures() { + return cart.resolveFeatures( + this.file.edition, + (this.parent ?? this.file).getFeatures(), + this.proto.options?.features, + ); + }, }; } @@ -850,47 +949,77 @@ function isOptionalField( } /** - * Get the default `packed` state of a repeated field. + * Is this field packed by default? Only valid for repeated enum fields, and + * for repeated scalar fields except BYTES and STRING. + * + * In proto3 syntax, fields are packed by default. In proto2 syntax, fields + * are unpacked by default. With editions, the default is whatever the edition + * specifies as a default. In edition 2023, fields are packed by default. */ -export function isPackedFieldByDefault( +function isPackedFieldByDefault( + file: DescFile, proto: FieldDescriptorProto, - syntax: "proto2" | "proto3" | "editions", -): boolean { - assert(proto.type, `invalid FieldDescriptorProto: missing type`); - switch (syntax) { - case "proto3": - switch (proto.type) { - case FieldDescriptorProto_Type.DOUBLE: - case FieldDescriptorProto_Type.FLOAT: - case FieldDescriptorProto_Type.INT64: - case FieldDescriptorProto_Type.UINT64: - case FieldDescriptorProto_Type.INT32: - case FieldDescriptorProto_Type.FIXED64: - case FieldDescriptorProto_Type.FIXED32: - case FieldDescriptorProto_Type.UINT32: - case FieldDescriptorProto_Type.SFIXED32: - case FieldDescriptorProto_Type.SFIXED64: - case FieldDescriptorProto_Type.SINT32: - case FieldDescriptorProto_Type.SINT64: - case FieldDescriptorProto_Type.BOOL: - case FieldDescriptorProto_Type.ENUM: - // From the proto3 language guide: - // > In proto3, repeated fields of scalar numeric types are packed by default. - // This information is incomplete - according to the conformance tests, BOOL - // and ENUM are packed by default as well. This means only STRING and BYTES - // are not packed by default, which makes sense because they are length-delimited. - return true; - default: - return false; - } - case "proto2": + resolveFeatures: FeatureResolverFn, +) { + const { repeatedFieldEncoding } = resolveFeatures(file.edition); + if (repeatedFieldEncoding == FeatureSet_RepeatedFieldEncoding.EXPANDED) { + return false; + } + // From the proto3 language guide: + // > In proto3, repeated fields of scalar numeric types are packed by default. + // This information is incomplete - according to the conformance tests, BOOL + // and ENUM are packed by default as well. This means only STRING and BYTES + // are not packed by default, which makes sense because they are length-delimited. + switch (proto.type) { + case FieldDescriptorProto_Type.STRING: + case FieldDescriptorProto_Type.BYTES: + case FieldDescriptorProto_Type.GROUP: + case FieldDescriptorProto_Type.MESSAGE: return false; - case "editions": - // TODO support edition featureset + default: return true; } } +/** + * Pack this repeated field? + * + * Respects field type, proto2/proto3 defaults and the `packed` option, or + * edition defaults and the edition features.repeated_field_encoding options. + */ +function isPackedField( + file: DescFile, + parent: DescMessage | undefined, + proto: FieldDescriptorProto, + resolveFeatures: FeatureResolverFn, +) { + switch (proto.type) { + case FieldDescriptorProto_Type.STRING: + case FieldDescriptorProto_Type.BYTES: + case FieldDescriptorProto_Type.GROUP: + case FieldDescriptorProto_Type.MESSAGE: + // length-delimited types cannot be packed + return false; + default: + switch (file.edition) { + case Edition.EDITION_PROTO2: + return proto.options?.packed ?? false; + case Edition.EDITION_PROTO3: + return proto.options?.packed ?? true; + default: { + const { repeatedFieldEncoding } = resolveFeatures( + file.edition, + parent?.getFeatures() ?? file.getFeatures(), + proto.options?.features, + ); + return ( + repeatedFieldEncoding == FeatureSet_RepeatedFieldEncoding.PACKED + ); + } + } + } +} + /** * Map from a compiler-generated field type to our ScalarType, which is a * subset of field types declared by protobuf enum google.protobuf.FieldDescriptorProto. diff --git a/packages/protobuf/src/descriptor-set.ts b/packages/protobuf/src/descriptor-set.ts index 9cc1ab8c3..f0dd71281 100644 --- a/packages/protobuf/src/descriptor-set.ts +++ b/packages/protobuf/src/descriptor-set.ts @@ -23,8 +23,9 @@ import type { OneofDescriptorProto, ServiceDescriptorProto, } from "./google/protobuf/descriptor_pb.js"; -import type { ScalarType, LongType } from "./field.js"; +import type { LongType, ScalarType } from "./field.js"; import type { MethodIdempotency, MethodKind } from "./service-type.js"; +import type { MergedFeatureSet } from "./private/feature-set.js"; /** * DescriptorSet provides a convenient interface for working with a set @@ -142,6 +143,11 @@ export interface DescFile { */ getPackageComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -189,6 +195,11 @@ export interface DescEnum { */ getComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -229,6 +240,11 @@ export interface DescEnumValue { */ getComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -295,6 +311,11 @@ export interface DescMessage { */ getComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -358,10 +379,14 @@ interface DescFieldCommon { */ readonly packed: boolean; /** - * Is this field packed by default? Only valid for enum fields, and for - * scalar fields except BYTES and STRING. + * Is this field packed by default? Only valid for repeated enum fields, and + * for repeated scalar fields except BYTES and STRING. + * * In proto3 syntax, fields are packed by default. In proto2 syntax, fields * are unpacked by default. + * + * With editions, the default is whatever the edition specifies as a default. + * In edition 2023, fields are packed by default. */ readonly packedByDefault: boolean; /** @@ -389,6 +414,11 @@ interface DescFieldCommon { */ declarationString(): string; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -636,6 +666,11 @@ export interface DescOneof { */ getComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -674,6 +709,11 @@ export interface DescService { */ getComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } @@ -720,6 +760,11 @@ export interface DescMethod { */ getComments(): DescComments; + /** + * Get the edition features for this protobuf element. + */ + getFeatures(): MergedFeatureSet; + toString(): string; } diff --git a/packages/protobuf/src/private/feature-set-defaults.ts b/packages/protobuf/src/private/feature-set-defaults.ts deleted file mode 100644 index 938455838..000000000 --- a/packages/protobuf/src/private/feature-set-defaults.ts +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2021-2023 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import { FeatureSetDefaults } from "../google/protobuf/descriptor_pb.js"; -import { protoBase64 } from "../proto-base64.js"; - -export const featureSetDefaults = FeatureSetDefaults.fromBinary( - protoBase64.dec( - /*upstream-inject-feature-defaults-start*/ "ChESDAgBEAIYAiABKAEwAhjmBwoREgwIAhABGAEgAigBMAEY5wcKERIMCAEQARgBIAIoATABGOgHIOYHKOgH" /*upstream-inject-feature-defaults-end*/, - ), -); diff --git a/packages/protobuf/src/private/feature-set.ts b/packages/protobuf/src/private/feature-set.ts new file mode 100644 index 000000000..56318d41f --- /dev/null +++ b/packages/protobuf/src/private/feature-set.ts @@ -0,0 +1,122 @@ +// Copyright 2021-2023 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { + Edition, + FeatureSet, + FeatureSetDefaults, +} from "../google/protobuf/descriptor_pb.js"; +import { protoBase64 } from "../proto-base64.js"; + +/** + * Static edition feature defaults supported by @bufbuild/protobuf. + */ +export const featureSetDefaults = FeatureSetDefaults.fromBinary( + protoBase64.dec( + /*upstream-inject-feature-defaults-start*/ "ChESDAgBEAIYAiABKAEwAhjmBwoREgwIAhABGAEgAigBMAEY5wcKERIMCAEQARgBIAIoATABGOgHIOYHKOgH" /*upstream-inject-feature-defaults-end*/, + ), +); + +/** + * A merged google.protobuf.FeaturesSet, with all fields guaranteed to be set. + */ +export type MergedFeatureSet = FeatureSet & Required; + +/** + * A function that resolves features for the given edition. + * + * If no feature set is provided, the default feature set for the edition is + * returned. If features are provided, they are merged into the edition default + * features. + */ +export type FeatureResolverFn = ( + edition: Edition, + a?: FeatureSet, + b?: FeatureSet, +) => MergedFeatureSet; + +/** + * Create an edition feature resolver with the given feature set defaults. + */ +export function createFeatureResolver( + compiledFeatureSetDefaults: FeatureSetDefaults, +): FeatureResolverFn { + const min = compiledFeatureSetDefaults.minimumEdition ?? 0; + const max = compiledFeatureSetDefaults.maximumEdition ?? 0; + const defaultsBinByEdition = new Map(); + return (edition: Edition, ...rest): MergedFeatureSet => { + let defaultsBin: Uint8Array | undefined = defaultsBinByEdition.get(edition); + if (defaultsBin === undefined) { + if (edition < min) { + throw new Error( + `Edition ${Edition[edition]} is earlier than the minimum supported edition ${Edition[min]}`, + ); + } + if (max < edition) { + throw new Error( + `Edition ${Edition[edition]} is later than the maximum supported edition ${Edition[max]}`, + ); + } + let highestMatch: { e: Edition; f: FeatureSet } | undefined = undefined; + for (const c of compiledFeatureSetDefaults.defaults) { + const e = c.edition ?? 0; + if (e > edition) { + continue; + } + if (highestMatch !== undefined && highestMatch.e > e) { + continue; + } + highestMatch = { + e, + f: c.features ?? new FeatureSet(), + }; + } + if (highestMatch === undefined) { + throw new Error( + `No valid default found for edition ${Edition[edition]}`, + ); + } + defaultsBin = highestMatch.f.toBinary(); + defaultsBinByEdition.set(edition, defaultsBin); + } + const f = FeatureSet.fromBinary(defaultsBin); + for (const c of rest) { + if (c !== undefined) { + f.fromBinary(c.toBinary()); + } + } + if (!validateMergedFeatures(f)) { + throw new Error(`Invalid FeatureSet for edition ${Edition[edition]}`); + } + return f; + }; +} + +function validateMergedFeatures( + featureSet: FeatureSet, +): featureSet is MergedFeatureSet { + for (const p of [ + "fieldPresence", + "enumType", + "repeatedFieldEncoding", + "utf8Validation", + "messageEncoding", + "jsonFormat", + ] as const) { + if (featureSet[p] === undefined || (featureSet[p] as number) === 0) { + return false; + } + } + return true; +} diff --git a/packages/protobuf/tsconfig.json b/packages/protobuf/tsconfig.json index 9bcf60db8..c71a5ce4c 100644 --- a/packages/protobuf/tsconfig.json +++ b/packages/protobuf/tsconfig.json @@ -1,8 +1,7 @@ { "files": [ "src/index.ts", - "src/private/options-map.ts", // not yet exported, added here to satisfy the linter - "src/private/feature-set-defaults.ts" // not yet exported, added here to satisfy the linter + "src/private/options-map.ts" // not yet exported, added here to satisfy the linter ], "extends": "../../tsconfig.base.json", "compilerOptions": { diff --git a/packages/protoplugin-test/src/transpile.test.ts b/packages/protoplugin-test/src/transpile.test.ts index 266a0cc88..1e9a08556 100644 --- a/packages/protoplugin-test/src/transpile.test.ts +++ b/packages/protoplugin-test/src/transpile.test.ts @@ -18,7 +18,7 @@ import { Edition, FileDescriptorProto, } from "@bufbuild/protobuf"; -import type { DescFile } from "@bufbuild/protobuf"; +import type { DescFile, FeatureSet } from "@bufbuild/protobuf"; import { createEcmaScriptPlugin } from "@bufbuild/protoplugin"; import type { Schema } from "@bufbuild/protoplugin/ecmascript"; @@ -138,6 +138,9 @@ describe("transpile", function () { trailing: undefined, }; }, + getFeatures(): FeatureSet & Required { + return null as unknown as FeatureSet & Required; + }, }; test("generates correctly", () => { const linesOf = transpile((schema) => { From 9ee344c5a9c646326a0af4696094047ce6b637bd Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Wed, 22 Nov 2023 12:27:29 +0100 Subject: [PATCH 2/6] Update isPackedFieldByDefault to return false if not PACKED Instead of assuming that EXPANDED means that the field is not packed, be specific and check for PACKED, because it's unlikely but possible that more values are added to the enumeration in the future. --- packages/protobuf/src/create-descriptor-set.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protobuf/src/create-descriptor-set.ts b/packages/protobuf/src/create-descriptor-set.ts index 5e65cb328..b863492ef 100644 --- a/packages/protobuf/src/create-descriptor-set.ts +++ b/packages/protobuf/src/create-descriptor-set.ts @@ -962,7 +962,7 @@ function isPackedFieldByDefault( resolveFeatures: FeatureResolverFn, ) { const { repeatedFieldEncoding } = resolveFeatures(file.edition); - if (repeatedFieldEncoding == FeatureSet_RepeatedFieldEncoding.EXPANDED) { + if (repeatedFieldEncoding != FeatureSet_RepeatedFieldEncoding.PACKED) { return false; } // From the proto3 language guide: From 3bc7274a5b1fd62d4be588d5ee9ddff5dcd08dff Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Fri, 24 Nov 2023 18:37:11 +0100 Subject: [PATCH 3/6] Add some FeatureSetDefaults validation --- packages/protobuf/src/private/feature-set.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/protobuf/src/private/feature-set.ts b/packages/protobuf/src/private/feature-set.ts index 56318d41f..a8401249f 100644 --- a/packages/protobuf/src/private/feature-set.ts +++ b/packages/protobuf/src/private/feature-set.ts @@ -52,8 +52,15 @@ export type FeatureResolverFn = ( export function createFeatureResolver( compiledFeatureSetDefaults: FeatureSetDefaults, ): FeatureResolverFn { - const min = compiledFeatureSetDefaults.minimumEdition ?? 0; - const max = compiledFeatureSetDefaults.maximumEdition ?? 0; + const min = compiledFeatureSetDefaults.minimumEdition; + const max = compiledFeatureSetDefaults.maximumEdition; + if ( + min === undefined || + max === undefined || + compiledFeatureSetDefaults.defaults.some((d) => d.edition === undefined) + ) { + throw new Error("Invalid FeatureSetDefaults"); + } const defaultsBinByEdition = new Map(); return (edition: Edition, ...rest): MergedFeatureSet => { let defaultsBin: Uint8Array | undefined = defaultsBinByEdition.get(edition); From ad1965dc2ffff83e845e108520289a70032489d8 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Mon, 27 Nov 2023 11:59:39 +0100 Subject: [PATCH 4/6] Automatically pick up new known features invalidateMergedFeatures --- packages/protobuf/src/private/feature-set.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/protobuf/src/private/feature-set.ts b/packages/protobuf/src/private/feature-set.ts index a8401249f..00b893dbf 100644 --- a/packages/protobuf/src/private/feature-set.ts +++ b/packages/protobuf/src/private/feature-set.ts @@ -113,15 +113,12 @@ export function createFeatureResolver( function validateMergedFeatures( featureSet: FeatureSet, ): featureSet is MergedFeatureSet { - for (const p of [ - "fieldPresence", - "enumType", - "repeatedFieldEncoding", - "utf8Validation", - "messageEncoding", - "jsonFormat", - ] as const) { - if (featureSet[p] === undefined || (featureSet[p] as number) === 0) { + for (const fi of FeatureSet.fields.list()) { + const v = featureSet[fi.localName as keyof FeatureSet]; + if (v === undefined) { + return false; + } + if (fi.kind == "enum" && v === 0) { return false; } } From 169354c25b094ce370993ae6cc5a3457ce10590f Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Mon, 27 Nov 2023 12:01:57 +0100 Subject: [PATCH 5/6] Add docs for validateMergedFeatures --- packages/protobuf/src/private/feature-set.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/protobuf/src/private/feature-set.ts b/packages/protobuf/src/private/feature-set.ts index 00b893dbf..804f3d077 100644 --- a/packages/protobuf/src/private/feature-set.ts +++ b/packages/protobuf/src/private/feature-set.ts @@ -110,6 +110,23 @@ export function createFeatureResolver( }; } +// When protoc generates google.protobuf.FeatureSetDefaults, it ensures that +// fields are not repeated or required, do not use oneof, and have a default +// value. +// +// When features for an element are resolved, features of the element and its +// parents are merged into the default FeatureSet for the edition. Because unset +// fields in the FeatureSet of an element do not unset the default FeatureSet +// values, a resolved FeatureSet is guaranteed to have all fields set. This is +// also the case for extensions to FeatureSet that a user might provide, and for +// features from the future. +// +// We cannot exhaustively validate correctness of FeatureSetDefaults at runtime +// without knowing the schema: If no value for a feature is provided, we do not +// know that it exists at all. +// +// As a sanity check, we validate that all fields known to our version of +// FeatureSet are set. function validateMergedFeatures( featureSet: FeatureSet, ): featureSet is MergedFeatureSet { From 32771dc3e27c8e44d51d8b69c01ba1d8e7caab95 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Mon, 27 Nov 2023 12:54:38 +0100 Subject: [PATCH 6/6] make lint --- packages/protobuf/src/private/feature-set.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/protobuf/src/private/feature-set.ts b/packages/protobuf/src/private/feature-set.ts index 804f3d077..dc0b8ce4d 100644 --- a/packages/protobuf/src/private/feature-set.ts +++ b/packages/protobuf/src/private/feature-set.ts @@ -131,7 +131,7 @@ function validateMergedFeatures( featureSet: FeatureSet, ): featureSet is MergedFeatureSet { for (const fi of FeatureSet.fields.list()) { - const v = featureSet[fi.localName as keyof FeatureSet]; + const v = featureSet[fi.localName as keyof FeatureSet] as unknown; if (v === undefined) { return false; }