Skip to content

Commit

Permalink
Fix name collision with import in generated code (#688)
Browse files Browse the repository at this point in the history
  • Loading branch information
timostamm authored Jan 29, 2024
1 parent c912f62 commit 11e0f56
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 10 deletions.
7 changes: 7 additions & 0 deletions packages/protobuf-test/extra/name-clash.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
29 changes: 29 additions & 0 deletions packages/protobuf-test/src/gen/js/extra/name-clash_pb.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions packages/protobuf-test/src/gen/js/extra/name-clash_pb.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 42 additions & 0 deletions packages/protobuf-test/src/gen/ts/extra/name-clash_pb.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions packages/protoplugin-test/src/file-export-decl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 17 additions & 10 deletions packages/protoplugin/src/ecmascript/generated-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down

0 comments on commit 11e0f56

Please sign in to comment.