Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[new-rule] no-restricted-globals #3824

Merged
merged 12 commits into from
Feb 23, 2019
1 change: 1 addition & 0 deletions src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export const rules = {
"no-namespace": true,
"no-non-null-assertion": true,
"no-reference": true,
"no-restricted-globals": true,
"no-this-assignment": true,
"no-var-requires": true,
"only-arrow-functions": true,
Expand Down
115 changes: 115 additions & 0 deletions src/rules/noRestrictedGlobalsRule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/**
* @license
* Copyright 2014 Palantir Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as ts from "typescript";

import * as Lint from "../index";

export class Rule extends Lint.Rules.TypedRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
ruleName: "no-restricted-globals",
description: "Disallow specific global variables.",
rationale: Lint.Utils.dedent`
\`\`\`ts
function broken(evt: Event) {
// Meant to do something with \`evt\` but typed it incorrectly.
Event.target; // compiler error
event.target; // should be a lint failure
}

Early Internet Explorer versions exposed the current DOM event as a global variable 'event',
but using this variable has been considered a bad practice for a long time.
Restricting this will make sure this variable isn’t used in browser code.
\`\`\`
`,
descriptionDetails: Lint.Utils.dedent`
Disallowing usage of specific global variables can be useful if you want to allow
a set of global variables by enabling an environment, but still want to disallow
some of those.
`,
optionsDescription: Lint.Utils.dedent`
This rule takes a list of strings, where each string is a global to be restricted.
\`event\`, \`name\` and \`length\` are restricted by default.
`,
options: {
type: "list",
items: {type: "string"},
},
optionExamples: [
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
[
true,
"name",
"length",
"event",
],
],
type: "functionality",
typescriptOnly: false,
requiresTypeInfo: true,
};
/* tslint:enable:object-literal-sort-keys */

public static FAILURE_STRING(name: string) {
return `Unexpected global variable '${name}'. Use a local parameter or variable instead.`;
}

public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
const bannedList = this.ruleArguments.length > 0 ? this.ruleArguments : ["event", "name", "length"];
const bannedGlobals = new Set(bannedList);
if (sourceFile.isDeclarationFile) {
return [];
} else {
return this.applyWithFunction(sourceFile, walk, bannedGlobals, program.getTypeChecker());
}
}
}

function walk(ctx: Lint.WalkContext<Set<string>>, checker: ts.TypeChecker): void {
return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void {
switch (node.kind) {
Zzzen marked this conversation as resolved.
Show resolved Hide resolved
case ts.SyntaxKind.ObjectBindingPattern:
case ts.SyntaxKind.TypeReference:
return;
case ts.SyntaxKind.PropertyAccessExpression:
// Ignore `y` in `x.y`, but recurse to `x`.
return recur((node as ts.PropertyAccessExpression).expression);
case ts.SyntaxKind.Identifier:
return checkIdentifier(node as ts.Identifier);
default:
Zzzen marked this conversation as resolved.
Show resolved Hide resolved
return ts.forEachChild(node, recur);
}
});

function checkIdentifier(node: ts.Identifier): void {
if (!ctx.options.has(node.text)) {
return;
}

const symbol = checker.getSymbolAtLocation(node);
const declarations = symbol === undefined ? undefined : symbol.declarations;
if (declarations === undefined || declarations.length === 0) {
return;
}

const isAmbientGlobal = declarations.some((decl) => decl.getSourceFile().isDeclarationFile);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This errors too often if the declaration comes from a declaration file that is not lib.dom.d.ts. Consider the case of adone's types in Definitely Typed:

// @Filename: adone-tests.ts
namespace eventsTests {
    namespace EventEmitter {
        namespace static {
            const a: number = adone.event.Emitter.listenerCount(new adone.event.Emitter(), "event");
            const b: number = adone.event.Emitter.defaultMaxListeners;
        }
// @Filename: events.d.ts
declare namespace adone {
    namespace event {
        class Emitter {
            static listenerCount(emitter: Emitter, event: string | symbol): number;
            static defaultMaxListeners: number;
            // ...

You will get bogus errors on event.


if (isAmbientGlobal) {
ctx.addFailureAtNode(node, Rule.FAILURE_STRING(node.text));
}
}
}
2 changes: 2 additions & 0 deletions test/rules/no-restricted-globals/custom-global.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
declare var badGlobal: any;
declare var goodGlobal: any;
1 change: 1 addition & 0 deletions test/rules/no-restricted-globals/foo.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export var name: string;
21 changes: 21 additions & 0 deletions test/rules/no-restricted-globals/misc.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
function typeTest() {
Zzzen marked this conversation as resolved.
Show resolved Hide resolved
type name = any;
}

function functionTest () {
const foo = name;

function name () {

}
}

function interfaceTest() {
var foo: name;
interface name {}
}

function classTest() {
const foo = new name();
class name {}
}
10 changes: 10 additions & 0 deletions test/rules/no-restricted-globals/namespace.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
console.log(name);
~~~~ [Unexpected global variable 'name'. Use a local parameter or variable instead.]

namespace A {
const foo = name;
}

namespace A {
export var name = 23;
}
21 changes: 21 additions & 0 deletions test/rules/no-restricted-globals/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
function foo(evt: Event) {
let length: number = 1;
console.log(length);

Event.target;

event.target;
~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.]
}

console.log(length);
~~~~~~ [Unexpected global variable 'length'. Use a local parameter or variable instead.]

import { name } from "./foo";
import "./custom-global"
console.log(name);

console.log(badGlobal);
~~~~~~~~~ [Unexpected global variable 'badGlobal'. Use a local parameter or variable instead.]

console.log(goodGlobal);
8 changes: 8 additions & 0 deletions test/rules/no-restricted-globals/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"module": "commonjs",
"lib": [
"dom"
]
}
}
11 changes: 11 additions & 0 deletions test/rules/no-restricted-globals/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"rules": {
"no-restricted-globals": [
Zzzen marked this conversation as resolved.
Show resolved Hide resolved
true,
"name",
"length",
"event",
"badGlobal"
]
}
}