From a91331c96d95a7abc0523122f5160bf64795bf6b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 15 Aug 2019 17:21:20 +0200 Subject: [PATCH 1/7] trivial first cut --- build/lib/tslint/noUnsafeGlobalsRule.js | 32 +++++++++++++++++++ build/lib/tslint/noUnsafeGlobalsRule.ts | 42 +++++++++++++++++++++++++ tslint.json | 21 +++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 build/lib/tslint/noUnsafeGlobalsRule.js create mode 100644 build/lib/tslint/noUnsafeGlobalsRule.ts diff --git a/build/lib/tslint/noUnsafeGlobalsRule.js b/build/lib/tslint/noUnsafeGlobalsRule.js new file mode 100644 index 0000000000000..caf56d35d0948 --- /dev/null +++ b/build/lib/tslint/noUnsafeGlobalsRule.js @@ -0,0 +1,32 @@ +"use strict"; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +Object.defineProperty(exports, "__esModule", { value: true }); +const Lint = require("tslint"); +const minimatch = require("minimatch"); +class Rule extends Lint.Rules.AbstractRule { + apply(sourceFile) { + const configs = this.getOptions().ruleArguments; + for (const config of configs) { + if (minimatch(sourceFile.fileName, config.target)) { + return this.applyWithWalker(new NoUnsafeGlobalsRuleWalker(sourceFile, this.getOptions(), config)); + } + } + return []; + } +} +exports.Rule = Rule; +class NoUnsafeGlobalsRuleWalker extends Lint.RuleWalker { + constructor(file, opts, _config) { + super(file, opts); + this._config = _config; + } + visitIdentifier(node) { + if (this._config.unsafe.some(unsafe => unsafe === node.text)) { + this.addFailureAtNode(node, `Unsafe global usage of ${node.text} in ${this._config.target}`); + } + super.visitIdentifier(node); + } +} diff --git a/build/lib/tslint/noUnsafeGlobalsRule.ts b/build/lib/tslint/noUnsafeGlobalsRule.ts new file mode 100644 index 0000000000000..ae730309c632d --- /dev/null +++ b/build/lib/tslint/noUnsafeGlobalsRule.ts @@ -0,0 +1,42 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; + +interface NoUnsafeGlobalsConfig { + target: string; + unsafe: string[]; +} + +export class Rule extends Lint.Rules.AbstractRule { + apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + const configs = this.getOptions().ruleArguments; + + for (const config of configs) { + if (minimatch(sourceFile.fileName, config.target)) { + return this.applyWithWalker(new NoUnsafeGlobalsRuleWalker(sourceFile, this.getOptions(), config)); + } + } + + return []; + } +} + +class NoUnsafeGlobalsRuleWalker extends Lint.RuleWalker { + + constructor(file: ts.SourceFile, opts: Lint.IOptions, private _config: NoUnsafeGlobalsConfig) { + super(file, opts); + } + + visitIdentifier(node: ts.Identifier) { + if (this._config.unsafe.some(unsafe => unsafe === node.text)) { + this.addFailureAtNode(node, `Unsafe global usage of ${node.text} in ${this._config.target}`); + } + + super.visitIdentifier(node); + } +} diff --git a/tslint.json b/tslint.json index 4b23918f0fcdf..4d2a6da33b726 100644 --- a/tslint.json +++ b/tslint.json @@ -624,6 +624,27 @@ "restrictions": "**/vs/**" } ], + "no-unsafe-globals": [ + true, + { + "target": "**/vs/base/common/{path,process,platform}.ts", + "unsafe": [] + }, + { + "target": "**/vs/**/{common,browser}/**", + "unsafe": [ + "Buffer", + "process", + "__filename", + "__dirname", + "clearImmediate", + "exports", + "global", + "module", + "setImmediate" + ] + } + ], "duplicate-imports": true, "no-new-buffer": true, "translation-remind": true, From 8488f763aaa6ce9685cc2d8347220c12c6bab8f6 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Thu, 15 Aug 2019 17:27:34 +0200 Subject: [PATCH 2/7] document where globals are from --- tslint.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tslint.json b/tslint.json index 4d2a6da33b726..859a9663226d3 100644 --- a/tslint.json +++ b/tslint.json @@ -633,6 +633,7 @@ { "target": "**/vs/**/{common,browser}/**", "unsafe": [ + // https://nodejs.org/api/globals.html#globals_global_objects "Buffer", "process", "__filename", From ef152eac8b670cd31cdd4d8842e0b9e109376795 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 17 Aug 2019 13:32:09 +0200 Subject: [PATCH 3/7] improve rule detection --- build/lib/tslint/noNodeJSGlobalsRule.js | 64 +++++++++++++++++++++ build/lib/tslint/noNodeJSGlobalsRule.ts | 76 +++++++++++++++++++++++++ build/lib/tslint/noUnsafeGlobalsRule.js | 32 ----------- build/lib/tslint/noUnsafeGlobalsRule.ts | 42 -------------- tslint.json | 30 ++++++---- 5 files changed, 158 insertions(+), 86 deletions(-) create mode 100644 build/lib/tslint/noNodeJSGlobalsRule.js create mode 100644 build/lib/tslint/noNodeJSGlobalsRule.ts delete mode 100644 build/lib/tslint/noUnsafeGlobalsRule.js delete mode 100644 build/lib/tslint/noUnsafeGlobalsRule.ts diff --git a/build/lib/tslint/noNodeJSGlobalsRule.js b/build/lib/tslint/noNodeJSGlobalsRule.js new file mode 100644 index 0000000000000..7dd5856602bc5 --- /dev/null +++ b/build/lib/tslint/noNodeJSGlobalsRule.js @@ -0,0 +1,64 @@ +"use strict"; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +Object.defineProperty(exports, "__esModule", { value: true }); +const Lint = require("tslint"); +const minimatch = require("minimatch"); +// https://nodejs.org/api/globals.html#globals_global_objects +const nodeJSGlobals = [ + "Buffer", + "__dirname", + "__filename", + "clearImmediate", + "exports", + "global", + "module", + "process", + "setImmediate" +]; +class Rule extends Lint.Rules.TypedRule { + applyWithProgram(sourceFile, program) { + const configs = this.getOptions().ruleArguments; + for (const config of configs) { + if (minimatch(sourceFile.fileName, config.target)) { + return this.applyWithWalker(new NoNodejsGlobalsRuleWalker(sourceFile, program, this.getOptions(), config)); + } + } + return []; + } +} +exports.Rule = Rule; +class NoNodejsGlobalsRuleWalker extends Lint.RuleWalker { + constructor(file, program, opts, _config) { + super(file, opts); + this.program = program; + this._config = _config; + } + visitIdentifier(node) { + if (nodeJSGlobals.some(nodeJSGlobal => nodeJSGlobal === node.text)) { + if (this._config.allowed && this._config.allowed.some(allowed => allowed === node.text)) { + return; // override + } + const checker = this.program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const valueDeclaration = symbol.valueDeclaration; + if (valueDeclaration) { + const parent = valueDeclaration.parent; + if (parent) { + const sourceFile = parent.getSourceFile(); + if (sourceFile) { + const fileName = sourceFile.fileName; + if (fileName && fileName.indexOf('@types/node') >= 0) { + this.addFailureAtNode(node, `Cannot use node.js global '${node.text}' in '${this._config.target}'`); + } + } + } + } + } + } + super.visitIdentifier(node); + } +} diff --git a/build/lib/tslint/noNodeJSGlobalsRule.ts b/build/lib/tslint/noNodeJSGlobalsRule.ts new file mode 100644 index 0000000000000..b32700af3ea05 --- /dev/null +++ b/build/lib/tslint/noNodeJSGlobalsRule.ts @@ -0,0 +1,76 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; + +interface NoNodejsGlobalsConfig { + target: string; + allowed: string[]; +} + +// https://nodejs.org/api/globals.html#globals_global_objects +const nodeJSGlobals = [ + "Buffer", + "__dirname", + "__filename", + "clearImmediate", + "exports", + "global", + "module", + "process", + "setImmediate" +]; + +export class Rule extends Lint.Rules.TypedRule { + + applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const configs = this.getOptions().ruleArguments; + + for (const config of configs) { + if (minimatch(sourceFile.fileName, config.target)) { + return this.applyWithWalker(new NoNodejsGlobalsRuleWalker(sourceFile, program, this.getOptions(), config)); + } + } + + return []; + } +} + +class NoNodejsGlobalsRuleWalker extends Lint.RuleWalker { + + constructor(file: ts.SourceFile, private program: ts.Program, opts: Lint.IOptions, private _config: NoNodejsGlobalsConfig) { + super(file, opts); + } + + visitIdentifier(node: ts.Identifier) { + if (nodeJSGlobals.some(nodeJSGlobal => nodeJSGlobal === node.text)) { + if (this._config.allowed && this._config.allowed.some(allowed => allowed === node.text)) { + return; // override + } + + const checker = this.program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const valueDeclaration = symbol.valueDeclaration; + if (valueDeclaration) { + const parent = valueDeclaration.parent; + if (parent) { + const sourceFile = parent.getSourceFile(); + if (sourceFile) { + const fileName = sourceFile.fileName; + if (fileName && fileName.indexOf('@types/node') >= 0) { + this.addFailureAtNode(node, `Cannot use node.js global '${node.text}' in '${this._config.target}'`); + } + } + } + } + } + } + + super.visitIdentifier(node); + } +} diff --git a/build/lib/tslint/noUnsafeGlobalsRule.js b/build/lib/tslint/noUnsafeGlobalsRule.js deleted file mode 100644 index caf56d35d0948..0000000000000 --- a/build/lib/tslint/noUnsafeGlobalsRule.js +++ /dev/null @@ -1,32 +0,0 @@ -"use strict"; -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ -Object.defineProperty(exports, "__esModule", { value: true }); -const Lint = require("tslint"); -const minimatch = require("minimatch"); -class Rule extends Lint.Rules.AbstractRule { - apply(sourceFile) { - const configs = this.getOptions().ruleArguments; - for (const config of configs) { - if (minimatch(sourceFile.fileName, config.target)) { - return this.applyWithWalker(new NoUnsafeGlobalsRuleWalker(sourceFile, this.getOptions(), config)); - } - } - return []; - } -} -exports.Rule = Rule; -class NoUnsafeGlobalsRuleWalker extends Lint.RuleWalker { - constructor(file, opts, _config) { - super(file, opts); - this._config = _config; - } - visitIdentifier(node) { - if (this._config.unsafe.some(unsafe => unsafe === node.text)) { - this.addFailureAtNode(node, `Unsafe global usage of ${node.text} in ${this._config.target}`); - } - super.visitIdentifier(node); - } -} diff --git a/build/lib/tslint/noUnsafeGlobalsRule.ts b/build/lib/tslint/noUnsafeGlobalsRule.ts deleted file mode 100644 index ae730309c632d..0000000000000 --- a/build/lib/tslint/noUnsafeGlobalsRule.ts +++ /dev/null @@ -1,42 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as ts from 'typescript'; -import * as Lint from 'tslint'; -import * as minimatch from 'minimatch'; - -interface NoUnsafeGlobalsConfig { - target: string; - unsafe: string[]; -} - -export class Rule extends Lint.Rules.AbstractRule { - apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const configs = this.getOptions().ruleArguments; - - for (const config of configs) { - if (minimatch(sourceFile.fileName, config.target)) { - return this.applyWithWalker(new NoUnsafeGlobalsRuleWalker(sourceFile, this.getOptions(), config)); - } - } - - return []; - } -} - -class NoUnsafeGlobalsRuleWalker extends Lint.RuleWalker { - - constructor(file: ts.SourceFile, opts: Lint.IOptions, private _config: NoUnsafeGlobalsConfig) { - super(file, opts); - } - - visitIdentifier(node: ts.Identifier) { - if (this._config.unsafe.some(unsafe => unsafe === node.text)) { - this.addFailureAtNode(node, `Unsafe global usage of ${node.text} in ${this._config.target}`); - } - - super.visitIdentifier(node); - } -} diff --git a/tslint.json b/tslint.json index 859a9663226d3..8d3caae57e3d1 100644 --- a/tslint.json +++ b/tslint.json @@ -624,26 +624,32 @@ "restrictions": "**/vs/**" } ], - "no-unsafe-globals": [ + "no-nodejs-globals": [ true, { "target": "**/vs/base/common/{path,process,platform}.ts", - "unsafe": [] + "allowed": [ + "process" + ] }, { - "target": "**/vs/**/{common,browser}/**", - "unsafe": [ - // https://nodejs.org/api/globals.html#globals_global_objects - "Buffer", + "target": "**/vs/**/test/{common,browser}/**", + "allowed": [ "process", + "Buffer", "__filename", - "__dirname", - "clearImmediate", - "exports", - "global", - "module", - "setImmediate" + "__dirname" + ] + }, + { + "target": "**/vs/workbench/api/common/extHostExtensionService.ts", + "allowed": [ + "global" ] + }, + { + "target": "**/vs/**/{common,browser}/**", + "allowed": [ /* none */] } ], "duplicate-imports": true, From 4895470729e71ad4a7246d442c19297fda022708 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 17 Aug 2019 14:38:56 +0200 Subject: [PATCH 4/7] fix "gulp tslint" task --- build/gulpfile.hygiene.js | 46 ++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/build/gulpfile.hygiene.js b/build/gulpfile.hygiene.js index f6594804322e0..c6d8718cdcf11 100644 --- a/build/gulpfile.hygiene.js +++ b/build/gulpfile.hygiene.js @@ -149,6 +149,32 @@ const tslintFilter = [ '!extensions/html-language-features/server/lib/jquery.d.ts' ]; +const tslintBaseFilter = [ + '!**/fixtures/**', + '!**/typings/**', + '!**/node_modules/**', + '!extensions/typescript-basics/test/colorize-fixtures/**', + '!extensions/vscode-api-tests/testWorkspace/**', + '!extensions/vscode-api-tests/testWorkspace2/**', + '!extensions/**/*.test.ts', + '!extensions/html-language-features/server/lib/jquery.d.ts' +]; + +const tslintCoreFilter = [ + 'src/**/*.ts', + 'test/**/*.ts', + '!extensions/**/*.ts', + '!test/smoke/**', + ...tslintBaseFilter +]; + +const tslintExtensionsFilter = [ + 'extensions/**/*.ts', + '!src/**/*.ts', + '!test/**/*.ts', + ...tslintBaseFilter +]; + const copyrightHeaderLines = [ '/*---------------------------------------------------------------------------------------------', ' * Copyright (c) Microsoft Corporation. All rights reserved.', @@ -165,12 +191,20 @@ gulp.task('eslint', () => { }); gulp.task('tslint', () => { - const options = { emitError: true }; - - return vfs.src(all, { base: '.', follow: true, allowEmpty: true }) - .pipe(filter(tslintFilter)) - .pipe(gulptslint.default({ rulesDirectory: 'build/lib/tslint' })) - .pipe(gulptslint.default.report(options)); + return es.merge([ + + // Core: include type information (required by certain rules like no-nodejs-globals) + vfs.src(all, { base: '.', follow: true, allowEmpty: true }) + .pipe(filter(tslintCoreFilter)) + .pipe(gulptslint.default({ rulesDirectory: 'build/lib/tslint', program: tslint.Linter.createProgram('src/tsconfig.json') })) + .pipe(gulptslint.default.report({ emitError: true })), + + // Exenstions: do not include type information + vfs.src(all, { base: '.', follow: true, allowEmpty: true }) + .pipe(filter(tslintExtensionsFilter)) + .pipe(gulptslint.default({ rulesDirectory: 'build/lib/tslint' })) + .pipe(gulptslint.default.report({ emitError: true })) + ]); }); function hygiene(some) { From 582b187f88ef40c354d37ee792b621be3fc03a91 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Sat, 17 Aug 2019 14:42:56 +0200 Subject: [PATCH 5/7] share rules --- build/gulpfile.hygiene.js | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/build/gulpfile.hygiene.js b/build/gulpfile.hygiene.js index c6d8718cdcf11..a05fff4a1bd6c 100644 --- a/build/gulpfile.hygiene.js +++ b/build/gulpfile.hygiene.js @@ -135,20 +135,6 @@ const eslintFilter = [ '!**/test/**' ]; -const tslintFilter = [ - 'src/**/*.ts', - 'test/**/*.ts', - 'extensions/**/*.ts', - '!**/fixtures/**', - '!**/typings/**', - '!**/node_modules/**', - '!extensions/typescript/test/colorize-fixtures/**', - '!extensions/vscode-api-tests/testWorkspace/**', - '!extensions/vscode-api-tests/testWorkspace2/**', - '!extensions/**/*.test.ts', - '!extensions/html-language-features/server/lib/jquery.d.ts' -]; - const tslintBaseFilter = [ '!**/fixtures/**', '!**/typings/**', @@ -175,6 +161,13 @@ const tslintExtensionsFilter = [ ...tslintBaseFilter ]; +const tslintHygieneFilter = [ + 'src/**/*.ts', + 'test/**/*.ts', + 'extensions/**/*.ts', + ...tslintBaseFilter +]; + const copyrightHeaderLines = [ '/*---------------------------------------------------------------------------------------------', ' * Copyright (c) Microsoft Corporation. All rights reserved.', @@ -317,7 +310,7 @@ function hygiene(some) { .pipe(copyrights); const typescript = result - .pipe(filter(tslintFilter)) + .pipe(filter(tslintHygieneFilter)) .pipe(formatting) .pipe(tsl); From da0c6f5fe4e7cdcb7d13417ede5fa47d6b5fe02f Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 19 Aug 2019 08:54:58 +0200 Subject: [PATCH 6/7] enable more rules --- tslint.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tslint.json b/tslint.json index 8d3caae57e3d1..4e60e80431ac2 100644 --- a/tslint.json +++ b/tslint.json @@ -8,8 +8,10 @@ "no-duplicate-super": true, "no-duplicate-switch-case": true, "no-duplicate-variable": true, + "no-for-in-array": true, "no-eval": true, "no-redundant-jsdoc": true, + "no-restricted-globals": true, "no-sparse-arrays": true, "no-string-throw": true, "no-unsafe-finally": true, From 7bb912dee473030b0cbdce89b5b3fb34a1b4780b Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 19 Aug 2019 10:26:18 +0200 Subject: [PATCH 7/7] also add a rule for DOM --- build/lib/tslint/abstractGlobalsRule.js | 40 +++++++++++++++++ build/lib/tslint/abstractGlobalsRule.ts | 51 +++++++++++++++++++++ build/lib/tslint/noDOMGlobalsRule.js | 34 ++++++++++++++ build/lib/tslint/noDOMGlobalsRule.ts | 45 +++++++++++++++++++ build/lib/tslint/noNodeJSGlobalsRule.js | 58 +++++++----------------- build/lib/tslint/noNodeJSGlobalsRule.ts | 59 +++++++------------------ tslint.json | 18 +++++++- 7 files changed, 220 insertions(+), 85 deletions(-) create mode 100644 build/lib/tslint/abstractGlobalsRule.js create mode 100644 build/lib/tslint/abstractGlobalsRule.ts create mode 100644 build/lib/tslint/noDOMGlobalsRule.js create mode 100644 build/lib/tslint/noDOMGlobalsRule.ts diff --git a/build/lib/tslint/abstractGlobalsRule.js b/build/lib/tslint/abstractGlobalsRule.js new file mode 100644 index 0000000000000..2aadf2c3bd965 --- /dev/null +++ b/build/lib/tslint/abstractGlobalsRule.js @@ -0,0 +1,40 @@ +"use strict"; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +Object.defineProperty(exports, "__esModule", { value: true }); +const Lint = require("tslint"); +class AbstractGlobalsRuleWalker extends Lint.RuleWalker { + constructor(file, program, opts, _config) { + super(file, opts); + this.program = program; + this._config = _config; + } + visitIdentifier(node) { + if (this.getDisallowedGlobals().some(disallowedGlobal => disallowedGlobal === node.text)) { + if (this._config.allowed && this._config.allowed.some(allowed => allowed === node.text)) { + return; // override + } + const checker = this.program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const valueDeclaration = symbol.valueDeclaration; + if (valueDeclaration) { + const parent = valueDeclaration.parent; + if (parent) { + const sourceFile = parent.getSourceFile(); + if (sourceFile) { + const fileName = sourceFile.fileName; + if (fileName && fileName.indexOf(this.getDefinitionPattern()) >= 0) { + this.addFailureAtNode(node, `Cannot use global '${node.text}' in '${this._config.target}'`); + } + } + } + } + } + } + super.visitIdentifier(node); + } +} +exports.AbstractGlobalsRuleWalker = AbstractGlobalsRuleWalker; diff --git a/build/lib/tslint/abstractGlobalsRule.ts b/build/lib/tslint/abstractGlobalsRule.ts new file mode 100644 index 0000000000000..8c80b3e0cfd76 --- /dev/null +++ b/build/lib/tslint/abstractGlobalsRule.ts @@ -0,0 +1,51 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as ts from 'typescript'; +import * as Lint from 'tslint'; + +interface AbstractGlobalsRuleConfig { + target: string; + allowed: string[]; +} + +export abstract class AbstractGlobalsRuleWalker extends Lint.RuleWalker { + + constructor(file: ts.SourceFile, private program: ts.Program, opts: Lint.IOptions, private _config: AbstractGlobalsRuleConfig) { + super(file, opts); + } + + protected abstract getDisallowedGlobals(): string[]; + + protected abstract getDefinitionPattern(): string; + + visitIdentifier(node: ts.Identifier) { + if (this.getDisallowedGlobals().some(disallowedGlobal => disallowedGlobal === node.text)) { + if (this._config.allowed && this._config.allowed.some(allowed => allowed === node.text)) { + return; // override + } + + const checker = this.program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const valueDeclaration = symbol.valueDeclaration; + if (valueDeclaration) { + const parent = valueDeclaration.parent; + if (parent) { + const sourceFile = parent.getSourceFile(); + if (sourceFile) { + const fileName = sourceFile.fileName; + if (fileName && fileName.indexOf(this.getDefinitionPattern()) >= 0) { + this.addFailureAtNode(node, `Cannot use global '${node.text}' in '${this._config.target}'`); + } + } + } + } + } + } + + super.visitIdentifier(node); + } +} diff --git a/build/lib/tslint/noDOMGlobalsRule.js b/build/lib/tslint/noDOMGlobalsRule.js new file mode 100644 index 0000000000000..a83ac8f7f5958 --- /dev/null +++ b/build/lib/tslint/noDOMGlobalsRule.js @@ -0,0 +1,34 @@ +"use strict"; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +Object.defineProperty(exports, "__esModule", { value: true }); +const Lint = require("tslint"); +const minimatch = require("minimatch"); +const abstractGlobalsRule_1 = require("./abstractGlobalsRule"); +class Rule extends Lint.Rules.TypedRule { + applyWithProgram(sourceFile, program) { + const configs = this.getOptions().ruleArguments; + for (const config of configs) { + if (minimatch(sourceFile.fileName, config.target)) { + return this.applyWithWalker(new NoDOMGlobalsRuleWalker(sourceFile, program, this.getOptions(), config)); + } + } + return []; + } +} +exports.Rule = Rule; +class NoDOMGlobalsRuleWalker extends abstractGlobalsRule_1.AbstractGlobalsRuleWalker { + getDefinitionPattern() { + return 'lib.dom.d.ts'; + } + getDisallowedGlobals() { + // intentionally not complete + return [ + "window", + "document", + "HTMLElement" + ]; + } +} diff --git a/build/lib/tslint/noDOMGlobalsRule.ts b/build/lib/tslint/noDOMGlobalsRule.ts new file mode 100644 index 0000000000000..df9e67bf78b6f --- /dev/null +++ b/build/lib/tslint/noDOMGlobalsRule.ts @@ -0,0 +1,45 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as ts from 'typescript'; +import * as Lint from 'tslint'; +import * as minimatch from 'minimatch'; +import { AbstractGlobalsRuleWalker } from './abstractGlobalsRule'; + +interface NoDOMGlobalsRuleConfig { + target: string; + allowed: string[]; +} + +export class Rule extends Lint.Rules.TypedRule { + + applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const configs = this.getOptions().ruleArguments; + + for (const config of configs) { + if (minimatch(sourceFile.fileName, config.target)) { + return this.applyWithWalker(new NoDOMGlobalsRuleWalker(sourceFile, program, this.getOptions(), config)); + } + } + + return []; + } +} + +class NoDOMGlobalsRuleWalker extends AbstractGlobalsRuleWalker { + + getDefinitionPattern(): string { + return 'lib.dom.d.ts'; + } + + getDisallowedGlobals(): string[] { + // intentionally not complete + return [ + "window", + "document", + "HTMLElement" + ]; + } +} diff --git a/build/lib/tslint/noNodeJSGlobalsRule.js b/build/lib/tslint/noNodeJSGlobalsRule.js index 7dd5856602bc5..1e955a41e1b47 100644 --- a/build/lib/tslint/noNodeJSGlobalsRule.js +++ b/build/lib/tslint/noNodeJSGlobalsRule.js @@ -6,18 +6,7 @@ Object.defineProperty(exports, "__esModule", { value: true }); const Lint = require("tslint"); const minimatch = require("minimatch"); -// https://nodejs.org/api/globals.html#globals_global_objects -const nodeJSGlobals = [ - "Buffer", - "__dirname", - "__filename", - "clearImmediate", - "exports", - "global", - "module", - "process", - "setImmediate" -]; +const abstractGlobalsRule_1 = require("./abstractGlobalsRule"); class Rule extends Lint.Rules.TypedRule { applyWithProgram(sourceFile, program) { const configs = this.getOptions().ruleArguments; @@ -30,35 +19,22 @@ class Rule extends Lint.Rules.TypedRule { } } exports.Rule = Rule; -class NoNodejsGlobalsRuleWalker extends Lint.RuleWalker { - constructor(file, program, opts, _config) { - super(file, opts); - this.program = program; - this._config = _config; +class NoNodejsGlobalsRuleWalker extends abstractGlobalsRule_1.AbstractGlobalsRuleWalker { + getDefinitionPattern() { + return '@types/node'; } - visitIdentifier(node) { - if (nodeJSGlobals.some(nodeJSGlobal => nodeJSGlobal === node.text)) { - if (this._config.allowed && this._config.allowed.some(allowed => allowed === node.text)) { - return; // override - } - const checker = this.program.getTypeChecker(); - const symbol = checker.getSymbolAtLocation(node); - if (symbol) { - const valueDeclaration = symbol.valueDeclaration; - if (valueDeclaration) { - const parent = valueDeclaration.parent; - if (parent) { - const sourceFile = parent.getSourceFile(); - if (sourceFile) { - const fileName = sourceFile.fileName; - if (fileName && fileName.indexOf('@types/node') >= 0) { - this.addFailureAtNode(node, `Cannot use node.js global '${node.text}' in '${this._config.target}'`); - } - } - } - } - } - } - super.visitIdentifier(node); + getDisallowedGlobals() { + // https://nodejs.org/api/globals.html#globals_global_objects + return [ + "Buffer", + "__dirname", + "__filename", + "clearImmediate", + "exports", + "global", + "module", + "process", + "setImmediate" + ]; } } diff --git a/build/lib/tslint/noNodeJSGlobalsRule.ts b/build/lib/tslint/noNodeJSGlobalsRule.ts index b32700af3ea05..48b229333e9e8 100644 --- a/build/lib/tslint/noNodeJSGlobalsRule.ts +++ b/build/lib/tslint/noNodeJSGlobalsRule.ts @@ -6,25 +6,13 @@ import * as ts from 'typescript'; import * as Lint from 'tslint'; import * as minimatch from 'minimatch'; +import { AbstractGlobalsRuleWalker } from './abstractGlobalsRule'; interface NoNodejsGlobalsConfig { target: string; allowed: string[]; } -// https://nodejs.org/api/globals.html#globals_global_objects -const nodeJSGlobals = [ - "Buffer", - "__dirname", - "__filename", - "clearImmediate", - "exports", - "global", - "module", - "process", - "setImmediate" -]; - export class Rule extends Lint.Rules.TypedRule { applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { @@ -40,37 +28,24 @@ export class Rule extends Lint.Rules.TypedRule { } } -class NoNodejsGlobalsRuleWalker extends Lint.RuleWalker { +class NoNodejsGlobalsRuleWalker extends AbstractGlobalsRuleWalker { - constructor(file: ts.SourceFile, private program: ts.Program, opts: Lint.IOptions, private _config: NoNodejsGlobalsConfig) { - super(file, opts); + getDefinitionPattern(): string { + return '@types/node'; } - visitIdentifier(node: ts.Identifier) { - if (nodeJSGlobals.some(nodeJSGlobal => nodeJSGlobal === node.text)) { - if (this._config.allowed && this._config.allowed.some(allowed => allowed === node.text)) { - return; // override - } - - const checker = this.program.getTypeChecker(); - const symbol = checker.getSymbolAtLocation(node); - if (symbol) { - const valueDeclaration = symbol.valueDeclaration; - if (valueDeclaration) { - const parent = valueDeclaration.parent; - if (parent) { - const sourceFile = parent.getSourceFile(); - if (sourceFile) { - const fileName = sourceFile.fileName; - if (fileName && fileName.indexOf('@types/node') >= 0) { - this.addFailureAtNode(node, `Cannot use node.js global '${node.text}' in '${this._config.target}'`); - } - } - } - } - } - } - - super.visitIdentifier(node); + getDisallowedGlobals(): string[] { + // https://nodejs.org/api/globals.html#globals_global_objects + return [ + "Buffer", + "__dirname", + "__filename", + "clearImmediate", + "exports", + "global", + "module", + "process", + "setImmediate" + ]; } } diff --git a/tslint.json b/tslint.json index 4e60e80431ac2..6ccbc6ce1ed82 100644 --- a/tslint.json +++ b/tslint.json @@ -631,7 +631,7 @@ { "target": "**/vs/base/common/{path,process,platform}.ts", "allowed": [ - "process" + "process" // -> defines safe access to process ] }, { @@ -646,7 +646,7 @@ { "target": "**/vs/workbench/api/common/extHostExtensionService.ts", "allowed": [ - "global" + "global" // -> safe access to 'global' ] }, { @@ -654,6 +654,20 @@ "allowed": [ /* none */] } ], + "no-dom-globals": [ + true, + { + "target": "**/vs/**/test/{common,node,electron-main}/**", + "allowed": [ + "document", + "HTMLElement" + ] + }, + { + "target": "**/vs/**/{common,node,electron-main}/**", + "allowed": [ /* none */] + } + ], "duplicate-imports": true, "no-new-buffer": true, "translation-remind": true,