TSLint: show a warning when accessing node.js globals in common|browser (#79222)

* trivial first cut

* document where globals are from

* improve rule detection

* fix "gulp tslint" task

* share rules

* enable more rules

* also add a rule for DOM

build/gulpfile.hygiene.js

@@ -135,20 +135,39 @@ const eslintFilter = [
 	'!**/test/**'
 ];
 
-const tslintFilter = [
-	'src/**/*.ts',
-	'test/**/*.ts',
-	'extensions/**/*.ts',
+const tslintBaseFilter = [
 	'!**/fixtures/**',
 	'!**/typings/**',
 	'!**/node_modules/**',
-	'!extensions/typescript/test/colorize-fixtures/**',
+	'!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 tslintHygieneFilter = [
+	'src/**/*.ts',
+	'test/**/*.ts',
+	'extensions/**/*.ts',
+	...tslintBaseFilter
+];
+
 const copyrightHeaderLines = [
 	'/*---------------------------------------------------------------------------------------------',
 	' *  Copyright (c) Microsoft Corporation. All rights reserved.',
@@ -165,12 +184,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) {
@@ -283,7 +310,7 @@ function hygiene(some) {
 		.pipe(copyrights);
 
 	const typescript = result
-		.pipe(filter(tslintFilter))
+		.pipe(filter(tslintHygieneFilter))
 		.pipe(formatting)
 		.pipe(tsl);
 

build/lib/tslint/abstractGlobalsRule.js

+"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;

build/lib/tslint/abstractGlobalsRule.ts

+/*---------------------------------------------------------------------------------------------
+ *  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);
+	}
+}

build/lib/tslint/noDOMGlobalsRule.js

+"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"
+        ];
+    }
+}

build/lib/tslint/noDOMGlobalsRule.ts

+/*---------------------------------------------------------------------------------------------
+ *  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 = <NoDOMGlobalsRuleConfig[]>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"
+		];
+	}
+}

build/lib/tslint/noNodeJSGlobalsRule.js

+"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 NoNodejsGlobalsRuleWalker(sourceFile, program, this.getOptions(), config));
+            }
+        }
+        return [];
+    }
+}
+exports.Rule = Rule;
+class NoNodejsGlobalsRuleWalker extends abstractGlobalsRule_1.AbstractGlobalsRuleWalker {
+    getDefinitionPattern() {
+        return '@types/node';
+    }
+    getDisallowedGlobals() {
+        // https://nodejs.org/api/globals.html#globals_global_objects
+        return [
+            "Buffer",
+            "__dirname",
+            "__filename",
+            "clearImmediate",
+            "exports",
+            "global",
+            "module",
+            "process",
+            "setImmediate"
+        ];
+    }
+}

build/lib/tslint/noNodeJSGlobalsRule.ts

+/*---------------------------------------------------------------------------------------------
+ *  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 NoNodejsGlobalsConfig {
+	target: string;
+	allowed: string[];
+}
+
+export class Rule extends Lint.Rules.TypedRule {
+
+	applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+		const configs = <NoNodejsGlobalsConfig[]>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 AbstractGlobalsRuleWalker {
+
+	getDefinitionPattern(): string {
+		return '@types/node';
+	}
+
+	getDisallowedGlobals(): string[] {
+		// https://nodejs.org/api/globals.html#globals_global_objects
+		return [
+			"Buffer",
+			"__dirname",
+			"__filename",
+			"clearImmediate",
+			"exports",
+			"global",
+			"module",
+			"process",
+			"setImmediate"
+		];
+	}
+}

tslint.json

@@ -9,8 +9,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,
@@ -625,6 +627,48 @@
 				"restrictions": "**/vs/**"
 			}
 		],
+		"no-nodejs-globals": [
+			true,
+			{
+				"target": "**/vs/base/common/{path,process,platform}.ts",
+				"allowed": [
+					"process" // -> defines safe access to process
+				]
+			},
+			{
+				"target": "**/vs/**/test/{common,browser}/**",
+				"allowed": [
+					"process",
+					"Buffer",
+					"__filename",
+					"__dirname"
+				]
+			},
+			{
+				"target": "**/vs/workbench/api/common/extHostExtensionService.ts",
+				"allowed": [
+					"global" // -> safe access to 'global'
+				]
+			},
+			{
+				"target": "**/vs/**/{common,browser}/**",
+				"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,