Skip to content

Commit

Permalink
TSLint: show a warning when accessing node.js globals in common|brows…
Browse files Browse the repository at this point in the history
…er (#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
  • Loading branch information
bpasero authored Aug 19, 2019
1 parent 9fed4cf commit 90a35ec
Show file tree
Hide file tree
Showing 8 changed files with 344 additions and 12 deletions.
51 changes: 39 additions & 12 deletions build/gulpfile.hygiene.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand All @@ -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) {
Expand Down Expand Up @@ -283,7 +310,7 @@ function hygiene(some) {
.pipe(copyrights);

const typescript = result
.pipe(filter(tslintFilter))
.pipe(filter(tslintHygieneFilter))
.pipe(formatting)
.pipe(tsl);

Expand Down
40 changes: 40 additions & 0 deletions build/lib/tslint/abstractGlobalsRule.js
Original file line number Diff line number Diff line change
@@ -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;
51 changes: 51 additions & 0 deletions build/lib/tslint/abstractGlobalsRule.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}
34 changes: 34 additions & 0 deletions build/lib/tslint/noDOMGlobalsRule.js
Original file line number Diff line number Diff line change
@@ -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"
];
}
}
45 changes: 45 additions & 0 deletions build/lib/tslint/noDOMGlobalsRule.ts
Original file line number Diff line number Diff line change
@@ -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 = <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"
];
}
}
40 changes: 40 additions & 0 deletions build/lib/tslint/noNodeJSGlobalsRule.js
Original file line number Diff line number Diff line change
@@ -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");
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"
];
}
}
51 changes: 51 additions & 0 deletions build/lib/tslint/noNodeJSGlobalsRule.ts
Original file line number Diff line number Diff line change
@@ -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';
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"
];
}
}
Loading

0 comments on commit 90a35ec

Please sign in to comment.