From 745a644a25788184a1da7cbe0b82b3202a21a97d Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 27 Nov 2019 15:33:46 +0200 Subject: [PATCH 1/6] feat(jsii): switch to disable reserved words warnings By default, jsii issues a warning when it encounters a symbol that uses a word that is reserved in one of the supported languages. Practically this creates a tremendous amount of noise for large existing projects. #1073 proposes to add support for a warning exclusion file, which is probably a better long term solution, but in the meantime, this switch allows opting-out of reserved words warnings in order to save the planet. Tested by adding `--disable-reserved-words-warnings` to `jsii-calc` (which uses many reserved words already). --- packages/jsii-calc/package.json | 2 +- packages/jsii/bin/jsii.ts | 7 ++++++- packages/jsii/lib/assembler.ts | 18 +++++++++++++++++- packages/jsii/lib/compiler.ts | 9 ++++++++- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/jsii-calc/package.json b/packages/jsii-calc/package.json index dc02899082..ac58259e12 100644 --- a/packages/jsii-calc/package.json +++ b/packages/jsii-calc/package.json @@ -25,7 +25,7 @@ "main": "lib/index.js", "types": "lib/index.d.ts", "scripts": { - "build": "jsii && jsii-rosetta --compile", + "build": "jsii --disable-reserved-words-warnings && jsii-rosetta --compile", "watch": "jsii -w", "test": "node test/test.calc.js && diff-test test/assembly.jsii .jsii", "test:update": "npm run build && UPDATE_DIFF=1 npm run test" diff --git a/packages/jsii/bin/jsii.ts b/packages/jsii/bin/jsii.ts index 22c5387ba4..aa05f93367 100644 --- a/packages/jsii/bin/jsii.ts +++ b/packages/jsii/bin/jsii.ts @@ -27,6 +27,10 @@ import { VERSION } from '../lib/version'; type: 'boolean', desc: 'Treat warnings as errors' }) + .option('disable-reserved-words-warnings', { + type: 'boolean', + desc: 'Do not emit warnings for symbols that are reserved words in one of the supported languages', + }) .help() .version(`${VERSION}, typescript ${require('typescript/package.json').version}`) .argv; @@ -41,7 +45,8 @@ import { VERSION } from '../lib/version'; projectInfo, watch: argv.watch, projectReferences: argv['project-references'], - failOnWarnings: argv['fail-on-warnings'] + failOnWarnings: argv['fail-on-warnings'], + reservedWordsWarningsDisabled: argv['disable-reserved-words-warnings'] }); return { projectRoot, emitResult: await compiler.emit() }; diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index ff76eb2ab8..de23b76ff3 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -22,6 +22,14 @@ const sortJson = require('sort-json'); const LOG = log4js.getLogger('jsii/assembler'); +export interface AssemblerOptions { + /** + * Do not emit warnings for reserved words. + * @default false (warnings are emitted) + */ + readonly reservedWordsWarningsDisabled?: boolean; +} + /** * The JSII Assembler consumes a ``ts.Program`` instance and emits a JSII assembly. */ @@ -29,6 +37,7 @@ export class Assembler implements Emitter { private _diagnostics = new Array(); private _deferred = new Array(); private _types: { [fqn: string]: spec.Type } = {}; + private readonly _reservedWordsWarningsDisabled: boolean = false; /** * @param projectInfo information about the package being assembled @@ -38,7 +47,11 @@ export class Assembler implements Emitter { public constructor( public readonly projectInfo: ProjectInfo, public readonly program: ts.Program, - public readonly stdlib: string) { } + public readonly stdlib: string, + options: AssemblerOptions) { + + this._reservedWordsWarningsDisabled = !!options.reservedWordsWarningsDisabled; + } private get _typeChecker(): ts.TypeChecker { return this.program.getTypeChecker(); @@ -1026,6 +1039,9 @@ export class Assembler implements Emitter { } private _warnAboutReservedWords(symbol: ts.Symbol) { + if (this._reservedWordsWarningsDisabled) { + return; + } const reservingLanguages = isReservedName(symbol.name); if (reservingLanguages) { this._diagnostic(ts.getNameOfDeclaration(symbol.valueDeclaration) || symbol.valueDeclaration, diff --git a/packages/jsii/lib/compiler.ts b/packages/jsii/lib/compiler.ts index 331e3bcbd7..f06ea52083 100644 --- a/packages/jsii/lib/compiler.ts +++ b/packages/jsii/lib/compiler.ts @@ -46,6 +46,11 @@ export interface CompilerOptions { projectReferences?: boolean; /** Whether to fail when a warning is emitted */ failOnWarnings?: boolean; + /** + * Do not emit warnings for reserved words. + * @default false (warnings are emitted) + */ + readonly reservedWordsWarningsDisabled?: boolean; } export interface TypescriptConfig { @@ -159,7 +164,9 @@ export class Compiler implements Emitter { // jsii warnings will appear. However, the Assembler might throw an exception // because broken/missing type information might lead it to fail completely. try { - const assembler = new Assembler(this.options.projectInfo, program, stdlib); + const assembler = new Assembler(this.options.projectInfo, program, stdlib, { + reservedWordsWarningsDisabled: this.options.reservedWordsWarningsDisabled + }); const assmEmit = await assembler.emit(); if (assmEmit.emitSkipped) { LOG.error('Type model errors prevented the JSII assembly from being created'); From ed609359bb02931a74ac4a46d0b5112cdbe1ad4a Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 17 Dec 2019 15:31:10 +0200 Subject: [PATCH 2/6] change boolean switch to --silence-warnings array --- packages/jsii-calc/package.json | 2 +- packages/jsii/bin/jsii.ts | 20 ++++++++++++++++---- packages/jsii/lib/assembler.ts | 21 +++++---------------- packages/jsii/lib/compiler.ts | 9 +-------- packages/jsii/lib/warnings.ts | 7 +++++++ 5 files changed, 30 insertions(+), 29 deletions(-) create mode 100644 packages/jsii/lib/warnings.ts diff --git a/packages/jsii-calc/package.json b/packages/jsii-calc/package.json index b64eecc70e..9f630d0657 100644 --- a/packages/jsii-calc/package.json +++ b/packages/jsii-calc/package.json @@ -30,7 +30,7 @@ "main": "lib/index.js", "types": "lib/index.d.ts", "scripts": { - "build": "jsii --disable-reserved-words-warnings && jsii-rosetta --compile", + "build": "jsii --silence-warnings reserved-word && jsii-rosetta --compile", "watch": "jsii -w", "test": "node test/test.calc.js && diff-test test/assembly.jsii .jsii", "test:update": "npm run build && UPDATE_DIFF=1 npm run test" diff --git a/packages/jsii/bin/jsii.ts b/packages/jsii/bin/jsii.ts index cd5ae43dd3..c06176548d 100644 --- a/packages/jsii/bin/jsii.ts +++ b/packages/jsii/bin/jsii.ts @@ -6,6 +6,9 @@ import { Compiler, DIAGNOSTICS } from '../lib/compiler'; import { loadProjectInfo } from '../lib/project-info'; import * as utils from '../lib/utils'; import { VERSION } from '../lib/version'; +import { enabledWarnings } from '../lib/warnings'; + +const warningTypes = Object.keys(enabledWarnings); (async () => { const argv = yargs @@ -27,9 +30,10 @@ import { VERSION } from '../lib/version'; type: 'boolean', desc: 'Treat warnings as errors' }) - .option('disable-reserved-words-warnings', { - type: 'boolean', - desc: 'Do not emit warnings for symbols that are reserved words in one of the supported languages', + .option('silence-warnings', { + type: 'array', + default: [], + desc: `List of warnings to silence (warnings: ${warningTypes})`, }) .help() // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -42,12 +46,20 @@ import { VERSION } from '../lib/version'; const projectInfo = await loadProjectInfo(projectRoot, { fixPeerDependencies: argv['fix-peer-dependencies'] }); + // disable all silenced warnings + for (const key of argv['silence-warnings']) { + if (!(key in enabledWarnings)) { + throw new Error(`Unknown warning type ${key}. Must be one of: ${warningTypes}`); + } + + (enabledWarnings as any)[key] = false; + } + const compiler = new Compiler({ projectInfo, watch: argv.watch, projectReferences: argv['project-references'], failOnWarnings: argv['fail-on-warnings'], - reservedWordsWarningsDisabled: argv['disable-reserved-words-warnings'] }); return { projectRoot, emitResult: await compiler.emit() }; diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 232cef2ab2..67facf3344 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -16,20 +16,13 @@ import { ProjectInfo } from './project-info'; import { isReservedName } from './reserved-words'; import { Validator } from './validator'; import { SHORT_VERSION, VERSION } from './version'; +import { enabledWarnings } from './warnings'; // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports const sortJson = require('sort-json'); const LOG = log4js.getLogger('jsii/assembler'); -export interface AssemblerOptions { - /** - * Do not emit warnings for reserved words. - * @default false (warnings are emitted) - */ - readonly reservedWordsWarningsDisabled?: boolean; -} - /** * The JSII Assembler consumes a ``ts.Program`` instance and emits a JSII assembly. */ @@ -37,7 +30,6 @@ export class Assembler implements Emitter { private _diagnostics = new Array(); private _deferred = new Array(); private _types: { [fqn: string]: spec.Type } = {}; - private readonly _reservedWordsWarningsDisabled: boolean = false; /** * @param projectInfo information about the package being assembled @@ -47,11 +39,7 @@ export class Assembler implements Emitter { public constructor( public readonly projectInfo: ProjectInfo, public readonly program: ts.Program, - public readonly stdlib: string, - options: AssemblerOptions) { - - this._reservedWordsWarningsDisabled = !!options.reservedWordsWarningsDisabled; - } + public readonly stdlib: string) { } private get _typeChecker(): ts.TypeChecker { return this.program.getTypeChecker(); @@ -221,7 +209,7 @@ export class Assembler implements Emitter { * been executed. * * @param fqn FQN of the current type. - * @param deps List of FQNs of types this callback depends on. All deferreds for all + * @param dependedFqns List of FQNs of types this callback depends on. All deferreds for all * @param cb the function to be called in a deferred way. It will be bound with ``this``, so it can depend on using * ``this``. */ @@ -1037,9 +1025,10 @@ export class Assembler implements Emitter { } private _warnAboutReservedWords(symbol: ts.Symbol) { - if (this._reservedWordsWarningsDisabled) { + if (!enabledWarnings['reserved-word']) { return; } + const reservingLanguages = isReservedName(symbol.name); if (reservingLanguages) { this._diagnostic(ts.getNameOfDeclaration(symbol.valueDeclaration) || symbol.valueDeclaration, diff --git a/packages/jsii/lib/compiler.ts b/packages/jsii/lib/compiler.ts index c6d305b589..51131b675e 100644 --- a/packages/jsii/lib/compiler.ts +++ b/packages/jsii/lib/compiler.ts @@ -46,11 +46,6 @@ export interface CompilerOptions { projectReferences?: boolean; /** Whether to fail when a warning is emitted */ failOnWarnings?: boolean; - /** - * Do not emit warnings for reserved words. - * @default false (warnings are emitted) - */ - readonly reservedWordsWarningsDisabled?: boolean; } export interface TypescriptConfig { @@ -164,9 +159,7 @@ export class Compiler implements Emitter { // jsii warnings will appear. However, the Assembler might throw an exception // because broken/missing type information might lead it to fail completely. try { - const assembler = new Assembler(this.options.projectInfo, program, stdlib, { - reservedWordsWarningsDisabled: this.options.reservedWordsWarningsDisabled - }); + const assembler = new Assembler(this.options.projectInfo, program, stdlib); const assmEmit = await assembler.emit(); if (assmEmit.emitSkipped) { LOG.error('Type model errors prevented the JSII assembly from being created'); diff --git a/packages/jsii/lib/warnings.ts b/packages/jsii/lib/warnings.ts new file mode 100644 index 0000000000..f9795760cf --- /dev/null +++ b/packages/jsii/lib/warnings.ts @@ -0,0 +1,7 @@ +/** + * Indicates which warnings are currently enabled. By default all warnings are + * enabled, and can be silenced through the --silence-warning option. + */ +export const enabledWarnings = { + 'reserved-word': true +}; \ No newline at end of file From c66c8fe86af56f2bb1992e0bef4d64c80e1a6ab2 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 17 Dec 2019 16:26:26 +0200 Subject: [PATCH 3/6] Update jsii.ts --- packages/jsii/bin/jsii.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsii/bin/jsii.ts b/packages/jsii/bin/jsii.ts index c06176548d..b7f8c9295e 100644 --- a/packages/jsii/bin/jsii.ts +++ b/packages/jsii/bin/jsii.ts @@ -33,7 +33,7 @@ const warningTypes = Object.keys(enabledWarnings); .option('silence-warnings', { type: 'array', default: [], - desc: `List of warnings to silence (warnings: ${warningTypes})`, + desc: `List of warnings to silence (warnings: ${warningTypes.join(',')})`, }) .help() // eslint-disable-next-line @typescript-eslint/no-require-imports From 856aa52b8847dcc65675618f0e06e27f335d4a0e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 18 Dec 2019 14:11:32 +0200 Subject: [PATCH 4/6] Update packages/jsii/lib/warnings.ts Co-Authored-By: Romain Marcadier-Muller --- packages/jsii/lib/warnings.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jsii/lib/warnings.ts b/packages/jsii/lib/warnings.ts index f9795760cf..6e9748e26d 100644 --- a/packages/jsii/lib/warnings.ts +++ b/packages/jsii/lib/warnings.ts @@ -2,6 +2,6 @@ * Indicates which warnings are currently enabled. By default all warnings are * enabled, and can be silenced through the --silence-warning option. */ -export const enabledWarnings = { +export const enabledWarnings: { [name]: boolean } = { 'reserved-word': true -}; \ No newline at end of file +}; From 033a3c174ba511db59771f5f3dbf96efd7a7fe66 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 18 Dec 2019 14:12:10 +0200 Subject: [PATCH 5/6] Update jsii.ts --- packages/jsii/bin/jsii.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsii/bin/jsii.ts b/packages/jsii/bin/jsii.ts index b7f8c9295e..470893b082 100644 --- a/packages/jsii/bin/jsii.ts +++ b/packages/jsii/bin/jsii.ts @@ -52,7 +52,7 @@ const warningTypes = Object.keys(enabledWarnings); throw new Error(`Unknown warning type ${key}. Must be one of: ${warningTypes}`); } - (enabledWarnings as any)[key] = false; + enabledWarnings[key] = false; } const compiler = new Compiler({ From 93cc2ffad12fa69df649902f05b68781aea5a822 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 18 Dec 2019 14:12:32 +0200 Subject: [PATCH 6/6] Update warnings.ts --- packages/jsii/lib/warnings.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jsii/lib/warnings.ts b/packages/jsii/lib/warnings.ts index 6e9748e26d..392dc73a1f 100644 --- a/packages/jsii/lib/warnings.ts +++ b/packages/jsii/lib/warnings.ts @@ -2,6 +2,6 @@ * Indicates which warnings are currently enabled. By default all warnings are * enabled, and can be silenced through the --silence-warning option. */ -export const enabledWarnings: { [name]: boolean } = { +export const enabledWarnings: { [name: string]: boolean } = { 'reserved-word': true };