From 11e0f56f213870ba0c330a1cb844ca9f70cca12a Mon Sep 17 00:00:00 2001 From: Timo Stamm Date: Mon, 29 Jan 2024 19:27:11 +0100 Subject: [PATCH] Fix name collision with import in generated code (#688) --- packages/protobuf-test/extra/name-clash.proto | 7 ++++ .../src/gen/js/extra/name-clash_pb.d.ts | 29 +++++++++++++ .../src/gen/js/extra/name-clash_pb.js | 13 ++++++ .../src/gen/ts/extra/name-clash_pb.ts | 42 +++++++++++++++++++ .../src/file-export-decl.test.ts | 13 ++++++ .../src/ecmascript/generated-file.ts | 27 +++++++----- 6 files changed, 121 insertions(+), 10 deletions(-) diff --git a/packages/protobuf-test/extra/name-clash.proto b/packages/protobuf-test/extra/name-clash.proto index 139b5ffc7..64bc2dc5b 100644 --- a/packages/protobuf-test/extra/name-clash.proto +++ b/packages/protobuf-test/extra/name-clash.proto @@ -15,6 +15,13 @@ syntax = "proto3"; package spec; +import "extra/example.proto"; + +// This message class will clash with the imported message class +message User { + docs.User u = 1; // Reference the import with the same name to trigger a clash +} + message ReservedPropertyNames { message BuiltIn { string constructor = 2; // built-in constructor() diff --git a/packages/protobuf-test/src/gen/js/extra/name-clash_pb.d.ts b/packages/protobuf-test/src/gen/js/extra/name-clash_pb.d.ts index 443528e70..fd1fdc7e3 100644 --- a/packages/protobuf-test/src/gen/js/extra/name-clash_pb.d.ts +++ b/packages/protobuf-test/src/gen/js/extra/name-clash_pb.d.ts @@ -18,6 +18,35 @@ import type { BinaryReadOptions, FieldList, JsonReadOptions, JsonValue, PartialMessage as PartialMessage$1, PlainMessage as PlainMessage$1 } from "@bufbuild/protobuf"; import { Message as Message$1, proto3 } from "@bufbuild/protobuf"; +import type { User as User$1 } from "./example_pb.js"; + +/** + * This message class will clash with the imported message class + * + * @generated from message spec.User + */ +export declare class User extends Message$1 { + /** + * Reference the import with the same name to trigger a clash + * + * @generated from field: docs.User u = 1; + */ + u?: User$1; + + constructor(data?: PartialMessage$1); + + static readonly runtime: typeof proto3; + static readonly typeName = "spec.User"; + static readonly fields: FieldList; + + static fromBinary(bytes: Uint8Array, options?: Partial): User; + + static fromJson(jsonValue: JsonValue, options?: Partial): User; + + static fromJsonString(jsonString: string, options?: Partial): User; + + static equals(a: User | PlainMessage$1 | undefined, b: User | PlainMessage$1 | undefined): boolean; +} /** * @generated from message spec.ReservedPropertyNames diff --git a/packages/protobuf-test/src/gen/js/extra/name-clash_pb.js b/packages/protobuf-test/src/gen/js/extra/name-clash_pb.js index e877750f5..a5ba69202 100644 --- a/packages/protobuf-test/src/gen/js/extra/name-clash_pb.js +++ b/packages/protobuf-test/src/gen/js/extra/name-clash_pb.js @@ -17,6 +17,19 @@ /* eslint-disable */ import { proto3 } from "@bufbuild/protobuf"; +import { User as User$1 } from "./example_pb.js"; + +/** + * This message class will clash with the imported message class + * + * @generated from message spec.User + */ +export const User = proto3.makeMessageType( + "spec.User", + () => [ + { no: 1, name: "u", kind: "message", T: User$1 }, + ], +); /** * @generated from message spec.ReservedPropertyNames diff --git a/packages/protobuf-test/src/gen/ts/extra/name-clash_pb.ts b/packages/protobuf-test/src/gen/ts/extra/name-clash_pb.ts index d80468294..6ddd22b9d 100644 --- a/packages/protobuf-test/src/gen/ts/extra/name-clash_pb.ts +++ b/packages/protobuf-test/src/gen/ts/extra/name-clash_pb.ts @@ -18,6 +18,48 @@ import type { BinaryReadOptions, FieldList, JsonReadOptions, JsonValue, PartialMessage as PartialMessage$1, PlainMessage as PlainMessage$1 } from "@bufbuild/protobuf"; import { Message as Message$1, proto3 } from "@bufbuild/protobuf"; +import { User as User$1 } from "./example_pb.js"; + +/** + * This message class will clash with the imported message class + * + * @generated from message spec.User + */ +export class User extends Message$1 { + /** + * Reference the import with the same name to trigger a clash + * + * @generated from field: docs.User u = 1; + */ + u?: User$1; + + constructor(data?: PartialMessage$1) { + super(); + proto3.util.initPartial(data, this); + } + + static readonly runtime: typeof proto3 = proto3; + static readonly typeName = "spec.User"; + static readonly fields: FieldList = proto3.util.newFieldList(() => [ + { no: 1, name: "u", kind: "message", T: User$1 }, + ]); + + static fromBinary(bytes: Uint8Array, options?: Partial): User { + return new User().fromBinary(bytes, options); + } + + static fromJson(jsonValue: JsonValue, options?: Partial): User { + return new User().fromJson(jsonValue, options); + } + + static fromJsonString(jsonString: string, options?: Partial): User { + return new User().fromJsonString(jsonString, options); + } + + static equals(a: User | PlainMessage$1 | undefined, b: User | PlainMessage$1 | undefined): boolean { + return proto3.util.equals(User, a, b); + } +} /** * @generated from message spec.ReservedPropertyNames diff --git a/packages/protoplugin-test/src/file-export-decl.test.ts b/packages/protoplugin-test/src/file-export-decl.test.ts index 85eae889f..a40a5df85 100644 --- a/packages/protoplugin-test/src/file-export-decl.test.ts +++ b/packages/protoplugin-test/src/file-export-decl.test.ts @@ -48,6 +48,19 @@ describe("file exportDecl", () => { expect(lines).toStrictEqual(["export const SomeEnum = 123;"]); }); + test("forces import with same name to be aliased", async () => { + const lines = await testGenerate((f) => { + f.print(f.import("Foo", "pkg")); + f.print(f.exportDecl("const", "Foo"), " = 123;"); + }); + expect(lines).toStrictEqual([ + `import { Foo as Foo$1 } from "pkg";`, + ``, + `Foo$1`, + `export const Foo = 123;`, + ]); + }); + async function testGenerate( gen: ( f: GeneratedFile, diff --git a/packages/protoplugin/src/ecmascript/generated-file.ts b/packages/protoplugin/src/ecmascript/generated-file.ts index a8e99b135..a37a536ab 100644 --- a/packages/protoplugin/src/ecmascript/generated-file.ts +++ b/packages/protoplugin/src/ecmascript/generated-file.ts @@ -431,18 +431,25 @@ function processImports( // Walk through all symbols used and populate the collections above. for (const s of el) { - if (typeof s != "object" || s.kind !== "es_symbol") { + if (typeof s != "object") { continue; } - symbolToIdentifier.set(s.id, s.name); - if (!s.typeOnly) { - // a symbol is only type-imported as long as all uses are type-only - symbolToIsValue.set(s.id, true); - } - if (s.from === importerPath) { - identifiersTaken.add(s.name); - } else { - foreignSymbols.push(s); + switch (s.kind) { + case "es_symbol": + symbolToIdentifier.set(s.id, s.name); + if (!s.typeOnly) { + // a symbol is only type-imported as long as all uses are type-only + symbolToIsValue.set(s.id, true); + } + if (s.from === importerPath) { + identifiersTaken.add(s.name); + } else { + foreignSymbols.push(s); + } + break; + case "es_export_stmt": + identifiersTaken.add(s.name); + break; } }