From 66a19d95f938d2fc2e2445ec75dde3e707192d42 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Thu, 9 May 2024 11:46:11 +0200 Subject: [PATCH 1/2] Add localName property to DescField and others --- packages/protobuf-bench/README.md | 2 +- packages/protobuf-test/src/create.test.ts | 6 +- packages/protobuf-test/src/fields.test.ts | 7 +- packages/protobuf-test/src/helpers.ts | 4 +- .../protobuf-test/src/reflect/names.test.ts | 130 +----------- .../src/reflect/registry.test.ts | 187 +++++++++++++++++- .../protobuf/scripts/bootstrap-inject.mjs | 10 +- packages/protobuf/src/codegenv1/enum.ts | 6 +- packages/protobuf/src/create.ts | 9 +- packages/protobuf/src/desc-types.ts | 16 ++ packages/protobuf/src/extensions.ts | 8 +- packages/protobuf/src/fields.ts | 17 +- packages/protobuf/src/reflect/names.ts | 43 ---- packages/protobuf/src/reflect/registry.ts | 29 ++- packages/protobuf/src/reflect/unsafe.ts | 33 ++-- .../protoc-gen-es/src/protoc-gen-es-plugin.ts | 14 +- .../src/protoc-gen-twirp-es.ts | 3 +- 17 files changed, 276 insertions(+), 248 deletions(-) diff --git a/packages/protobuf-bench/README.md b/packages/protobuf-bench/README.md index 9872fcfef..5473dbf15 100644 --- a/packages/protobuf-bench/README.md +++ b/packages/protobuf-bench/README.md @@ -10,5 +10,5 @@ server would usually do. | code generator | bundle size | minified | compressed | |---------------------|------------------------:|-----------------------:|-------------------:| -| protobuf-es | 126,284 b | 65,142 b | 15,854 b | +| protobuf-es | 125,973 b | 65,131 b | 15,859 b | | protobuf-javascript | 394,384 b | 288,654 b | 45,122 b | diff --git a/packages/protobuf-test/src/create.test.ts b/packages/protobuf-test/src/create.test.ts index 1bc5851ac..f93f6a13e 100644 --- a/packages/protobuf-test/src/create.test.ts +++ b/packages/protobuf-test/src/create.test.ts @@ -17,7 +17,7 @@ import { protoInt64 } from "@bufbuild/protobuf"; import type { MessageInitShape } from "@bufbuild/protobuf"; import { create, isFieldSet, isMessage } from "@bufbuild/protobuf"; import { describe, expect, test } from "@jest/globals"; -import { localName, reflect } from "@bufbuild/protobuf/reflect"; +import { reflect } from "@bufbuild/protobuf/reflect"; import * as example_ts from "./gen/ts/extra/example_pb.js"; import * as proto3_ts from "./gen/ts/extra/proto3_pb.js"; import * as proto2_ts from "./gen/ts/extra/proto2_pb.js"; @@ -596,7 +596,7 @@ describe("create()", () => { test.each(desc.fields)("$name", (f) => { const init: Record = {}; for (const f of desc.members) { - init[localName(f)] = null; + init[f.localName] = null; } const msg = create(desc, init); const r = reflect(desc, msg); @@ -613,7 +613,7 @@ describe("create()", () => { test.each(desc.fields)("$name", (f) => { const init: Record = {}; for (const f of desc.members) { - init[localName(f)] = undefined; + init[f.localName] = undefined; } const msg = create(desc, init); const r = reflect(desc, msg); diff --git a/packages/protobuf-test/src/fields.test.ts b/packages/protobuf-test/src/fields.test.ts index 3b9ab943f..f8a9c9642 100644 --- a/packages/protobuf-test/src/fields.test.ts +++ b/packages/protobuf-test/src/fields.test.ts @@ -15,7 +15,6 @@ import { beforeEach, describe, expect, test } from "@jest/globals"; import type { DescMessage } from "@bufbuild/protobuf"; import { clearField, create, isFieldSet } from "@bufbuild/protobuf"; -import { localName } from "@bufbuild/protobuf/reflect"; import * as proto3_ts from "./gen/ts/extra/proto3_pb.js"; import * as proto2_ts from "./gen/ts/extra/proto2_pb.js"; import * as edition2023_ts from "./gen/ts/extra/edition2023_pb.js"; @@ -55,7 +54,7 @@ describe("isFieldSet()", () => { const desc = proto3_ts.Proto3MessageDesc; test.each(desc.fields)("%s is initially unset", (field) => { const msg = create(desc); - const set = isFieldSet(desc as DescMessage, msg, localName(field)); + const set = isFieldSet(desc as DescMessage, msg, field.localName); expect(set).toBe(false); }); test.each(fillProto3MessageNames())("%s is set", (name) => { @@ -69,7 +68,7 @@ describe("isFieldSet()", () => { const desc = proto2_ts.Proto2MessageDesc; test.each(desc.fields)("%s is initially unset", (field) => { const msg = create(desc); - const set = isFieldSet(desc as DescMessage, msg, localName(field)); + const set = isFieldSet(desc as DescMessage, msg, field.localName); expect(set).toBe(false); }); test.each(fillProto2MessageNames())("%s is set", (name) => { @@ -83,7 +82,7 @@ describe("isFieldSet()", () => { const desc = edition2023_ts.Edition2023MessageDesc; test.each(desc.fields)("%s is initially unset", (field) => { const msg = create(desc); - const set = isFieldSet(desc as DescMessage, msg, localName(field)); + const set = isFieldSet(desc as DescMessage, msg, field.localName); expect(set).toBe(false); }); test.each(fillEdition2023MessageNames())("%s is set", (name) => { diff --git a/packages/protobuf-test/src/helpers.ts b/packages/protobuf-test/src/helpers.ts index 76db2742f..dbfa64156 100644 --- a/packages/protobuf-test/src/helpers.ts +++ b/packages/protobuf-test/src/helpers.ts @@ -14,7 +14,7 @@ import type { DescMessage } from "@bufbuild/protobuf"; import { UpstreamProtobuf } from "upstream-protobuf"; -import { createFileRegistry, localName } from "@bufbuild/protobuf/reflect"; +import { createFileRegistry } from "@bufbuild/protobuf/reflect"; import * as proto3_ts from "./gen/ts/extra/proto3_pb.js"; import type { DescField } from "@bufbuild/protobuf"; import { fromBinary } from "@bufbuild/protobuf"; @@ -118,7 +118,7 @@ export function getFieldByLocalName( fieldKind?: string, ): DescField { const field = proto3_ts.Proto3MessageDesc.fields.find( - (f) => localName(f) === name, + (f) => f.localName === name, ); if (!field) { throw new Error(`getFieldByLocalName: ${name} not found`); diff --git a/packages/protobuf-test/src/reflect/names.test.ts b/packages/protobuf-test/src/reflect/names.test.ts index 38fe3e469..fc3b8e795 100644 --- a/packages/protobuf-test/src/reflect/names.test.ts +++ b/packages/protobuf-test/src/reflect/names.test.ts @@ -13,134 +13,8 @@ // limitations under the License. import { describe, expect, test } from "@jest/globals"; -import { - safeObjectProperty, - protoCamelCase, - localName, -} from "@bufbuild/protobuf/reflect"; -import { compileEnum, compileField, compileService } from "../helpers.js"; - -describe("localName", () => { - describe("with field", () => { - test("applies protoCamelCase", async () => { - const field = await compileField(` - syntax="proto3"; - message M { - int32 __proto__ = 1; - } - `); - expect(localName(field)).toBe("Proto"); - }); - test("escapes reserved property name", async () => { - const field = await compileField(` - syntax="proto3"; - message M { - int32 constructor = 1; - } - `); - expect(localName(field)).toBe("constructor$"); - }); - }); - describe("with field in oneof", () => { - test("applies protoCamelCase", async () => { - const field = await compileField(` - syntax="proto3"; - message M { - oneof kind { - int32 __proto__ = 1; - } - } - `); - expect(field.oneof).toBeDefined(); - expect(localName(field)).toBe("Proto"); - }); - test("does not escape reserved property name", async () => { - const field = await compileField(` - syntax="proto3"; - message M { - oneof kind { - int32 constructor = 1; - } - } - `); - expect(field.oneof).toBeDefined(); - expect(localName(field)).toBe("constructor"); - }); - }); - describe("with enum value", () => { - test("does not change case", async () => { - const value = ( - await compileEnum(` - syntax="proto3"; - enum E { - FooBAR_baz_1 = 0; - } - `) - ).values[0]; - expect(localName(value)).toBe("FooBAR_baz_1"); - }); - test("drops prefix", async () => { - const value = ( - await compileEnum(` - syntax="proto3"; - enum PrefixEnum { - PREFIX_ENUM_ZERO = 0; - PREFIX_ENUM_ONE = 1; - } - `) - ).values[0]; - expect(localName(value)).toBe("ZERO"); - }); - test("escapes reserved property name", async () => { - const value = ( - await compileEnum(` - syntax="proto3"; - enum EnumBuiltIn { - constructor = 0; - } - `) - ).values[0]; - expect(localName(value)).toBe("constructor$"); - }); - test("escapes reserved property name with dropped prefix", async () => { - const value = ( - await compileEnum(` - syntax="proto3"; - enum EnumBuiltInPrefixed { - ENUM_BUILT_IN_PREFIXED_constructor = 0; - } - `) - ).values[0]; - expect(localName(value)).toBe("constructor$"); - }); - }); - describe("with rpc", () => { - test("makes first letter lowerCase", async () => { - const rpc = ( - await compileService(` - syntax="proto3"; - service Srv { - rpc Foo_bar_BAZ(E) returns (E); - } - message E {} - `) - ).methods[0]; - expect(localName(rpc)).toBe("foo_bar_BAZ"); - }); - test("escapes reserved property name", async () => { - const rpc = ( - await compileService(` - syntax="proto3"; - service Srv { - rpc constructor(E) returns (E); - } - message E {} - `) - ).methods[0]; - expect(localName(rpc)).toBe("constructor$"); - }); - }); -}); +import { safeObjectProperty, protoCamelCase } from "@bufbuild/protobuf/reflect"; +import { compileField } from "../helpers.js"; describe("safeObjectProperty", () => { test("escapes reserved object property names", () => { diff --git a/packages/protobuf-test/src/reflect/registry.test.ts b/packages/protobuf-test/src/reflect/registry.test.ts index e23d74f09..608d4f50a 100644 --- a/packages/protobuf-test/src/reflect/registry.test.ts +++ b/packages/protobuf-test/src/reflect/registry.test.ts @@ -14,8 +14,8 @@ import { beforeAll, beforeEach, describe, expect, test } from "@jest/globals"; import assert from "node:assert"; -import type { FileRegistry } from "@bufbuild/protobuf/reflect"; import { + type FileRegistry, createFileRegistry, createRegistry, LongType, @@ -30,8 +30,8 @@ import type { DescOneof, DescService, } from "@bufbuild/protobuf"; -import type { FileDescriptorSet } from "@bufbuild/protobuf/wkt"; import { + type FileDescriptorSet, Edition, FeatureSet_FieldPresence, MethodOptions_IdempotencyLevel, @@ -637,6 +637,70 @@ describe("DescEnum", () => { }); }); +describe("DescEnumValue", () => { + describe("name", () => { + test.each(["MY_ENUM_A", "foo", "__proto__"])( + "is proto name %s", + async (name) => { + const descEnum = await compileEnum(` + syntax="proto3"; + enum MyEnum { + ${name} = 0; + } + `); + expect(descEnum.values[0].name).toBe(name); + }, + ); + }); + describe("localName", () => { + test("does not change case", async () => { + const value = ( + await compileEnum(` + syntax="proto3"; + enum E { + FooBAR_baz_1 = 0; + } + `) + ).values[0]; + expect(value.localName).toBe("FooBAR_baz_1"); + }); + test("drops shared prefix", async () => { + const value = ( + await compileEnum(` + syntax="proto3"; + enum PrefixEnum { + PREFIX_ENUM_ZERO = 0; + PREFIX_ENUM_ONE = 1; + } + `) + ).values[0]; + expect(value.localName).toBe("ZERO"); + }); + test("escapes reserved property name", async () => { + const value = ( + await compileEnum(` + syntax="proto3"; + enum EnumBuiltIn { + constructor = 0; + } + `) + ).values[0]; + expect(value.localName).toBe("constructor$"); + }); + test("escapes reserved property name with dropped prefix", async () => { + const value = ( + await compileEnum(` + syntax="proto3"; + enum EnumBuiltInPrefixed { + ENUM_BUILT_IN_PREFIXED_constructor = 0; + } + `) + ).values[0]; + expect(value.localName).toBe("constructor$"); + }); + }); +}); + describe("DescField", () => { describe("presence", () => { test("proto2 optional is EXPLICIT", async () => { @@ -919,6 +983,52 @@ describe("DescField", () => { }, ); }); + describe("localName", () => { + test("applies protoCamelCase", async () => { + const field = await compileField(` + syntax="proto3"; + message M { + int32 __proto__ = 1; + } + `); + expect(field.localName).toBe("Proto"); + }); + test("escapes reserved property name", async () => { + const field = await compileField(` + syntax="proto3"; + message M { + int32 constructor = 1; + } + `); + expect(field.localName).toBe("constructor$"); + }); + describe("with field in oneof", () => { + test("applies protoCamelCase", async () => { + const field = await compileField(` + syntax="proto3"; + message M { + oneof kind { + int32 __proto__ = 1; + } + } + `); + expect(field.oneof).toBeDefined(); + expect(field.localName).toBe("Proto"); + }); + test("does not escape reserved property name", async () => { + const field = await compileField(` + syntax="proto3"; + message M { + oneof kind { + int32 constructor = 1; + } + } + `); + expect(field.oneof).toBeDefined(); + expect(field.localName).toBe("constructor"); + }); + }); + }); describe("jsonName", () => { test.each(["field", "foo_bar", "__proto__", "constructor"])( "returns compiler-provided json_name for %s", @@ -1335,6 +1445,53 @@ describe("DescField", () => { }); }); +describe("DescOneof", () => { + describe("localName", () => { + test("applies protoCamelCase", async () => { + const oneof = ( + await compileMessage(` + syntax="proto3"; + message M { + oneof __proto__ { + bool placeholder = 1; + } + } + `) + ).oneofs[0]; + expect(oneof.localName).toBe("Proto"); + }); + test("escapes reserved property name", async () => { + const oneof = ( + await compileMessage(` + syntax="proto3"; + message M { + oneof constructor { + bool placeholder = 1; + } + } + `) + ).oneofs[0]; + expect(oneof.localName).toBe("constructor$"); + }); + }); + describe("fields", () => { + test("has member fields", async () => { + const oneof = ( + await compileMessage(` + syntax="proto3"; + message M { + oneof tst { + bool a = 1; + bool b = 2; + } + } + `) + ).oneofs[0]; + expect(oneof.fields.length).toBe(2); + }); + }); +}); + describe("DescExtension", () => { test("typeName", async () => { const ext = await compileExtension(` @@ -1637,4 +1794,30 @@ describe("DescMethod", () => { expect(method.deprecated).toBe(true); }); }); + describe("localName", () => { + test("makes first letter lowerCase", async () => { + const rpc = ( + await compileService(` + syntax="proto3"; + service Srv { + rpc Foo_bar_BAZ(E) returns (E); + } + message E {} + `) + ).methods[0]; + expect(rpc.localName).toBe("foo_bar_BAZ"); + }); + test("escapes reserved property name", async () => { + const rpc = ( + await compileService(` + syntax="proto3"; + service Srv { + rpc constructor(E) returns (E); + } + message E {} + `) + ).methods[0]; + expect(rpc.localName).toBe("constructor$"); + }); + }); }); diff --git a/packages/protobuf/scripts/bootstrap-inject.mjs b/packages/protobuf/scripts/bootstrap-inject.mjs index 0ff186590..fa6ab4c9c 100644 --- a/packages/protobuf/scripts/bootstrap-inject.mjs +++ b/packages/protobuf/scripts/bootstrap-inject.mjs @@ -17,11 +17,7 @@ import { join as joinPath } from "node:path"; import assert from "node:assert"; import { stdout, stderr, argv } from "node:process"; import { UpstreamProtobuf } from "upstream-protobuf"; -import { - createFileRegistry, - localName, - reflect, -} from "@bufbuild/protobuf/reflect"; +import { createFileRegistry, reflect } from "@bufbuild/protobuf/reflect"; import { fromBinary } from "@bufbuild/protobuf"; import { Edition, @@ -116,7 +112,7 @@ async function processFile(filePath, content, descriptorProto, upstream) { lines[i] = injectVars(template, { $name: enumValueDesc.name, $number: enumValueDesc.number, - $localName: localName(enumValueDesc), + $localName: enumValueDesc.localName, }); continue; } @@ -177,7 +173,7 @@ async function processFile(filePath, content, descriptorProto, upstream) { assert(val !== undefined); const valDesc = f.enum.values.find((e) => e.number === val); assert(valDesc !== undefined); - lines.push(` ${localName(f)}: ${val}, // ${valDesc.name},`); + lines.push(` ${f.localName}: ${val}, // ${valDesc.name},`); } lines.push(` },`); } diff --git a/packages/protobuf/src/codegenv1/enum.ts b/packages/protobuf/src/codegenv1/enum.ts index 3644566dd..555bb3585 100644 --- a/packages/protobuf/src/codegenv1/enum.ts +++ b/packages/protobuf/src/codegenv1/enum.ts @@ -13,7 +13,6 @@ // limitations under the License. import type { DescEnum, DescFile } from "../desc-types.js"; -import { localName } from "../reflect/names.js"; import type { GenDescEnum } from "./types.js"; /** @@ -42,9 +41,8 @@ export function enumDesc( export function tsEnum(desc: DescEnum) { const enumObject = {} as enumObject; for (const value of desc.values) { - const name = localName(value); - enumObject[name] = value.number; - enumObject[value.number] = name; + enumObject[value.localName] = value.number; + enumObject[value.number] = value.localName; } return enumObject; } diff --git a/packages/protobuf/src/create.ts b/packages/protobuf/src/create.ts index 5508e9be8..392e8ba4e 100644 --- a/packages/protobuf/src/create.ts +++ b/packages/protobuf/src/create.ts @@ -15,7 +15,6 @@ import { isMessage } from "./is-message.js"; import type { DescField, DescMessage, DescOneof } from "./desc-types.js"; import type { Message, MessageInitShape, MessageShape } from "./types.js"; -import { localName } from "./reflect/names.js"; import { LongType, ScalarType, scalarZeroValue } from "./reflect/scalar.js"; import { FieldError } from "./reflect/error.js"; import { isObject, type OneofADT } from "./reflect/guard.js"; @@ -60,7 +59,7 @@ function initMessage( init: MessageInitShape, ): MessageShape | FieldError { for (const member of messageDesc.members) { - let value = (init as Record)[localName(member)]; + let value = (init as Record)[member.localName]; if (value == null) { // intentionally ignore undefined and null continue; @@ -181,7 +180,7 @@ function createZeroMessage(desc: DescMessage): Message { msg = {}; for (const member of desc.members) { if (member.kind == "oneof" || member.presence == IMPLICIT) { - msg[localName(member)] = createZeroField(member); + msg[member.localName] = createZeroField(member); } } } else { @@ -212,7 +211,7 @@ function createZeroMessage(desc: DescMessage): Message { continue; } members.add(member); - prototype[localName(member)] = createZeroField(member); + prototype[member.localName] = createZeroField(member); } messagePrototypes.set(desc, { prototype, members }); } @@ -231,7 +230,7 @@ function createZeroMessage(desc: DescMessage): Message { } } } - msg[localName(member)] = createZeroField(member); + msg[member.localName] = createZeroField(member); } } msg.$typeName = desc.typeName; diff --git a/packages/protobuf/src/desc-types.ts b/packages/protobuf/src/desc-types.ts index e74c97558..8dd6045d9 100644 --- a/packages/protobuf/src/desc-types.ts +++ b/packages/protobuf/src/desc-types.ts @@ -159,6 +159,10 @@ export interface DescEnumValue { * The name of the enumeration value, as specified in the protobuf source. */ readonly name: string; + /** + * A safe and idiomatic name for the value in a TypeScript enum. + */ + readonly localName: string; /** * The enumeration this value belongs to. */ @@ -256,6 +260,10 @@ type descFieldCommon = descFieldAndExtensionShared & { * The message this field is declared on. */ readonly parent: DescMessage; + /** + * A safe and idiomatic name for the field as a property in ECMAScript. + */ + readonly localName: string; }; /** @@ -563,6 +571,10 @@ export interface DescOneof { * The name of the oneof group, as specified in the protobuf source. */ readonly name: string; + /** + * A safe and idiomatic name for the oneof group as a property in ECMAScript. + */ + readonly localName: string; /** * The message this oneof group was declared in. */ @@ -627,6 +639,10 @@ export interface DescMethod { * The name of the RPC, as specified in the protobuf source. */ readonly name: string; + /** + * A safe and idiomatic name for the RPC as a method in ECMAScript. + */ + readonly localName: string; /** * The parent service. */ diff --git a/packages/protobuf/src/extensions.ts b/packages/protobuf/src/extensions.ts index 3f290394d..eab919f43 100644 --- a/packages/protobuf/src/extensions.ts +++ b/packages/protobuf/src/extensions.ts @@ -27,7 +27,6 @@ import type { import { assert } from "./reflect/assert.js"; import { create } from "./create.js"; import { readField } from "./from-binary.js"; -import { localName } from "./reflect/names.js"; import type { ReflectMessage } from "./reflect/reflect-types.js"; import { reflect } from "./reflect/reflect.js"; import { scalarZeroValue } from "./reflect/scalar.js"; @@ -214,10 +213,12 @@ export function createExtensionContainer( extension: Desc, value?: ExtensionValueShape, ): [ReflectMessage, DescField, () => ExtensionValueShape] { + const localName = extension.typeName; const field = { ...extension, kind: "field", parent: extension.extendee, + localName, } as DescField; const desc = { ...extension.extendee, @@ -225,16 +226,15 @@ export function createExtensionContainer( members: [field], oneofs: [], }; - const fieldName = localName(field); const container = create( desc, - value !== undefined ? { [fieldName]: value } : undefined, + value !== undefined ? { [localName]: value } : undefined, ); return [ reflect(desc, container), field, () => { - const value = (container as Record)[fieldName]; + const value = (container as Record)[localName]; if (value === undefined) { // Only message fields are undefined, rest will have a zero value. const desc = extension.message!; diff --git a/packages/protobuf/src/fields.ts b/packages/protobuf/src/fields.ts index 25ff5c69c..cdc5413b4 100644 --- a/packages/protobuf/src/fields.ts +++ b/packages/protobuf/src/fields.ts @@ -13,7 +13,6 @@ // limitations under the License. import type { Message, MessageShape } from "./types.js"; -import { localName } from "./reflect/names.js"; import { unsafeClear, unsafeIsSet } from "./reflect/unsafe.js"; import type { DescMessage } from "./desc-types.js"; @@ -38,7 +37,7 @@ export function isFieldSet( message: MessageShape, fieldName: MessageFieldNames>, ): boolean { - const field = messageDesc.fields.find((f) => localName(f) === fieldName); + const field = messageDesc.fields.find((f) => f.localName === fieldName); if (field) { return unsafeIsSet(message, field); } @@ -53,7 +52,7 @@ export function clearField( message: MessageShape, fieldName: MessageFieldNames>, ): void { - const field = messageDesc.fields.find((f) => localName(f) === fieldName); + const field = messageDesc.fields.find((f) => f.localName === fieldName); if (field) { unsafeClear(message, field); } @@ -66,12 +65,12 @@ export function clearField( // prettier-ignore type MessageFieldNames = Message extends T ? string : Exclude ? K - : P - ]-?: true; -}, number | symbol> + [P in keyof T as + P extends ("$typeName" | "$unknown") ? never + : T[P] extends Oneof ? K + : P + ]-?: true; + }, number | symbol> type Oneof = { case: K | undefined; diff --git a/packages/protobuf/src/reflect/names.ts b/packages/protobuf/src/reflect/names.ts index b668d0cc1..5a52c8d7e 100644 --- a/packages/protobuf/src/reflect/names.ts +++ b/packages/protobuf/src/reflect/names.ts @@ -12,49 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import type { DescEnumValue, DescField } from "../desc-types.js"; -import type { DescMethod, DescOneof } from "../desc-types.js"; - -/** - * Returns the name of a protobuf element in generated code. - * - * Field names - including oneofs - are converted to lowerCamelCase. For methods, - * the first character is made lowercase. All names are escaped to be safe object - * property names. - * - * This function does not provide names for messages, enumerations, and services. - */ -export function localName( - desc: DescEnumValue | DescOneof | DescField | DescMethod, -): string { - const name = desc.name; - switch (desc.kind) { - // @ts-expect-error TS7029 - case "field": - if (desc.oneof !== undefined) { - // oneof member names are not properties, but values of the `case` property. - return protoCamelCase(name); - } - // eslint-disable-next-line no-fallthrough - case "oneof": - return safeObjectProperty(protoCamelCase(name)); - case "enum_value": { - let name = desc.name; - const sharedPrefix = desc.parent.sharedPrefix; - if (sharedPrefix !== undefined) { - name = name.substring(sharedPrefix.length); - } - return safeObjectProperty(name); - } - case "rpc": { - if (name.length == 0) { - return name; - } - return safeObjectProperty(name[0].toLowerCase() + name.substring(1)); - } - } -} - /** * Converts snake_case to protoCamelCase according to the convention * used by protoc to convert a field name to a JSON name. diff --git a/packages/protobuf/src/reflect/registry.ts b/packages/protobuf/src/reflect/registry.ts index 0434b335c..5e469319d 100644 --- a/packages/protobuf/src/reflect/registry.ts +++ b/packages/protobuf/src/reflect/registry.ts @@ -31,6 +31,7 @@ import type { MethodOptions_IdempotencyLevel, OneofDescriptorProto, ServiceDescriptorProto, + EnumValueDescriptorProto, } from "../wkt/gen/google/protobuf/descriptor_pb.js"; import { assert } from "./assert.js"; import type { @@ -51,6 +52,7 @@ import { import { LongType, ScalarType } from "./scalar.js"; import { nestedTypes } from "./nested-types.js"; import { unsafeIsSetExplicit } from "./unsafe.js"; +import { protoCamelCase, safeObjectProperty } from "./names.js"; /** * A set of descriptors for messages, enumerations, extensions, @@ -569,6 +571,7 @@ function addEnum( parent: DescMessage | undefined, reg: MutableRegistry, ): void { + const sharedPrefix = findEnumSharedPrefix(proto.name, proto.value); const desc: { -readonly [P in keyof DescEnum]: DescEnum[P] } = { kind: "enum", proto, @@ -579,10 +582,7 @@ function addEnum( name: proto.name, typeName: makeTypeName(proto, parent, file), values: [], - sharedPrefix: findEnumSharedPrefix( - proto.name, - proto.value.map((v) => v.name), - ), + sharedPrefix, toString(): string { return `enum ${this.typeName}`; }, @@ -590,12 +590,16 @@ function addEnum( desc.open = isEnumOpen(desc); reg.add(desc); proto.value.forEach((proto) => { + const name = proto.name; desc.values.push({ kind: "enum_value", proto, deprecated: proto.options?.deprecated ?? false, parent: desc, name: proto.name, + localName: safeObjectProperty( + sharedPrefix == undefined ? name : name.substring(sharedPrefix.length), + ), number: proto.number, toString() { return `enum value ${desc.typeName}.${this.name}`; @@ -711,6 +715,11 @@ function newMethod( deprecated: proto.options?.deprecated ?? false, parent, name, + localName: safeObjectProperty( + name.length + ? safeObjectProperty(name[0].toLowerCase() + name.substring(1)) + : name, + ), methodKind, input, output, @@ -732,6 +741,7 @@ function newOneof(proto: OneofDescriptorProto, parent: DescMessage): DescOneof { parent, fields: [], name: proto.name, + localName: safeObjectProperty(protoCamelCase(proto.name)), toString(): string { return `oneof ${parent.typeName}.${this.name}`; }, @@ -808,6 +818,9 @@ function newField( field.kind = "field"; field.parent = parent; field.oneof = oneof; + field.localName = oneof + ? protoCamelCase(proto.name) + : safeObjectProperty(protoCamelCase(proto.name)); field.jsonName = proto.jsonName; field.toString = () => `field ${parent.typeName}.${proto.name}`; } @@ -956,14 +969,14 @@ function findFileDependencies( */ function findEnumSharedPrefix( enumName: string, - valueNames: string[], + values: EnumValueDescriptorProto[], ): string | undefined { const prefix = camelToSnakeCase(enumName) + "_"; - for (const name of valueNames) { - if (!name.toLowerCase().startsWith(prefix)) { + for (const value of values) { + if (!value.name.toLowerCase().startsWith(prefix)) { return undefined; } - const shortName = name.substring(prefix.length); + const shortName = value.name.substring(prefix.length); if (shortName.length == 0) { return undefined; } diff --git a/packages/protobuf/src/reflect/unsafe.ts b/packages/protobuf/src/reflect/unsafe.ts index 7d2dd8069..b6656bbab 100644 --- a/packages/protobuf/src/reflect/unsafe.ts +++ b/packages/protobuf/src/reflect/unsafe.ts @@ -13,7 +13,6 @@ // limitations under the License. import type { DescField, DescOneof } from "../desc-types.js"; -import { localName } from "./names.js"; import type { OneofADT } from "./guard.js"; import { isScalarZeroValue, scalarZeroValue } from "./scalar.js"; import type { FeatureSet_FieldPresence } from "../wkt/gen/google/protobuf/descriptor_pb.js"; @@ -32,11 +31,11 @@ export function unsafeOneofCase( target: Record, // eslint-disable-line @typescript-eslint/no-explicit-any -- `any` is the best choice for dynamic access oneof: DescOneof, ) { - const c = (target[localName(oneof)] as OneofADT).case; + const c = (target[oneof.localName] as OneofADT).case; if (c === undefined) { return c; } - return oneof.fields.find((f) => localName(f) === c); + return oneof.fields.find((f) => f.localName === c); } /** @@ -48,9 +47,9 @@ export function unsafeIsSet( target: Record, // eslint-disable-line @typescript-eslint/no-explicit-any -- `any` is the best choice for dynamic access field: DescField, ) { - const name = localName(field); + const name = field.localName; if (field.oneof) { - return target[localName(field.oneof)].case === name; // eslint-disable-line @typescript-eslint/no-unsafe-member-access + return target[field.oneof.localName].case === name; // eslint-disable-line @typescript-eslint/no-unsafe-member-access } if (field.presence != IMPLICIT) { // Fields with explicit presence have properties on the prototype chain @@ -96,15 +95,14 @@ export function unsafeGet( target: Record, field: DescField, ): unknown { - const name = localName(field); if (field.oneof) { - const oneof = target[localName(field.oneof)] as OneofADT; - if (oneof.case === name) { + const oneof = target[field.oneof.localName] as OneofADT; + if (oneof.case === field.localName) { return oneof.value; } return undefined; } - return target[name]; + return target[field.localName]; } /** @@ -117,14 +115,13 @@ export function unsafeSet( field: DescField, value: unknown, ) { - const name = localName(field); if (field.oneof) { - target[localName(field.oneof)] = { - case: name, + target[field.oneof.localName] = { + case: field.localName, value: value, }; } else { - target[name] = value; + target[field.localName] = value; } } @@ -137,9 +134,9 @@ export function unsafeClear( target: Record, // eslint-disable-line @typescript-eslint/no-explicit-any -- `any` is the best choice for dynamic access field: DescField, ) { - const name = localName(field); + const name = field.localName; if (field.oneof) { - const oneofLocalName = localName(field.oneof); + const oneofLocalName = field.oneof.localName; if ((target[oneofLocalName] as OneofADT).case === name) { target[oneofLocalName] = { case: undefined }; } @@ -177,8 +174,7 @@ export function unsafeAddListItem( field: DescField & { fieldKind: "list" }, value: unknown, ) { - const name = localName(field); - (target[name] as unknown[]).push(value); + (target[field.localName] as unknown[]).push(value); } /** @@ -192,6 +188,5 @@ export function unsafeSetMapEntry( key: string | number, value: unknown, ) { - const name = localName(field); - (target[name] as Record)[key] = value; + (target[field.localName] as Record)[key] = value; } diff --git a/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts b/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts index 88e55a0c7..204c8b29e 100644 --- a/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts +++ b/packages/protoc-gen-es/src/protoc-gen-es-plugin.ts @@ -18,7 +18,7 @@ import type { DescMessage, DescService, } from "@bufbuild/protobuf"; -import { localName, parentTypes } from "@bufbuild/protobuf/reflect"; +import { parentTypes } from "@bufbuild/protobuf/reflect"; import { embedFileDesc, pathInFileDesc } from "@bufbuild/protobuf/codegenv1"; import { createEcmaScriptPlugin } from "@bufbuild/protoplugin"; import type { @@ -298,7 +298,7 @@ function getServiceShapeExpr(f: GeneratedFile, service: DescService): Printable print("{"); for (const method of service.methods) { print(f.jsDoc(method, " ")); - print(" ", localName(method), ": {"); + print(" ", method.localName, ": {"); print(" kind: ", f.string(method.methodKind), ";"); print(" I: ", f.importShape(method.input), ";"); print(" O: ", f.importShape(method.output), ";"); @@ -317,7 +317,7 @@ function generateEnumShape(f: GeneratedFile, enumeration: DescEnum) { f.print(); } f.print(f.jsDoc(value, " ")); - f.print(" ", localName(value), " = ", value.number, ","); + f.print(" ", value.localName, " = ", value.number, ","); } f.print("}"); f.print(); @@ -333,7 +333,7 @@ function generateMessageShape(f: GeneratedFile, message: DescMessage, target: Ex switch (member.kind) { case "oneof": f.print(f.jsDoc(member, " ")); - f.print(" ", localName(member), ": {"); + f.print(" ", member.localName, ": {"); for (const field of member.fields) { if (member.fields.indexOf(field) > 0) { f.print(` } | {`); @@ -341,7 +341,7 @@ function generateMessageShape(f: GeneratedFile, message: DescMessage, target: Ex f.print(f.jsDoc(field, " ")); const { typing } = fieldTypeScriptType(field); f.print(` value: `, typing, `;`); - f.print(` case: "`, localName(field), `";`); + f.print(` case: "`, field.localName, `";`); } f.print(` } | { case: undefined; value?: undefined };`); break; @@ -349,9 +349,9 @@ function generateMessageShape(f: GeneratedFile, message: DescMessage, target: Ex f.print(f.jsDoc(member, " ")); const { typing, optional } = fieldTypeScriptType(member); if (optional) { - f.print(" ", localName(member), "?: ", typing, ";"); + f.print(" ", member.localName, "?: ", typing, ";"); } else { - f.print(" ", localName(member), ": ", typing, ";"); + f.print(" ", member.localName, ": ", typing, ";"); } break; } diff --git a/packages/protoplugin-example/src/protoc-gen-twirp-es.ts b/packages/protoplugin-example/src/protoc-gen-twirp-es.ts index 846c21d59..2064ebe44 100755 --- a/packages/protoplugin-example/src/protoc-gen-twirp-es.ts +++ b/packages/protoplugin-example/src/protoc-gen-twirp-es.ts @@ -16,7 +16,6 @@ import { createEcmaScriptPlugin, runNodeJs } from "@bufbuild/protoplugin"; import { getOption, hasOption } from "@bufbuild/protobuf"; -import { localName } from "@bufbuild/protobuf/reflect"; import { type Schema, safeIdentifier } from "@bufbuild/protoplugin/ecmascript"; import { default_host } from "./gen/customoptions/default_host_pb.js"; import { version } from "../package.json"; @@ -60,7 +59,7 @@ function generateTs(schema: Schema) { const outputType = f.importShape(method.output); const outputDesc = f.importDesc(method.output); const jsonValue = f.import("JsonValue", "@bufbuild/protobuf").toTypeOnly(); - f.print(" async ", localName(method), "(request: ", inputType, "): Promise<", outputType, "> {"); + f.print(" async ", method.localName, "(request: ", inputType, "): Promise<", outputType, "> {"); f.print(" const headers = new Headers([]);"); f.print(" headers.set('content-type', 'application/json');"); f.print(" const fetchResponse = await fetch("); From 2bd8de62c49351f7a08430ded8768db34ce20453 Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Thu, 9 May 2024 11:49:52 +0200 Subject: [PATCH 2/2] Revert formatting change --- packages/protobuf/src/fields.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/protobuf/src/fields.ts b/packages/protobuf/src/fields.ts index cdc5413b4..028591942 100644 --- a/packages/protobuf/src/fields.ts +++ b/packages/protobuf/src/fields.ts @@ -65,12 +65,12 @@ export function clearField( // prettier-ignore type MessageFieldNames = Message extends T ? string : Exclude ? K - : P - ]-?: true; - }, number | symbol> + [P in keyof T as + P extends ("$typeName" | "$unknown") ? never + : T[P] extends Oneof ? K + : P + ]-?: true; +}, number | symbol>; type Oneof = { case: K | undefined;