From d5309cb6b6983913dea192a1f0a715df7e78f815 Mon Sep 17 00:00:00 2001 From: cataggar <87583576+cataggar@users.noreply.github.com> Date: Fri, 2 Oct 2026 22:53:18 +0000 Subject: [PATCH 1/2] feat(protobuf): add opt-in enum value prefixes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../protobuf-enum-value-prefix-2026-10-02.md | 17 ++ packages/protobuf/README.md | 8 + packages/protobuf/package.json | 3 + packages/protobuf/src/lib.ts | 22 ++ packages/protobuf/src/transform/index.ts | 51 +++- packages/protobuf/test/enum-prefix.test.ts | 250 ++++++++++++++++++ pnpm-lock.yaml | 4 + .../docs/docs/emitters/protobuf/guide.md | 38 +++ .../emitters/protobuf/reference/emitter.md | 8 + 9 files changed, 395 insertions(+), 6 deletions(-) create mode 100644 .chronus/changes/protobuf-enum-value-prefix-2026-10-02.md create mode 100644 packages/protobuf/test/enum-prefix.test.ts diff --git a/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md b/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md new file mode 100644 index 00000000000..07182f10da9 --- /dev/null +++ b/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md @@ -0,0 +1,17 @@ +--- +changeKind: feature +packages: + - "@typespec/protobuf" +--- + +Add an opt-in `enum-value-prefix: enum-name` emitter option to prefix enum values with the enum name in `UPPER_SNAKE_CASE`, without repeating the prefix in TypeSpec members. + +```yaml +options: + "@typespec/protobuf": + enum-value-prefix: enum-name +``` + +For example, the `Shipped: 2` member of `OrderState` emits `ORDER_STATE_SHIPPED = 2`. Already-prefixed names are preserved, and collisions in the emitted package scope are reported as errors. + +Existing output remains unchanged by default. Enum members and numeric values remain explicit; this option does not add an `Unspecified` member or infer numbers. diff --git a/packages/protobuf/README.md b/packages/protobuf/README.md index 1e407639097..e867ccb4f19 100644 --- a/packages/protobuf/README.md +++ b/packages/protobuf/README.md @@ -54,6 +54,14 @@ If set to `true`, this emitter will not write any files. It will still validate By default, the emitter will create `message` declarations for any models in a namespace decorated with `@package` that have an `@field` decorator on every property. If this option is set to true, this behavior will be disabled, and only messages that are explicitly decorated with `@message` or that are reachable from a service operation will be emitted. +### `enum-value-prefix` + +**Type:** `"none" | "enum-name"` + +**Default:** `"none"` + +When set to `enum-name`, enum values are prefixed with the enum name in UPPER_SNAKE_CASE. Already-prefixed names are preserved. By default (`none`), member names are emitted unchanged. This option only changes names; explicit integer values and a first member set to zero are still required. + ## Decorators ### TypeSpec.Protobuf diff --git a/packages/protobuf/package.json b/packages/protobuf/package.json index 8a600a7eb41..67d9bd6a900 100644 --- a/packages/protobuf/package.json +++ b/packages/protobuf/package.json @@ -46,6 +46,9 @@ "lint:fix": "oxlint . --fix", "regen-docs": "tspd doc . --enable-experimental --llmstxt --output-dir ../../website/src/content/docs/docs/emitters/protobuf/reference" }, + "dependencies": { + "change-case": "catalog:" + }, "peerDependencies": { "@typespec/compiler": "workspace:^" }, diff --git a/packages/protobuf/src/lib.ts b/packages/protobuf/src/lib.ts index 9e153b93b7f..2b840a07549 100644 --- a/packages/protobuf/src/lib.ts +++ b/packages/protobuf/src/lib.ts @@ -21,6 +21,14 @@ export interface ProtobufEmitterOptions { * in an interface decoarated with `@service` will be emitted. */ "omit-unreachable-types"?: boolean; + + /** + * Prefix enum values with the enum name in UPPER_SNAKE_CASE. + * + * By default, member names are emitted unchanged. This option only changes names; + * explicit integer values and a first member set to zero are still required. + */ + "enum-value-prefix"?: "none" | "enum-name"; } const EmitterOptionsSchema: JSONSchemaType = { @@ -39,6 +47,14 @@ const EmitterOptionsSchema: JSONSchemaType = { description: "By default, the emitter will create `message` declarations for any models in a namespace decorated with `@package` that have an `@field` decorator on every property. If this option is set to true, this behavior will be disabled, and only messages that are explicitly decorated with `@message` or that are reachable from a service operation will be emitted.", }, + "enum-value-prefix": { + type: "string", + enum: ["none", "enum-name"], + nullable: true, + default: "none", + description: + "When set to `enum-name`, enum values are prefixed with the enum name in UPPER_SNAKE_CASE. Already-prefixed names are preserved. By default (`none`), member names are emitted unchanged. This option only changes names; explicit integer values and a first member set to zero are still required.", + }, }, required: [], }; @@ -136,6 +152,12 @@ export const TypeSpecProtobufLibrary = createTypeSpecLibrary({ "the first variant of an enum must be set to zero to be used in a Protobuf message", }, }, + "enum-value-name-collision": { + severity: "error", + messages: { + default: paramMessage`enum value name '${"name"}' is already used in this Protobuf package`, + }, + }, "nested-array": { severity: "error", messages: { diff --git a/packages/protobuf/src/transform/index.ts b/packages/protobuf/src/transform/index.ts index c56cafd50f0..79a74d127ba 100644 --- a/packages/protobuf/src/transform/index.ts +++ b/packages/protobuf/src/transform/index.ts @@ -4,6 +4,7 @@ import type { DiagnosticTarget, Enum, + EnumMember, Interface, IntrinsicType, Model, @@ -30,6 +31,7 @@ import { } from "@typespec/compiler"; import { SyntaxKind } from "@typespec/compiler/ast"; import { capitalize } from "@typespec/compiler/casing"; +import { constantCase } from "change-case"; import type { ProtoEnumDeclaration, ProtoEnumVariantDeclaration, @@ -104,6 +106,10 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P [...packages].map((p) => [p, []]), ); + const enumValueNames = new Map( + [...packages].map((p) => [p, []]), + ); + const visitedTypes = new Set(); /** @@ -218,6 +224,20 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P checkForNamespaceCollisions(files); + for (const file of files) { + const names = new Set([...file.declarations].map((declaration) => declaration.name)); + for (const { name, member } of enumValueNames.get(file.source) ?? []) { + if (names.has(name)) { + reportDiagnostic(program, { + target: member, + code: "enum-value-name-collision", + format: { name }, + }); + } + names.add(name); + } + } + return files; /** @@ -879,17 +899,36 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P */ function toEnum(e: Enum): ProtoEnumDeclaration { const needsAlias = new Set([...e.members.values()].map((v) => v.value)).size !== e.members.size; + const prefix = + emitterOptions["enum-value-prefix"] === "enum-name" + ? constantCase(e.name, { prefixCharacters: "_" }) + "_" + : undefined; + const enumPackage = getPackageOfType(program, e); return { kind: "enum", name: e.name, allowAlias: needsAlias, - variants: [...e.members.values()].map((variant): ProtoEnumVariantDeclaration => ({ - kind: "variant", - name: variant.name, - value: variant.value as number, - doc: getDoc(program, variant), - })), + variants: [...e.members.values()].map((variant): ProtoEnumVariantDeclaration => { + let name = variant.name; + if (prefix !== undefined) { + if (!name.startsWith(prefix)) { + name = constantCase(name, { prefixCharacters: "_" }); + if (!name.startsWith(prefix)) { + name = prefix + name; + } + } + if (enumPackage) { + enumValueNames.get(enumPackage)?.push({ name, member: variant }); + } + } + return { + kind: "variant", + name, + value: variant.value as number, + doc: getDoc(program, variant), + }; + }), doc: getDoc(program, e), }; } diff --git a/packages/protobuf/test/enum-prefix.test.ts b/packages/protobuf/test/enum-prefix.test.ts new file mode 100644 index 00000000000..ae9185633a3 --- /dev/null +++ b/packages/protobuf/test/enum-prefix.test.ts @@ -0,0 +1,250 @@ +import { expectDiagnostics } from "@typespec/compiler/testing"; +import { describe, expect, it } from "vitest"; +import { Tester as BaseTester } from "./tester.js"; + +const Tester = BaseTester.importLibraries().using("Protobuf"); +const PrefixTester = Tester.emit("@typespec/protobuf", { "enum-value-prefix": "enum-name" }); + +describe("enum-value-prefix", () => { + it.each([undefined, "none"])("preserves member names with option %s", async (option) => { + const result = await Tester.emit( + "@typespec/protobuf", + option === undefined ? {} : { "enum-value-prefix": option }, + ).compile(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Unknown: 0, + pendingShipment: 5, + } + `); + + expect(result.outputs["main.proto"]).toContain(`enum OrderState { + Unknown = 0; + pendingShipment = 5; +}`); + }); + + it("prefixes shared short names while preserving numbers, aliases, and documentation", async () => { + const result = await PrefixTester.compile(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + @field(2) payment: PaymentMethod; + } + enum OrderState { + Unspecified: 0, + /** Awaiting shipment. */ + Pending: 5, + AwaitingShipment: 5, + ORDER_STATE_SHIPPED: 10, + } + enum PaymentMethod { + Unspecified: 0, + CreditCard: 20, + } + `); + + expect(result.outputs["main.proto"]).toContain(`enum OrderState { + option allow_alias = true; + + ORDER_STATE_UNSPECIFIED = 0; + ORDER_STATE_PENDING = 5; // Awaiting shipment. + ORDER_STATE_AWAITING_SHIPMENT = 5; + ORDER_STATE_SHIPPED = 10; +}`); + expect(result.outputs["main.proto"]).toContain(`enum PaymentMethod { + PAYMENT_METHOD_UNSPECIFIED = 0; + PAYMENT_METHOD_CREDIT_CARD = 20; +}`); + expect(result.outputs["main.proto"]).toContain(`message Example { + OrderState state = 1; + PaymentMethod payment = 2; +}`); + }); + + it.each([ + ["OrderState", "pendingShipment", "ORDER_STATE_PENDING_SHIPMENT"], + ["HTTPStatus", "HTTPTimeout", "HTTP_STATUS_HTTP_TIMEOUT"], + ["Http2Status", "retryAfter2Seconds", "HTTP2_STATUS_RETRY_AFTER2_SECONDS"], + ["order_state", "in_progress", "ORDER_STATE_IN_PROGRESS"], + ["OrderState", "ORDER_STATE_SHIPPED", "ORDER_STATE_SHIPPED"], + ["OrderState", "ORDER_STATE_READY__NOW", "ORDER_STATE_READY__NOW"], + ["OrderState", "OrderStateShipped", "ORDER_STATE_SHIPPED"], + ["OrderState", "OrderStateful", "ORDER_STATE_ORDER_STATEFUL"], + ])("converts %s.%s to %s", async (enumName, memberName, expectedName) => { + const result = await PrefixTester.compile(` + @package + namespace Test; + model Example { + @field(1) value: ${enumName}; + } + enum ${enumName} { + Unknown: 0, + ${memberName}: 5, + } + `); + + expect(result.outputs["main.proto"]).toContain(`enum ${enumName} {`); + expect(result.outputs["main.proto"]).toContain(` ${expectedName} = 5;`); + expect(result.outputs["main.proto"]).not.toContain("_UNSPECIFIED"); + }); + + it("diagnoses member names that collide after normalization", async () => { + const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Unspecified: 0, + InProgress: 1, + in_progress: 2, + } + `); + + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/enum-value-name-collision", + message: "enum value name 'ORDER_STATE_IN_PROGRESS' is already used in this Protobuf package", + }); + expect(result.outputs).toEqual({}); + }); + + it("diagnoses a collision with an already-prefixed alias", async () => { + const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Unspecified: 0, + Shipped: 1, + ORDER_STATE_SHIPPED: 1, + } + `); + + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/enum-value-name-collision", + message: "enum value name 'ORDER_STATE_SHIPPED' is already used in this Protobuf package", + }); + expect(result.outputs).toEqual({}); + }); + + it("diagnoses collisions across enums in nested namespaces of the same package", async () => { + const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` + @package + namespace Test; + model Example { + @field(1) first: HTTPStatus; + @field(2) second: Nested.HttpStatus; + } + enum HTTPStatus { + Unknown: 0, + } + namespace Nested { + enum HttpStatus { + Unknown: 0, + } + } + `); + + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/enum-value-name-collision", + message: "enum value name 'HTTP_STATUS_UNKNOWN' is already used in this Protobuf package", + }); + expect(result.outputs).toEqual({}); + }); + + it("allows identical enum value names in different packages", async () => { + const result = await PrefixTester.compile(` + @package({ name: "first" }) + namespace First { + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Unknown: 0, + } + } + @package({ name: "second" }) + namespace Second { + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Unknown: 0, + } + } + `); + + expect(result.outputs["first.proto"]).toContain(" ORDER_STATE_UNKNOWN = 0;"); + expect(result.outputs["second.proto"]).toContain(" ORDER_STATE_UNKNOWN = 0;"); + }); + + it("diagnoses a collision with a package-level message name", async () => { + const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + } + @message + model ORDER_STATE_SHIPPED {} + enum OrderState { + Unknown: 0, + Shipped: 1, + } + `); + + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/enum-value-name-collision", + message: "enum value name 'ORDER_STATE_SHIPPED' is already used in this Protobuf package", + }); + expect(result.outputs).toEqual({}); + }); + + it("still requires an explicit integer on every member", async () => { + const [, diagnostics] = await PrefixTester.compileAndDiagnose(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Unknown: 0, + Shipped, + } + `); + + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/unconvertible-enum", + message: + "enums must explicitly assign exactly one integer to each member to be used in a Protobuf message", + }); + }); + + it("does not insert a zero member when the first value is nonzero", async () => { + const [, diagnostics] = await PrefixTester.compileAndDiagnose(` + @package + namespace Test; + model Example { + @field(1) state: OrderState; + } + enum OrderState { + Pending: 1, + Shipped: 2, + } + `); + + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/unconvertible-enum", + message: "the first variant of an enum must be set to zero to be used in a Protobuf message", + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f787ae85ea0..595c1941426 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2106,6 +2106,10 @@ importers: version: 5.0.0(@types/node@26.4.1)(@vitest/coverage-v8@5.0.0)(@vitest/ui@5.0.0)(happy-dom@20.11.6)(jsdom@25.0.1)(vite@8.2.2(@types/node@26.4.1)(esbuild@0.28.2)(yaml@2.9.0)) packages/protobuf: + dependencies: + change-case: + specifier: 'catalog:' + version: 5.4.4 devDependencies: '@types/micromatch': specifier: 'catalog:' diff --git a/website/src/content/docs/docs/emitters/protobuf/guide.md b/website/src/content/docs/docs/emitters/protobuf/guide.md index 987742f224f..4120151ecab 100644 --- a/website/src/content/docs/docs/emitters/protobuf/guide.md +++ b/website/src/content/docs/docs/emitters/protobuf/guide.md @@ -107,6 +107,44 @@ Optional message fields (TypeSpec optional properties where the type is a model) Optional `repeated` and `map` fields are emitted without `optional` and produce a warning; protobuf cannot distinguish between _unset_ and _empty_ for those shapes because they are represented by "repeating" a field index within the protobuf message payload (if the field isn't set in the message, that means "_empty_" and there is no alternative to represent "_this field is not set_"). +### Enums + +Enums referenced by messages are translated into Protobuf enums. Every member must have an explicit integer value, and the first member must have value `0`. Values do not need to be consecutive. If multiple members share a number, the emitter adds `option allow_alias = true`. + +By default, enum member names are emitted unchanged. Protobuf enum values share their containing scope, so values from different package-level enums cannot have the same name. Both [AIP-126](https://google.aip.dev/126) and the [Protobuf style guide](https://protobuf.dev/programming-guides/style/#enum-value-prefixing) recommend prefixing all package-level enum values with the enum name. + +To write short TypeSpec member names while emitting prefixed `UPPER_SNAKE_CASE` values, enable `enum-value-prefix`: + +```yaml +options: + "@typespec/protobuf": + enum-value-prefix: enum-name +``` + +For example, this enum referenced by a message: + +```typespec +enum OrderState { + Unspecified: 0, + Pending: 1, + Shipped: 2, +} +``` + +is emitted as: + +```protobuf +enum OrderState { + ORDER_STATE_UNSPECIFIED = 0; + ORDER_STATE_PENDING = 1; + ORDER_STATE_SHIPPED = 2; +} +``` + +The enum name and member name are converted to `UPPER_SNAKE_CASE`, with acronyms treated as words: `HTTPStatus.HttpTimeout` becomes `HTTP_STATUS_HTTP_TIMEOUT`. Names that already start with the emitted enum prefix are preserved unchanged. Names that acquire the prefix during case conversion are not prefixed twice. The emitter reports an error if the resulting value name collides with another enum value or a declaration in the same Protobuf package. + +This option changes names only. It does not insert an `Unspecified` member or infer numbers, and an existing zero member such as `Unknown: 0` keeps its meaning. Numeric values, member order, and enum type names are preserved. Changing value names can affect generated APIs and Protobuf JSON strings, even though the numeric binary values remain unchanged. + ### Services TypeSpec defines a "service" using the [`TypeSpec.service` decorator][native-service], but the Protobuf "service" concept is different and is denoted by the [`TypeSpec.Protobuf.service` decorator][protobuf-service]. diff --git a/website/src/content/docs/docs/emitters/protobuf/reference/emitter.md b/website/src/content/docs/docs/emitters/protobuf/reference/emitter.md index 63c8a5a20fa..6e56bd5c89a 100644 --- a/website/src/content/docs/docs/emitters/protobuf/reference/emitter.md +++ b/website/src/content/docs/docs/emitters/protobuf/reference/emitter.md @@ -47,3 +47,11 @@ If set to `true`, this emitter will not write any files. It will still validate **Type:** `boolean` By default, the emitter will create `message` declarations for any models in a namespace decorated with `@package` that have an `@field` decorator on every property. If this option is set to true, this behavior will be disabled, and only messages that are explicitly decorated with `@message` or that are reachable from a service operation will be emitted. + +### `enum-value-prefix` + +**Type:** `"none" | "enum-name"` + +**Default:** `"none"` + +When set to `enum-name`, enum values are prefixed with the enum name in UPPER_SNAKE_CASE. Already-prefixed names are preserved. By default (`none`), member names are emitted unchanged. This option only changes names; explicit integer values and a first member set to zero are still required. From ffce982497f81d6870b32d073c4e5ba2224f263b Mon Sep 17 00:00:00 2001 From: cataggar <87583576+cataggar@users.noreply.github.com> Date: Fri, 2 Oct 2026 23:49:32 +0000 Subject: [PATCH 2/2] fix(protobuf): validate emitted enum scopes and identify collisions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../protobuf-enum-value-prefix-2026-10-02.md | 2 +- packages/protobuf/src/lib.ts | 2 +- packages/protobuf/src/transform/index.ts | 60 ++++++---- packages/protobuf/test/enum-prefix.test.ts | 105 ++++++++++++++---- .../docs/docs/emitters/protobuf/guide.md | 2 +- 5 files changed, 124 insertions(+), 47 deletions(-) diff --git a/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md b/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md index 07182f10da9..664bfd57a66 100644 --- a/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md +++ b/.chronus/changes/protobuf-enum-value-prefix-2026-10-02.md @@ -12,6 +12,6 @@ options: enum-value-prefix: enum-name ``` -For example, the `Shipped: 2` member of `OrderState` emits `ORDER_STATE_SHIPPED = 2`. Already-prefixed names are preserved, and collisions in the emitted package scope are reported as errors. +For example, the `Shipped: 2` member of `OrderState` emits `ORDER_STATE_SHIPPED = 2`. Already-prefixed names are preserved, and collisions in the emitted package scope are reported as errors identifying the original conflicting symbol. Existing output remains unchanged by default. Enum members and numeric values remain explicit; this option does not add an `Unspecified` member or infer numbers. diff --git a/packages/protobuf/src/lib.ts b/packages/protobuf/src/lib.ts index 2b840a07549..8a40aa72a63 100644 --- a/packages/protobuf/src/lib.ts +++ b/packages/protobuf/src/lib.ts @@ -155,7 +155,7 @@ export const TypeSpecProtobufLibrary = createTypeSpecLibrary({ "enum-value-name-collision": { severity: "error", messages: { - default: paramMessage`enum value name '${"name"}' is already used in this Protobuf package`, + default: paramMessage`enum value name '${"name"}' collides with ${"kind"} '${"owner"}' in this Protobuf package`, }, }, "nested-array": { diff --git a/packages/protobuf/src/transform/index.ts b/packages/protobuf/src/transform/index.ts index 79a74d127ba..8d3dc21c291 100644 --- a/packages/protobuf/src/transform/index.ts +++ b/packages/protobuf/src/transform/index.ts @@ -106,9 +106,7 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P [...packages].map((p) => [p, []]), ); - const enumValueNames = new Map( - [...packages].map((p) => [p, []]), - ); + const enumMemberSources = new WeakMap(); const visitedTypes = new Set(); @@ -224,17 +222,9 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P checkForNamespaceCollisions(files); - for (const file of files) { - const names = new Set([...file.declarations].map((declaration) => declaration.name)); - for (const { name, member } of enumValueNames.get(file.source) ?? []) { - if (names.has(name)) { - reportDiagnostic(program, { - target: member, - code: "enum-value-name-collision", - format: { name }, - }); - } - names.add(name); + if (emitterOptions["enum-value-prefix"] === "enum-name") { + for (const file of files) { + checkForEnumValueNameCollisions(file); } } @@ -709,6 +699,39 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P } } + function checkForEnumValueNameCollisions(file: ProtoFile) { + const declarations = [...file.declarations]; + const names = new Map< + string, + { kind: ProtoTopLevelDeclaration["kind"] | "enum value"; owner: string } + >(); + + for (const declaration of declarations) { + if (!names.has(declaration.name)) { + names.set(declaration.name, { kind: declaration.kind, owner: declaration.name }); + } + } + + for (const declaration of declarations) { + if (declaration.kind !== "enum") continue; + + for (const variant of declaration.variants) { + const member = enumMemberSources.get(variant); + compilerAssert(member, "Missing TypeSpec source for emitted enum value."); + const existing = names.get(variant.name); + if (existing) { + reportDiagnostic(program, { + target: member, + code: "enum-value-name-collision", + format: { name: variant.name, kind: existing.kind, owner: existing.owner }, + }); + } else { + names.set(variant.name, { kind: "enum value", owner: getTypeName(member) }); + } + } + } + } + /** * @param model - the Model to convert * @returns a corresponding message declaration @@ -903,8 +926,6 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P emitterOptions["enum-value-prefix"] === "enum-name" ? constantCase(e.name, { prefixCharacters: "_" }) + "_" : undefined; - const enumPackage = getPackageOfType(program, e); - return { kind: "enum", name: e.name, @@ -918,16 +939,15 @@ function tspToProto(program: Program, emitterOptions: ProtobufEmitterOptions): P name = prefix + name; } } - if (enumPackage) { - enumValueNames.get(enumPackage)?.push({ name, member: variant }); - } } - return { + const declaration: ProtoEnumVariantDeclaration = { kind: "variant", name, value: variant.value as number, doc: getDoc(program, variant), }; + enumMemberSources.set(declaration, variant); + return declaration; }), doc: getDoc(program, e), }; diff --git a/packages/protobuf/test/enum-prefix.test.ts b/packages/protobuf/test/enum-prefix.test.ts index ae9185633a3..bdf5961654c 100644 --- a/packages/protobuf/test/enum-prefix.test.ts +++ b/packages/protobuf/test/enum-prefix.test.ts @@ -94,7 +94,7 @@ describe("enum-value-prefix", () => { expect(result.outputs["main.proto"]).not.toContain("_UNSPECIFIED"); }); - it("diagnoses member names that collide after normalization", async () => { + it("diagnoses member names that collide after normalization and identifies the first use", async () => { const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` @package namespace Test; @@ -105,13 +105,22 @@ describe("enum-value-prefix", () => { Unspecified: 0, InProgress: 1, in_progress: 2, + ORDER_STATE_IN_PROGRESS: 3, } `); - expectDiagnostics(diagnostics, { - code: "@typespec/protobuf/enum-value-name-collision", - message: "enum value name 'ORDER_STATE_IN_PROGRESS' is already used in this Protobuf package", - }); + expectDiagnostics(diagnostics, [ + { + code: "@typespec/protobuf/enum-value-name-collision", + message: + "enum value name 'ORDER_STATE_IN_PROGRESS' collides with enum value 'Test.OrderState.InProgress' in this Protobuf package", + }, + { + code: "@typespec/protobuf/enum-value-name-collision", + message: + "enum value name 'ORDER_STATE_IN_PROGRESS' collides with enum value 'Test.OrderState.InProgress' in this Protobuf package", + }, + ]); expect(result.outputs).toEqual({}); }); @@ -131,14 +140,17 @@ describe("enum-value-prefix", () => { expectDiagnostics(diagnostics, { code: "@typespec/protobuf/enum-value-name-collision", - message: "enum value name 'ORDER_STATE_SHIPPED' is already used in this Protobuf package", + message: + "enum value name 'ORDER_STATE_SHIPPED' collides with enum value 'Test.OrderState.Shipped' in this Protobuf package", }); expect(result.outputs).toEqual({}); }); - it("diagnoses collisions across enums in nested namespaces of the same package", async () => { - const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` - @package + it.each(["@package", '@package({ name: "example.v1" })'])( + "diagnoses collisions across nested enums with %s", + async (packageDecorator) => { + const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` + ${packageDecorator} namespace Test; model Example { @field(1) first: HTTPStatus; @@ -154,12 +166,14 @@ describe("enum-value-prefix", () => { } `); - expectDiagnostics(diagnostics, { - code: "@typespec/protobuf/enum-value-name-collision", - message: "enum value name 'HTTP_STATUS_UNKNOWN' is already used in this Protobuf package", - }); - expect(result.outputs).toEqual({}); - }); + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/enum-value-name-collision", + message: + "enum value name 'HTTP_STATUS_UNKNOWN' collides with enum value 'Test.HTTPStatus.Unknown' in this Protobuf package", + }); + expect(result.outputs).toEqual({}); + }, + ); it("allows identical enum value names in different packages", async () => { const result = await PrefixTester.compile(` @@ -187,26 +201,69 @@ describe("enum-value-prefix", () => { expect(result.outputs["second.proto"]).toContain(" ORDER_STATE_UNKNOWN = 0;"); }); - it("diagnoses a collision with a package-level message name", async () => { - const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` + it.each([ + { + kind: "message", + declaration: "@message model ORDER_STATE_SHIPPED {}", + field: "", + }, + { + kind: "enum", + declaration: "enum ORDER_STATE_SHIPPED { Unknown: 0 }", + field: "@field(2) other: ORDER_STATE_SHIPPED;", + }, + { + kind: "service", + declaration: "@Protobuf.service interface ORDER_STATE_SHIPPED { ping(): void; }", + field: "", + }, + ])( + "diagnoses a collision with a package-level $kind and identifies its kind and name", + async ({ kind, declaration, field }) => { + const [result, diagnostics] = await PrefixTester.compileAndDiagnose(` @package namespace Test; model Example { @field(1) state: OrderState; + ${field} } - @message - model ORDER_STATE_SHIPPED {} + ${declaration} enum OrderState { Unknown: 0, Shipped: 1, } `); - expectDiagnostics(diagnostics, { - code: "@typespec/protobuf/enum-value-name-collision", - message: "enum value name 'ORDER_STATE_SHIPPED' is already used in this Protobuf package", - }); - expect(result.outputs).toEqual({}); + expectDiagnostics(diagnostics, { + code: "@typespec/protobuf/enum-value-name-collision", + message: `enum value name 'ORDER_STATE_SHIPPED' collides with ${kind} 'ORDER_STATE_SHIPPED' in this Protobuf package`, + }); + expect(result.outputs).toEqual({}); + }, + ); + + it("keeps collision validation opt-in when default names overlap across enums", async () => { + const result = await Tester.emit("@typespec/protobuf").compile(` + @package + namespace Test; + model Example { + @field(1) order: OrderState; + @field(2) payment: PaymentState; + } + enum OrderState { + Unknown: 0, + } + enum PaymentState { + Unknown: 0, + } + `); + + expect(result.outputs["main.proto"]).toContain(`enum OrderState { + Unknown = 0; +}`); + expect(result.outputs["main.proto"]).toContain(`enum PaymentState { + Unknown = 0; +}`); }); it("still requires an explicit integer on every member", async () => { diff --git a/website/src/content/docs/docs/emitters/protobuf/guide.md b/website/src/content/docs/docs/emitters/protobuf/guide.md index 4120151ecab..ce51a22337e 100644 --- a/website/src/content/docs/docs/emitters/protobuf/guide.md +++ b/website/src/content/docs/docs/emitters/protobuf/guide.md @@ -141,7 +141,7 @@ enum OrderState { } ``` -The enum name and member name are converted to `UPPER_SNAKE_CASE`, with acronyms treated as words: `HTTPStatus.HttpTimeout` becomes `HTTP_STATUS_HTTP_TIMEOUT`. Names that already start with the emitted enum prefix are preserved unchanged. Names that acquire the prefix during case conversion are not prefixed twice. The emitter reports an error if the resulting value name collides with another enum value or a declaration in the same Protobuf package. +The enum name and member name are converted to `UPPER_SNAKE_CASE`, with acronyms treated as words: `HTTPStatus.HttpTimeout` becomes `HTTP_STATUS_HTTP_TIMEOUT`. Names that already start with the emitted enum prefix are preserved unchanged. Names that acquire the prefix during case conversion are not prefixed twice. The emitter reports an error if the resulting value name collides with another enum value or a declaration in the same Protobuf package, including packages without a name. The diagnostic identifies the original conflicting enum member, or the kind and emitted name of the conflicting message, enum, or service. This option changes names only. It does not insert an `Unspecified` member or infer numbers, and an existing zero member such as `Unknown: 0` keeps its meaning. Numeric values, member order, and enum type names are preserved. Changing value names can affect generated APIs and Protobuf JSON strings, even though the numeric binary values remain unchanged.