Skip to content

Commit 68a80d9

Browse files
authored
fix(go): unused imports emitted for type unions (#3664)
When imported types are solely referenced by type unions, a go import is emitted, but is never used (type unions end up represented as opaque `interface{}` type). This causes compilation failures. Added a test case for this scenario in particular, and adjusted go code generation to not emit dependency imports for type unions. These imports may be re-introduced soon, as we are working to add dynamic type checking around type unions in go (at which point those imports would no longer be unused). Fixes #3399
1 parent 3abe20f commit 68a80d9

File tree

5 files changed

+148
-3
lines changed

5 files changed

+148
-3
lines changed

Diff for: packages/jsii-pacmak/jest.config.mjs

+1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ import { overriddenConfig } from '../../jest.config.mjs';
22

33
export default overriddenConfig({
44
coveragePathIgnorePatterns: ['/node_modules/', '<rootDir>/test'],
5+
watchPathIgnorePatterns: ['.*\\.tsx?$'],
56
});

Diff for: packages/jsii-pacmak/lib/targets/go/types/go-type-reference.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,13 @@ export class GoTypeRef {
137137
break;
138138

139139
case 'union':
140-
for (const t of this.typeMap.value) {
141-
ret.push(...t.dependencies);
142-
}
140+
// Unions ultimately result in `interface{}` being rendered, so no import is needed. We
141+
// hence ignore them entirely here for now. In the future, we may want to inject specific
142+
// runtime type checks around use of unions, which may result in imports being useful.
143+
144+
// for (const t of this.typeMap.value) {
145+
// ret.push(...t.dependencies);
146+
// }
143147
break;
144148

145149
case 'void':
+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
{
2+
"author": {
3+
"email": "[email protected]",
4+
"name": "Amazon Web Services, Inc.",
5+
"organization": true,
6+
"roles": [
7+
"owner"
8+
]
9+
},
10+
"description": "A dummy test package",
11+
"fingerprint": "PHONY",
12+
"homepage": "https://aws.github.io/jsii",
13+
"jsiiVersion": "0.0.0-dev",
14+
"license": "Apache-2.0",
15+
"name": "base",
16+
"repository": {
17+
"directory": "packages/jsii-pacmak/test/targets/fixtures",
18+
"type": "git",
19+
"url": "https://github.com/aws/jsii"
20+
},
21+
"schema": "jsii/0.10.0",
22+
"targets": {
23+
"go": {
24+
"moduleName": "github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures"
25+
}
26+
},
27+
"types": {
28+
"BaseA": {
29+
"assembly": "base",
30+
"fqn": "base.BaseA",
31+
"kind": "interface",
32+
"name": "BaseA"
33+
},
34+
"BaseB": {
35+
"assembly": "base",
36+
"fqn": "base.BaseB",
37+
"kind": "interface",
38+
"name": "BaseB"
39+
}
40+
},
41+
"version": "1.2.3"
42+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
{
2+
"author": {
3+
"email": "[email protected]",
4+
"name": "Amazon Web Services, Inc.",
5+
"organization": true,
6+
"roles": [
7+
"owner"
8+
]
9+
},
10+
"dependencies": {
11+
"base": "1.2.3"
12+
},
13+
"description": "A dummy test package",
14+
"fingerprint": "PHONY",
15+
"homepage": "https://aws.github.io/jsii",
16+
"jsiiVersion": "0.0.0-dev",
17+
"license": "Apache-2.0",
18+
"name": "dependent",
19+
"repository": {
20+
"directory": "packages/jsii-pacmak/test/targets/fixtures",
21+
"type": "git",
22+
"url": "https://github.com/aws/jsii"
23+
},
24+
"schema": "jsii/0.10.0",
25+
"targets": {
26+
"go": {
27+
"moduleName": "github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures"
28+
}
29+
},
30+
"types": {
31+
"Dependent": {
32+
"assembly": "dependent",
33+
"fqn": "dependent.Dependent",
34+
"kind": "class",
35+
"name": "dependent",
36+
"methods": [
37+
{
38+
"name": "getBaseUnion",
39+
"returns": {
40+
"type": {
41+
"union": {
42+
"types": [
43+
{
44+
"fqn": "base.BaseA"
45+
},
46+
{
47+
"fqn": "base.BaseB"
48+
}
49+
]
50+
}
51+
}
52+
}
53+
}
54+
]
55+
}
56+
},
57+
"version": "4.5.6"
58+
}

Diff for: packages/jsii-pacmak/test/targets/go.test.ts

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { promises as fs } from 'fs';
2+
import { TypeSystem } from 'jsii-reflect';
3+
import { Rosetta } from 'jsii-rosetta';
4+
import { tmpdir } from 'os';
5+
import { join } from 'path';
6+
7+
import { Golang } from '../../lib/targets/go';
8+
9+
test('does not generate imports for unused types', async () => {
10+
const outDir = await fs.mkdtemp(join(tmpdir(), 'pacmak-golang-'));
11+
try {
12+
const tarball = join(outDir, 'mock-tarball.tgz');
13+
await fs.writeFile(tarball, 'Mock Tarball');
14+
15+
const typeSystem = new TypeSystem();
16+
await typeSystem.load(require.resolve('./fixtures/base.jsii.json'));
17+
const assembly = await typeSystem.load(
18+
require.resolve('./fixtures/dependent.jsii.json'),
19+
);
20+
21+
const rosetta = new Rosetta();
22+
const subject = new Golang({
23+
arguments: {},
24+
assembly,
25+
packageDir: '',
26+
rosetta,
27+
targetName: 'golang',
28+
});
29+
30+
await subject.generateCode(outDir, tarball);
31+
32+
await expect(
33+
fs.readFile(join(outDir, assembly.name, `${assembly.name}.go`), 'utf-8'),
34+
).resolves.not.toContain(
35+
'github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures/base',
36+
);
37+
} finally {
38+
await fs.rm(outDir, { recursive: true });
39+
}
40+
});

0 commit comments

Comments
 (0)