From 68a80d98f1faa5772862c18b6907ca2298dccc76 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 21 Jul 2022 09:54:39 +0200 Subject: [PATCH] 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 --- packages/jsii-pacmak/jest.config.mjs | 1 + .../lib/targets/go/types/go-type-reference.ts | 10 +++- .../test/targets/fixtures/base.jsii.json | 42 ++++++++++++++ .../test/targets/fixtures/dependent.jsii.json | 58 +++++++++++++++++++ packages/jsii-pacmak/test/targets/go.test.ts | 40 +++++++++++++ 5 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 packages/jsii-pacmak/test/targets/fixtures/base.jsii.json create mode 100644 packages/jsii-pacmak/test/targets/fixtures/dependent.jsii.json create mode 100644 packages/jsii-pacmak/test/targets/go.test.ts diff --git a/packages/jsii-pacmak/jest.config.mjs b/packages/jsii-pacmak/jest.config.mjs index c8a833168e..977f8c7cf9 100644 --- a/packages/jsii-pacmak/jest.config.mjs +++ b/packages/jsii-pacmak/jest.config.mjs @@ -2,4 +2,5 @@ import { overriddenConfig } from '../../jest.config.mjs'; export default overriddenConfig({ coveragePathIgnorePatterns: ['/node_modules/', '/test'], + watchPathIgnorePatterns: ['.*\\.tsx?$'], }); diff --git a/packages/jsii-pacmak/lib/targets/go/types/go-type-reference.ts b/packages/jsii-pacmak/lib/targets/go/types/go-type-reference.ts index c163bd5109..db10d16c75 100644 --- a/packages/jsii-pacmak/lib/targets/go/types/go-type-reference.ts +++ b/packages/jsii-pacmak/lib/targets/go/types/go-type-reference.ts @@ -137,9 +137,13 @@ export class GoTypeRef { break; case 'union': - for (const t of this.typeMap.value) { - ret.push(...t.dependencies); - } + // Unions ultimately result in `interface{}` being rendered, so no import is needed. We + // hence ignore them entirely here for now. In the future, we may want to inject specific + // runtime type checks around use of unions, which may result in imports being useful. + + // for (const t of this.typeMap.value) { + // ret.push(...t.dependencies); + // } break; case 'void': diff --git a/packages/jsii-pacmak/test/targets/fixtures/base.jsii.json b/packages/jsii-pacmak/test/targets/fixtures/base.jsii.json new file mode 100644 index 0000000000..67c825f94c --- /dev/null +++ b/packages/jsii-pacmak/test/targets/fixtures/base.jsii.json @@ -0,0 +1,42 @@ +{ + "author": { + "email": "aws-jsii@amazon.com", + "name": "Amazon Web Services, Inc.", + "organization": true, + "roles": [ + "owner" + ] + }, + "description": "A dummy test package", + "fingerprint": "PHONY", + "homepage": "https://aws.github.io/jsii", + "jsiiVersion": "0.0.0-dev", + "license": "Apache-2.0", + "name": "base", + "repository": { + "directory": "packages/jsii-pacmak/test/targets/fixtures", + "type": "git", + "url": "https://github.com/aws/jsii" + }, + "schema": "jsii/0.10.0", + "targets": { + "go": { + "moduleName": "github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures" + } + }, + "types": { + "BaseA": { + "assembly": "base", + "fqn": "base.BaseA", + "kind": "interface", + "name": "BaseA" + }, + "BaseB": { + "assembly": "base", + "fqn": "base.BaseB", + "kind": "interface", + "name": "BaseB" + } + }, + "version": "1.2.3" +} diff --git a/packages/jsii-pacmak/test/targets/fixtures/dependent.jsii.json b/packages/jsii-pacmak/test/targets/fixtures/dependent.jsii.json new file mode 100644 index 0000000000..3704752df2 --- /dev/null +++ b/packages/jsii-pacmak/test/targets/fixtures/dependent.jsii.json @@ -0,0 +1,58 @@ +{ + "author": { + "email": "aws-jsii@amazon.com", + "name": "Amazon Web Services, Inc.", + "organization": true, + "roles": [ + "owner" + ] + }, + "dependencies": { + "base": "1.2.3" + }, + "description": "A dummy test package", + "fingerprint": "PHONY", + "homepage": "https://aws.github.io/jsii", + "jsiiVersion": "0.0.0-dev", + "license": "Apache-2.0", + "name": "dependent", + "repository": { + "directory": "packages/jsii-pacmak/test/targets/fixtures", + "type": "git", + "url": "https://github.com/aws/jsii" + }, + "schema": "jsii/0.10.0", + "targets": { + "go": { + "moduleName": "github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures" + } + }, + "types": { + "Dependent": { + "assembly": "dependent", + "fqn": "dependent.Dependent", + "kind": "class", + "name": "dependent", + "methods": [ + { + "name": "getBaseUnion", + "returns": { + "type": { + "union": { + "types": [ + { + "fqn": "base.BaseA" + }, + { + "fqn": "base.BaseB" + } + ] + } + } + } + } + ] + } + }, + "version": "4.5.6" +} diff --git a/packages/jsii-pacmak/test/targets/go.test.ts b/packages/jsii-pacmak/test/targets/go.test.ts new file mode 100644 index 0000000000..07c39eeeaa --- /dev/null +++ b/packages/jsii-pacmak/test/targets/go.test.ts @@ -0,0 +1,40 @@ +import { promises as fs } from 'fs'; +import { TypeSystem } from 'jsii-reflect'; +import { Rosetta } from 'jsii-rosetta'; +import { tmpdir } from 'os'; +import { join } from 'path'; + +import { Golang } from '../../lib/targets/go'; + +test('does not generate imports for unused types', async () => { + const outDir = await fs.mkdtemp(join(tmpdir(), 'pacmak-golang-')); + try { + const tarball = join(outDir, 'mock-tarball.tgz'); + await fs.writeFile(tarball, 'Mock Tarball'); + + const typeSystem = new TypeSystem(); + await typeSystem.load(require.resolve('./fixtures/base.jsii.json')); + const assembly = await typeSystem.load( + require.resolve('./fixtures/dependent.jsii.json'), + ); + + const rosetta = new Rosetta(); + const subject = new Golang({ + arguments: {}, + assembly, + packageDir: '', + rosetta, + targetName: 'golang', + }); + + await subject.generateCode(outDir, tarball); + + await expect( + fs.readFile(join(outDir, assembly.name, `${assembly.name}.go`), 'utf-8'), + ).resolves.not.toContain( + 'github.com/aws/jsii/packages/jsii-pacmak/test/targets/fixtures/base', + ); + } finally { + await fs.rm(outDir, { recursive: true }); + } +});