From eedabef75eb564ad772d1effed12249dea5629bc Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Mon, 9 Apr 2018 23:27:19 +0800 Subject: [PATCH 01/11] [new-rule] ban-dom-global --- src/configs/all.ts | 1 + src/rules/banDomGlobalRule.ts | 92 +++++++++++++++++++++++++ test/rules/ban-dom-global/test.ts.lint | 15 ++++ test/rules/ban-dom-global/tsconfig.json | 8 +++ test/rules/ban-dom-global/tslint.json | 10 +++ 5 files changed, 126 insertions(+) create mode 100644 src/rules/banDomGlobalRule.ts create mode 100644 test/rules/ban-dom-global/test.ts.lint create mode 100644 test/rules/ban-dom-global/tsconfig.json create mode 100644 test/rules/ban-dom-global/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index 63b14cae706..6bda8efba47 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -26,6 +26,7 @@ export const rules = { // TypeScript Specific "adjacent-overload-signatures": true, + "ban-dom-global": [true], "ban-types": { options: [ ["Object", "Avoid using the `Object` type. Did you mean `object`?"], diff --git a/src/rules/banDomGlobalRule.ts b/src/rules/banDomGlobalRule.ts new file mode 100644 index 00000000000..d99b6f21a45 --- /dev/null +++ b/src/rules/banDomGlobalRule.ts @@ -0,0 +1,92 @@ +/** + * @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: "ban-dom-global", + description: "Ban specific refrences to DOM globals.", + descriptionDetails: Lint.Utils.dedent` + For example, \`self\`, \`name\` + `, + optionsDescription: "Not configurable.", + options: { + type: "list", + items: {type: "string"}, + }, + optionExamples: [ + [ + true, + "name", + "length", + "event", + ], + ], + type: "functionality", + typescriptOnly: false, + requiresTypeInfo: true, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING(name: string) { + return `variable '${name}' is defined in lib.dom.d.ts.`; + } + + public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const bannedGlobals = new Set(this.ruleArguments as string[]); + return this.applyWithFunction(sourceFile, walk, bannedGlobals, program.getTypeChecker()); + } +} + +function walk(ctx: Lint.WalkContext>, checker: ts.TypeChecker): void { + return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void { + switch (node.kind) { + case ts.SyntaxKind.TypeReference: + // Ignore types. + 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: + 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 declaredInLibDom = declarations.some((decl) => decl.getSourceFile().fileName.endsWith("lib.dom.d.ts")); + + if (declaredInLibDom) { + ctx.addFailureAtNode(node, Rule.FAILURE_STRING(node.text)); + } + } +} diff --git a/test/rules/ban-dom-global/test.ts.lint b/test/rules/ban-dom-global/test.ts.lint new file mode 100644 index 00000000000..566139aae4e --- /dev/null +++ b/test/rules/ban-dom-global/test.ts.lint @@ -0,0 +1,15 @@ +function foo(evt: Event) { + let length: number = 1; + console.log(length); + + Event.target; + + event.target; + ~~~~~ [variable 'event' is defined in lib.dom.d.ts.] +} + +console.log(length); + ~~~~~~ [variable 'length' is defined in lib.dom.d.ts.] + +import { name } from "./foo"; +console.log(name); \ No newline at end of file diff --git a/test/rules/ban-dom-global/tsconfig.json b/test/rules/ban-dom-global/tsconfig.json new file mode 100644 index 00000000000..3039d056863 --- /dev/null +++ b/test/rules/ban-dom-global/tsconfig.json @@ -0,0 +1,8 @@ +{ + "compilerOptions": { + "module": "commonjs", + "lib": [ + "dom" + ] + } +} diff --git a/test/rules/ban-dom-global/tslint.json b/test/rules/ban-dom-global/tslint.json new file mode 100644 index 00000000000..28618c145dd --- /dev/null +++ b/test/rules/ban-dom-global/tslint.json @@ -0,0 +1,10 @@ +{ + "rules": { + "ban-dom-global": [ + true, + "name", + "length", + "event" + ] + } +} From 262b7ba972b9c9cc5bc4de469c0786e2b55508d0 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Tue, 10 Apr 2018 20:04:29 +0800 Subject: [PATCH 02/11] rename ban-dom-global to no-restricted-globals --- src/configs/all.ts | 2 +- ...nDomGlobalRule.ts => noRestrictedGlobalsRule.ts} | 13 +++++++++---- .../test.ts.lint | 0 .../tsconfig.json | 0 .../tslint.json | 2 +- 5 files changed, 11 insertions(+), 6 deletions(-) rename src/rules/{banDomGlobalRule.ts => noRestrictedGlobalsRule.ts} (80%) rename test/rules/{ban-dom-global => no-restricted-globals}/test.ts.lint (100%) rename test/rules/{ban-dom-global => no-restricted-globals}/tsconfig.json (100%) rename test/rules/{ban-dom-global => no-restricted-globals}/tslint.json (72%) diff --git a/src/configs/all.ts b/src/configs/all.ts index 6bda8efba47..7c31d728ae1 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -26,7 +26,6 @@ export const rules = { // TypeScript Specific "adjacent-overload-signatures": true, - "ban-dom-global": [true], "ban-types": { options: [ ["Object", "Avoid using the `Object` type. Did you mean `object`?"], @@ -52,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, diff --git a/src/rules/banDomGlobalRule.ts b/src/rules/noRestrictedGlobalsRule.ts similarity index 80% rename from src/rules/banDomGlobalRule.ts rename to src/rules/noRestrictedGlobalsRule.ts index d99b6f21a45..9d12758955b 100644 --- a/src/rules/banDomGlobalRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -22,12 +22,17 @@ import * as Lint from "../index"; export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { - ruleName: "ban-dom-global", - description: "Ban specific refrences to DOM globals.", + ruleName: "no-restricted-globals", + description: "Disallow specific global variables.", descriptionDetails: Lint.Utils.dedent` - For example, \`self\`, \`name\` + 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. For instance, early Internet Explorer versions exposed the current + DOM event as a global variable event, but using this variable has been considered + as a bad practice for a long time. Restricting this will make sure this variable + isn’t used in browser code. `, - optionsDescription: "Not configurable.", + optionsDescription: "This rule takes a list of strings, where each string is a global to be restricted.", options: { type: "list", items: {type: "string"}, diff --git a/test/rules/ban-dom-global/test.ts.lint b/test/rules/no-restricted-globals/test.ts.lint similarity index 100% rename from test/rules/ban-dom-global/test.ts.lint rename to test/rules/no-restricted-globals/test.ts.lint diff --git a/test/rules/ban-dom-global/tsconfig.json b/test/rules/no-restricted-globals/tsconfig.json similarity index 100% rename from test/rules/ban-dom-global/tsconfig.json rename to test/rules/no-restricted-globals/tsconfig.json diff --git a/test/rules/ban-dom-global/tslint.json b/test/rules/no-restricted-globals/tslint.json similarity index 72% rename from test/rules/ban-dom-global/tslint.json rename to test/rules/no-restricted-globals/tslint.json index 28618c145dd..d402b8d9d83 100644 --- a/test/rules/ban-dom-global/tslint.json +++ b/test/rules/no-restricted-globals/tslint.json @@ -1,6 +1,6 @@ { "rules": { - "ban-dom-global": [ + "no-restricted-globals": [ true, "name", "length", From 614353a11860aa79b70d0b367c1e7b66fd6509f4 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Tue, 10 Apr 2018 20:14:20 +0800 Subject: [PATCH 03/11] enable this rule for all definition files --- src/rules/noRestrictedGlobalsRule.ts | 4 ++-- test/rules/no-restricted-globals/test.ts.lint | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index 9d12758955b..ace42a59ffd 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -52,7 +52,7 @@ export class Rule extends Lint.Rules.TypedRule { /* tslint:enable:object-literal-sort-keys */ public static FAILURE_STRING(name: string) { - return `variable '${name}' is defined in lib.dom.d.ts.`; + return `Unexpected global variable '${name}'. Use local parameter instead.`; } public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { @@ -88,7 +88,7 @@ function walk(ctx: Lint.WalkContext>, checker: ts.TypeChecker): void return; } - const declaredInLibDom = declarations.some((decl) => decl.getSourceFile().fileName.endsWith("lib.dom.d.ts")); + const declaredInLibDom = declarations.some((decl) => decl.getSourceFile().fileName.endsWith(".d.ts")); if (declaredInLibDom) { ctx.addFailureAtNode(node, Rule.FAILURE_STRING(node.text)); diff --git a/test/rules/no-restricted-globals/test.ts.lint b/test/rules/no-restricted-globals/test.ts.lint index 566139aae4e..c1d57fcf960 100644 --- a/test/rules/no-restricted-globals/test.ts.lint +++ b/test/rules/no-restricted-globals/test.ts.lint @@ -5,11 +5,11 @@ function foo(evt: Event) { Event.target; event.target; - ~~~~~ [variable 'event' is defined in lib.dom.d.ts.] + ~~~~~ [Unexpected global variable 'event'. Use local parameter instead.] } console.log(length); - ~~~~~~ [variable 'length' is defined in lib.dom.d.ts.] + ~~~~~~ [Unexpected global variable 'length'. Use local parameter instead.] import { name } from "./foo"; console.log(name); \ No newline at end of file From 70743994e954ba8b7a6b48037de9063bcd6e2976 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Fri, 20 Apr 2018 23:45:30 +0800 Subject: [PATCH 04/11] improve doc and add tests --- src/rules/noRestrictedGlobalsRule.ts | 17 +++++++++++------ .../lib-dom/custom-global.d.ts.lint | 1 + .../lib-dom/foo.d.ts.lint | 1 + .../no-restricted-globals/lib-dom/misc.ts.lint | 15 +++++++++++++++ .../no-restricted-globals/lib-dom/test.ts.lint | 18 ++++++++++++++++++ .../{ => lib-dom}/tsconfig.json | 0 .../no-restricted-globals/lib-dom/tslint.json | 11 +++++++++++ .../namespace/core.d.ts.lint | 3 +++ .../namespace/lib.ts.lint | 8 ++++++++ .../namespace/tsconfig.json | 8 ++++++++ .../{ => namespace}/tslint.json | 0 test/rules/no-restricted-globals/test.ts.lint | 15 --------------- 12 files changed, 76 insertions(+), 21 deletions(-) create mode 100644 test/rules/no-restricted-globals/lib-dom/custom-global.d.ts.lint create mode 100644 test/rules/no-restricted-globals/lib-dom/foo.d.ts.lint create mode 100644 test/rules/no-restricted-globals/lib-dom/misc.ts.lint create mode 100644 test/rules/no-restricted-globals/lib-dom/test.ts.lint rename test/rules/no-restricted-globals/{ => lib-dom}/tsconfig.json (100%) create mode 100644 test/rules/no-restricted-globals/lib-dom/tslint.json create mode 100644 test/rules/no-restricted-globals/namespace/core.d.ts.lint create mode 100644 test/rules/no-restricted-globals/namespace/lib.ts.lint create mode 100644 test/rules/no-restricted-globals/namespace/tsconfig.json rename test/rules/no-restricted-globals/{ => namespace}/tslint.json (100%) delete mode 100644 test/rules/no-restricted-globals/test.ts.lint diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index ace42a59ffd..fbc1dd23cb1 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -19,6 +19,7 @@ import * as ts from "typescript"; import * as Lint from "../index"; +// TODO: add a default list export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { @@ -28,8 +29,8 @@ export class Rule extends Lint.Rules.TypedRule { 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. For instance, early Internet Explorer versions exposed the current - DOM event as a global variable event, but using this variable has been considered - as a bad practice for a long time. Restricting this will make sure this variable + 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. `, optionsDescription: "This rule takes a list of strings, where each string is a global to be restricted.", @@ -52,12 +53,16 @@ export class Rule extends Lint.Rules.TypedRule { /* tslint:enable:object-literal-sort-keys */ public static FAILURE_STRING(name: string) { - return `Unexpected global variable '${name}'. Use local parameter instead.`; + return `Unexpected global variable '${name}'. Use a local parameter or variable instead.`; } public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { const bannedGlobals = new Set(this.ruleArguments as string[]); - return this.applyWithFunction(sourceFile, walk, bannedGlobals, program.getTypeChecker()); + if (sourceFile.isDeclarationFile) { + return []; + } else { + return this.applyWithFunction(sourceFile, walk, bannedGlobals, program.getTypeChecker()); + } } } @@ -88,9 +93,9 @@ function walk(ctx: Lint.WalkContext>, checker: ts.TypeChecker): void return; } - const declaredInLibDom = declarations.some((decl) => decl.getSourceFile().fileName.endsWith(".d.ts")); + const isAmbientGlobal = declarations.some((decl) => decl.getSourceFile().isDeclarationFile); - if (declaredInLibDom) { + if (isAmbientGlobal) { ctx.addFailureAtNode(node, Rule.FAILURE_STRING(node.text)); } } diff --git a/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts.lint b/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts.lint new file mode 100644 index 00000000000..6672182513f --- /dev/null +++ b/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts.lint @@ -0,0 +1 @@ +declare var myGlobal: any; \ No newline at end of file diff --git a/test/rules/no-restricted-globals/lib-dom/foo.d.ts.lint b/test/rules/no-restricted-globals/lib-dom/foo.d.ts.lint new file mode 100644 index 00000000000..30f811d837f --- /dev/null +++ b/test/rules/no-restricted-globals/lib-dom/foo.d.ts.lint @@ -0,0 +1 @@ +export var name: string; \ No newline at end of file diff --git a/test/rules/no-restricted-globals/lib-dom/misc.ts.lint b/test/rules/no-restricted-globals/lib-dom/misc.ts.lint new file mode 100644 index 00000000000..ac44d0c6eb0 --- /dev/null +++ b/test/rules/no-restricted-globals/lib-dom/misc.ts.lint @@ -0,0 +1,15 @@ +type name = any; + +{ + function name () { + + } +} + +{ + interface name {} +} + +{ + class name {} +} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/lib-dom/test.ts.lint b/test/rules/no-restricted-globals/lib-dom/test.ts.lint new file mode 100644 index 00000000000..7355af766db --- /dev/null +++ b/test/rules/no-restricted-globals/lib-dom/test.ts.lint @@ -0,0 +1,18 @@ +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"; +console.log(name); + +console.log(myGlobal); + ~~~~~~~~ [Unexpected global variable 'myGlobal'. Use a local parameter or variable instead.] diff --git a/test/rules/no-restricted-globals/tsconfig.json b/test/rules/no-restricted-globals/lib-dom/tsconfig.json similarity index 100% rename from test/rules/no-restricted-globals/tsconfig.json rename to test/rules/no-restricted-globals/lib-dom/tsconfig.json diff --git a/test/rules/no-restricted-globals/lib-dom/tslint.json b/test/rules/no-restricted-globals/lib-dom/tslint.json new file mode 100644 index 00000000000..9f80463e279 --- /dev/null +++ b/test/rules/no-restricted-globals/lib-dom/tslint.json @@ -0,0 +1,11 @@ +{ + "rules": { + "no-restricted-globals": [ + true, + "name", + "length", + "event", + "myGlobal" + ] + } +} diff --git a/test/rules/no-restricted-globals/namespace/core.d.ts.lint b/test/rules/no-restricted-globals/namespace/core.d.ts.lint new file mode 100644 index 00000000000..0482f07ef5c --- /dev/null +++ b/test/rules/no-restricted-globals/namespace/core.d.ts.lint @@ -0,0 +1,3 @@ +namespace A { + export var name: string; +} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/namespace/lib.ts.lint b/test/rules/no-restricted-globals/namespace/lib.ts.lint new file mode 100644 index 00000000000..1f0cbc664e7 --- /dev/null +++ b/test/rules/no-restricted-globals/namespace/lib.ts.lint @@ -0,0 +1,8 @@ +import "./core" + +console.log(name); + ~~~~ [Unexpected global variable 'name'. Use a local parameter or variable instead.] + +namespace A { + const foo = name; +} diff --git a/test/rules/no-restricted-globals/namespace/tsconfig.json b/test/rules/no-restricted-globals/namespace/tsconfig.json new file mode 100644 index 00000000000..3039d056863 --- /dev/null +++ b/test/rules/no-restricted-globals/namespace/tsconfig.json @@ -0,0 +1,8 @@ +{ + "compilerOptions": { + "module": "commonjs", + "lib": [ + "dom" + ] + } +} diff --git a/test/rules/no-restricted-globals/tslint.json b/test/rules/no-restricted-globals/namespace/tslint.json similarity index 100% rename from test/rules/no-restricted-globals/tslint.json rename to test/rules/no-restricted-globals/namespace/tslint.json diff --git a/test/rules/no-restricted-globals/test.ts.lint b/test/rules/no-restricted-globals/test.ts.lint deleted file mode 100644 index c1d57fcf960..00000000000 --- a/test/rules/no-restricted-globals/test.ts.lint +++ /dev/null @@ -1,15 +0,0 @@ -function foo(evt: Event) { - let length: number = 1; - console.log(length); - - Event.target; - - event.target; - ~~~~~ [Unexpected global variable 'event'. Use local parameter instead.] -} - -console.log(length); - ~~~~~~ [Unexpected global variable 'length'. Use local parameter instead.] - -import { name } from "./foo"; -console.log(name); \ No newline at end of file From 049688e5eeec3c0ac0aad5dd92b842a5b4e746a9 Mon Sep 17 00:00:00 2001 From: Zzzen <843968788@qq.com> Date: Sat, 21 Apr 2018 11:54:27 +0800 Subject: [PATCH 05/11] rename *.d.ts.lint to *.d.ts --- .../lib-dom/{custom-global.d.ts.lint => custom-global.d.ts} | 0 .../no-restricted-globals/lib-dom/{foo.d.ts.lint => foo.d.ts} | 0 test/rules/no-restricted-globals/lib-dom/test.ts.lint | 1 + .../namespace/{core.d.ts.lint => core.d.ts} | 2 +- 4 files changed, 2 insertions(+), 1 deletion(-) rename test/rules/no-restricted-globals/lib-dom/{custom-global.d.ts.lint => custom-global.d.ts} (100%) rename test/rules/no-restricted-globals/lib-dom/{foo.d.ts.lint => foo.d.ts} (100%) rename test/rules/no-restricted-globals/namespace/{core.d.ts.lint => core.d.ts} (57%) diff --git a/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts.lint b/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts similarity index 100% rename from test/rules/no-restricted-globals/lib-dom/custom-global.d.ts.lint rename to test/rules/no-restricted-globals/lib-dom/custom-global.d.ts diff --git a/test/rules/no-restricted-globals/lib-dom/foo.d.ts.lint b/test/rules/no-restricted-globals/lib-dom/foo.d.ts similarity index 100% rename from test/rules/no-restricted-globals/lib-dom/foo.d.ts.lint rename to test/rules/no-restricted-globals/lib-dom/foo.d.ts diff --git a/test/rules/no-restricted-globals/lib-dom/test.ts.lint b/test/rules/no-restricted-globals/lib-dom/test.ts.lint index 7355af766db..83c0329c12a 100644 --- a/test/rules/no-restricted-globals/lib-dom/test.ts.lint +++ b/test/rules/no-restricted-globals/lib-dom/test.ts.lint @@ -12,6 +12,7 @@ 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(myGlobal); diff --git a/test/rules/no-restricted-globals/namespace/core.d.ts.lint b/test/rules/no-restricted-globals/namespace/core.d.ts similarity index 57% rename from test/rules/no-restricted-globals/namespace/core.d.ts.lint rename to test/rules/no-restricted-globals/namespace/core.d.ts index 0482f07ef5c..45920f312e4 100644 --- a/test/rules/no-restricted-globals/namespace/core.d.ts.lint +++ b/test/rules/no-restricted-globals/namespace/core.d.ts @@ -1,3 +1,3 @@ -namespace A { +declare namespace A { export var name: string; } \ No newline at end of file From ea5c8dedfa5b28da05e5d70ba23254b3a5dfc579 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Fri, 27 Apr 2018 23:35:22 +0800 Subject: [PATCH 06/11] refactor tests --- .../no-restricted-globals/custom-global.d.ts | 2 ++ .../{lib-dom => }/foo.d.ts | 0 .../lib-dom/custom-global.d.ts | 1 - .../lib-dom/misc.ts.lint | 15 ------------- test/rules/no-restricted-globals/misc.ts.lint | 21 +++++++++++++++++++ .../lib.ts.lint => namespace.ts.lint} | 6 ++++-- .../no-restricted-globals/namespace/core.d.ts | 3 --- .../namespace/tsconfig.json | 8 ------- .../namespace/tslint.json | 10 --------- .../{lib-dom => }/test.ts.lint | 6 ++++-- .../{lib-dom => }/tsconfig.json | 0 .../{lib-dom => }/tslint.json | 2 +- 12 files changed, 32 insertions(+), 42 deletions(-) create mode 100644 test/rules/no-restricted-globals/custom-global.d.ts rename test/rules/no-restricted-globals/{lib-dom => }/foo.d.ts (100%) delete mode 100644 test/rules/no-restricted-globals/lib-dom/custom-global.d.ts delete mode 100644 test/rules/no-restricted-globals/lib-dom/misc.ts.lint create mode 100644 test/rules/no-restricted-globals/misc.ts.lint rename test/rules/no-restricted-globals/{namespace/lib.ts.lint => namespace.ts.lint} (79%) delete mode 100644 test/rules/no-restricted-globals/namespace/core.d.ts delete mode 100644 test/rules/no-restricted-globals/namespace/tsconfig.json delete mode 100644 test/rules/no-restricted-globals/namespace/tslint.json rename test/rules/no-restricted-globals/{lib-dom => }/test.ts.lint (72%) rename test/rules/no-restricted-globals/{lib-dom => }/tsconfig.json (100%) rename test/rules/no-restricted-globals/{lib-dom => }/tslint.json (86%) diff --git a/test/rules/no-restricted-globals/custom-global.d.ts b/test/rules/no-restricted-globals/custom-global.d.ts new file mode 100644 index 00000000000..e981c8a88cf --- /dev/null +++ b/test/rules/no-restricted-globals/custom-global.d.ts @@ -0,0 +1,2 @@ +declare var badGlobal: any; +declare var goodGlobal: any; \ No newline at end of file diff --git a/test/rules/no-restricted-globals/lib-dom/foo.d.ts b/test/rules/no-restricted-globals/foo.d.ts similarity index 100% rename from test/rules/no-restricted-globals/lib-dom/foo.d.ts rename to test/rules/no-restricted-globals/foo.d.ts diff --git a/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts b/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts deleted file mode 100644 index 6672182513f..00000000000 --- a/test/rules/no-restricted-globals/lib-dom/custom-global.d.ts +++ /dev/null @@ -1 +0,0 @@ -declare var myGlobal: any; \ No newline at end of file diff --git a/test/rules/no-restricted-globals/lib-dom/misc.ts.lint b/test/rules/no-restricted-globals/lib-dom/misc.ts.lint deleted file mode 100644 index ac44d0c6eb0..00000000000 --- a/test/rules/no-restricted-globals/lib-dom/misc.ts.lint +++ /dev/null @@ -1,15 +0,0 @@ -type name = any; - -{ - function name () { - - } -} - -{ - interface name {} -} - -{ - class name {} -} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/misc.ts.lint b/test/rules/no-restricted-globals/misc.ts.lint new file mode 100644 index 00000000000..328f6bc07be --- /dev/null +++ b/test/rules/no-restricted-globals/misc.ts.lint @@ -0,0 +1,21 @@ +function typeTest() { + 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 {} +} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/namespace/lib.ts.lint b/test/rules/no-restricted-globals/namespace.ts.lint similarity index 79% rename from test/rules/no-restricted-globals/namespace/lib.ts.lint rename to test/rules/no-restricted-globals/namespace.ts.lint index 1f0cbc664e7..5fe8939d5c6 100644 --- a/test/rules/no-restricted-globals/namespace/lib.ts.lint +++ b/test/rules/no-restricted-globals/namespace.ts.lint @@ -1,8 +1,10 @@ -import "./core" - 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; +} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/namespace/core.d.ts b/test/rules/no-restricted-globals/namespace/core.d.ts deleted file mode 100644 index 45920f312e4..00000000000 --- a/test/rules/no-restricted-globals/namespace/core.d.ts +++ /dev/null @@ -1,3 +0,0 @@ -declare namespace A { - export var name: string; -} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/namespace/tsconfig.json b/test/rules/no-restricted-globals/namespace/tsconfig.json deleted file mode 100644 index 3039d056863..00000000000 --- a/test/rules/no-restricted-globals/namespace/tsconfig.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "compilerOptions": { - "module": "commonjs", - "lib": [ - "dom" - ] - } -} diff --git a/test/rules/no-restricted-globals/namespace/tslint.json b/test/rules/no-restricted-globals/namespace/tslint.json deleted file mode 100644 index d402b8d9d83..00000000000 --- a/test/rules/no-restricted-globals/namespace/tslint.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "rules": { - "no-restricted-globals": [ - true, - "name", - "length", - "event" - ] - } -} diff --git a/test/rules/no-restricted-globals/lib-dom/test.ts.lint b/test/rules/no-restricted-globals/test.ts.lint similarity index 72% rename from test/rules/no-restricted-globals/lib-dom/test.ts.lint rename to test/rules/no-restricted-globals/test.ts.lint index 83c0329c12a..b52f5c47a0e 100644 --- a/test/rules/no-restricted-globals/lib-dom/test.ts.lint +++ b/test/rules/no-restricted-globals/test.ts.lint @@ -15,5 +15,7 @@ import { name } from "./foo"; import "./custom-global" console.log(name); -console.log(myGlobal); - ~~~~~~~~ [Unexpected global variable 'myGlobal'. Use a local parameter or variable instead.] +console.log(badGlobal); + ~~~~~~~~~ [Unexpected global variable 'badGlobal'. Use a local parameter or variable instead.] + +console.log(goodGlobal); \ No newline at end of file diff --git a/test/rules/no-restricted-globals/lib-dom/tsconfig.json b/test/rules/no-restricted-globals/tsconfig.json similarity index 100% rename from test/rules/no-restricted-globals/lib-dom/tsconfig.json rename to test/rules/no-restricted-globals/tsconfig.json diff --git a/test/rules/no-restricted-globals/lib-dom/tslint.json b/test/rules/no-restricted-globals/tslint.json similarity index 86% rename from test/rules/no-restricted-globals/lib-dom/tslint.json rename to test/rules/no-restricted-globals/tslint.json index 9f80463e279..4c2950318f4 100644 --- a/test/rules/no-restricted-globals/lib-dom/tslint.json +++ b/test/rules/no-restricted-globals/tslint.json @@ -5,7 +5,7 @@ "name", "length", "event", - "myGlobal" + "badGlobal" ] } } From 78bf5eb97c357eddf9eedd94de3f4e5d3dbbacf5 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Sat, 28 Apr 2018 00:06:41 +0800 Subject: [PATCH 07/11] update doc --- src/rules/noRestrictedGlobalsRule.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index fbc1dd23cb1..a8393415986 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -25,13 +25,23 @@ export class Rule extends Lint.Rules.TypedRule { 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. For instance, 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. + some of those. `, optionsDescription: "This rule takes a list of strings, where each string is a global to be restricted.", options: { From bc9938401ec96580dbe2743021e4c898b9fbb0c9 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Sat, 28 Apr 2018 21:39:28 +0800 Subject: [PATCH 08/11] add default list --- src/rules/noRestrictedGlobalsRule.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index a8393415986..249da7f365e 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -43,7 +43,10 @@ export class Rule extends Lint.Rules.TypedRule { a set of global variables by enabling an environment, but still want to disallow some of those. `, - optionsDescription: "This rule takes a list of strings, where each string is a global to be restricted.", + 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"}, @@ -67,7 +70,8 @@ export class Rule extends Lint.Rules.TypedRule { } public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { - const bannedGlobals = new Set(this.ruleArguments as string[]); + const bannedList = this.ruleArguments.length ? this.ruleArguments : ["event", "name", "length"]; + const bannedGlobals = new Set(bannedList); if (sourceFile.isDeclarationFile) { return []; } else { From c018aaf913db1d8b4ba43f2ed7be1248bef86372 Mon Sep 17 00:00:00 2001 From: Zzzen <843968788@qq.com> Date: Tue, 26 Jun 2018 18:21:38 +0800 Subject: [PATCH 09/11] fix lint error --- src/rules/noRestrictedGlobalsRule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index 249da7f365e..8916aecb7ff 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -70,7 +70,7 @@ export class Rule extends Lint.Rules.TypedRule { } public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { - const bannedList = this.ruleArguments.length ? this.ruleArguments : ["event", "name", "length"]; + const bannedList = this.ruleArguments.length > 0 ? this.ruleArguments : ["event", "name", "length"]; const bannedGlobals = new Set(bannedList); if (sourceFile.isDeclarationFile) { return []; @@ -83,8 +83,8 @@ export class Rule extends Lint.Rules.TypedRule { function walk(ctx: Lint.WalkContext>, checker: ts.TypeChecker): void { return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void { switch (node.kind) { + case ts.SyntaxKind.ObjectBindingPattern: case ts.SyntaxKind.TypeReference: - // Ignore types. return; case ts.SyntaxKind.PropertyAccessExpression: // Ignore `y` in `x.y`, but recurse to `x`. From 109dfa6f0118c6fdb0dd52171605247a10da0fa8 Mon Sep 17 00:00:00 2001 From: Zen <843968788@qq.com> Date: Wed, 7 Nov 2018 23:21:12 +0800 Subject: [PATCH 10/11] remove outdated todo comment --- src/rules/noRestrictedGlobalsRule.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index 8916aecb7ff..1c507f336bf 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -19,7 +19,6 @@ import * as ts from "typescript"; import * as Lint from "../index"; -// TODO: add a default list export class Rule extends Lint.Rules.TypedRule { /* tslint:disable:object-literal-sort-keys */ public static metadata: Lint.IRuleMetadata = { From 1bcd788cde196dcbe36ccb3b120ab7ad77c4cfef Mon Sep 17 00:00:00 2001 From: Zzzen <843968788@qq.com> Date: Fri, 4 Jan 2019 21:21:56 +0800 Subject: [PATCH 11/11] add tests and recurse more nodes --- src/rules/noRestrictedGlobalsRule.ts | 35 +++++++----- test/rules/no-restricted-globals/misc.ts.lint | 21 ------- test/rules/no-restricted-globals/test.ts.lint | 56 ++++++++++++++++++- 3 files changed, 76 insertions(+), 36 deletions(-) delete mode 100644 test/rules/no-restricted-globals/misc.ts.lint diff --git a/src/rules/noRestrictedGlobalsRule.ts b/src/rules/noRestrictedGlobalsRule.ts index 1c507f336bf..a0a88a17316 100644 --- a/src/rules/noRestrictedGlobalsRule.ts +++ b/src/rules/noRestrictedGlobalsRule.ts @@ -15,8 +15,8 @@ * limitations under the License. */ +import { isBindingElement, isComputedPropertyName, isIdentifier, isPropertyAccessExpression } from "tsutils"; import * as ts from "typescript"; - import * as Lint from "../index"; export class Rule extends Lint.Rules.TypedRule { @@ -48,7 +48,7 @@ export class Rule extends Lint.Rules.TypedRule { `, options: { type: "list", - items: {type: "string"}, + items: { type: "string" }, }, optionExamples: [ [ @@ -80,18 +80,25 @@ export class Rule extends Lint.Rules.TypedRule { } function walk(ctx: Lint.WalkContext>, checker: ts.TypeChecker): void { - return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void { - switch (node.kind) { - 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: - return ts.forEachChild(node, recur); + return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node | undefined): void { + if (node == undefined) { + return; + } + + // Handles `const { bar, length: { x: y = () => event } } = foo` + if (isBindingElement(node)) { + recur(node.initializer); + recur(node.name); + if (node.propertyName != undefined && isComputedPropertyName(node.propertyName)) { + recur(node.propertyName); + } + } else if (isPropertyAccessExpression(node)) { + // Ignore `y` in `x.y`, but recurse to `x`. + recur(node.expression); + } else if (isIdentifier(node)) { + checkIdentifier(node); + } else { + ts.forEachChild(node, recur); } }); diff --git a/test/rules/no-restricted-globals/misc.ts.lint b/test/rules/no-restricted-globals/misc.ts.lint deleted file mode 100644 index 328f6bc07be..00000000000 --- a/test/rules/no-restricted-globals/misc.ts.lint +++ /dev/null @@ -1,21 +0,0 @@ -function typeTest() { - 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 {} -} \ No newline at end of file diff --git a/test/rules/no-restricted-globals/test.ts.lint b/test/rules/no-restricted-globals/test.ts.lint index b52f5c47a0e..2fb6b4c5521 100644 --- a/test/rules/no-restricted-globals/test.ts.lint +++ b/test/rules/no-restricted-globals/test.ts.lint @@ -18,4 +18,58 @@ console.log(name); console.log(badGlobal); ~~~~~~~~~ [Unexpected global variable 'badGlobal'. Use a local parameter or variable instead.] -console.log(goodGlobal); \ No newline at end of file +console.log(goodGlobal); + +let { foo = event } = bar + ~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.] + +let { foo = (() => event)() } = bar; + ~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.] + +let foo: typeof event; + ~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.] + +function rest() { + const { bar, ...event } = foo; +} + +function nested() { + const { bar, event: { x: y } } = foo; +} + +function initializer() { + const { bar, length = () => event } = foo; + ~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.] +} + +function nestedInitializer() { + const { bar, length: { x: y = () => event } } = foo; + ~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.] +} + +function computedProperty() { + const { [event.type]: x } = foo; + ~~~~~ [Unexpected global variable 'event'. Use a local parameter or variable instead.] +} + +function typeTest() { + 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 {} +} \ No newline at end of file