From 5893baded1e7a7e422adf2d38c7ecd6c753cda43 Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Tue, 22 Jan 2019 00:16:51 +0300 Subject: [PATCH 01/11] Implement static-this rule (#4428) --- src/configs/all.ts | 1 + src/rules/staticThisRule.ts | 91 +++++++++++++++++++++++++++++ test/rules/static-this/test.ts.lint | 14 +++++ test/rules/static-this/tslint.json | 5 ++ 4 files changed, 111 insertions(+) create mode 100644 src/rules/staticThisRule.ts create mode 100644 test/rules/static-this/test.ts.lint create mode 100644 test/rules/static-this/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index f59981ee30f..53353d216da 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -145,6 +145,7 @@ export const rules = { "prefer-conditional-expression": true, radix: true, "restrict-plus-operands": true, + "static-this": true, "strict-boolean-expressions": true, "strict-type-predicates": true, "switch-default": true, diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts new file mode 100644 index 00000000000..6a01061dbbc --- /dev/null +++ b/src/rules/staticThisRule.ts @@ -0,0 +1,91 @@ +/** + * @license + * Copyright 2019 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 utils from "tsutils"; +import * as ts from "typescript"; + +import * as Lint from "../index"; + +export class Rule extends Lint.Rules.AbstractRule { + /* tslint:disable:object-literal-sort-keys */ + public static metadata: Lint.IRuleMetadata = { + ruleName: "static-this", + description: "Ban the use of `this` in static methods.", + options: null, + optionsDescription: "", + optionExamples: [true], + type: "functionality", + typescriptOnly: false, + }; + /* tslint:enable:object-literal-sort-keys */ + + public static FAILURE_STRING = "Unallowed `this` usage in static context"; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new StaticThisWalker(sourceFile, this.getOptions())); + } +} + +const enum ParentType { + None, + Class, + ClassEntity, +} + +class StaticThisWalker extends Lint.RuleWalker { + public walk(sourceFile: ts.SourceFile) { + let currentParent: ParentType = ParentType.None; + + const cb = (node: ts.Node): void => { + const originalParent = currentParent; + + switch (node.kind) { + case ts.SyntaxKind.ClassDeclaration: + case ts.SyntaxKind.ClassExpression: + currentParent = ParentType.Class; + ts.forEachChild(node, cb); + currentParent = originalParent; + return; + + case ts.SyntaxKind.MethodDeclaration: + case ts.SyntaxKind.GetAccessor: + case ts.SyntaxKind.SetAccessor: + case ts.SyntaxKind.PropertyDeclaration: + case ts.SyntaxKind.FunctionDeclaration: + case ts.SyntaxKind.FunctionExpression: + if ( + ParentType.Class === currentParent && + utils.hasModifier(node.modifiers, ts.SyntaxKind.StaticKeyword) + ) { + currentParent = ParentType.ClassEntity; + ts.forEachChild(node, cb); + currentParent = originalParent; + } + return; + + case ts.SyntaxKind.ThisKeyword: + if (ParentType.ClassEntity === currentParent) { + return this.addFailureAtNode(node, Rule.FAILURE_STRING); + } + } + + ts.forEachChild(node, cb); + }; + + return ts.forEachChild(sourceFile, cb); + } +} diff --git a/test/rules/static-this/test.ts.lint b/test/rules/static-this/test.ts.lint new file mode 100644 index 00000000000..7713f919576 --- /dev/null +++ b/test/rules/static-this/test.ts.lint @@ -0,0 +1,14 @@ +class StaticThis { + static const staticValue = this.value; + ~~~~ [Unallowed `this` usage in static context] + static getInstance() { + return this; + ~~~~ [Unallowed `this` usage in static context] + } + + const value = 5; + + getValue() { + return this.value; + } +} diff --git a/test/rules/static-this/tslint.json b/test/rules/static-this/tslint.json new file mode 100644 index 00000000000..79bf2326026 --- /dev/null +++ b/test/rules/static-this/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "static-this": 1 + } +} From c5adf40fcdca1c84c031290976bad559ff70bdcc Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 27 Jan 2019 22:34:45 +0300 Subject: [PATCH 02/11] [static-this] Implement auto-fixed --- src/rules/staticThisRule.ts | 45 +++++++++++++++++++++++------ test/rules/static-this/test.ts.fix | 13 +++++++++ test/rules/static-this/test.ts.lint | 5 ++-- 3 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 test/rules/static-this/test.ts.fix diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index 6a01061dbbc..d0d3543821f 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -25,6 +25,7 @@ export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { ruleName: "static-this", description: "Ban the use of `this` in static methods.", + hasFix: true, options: null, optionsDescription: "", optionExamples: [true], @@ -33,7 +34,15 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_STRING = "Unallowed `this` usage in static context"; + public static FAILURE_STRING = (className?: string) => { + let message = + "`this` usage in static context could be a potential reason of runtime errors."; + if (undefined !== className) { + message = "`this` should be replaced with `" + className + "`."; + } + + return message; + }; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithWalker(new StaticThisWalker(sourceFile, this.getOptions())); @@ -49,18 +58,27 @@ const enum ParentType { class StaticThisWalker extends Lint.RuleWalker { public walk(sourceFile: ts.SourceFile) { let currentParent: ParentType = ParentType.None; + let currentClassName: string | undefined; const cb = (node: ts.Node): void => { const originalParent = currentParent; + const originalClassName = currentClassName; - switch (node.kind) { - case ts.SyntaxKind.ClassDeclaration: - case ts.SyntaxKind.ClassExpression: - currentParent = ParentType.Class; - ts.forEachChild(node, cb); - currentParent = originalParent; - return; + if (utils.isClassLikeDeclaration(node)) { + currentParent = ParentType.Class; + + if (undefined !== node.name) { + currentClassName = node.name!.text; + } + ts.forEachChild(node, cb); + + currentParent = originalParent; + currentClassName = originalClassName; + return; + } + + switch (node.kind) { case ts.SyntaxKind.MethodDeclaration: case ts.SyntaxKind.GetAccessor: case ts.SyntaxKind.SetAccessor: @@ -79,7 +97,16 @@ class StaticThisWalker extends Lint.RuleWalker { case ts.SyntaxKind.ThisKeyword: if (ParentType.ClassEntity === currentParent) { - return this.addFailureAtNode(node, Rule.FAILURE_STRING); + let fix: Lint.Fix | undefined; + if (undefined !== currentClassName) { + fix = Lint.Replacement.replaceNode(node, currentClassName); + } + + return this.addFailureAtNode( + node, + Rule.FAILURE_STRING(currentClassName), + fix, + ); } } diff --git a/test/rules/static-this/test.ts.fix b/test/rules/static-this/test.ts.fix new file mode 100644 index 00000000000..ad2755c6dc0 --- /dev/null +++ b/test/rules/static-this/test.ts.fix @@ -0,0 +1,13 @@ +class StaticThis { + static const staticValue = StaticThis.value; + + static getInstance() { + return StaticThis; + } + + const value = 5; + + getValue() { + return this.value; + } +} diff --git a/test/rules/static-this/test.ts.lint b/test/rules/static-this/test.ts.lint index 7713f919576..2fdbca45ce9 100644 --- a/test/rules/static-this/test.ts.lint +++ b/test/rules/static-this/test.ts.lint @@ -1,9 +1,10 @@ class StaticThis { static const staticValue = this.value; - ~~~~ [Unallowed `this` usage in static context] + ~~~~ [`this` should be replaced with `StaticThis`.] + static getInstance() { return this; - ~~~~ [Unallowed `this` usage in static context] + ~~~~ [`this` should be replaced with `StaticThis`.] } const value = 5; From a3e14cd54120588696625ef4b41d8ffc4aab21dc Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 27 Jan 2019 22:40:19 +0300 Subject: [PATCH 03/11] [static-this] Enable rule correctly --- test/rules/static-this/tslint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rules/static-this/tslint.json b/test/rules/static-this/tslint.json index 79bf2326026..76bded512b3 100644 --- a/test/rules/static-this/tslint.json +++ b/test/rules/static-this/tslint.json @@ -1,5 +1,5 @@ { "rules": { - "static-this": 1 + "static-this": true } } From 90632015ec4b70f8c200b56bef02fd7f8042df66 Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Mon, 28 Jan 2019 00:15:50 +0300 Subject: [PATCH 04/11] [static-this] Upgrade test cases --- test/rules/static-this/test.ts.fix | 38 ++++++++++++++++++++++--- test/rules/static-this/test.ts.lint | 44 +++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/test/rules/static-this/test.ts.fix b/test/rules/static-this/test.ts.fix index ad2755c6dc0..e1759a7e29b 100644 --- a/test/rules/static-this/test.ts.fix +++ b/test/rules/static-this/test.ts.fix @@ -1,13 +1,43 @@ class StaticThis { - static const staticValue = StaticThis.value; + static value = 0; + static staticValue = StaticThis.value; - static getInstance() { + static getTypeOf() { return StaticThis; } - const value = 5; + private _value = 5; getValue() { - return this.value; + return this._value; + } + + get length() { + return this._value; + } + + set length(value) { + this._value = value; + } +} + +class Child extends StaticThis { + static word = 'Hello'; + static staticValue = Child.word; + + static getTypeOf() { + return Child; + } +} + +const AnonymClass = class { + static getTypeOf() { + return this; + } +} + +const NamedClass = class Name { + static getTypeOf() { + return Name; } } diff --git a/test/rules/static-this/test.ts.lint b/test/rules/static-this/test.ts.lint index 2fdbca45ce9..aaa5b63a77d 100644 --- a/test/rules/static-this/test.ts.lint +++ b/test/rules/static-this/test.ts.lint @@ -1,15 +1,49 @@ class StaticThis { - static const staticValue = this.value; - ~~~~ [`this` should be replaced with `StaticThis`.] + static value = 0; + static staticValue = this.value; + ~~~~ [`this` should be replaced with `StaticThis`.] - static getInstance() { + static getTypeOf() { return this; ~~~~ [`this` should be replaced with `StaticThis`.] } - const value = 5; + private _value = 5; getValue() { - return this.value; + return this._value; + } + + get length() { + return this._value; + } + + set length(value) { + this._value = value; + } +} + +class Child extends StaticThis { + static word = 'Hello'; + static staticValue = this.word; + ~~~~ [`this` should be replaced with `Child`.] + + static getTypeOf() { + return this; + ~~~~ [`this` should be replaced with `Child`.] + } +} + +const AnonymClass = class { + static getTypeOf() { + return this; + ~~~~ [`this` usage in static context could be a potential reason of runtime errors.] + } +} + +const NamedClass = class Name { + static getTypeOf() { + return this; + ~~~~ [`this` should be replaced with `Name`.] } } From ad535fac152029dc16690989704794faab828d3a Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Mon, 28 Jan 2019 00:33:10 +0300 Subject: [PATCH 05/11] [static-this] Fix linting issues --- src/language/rule/rule.ts | 4 ++-- src/rules/staticThisRule.ts | 9 +++++---- src/rules/unifiedSignaturesRule.ts | 6 +++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts index df4ce2c1f26..a554e00cbd0 100644 --- a/src/language/rule/rule.ts +++ b/src/language/rule/rule.ts @@ -166,7 +166,7 @@ export interface ReplacementJson { } export class Replacement { public static applyFixes(content: string, fixes: Fix[]): string { - return this.applyAll(content, flatMap(fixes, arrayify)); + return Replacement.applyAll(content, flatMap(fixes, arrayify)); } public static applyAll(content: string, replacements: Replacement[]) { @@ -180,7 +180,7 @@ export class Replacement { text: string, sourceFile?: ts.SourceFile, ): Replacement { - return this.replaceFromTo(node.getStart(sourceFile), node.getEnd(), text); + return Replacement.replaceFromTo(node.getStart(sourceFile), node.getEnd(), text); } public static replaceFromTo(start: number, end: number, text: string) { diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index d0d3543821f..39c402dbb22 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -37,7 +37,8 @@ export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = (className?: string) => { let message = "`this` usage in static context could be a potential reason of runtime errors."; - if (undefined !== className) { + if (className !== undefined) { + /* tslint:disable-next-line:prefer-template */ message = "`this` should be replaced with `" + className + "`."; } @@ -67,8 +68,8 @@ class StaticThisWalker extends Lint.RuleWalker { if (utils.isClassLikeDeclaration(node)) { currentParent = ParentType.Class; - if (undefined !== node.name) { - currentClassName = node.name!.text; + if (node.name !== undefined) { + currentClassName = node.name.text; } ts.forEachChild(node, cb); @@ -98,7 +99,7 @@ class StaticThisWalker extends Lint.RuleWalker { case ts.SyntaxKind.ThisKeyword: if (ParentType.ClassEntity === currentParent) { let fix: Lint.Fix | undefined; - if (undefined !== currentClassName) { + if (currentClassName !== undefined) { fix = Lint.Replacement.replaceNode(node, currentClassName); } diff --git a/src/rules/unifiedSignaturesRule.ts b/src/rules/unifiedSignaturesRule.ts index cbdf04c3a7f..ab35dd6d77c 100644 --- a/src/rules/unifiedSignaturesRule.ts +++ b/src/rules/unifiedSignaturesRule.ts @@ -38,17 +38,17 @@ export class Rule extends Lint.Rules.AbstractRule { /* tslint:enable:object-literal-sort-keys */ public static FAILURE_STRING_OMITTING_SINGLE_PARAMETER(otherLine?: number) { - return `${this.FAILURE_STRING_START(otherLine)} with an optional parameter.`; + return `${Rule.FAILURE_STRING_START(otherLine)} with an optional parameter.`; } public static FAILURE_STRING_OMITTING_REST_PARAMETER(otherLine?: number) { - return `${this.FAILURE_STRING_START(otherLine)} with a rest parameter.`; + return `${Rule.FAILURE_STRING_START(otherLine)} with a rest parameter.`; } public static FAILURE_STRING_SINGLE_PARAMETER_DIFFERENCE( otherLine: number | undefined, type1: string, type2: string, ) { - return `${this.FAILURE_STRING_START(otherLine)} taking \`${type1} | ${type2}\`.`; + return `${Rule.FAILURE_STRING_START(otherLine)} taking \`${type1} | ${type2}\`.`; } private static FAILURE_STRING_START(otherLine?: number): string { // For only 2 overloads we don't need to specify which is the other one. From 99f1a469da78d31b023027ce6095db412b93e44e Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Thu, 7 Feb 2019 00:36:58 +0300 Subject: [PATCH 06/11] [static-this] Add code examples --- .../code-examples/staticThis.examples.ts | 50 +++++++++++++++++++ src/rules/staticThisRule.ts | 8 +++ 2 files changed, 58 insertions(+) create mode 100644 src/rules/code-examples/staticThis.examples.ts diff --git a/src/rules/code-examples/staticThis.examples.ts b/src/rules/code-examples/staticThis.examples.ts new file mode 100644 index 00000000000..1353ddea57b --- /dev/null +++ b/src/rules/code-examples/staticThis.examples.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2019 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 Lint from "../../index"; + +// tslint:disable: object-literal-sort-keys +export const codeExamples = [ + { + description: "Disallows the `this` keyword usage in static context.", + config: Lint.Utils.dedent` + "rules": { "static-this": true } + `, + pass: Lint.Utils.dedent` + class Pass { + static getName() { + return 'name' + } + + static getFullname() { + return \`full \${Pass.getName()}\` + } + } + `, + fail: Lint.Utils.dedent` + class Fail { + static getName() { + return 'name' + } + + static getFullname() { + return \`full \${this.getName()}\` + } + } + `, + }, +]; diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index 39c402dbb22..811f905503d 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -19,6 +19,7 @@ import * as utils from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +import { codeExamples } from "./code-examples/staticThis.examples"; export class Rule extends Lint.Rules.AbstractRule { /* tslint:disable:object-literal-sort-keys */ @@ -29,8 +30,15 @@ export class Rule extends Lint.Rules.AbstractRule { options: null, optionsDescription: "", optionExamples: [true], + /* tslint:disable:max-line-length */ + rationale: Lint.Utils.dedent` + Static \`this\` usage can be confusing for newcomers. + It can also become imprecise when used with extended classes when a static \`this\` of a parent class no longer specifically refers to the parent class. + `, + /* tslint:enable:max-line-length */ type: "functionality", typescriptOnly: false, + codeExamples, }; /* tslint:enable:object-literal-sort-keys */ From a3306ea61d6eb077e11b8ba1a9ef71d8757795d6 Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 24 Feb 2019 11:56:24 +0300 Subject: [PATCH 07/11] [static-this] Replace outdated walker --- src/rules/staticThisRule.ts | 103 +++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index 811f905503d..dc3cef5ca8f 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -54,7 +54,7 @@ export class Rule extends Lint.Rules.AbstractRule { }; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new StaticThisWalker(sourceFile, this.getOptions())); + return this.applyWithFunction(sourceFile, walk) } } @@ -64,64 +64,69 @@ const enum ParentType { ClassEntity, } -class StaticThisWalker extends Lint.RuleWalker { - public walk(sourceFile: ts.SourceFile) { - let currentParent: ParentType = ParentType.None; - let currentClassName: string | undefined; +function walk(ctx: Lint.WalkContext) { + let currentParent: ParentType = ParentType.None; + let currentClassName: string | undefined; - const cb = (node: ts.Node): void => { - const originalParent = currentParent; - const originalClassName = currentClassName; + const cb = (node: ts.Node): void => { + const originalParent = currentParent; + const originalClassName = currentClassName; - if (utils.isClassLikeDeclaration(node)) { - currentParent = ParentType.Class; + if (utils.isClassLikeDeclaration(node)) { + currentParent = ParentType.Class; - if (node.name !== undefined) { - currentClassName = node.name.text; - } - - ts.forEachChild(node, cb); - - currentParent = originalParent; - currentClassName = originalClassName; - return; + if (node.name !== undefined) { + currentClassName = node.name.text; } - switch (node.kind) { - case ts.SyntaxKind.MethodDeclaration: - case ts.SyntaxKind.GetAccessor: - case ts.SyntaxKind.SetAccessor: - case ts.SyntaxKind.PropertyDeclaration: - case ts.SyntaxKind.FunctionDeclaration: - case ts.SyntaxKind.FunctionExpression: - if ( - ParentType.Class === currentParent && - utils.hasModifier(node.modifiers, ts.SyntaxKind.StaticKeyword) - ) { - currentParent = ParentType.ClassEntity; - ts.forEachChild(node, cb); - currentParent = originalParent; - } - return; + ts.forEachChild(node, cb); - case ts.SyntaxKind.ThisKeyword: - if (ParentType.ClassEntity === currentParent) { - let fix: Lint.Fix | undefined; - if (currentClassName !== undefined) { - fix = Lint.Replacement.replaceNode(node, currentClassName); - } + currentParent = originalParent; + currentClassName = originalClassName; + return; + } - return this.addFailureAtNode( + switch (node.kind) { + case ts.SyntaxKind.MethodDeclaration: + case ts.SyntaxKind.GetAccessor: + case ts.SyntaxKind.SetAccessor: + case ts.SyntaxKind.PropertyDeclaration: + case ts.SyntaxKind.FunctionDeclaration: + case ts.SyntaxKind.FunctionExpression: + if ( + ParentType.Class === currentParent && + utils.hasModifier( + node.modifiers, + ts.SyntaxKind.StaticKeyword + ) + ) { + currentParent = ParentType.ClassEntity; + ts.forEachChild(node, cb); + currentParent = originalParent; + } + return; + + case ts.SyntaxKind.ThisKeyword: + if (ParentType.ClassEntity === currentParent) { + let fix: Lint.Fix | undefined; + if (currentClassName !== undefined) { + fix = Lint.Replacement.replaceNode( node, - Rule.FAILURE_STRING(currentClassName), - fix, + currentClassName ); } - } - ts.forEachChild(node, cb); - }; + ctx.addFailureAtNode( + node, + Rule.FAILURE_STRING(currentClassName), + fix, + ); + return; + } + } - return ts.forEachChild(sourceFile, cb); - } + ts.forEachChild(node, cb); + }; + + return ts.forEachChild(ctx.sourceFile, cb); } From a923d0cf1b39312c271f16b8333499a87f12446f Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 24 Feb 2019 12:27:24 +0300 Subject: [PATCH 08/11] [static-this] Simplify walker code --- src/rules/staticThisRule.ts | 78 ++++++++++--------------------------- 1 file changed, 20 insertions(+), 58 deletions(-) diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index dc3cef5ca8f..08f0880ddc8 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -54,79 +54,41 @@ export class Rule extends Lint.Rules.AbstractRule { }; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, walk) + return this.applyWithFunction(sourceFile, walk); } } -const enum ParentType { - None, - Class, - ClassEntity, -} - function walk(ctx: Lint.WalkContext) { - let currentParent: ParentType = ParentType.None; - let currentClassName: string | undefined; + let currentParentClass: ts.ClassLikeDeclaration | undefined; const cb = (node: ts.Node): void => { - const originalParent = currentParent; - const originalClassName = currentClassName; - - if (utils.isClassLikeDeclaration(node)) { - currentParent = ParentType.Class; - - if (node.name !== undefined) { - currentClassName = node.name.text; - } + const originalParentClass = currentParentClass; + if ( + utils.isClassLikeDeclaration(node.parent) && + utils.hasModifier(node.modifiers, ts.SyntaxKind.StaticKeyword) + ) { + currentParentClass = node.parent; ts.forEachChild(node, cb); - - currentParent = originalParent; - currentClassName = originalClassName; - return; + currentParentClass = originalParentClass; } - switch (node.kind) { - case ts.SyntaxKind.MethodDeclaration: - case ts.SyntaxKind.GetAccessor: - case ts.SyntaxKind.SetAccessor: - case ts.SyntaxKind.PropertyDeclaration: - case ts.SyntaxKind.FunctionDeclaration: - case ts.SyntaxKind.FunctionExpression: - if ( - ParentType.Class === currentParent && - utils.hasModifier( - node.modifiers, - ts.SyntaxKind.StaticKeyword - ) - ) { - currentParent = ParentType.ClassEntity; - ts.forEachChild(node, cb); - currentParent = originalParent; - } - return; + if (node.kind === ts.SyntaxKind.ThisKeyword && currentParentClass) { + let className: string | undefined; + if (currentParentClass.name !== undefined) { + className = currentParentClass.name.text; + } - case ts.SyntaxKind.ThisKeyword: - if (ParentType.ClassEntity === currentParent) { - let fix: Lint.Fix | undefined; - if (currentClassName !== undefined) { - fix = Lint.Replacement.replaceNode( - node, - currentClassName - ); - } + const fix = className + ? Lint.Replacement.replaceNode(node, className) + : undefined; - ctx.addFailureAtNode( - node, - Rule.FAILURE_STRING(currentClassName), - fix, - ); - return; - } + ctx.addFailureAtNode(node, Rule.FAILURE_STRING(className), fix); + return; } ts.forEachChild(node, cb); }; - return ts.forEachChild(ctx.sourceFile, cb); + ts.forEachChild(ctx.sourceFile, cb); } From 507a4a7b169c085e5be0ea0db91944d1b1f48fc4 Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 24 Feb 2019 17:22:31 +0300 Subject: [PATCH 09/11] [static-this] Update FAILURE_STRING --- src/rules/staticThisRule.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index 08f0880ddc8..aade4779e71 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -42,16 +42,7 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_STRING = (className?: string) => { - let message = - "`this` usage in static context could be a potential reason of runtime errors."; - if (className !== undefined) { - /* tslint:disable-next-line:prefer-template */ - message = "`this` should be replaced with `" + className + "`."; - } - - return message; - }; + public static FAILURE_STRING = 'Use the parent class name instead of `this` when in a `static` context.'; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walk); @@ -83,7 +74,7 @@ function walk(ctx: Lint.WalkContext) { ? Lint.Replacement.replaceNode(node, className) : undefined; - ctx.addFailureAtNode(node, Rule.FAILURE_STRING(className), fix); + ctx.addFailureAtNode(node, Rule.FAILURE_STRING, fix); return; } From dbe97dc11c99e2479d439e487655bbf7c5d6bcdb Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 24 Feb 2019 17:25:15 +0300 Subject: [PATCH 10/11] [static-this] Add test cases --- test/rules/static-this/test.ts.fix | 22 +++++++++++++++++ test/rules/static-this/test.ts.lint | 38 ++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/test/rules/static-this/test.ts.fix b/test/rules/static-this/test.ts.fix index e1759a7e29b..98510a2d587 100644 --- a/test/rules/static-this/test.ts.fix +++ b/test/rules/static-this/test.ts.fix @@ -2,6 +2,16 @@ class StaticThis { static value = 0; static staticValue = StaticThis.value; + static _color = undefined; + + static get color() { + return StaticThis._color; + } + + static set color(color) { + StaticThis._color = color; + } + static getTypeOf() { return StaticThis; } @@ -36,8 +46,20 @@ const AnonymClass = class { } } +class AnonymClassChild extends AnonymClass { + static getTypeOf() { + return AnonymClassChild; + } +} + const NamedClass = class Name { static getTypeOf() { return Name; } } + +class NamedClassChild extends NamedClass { + static getTypeOf() { + return NamedClassChild; + } +} diff --git a/test/rules/static-this/test.ts.lint b/test/rules/static-this/test.ts.lint index aaa5b63a77d..fd5a7a71d0b 100644 --- a/test/rules/static-this/test.ts.lint +++ b/test/rules/static-this/test.ts.lint @@ -1,11 +1,23 @@ class StaticThis { static value = 0; static staticValue = this.value; - ~~~~ [`this` should be replaced with `StaticThis`.] + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + + static _color = undefined; + + static get color() { + return this._color; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + } + + static set color(color) { + this._color = color; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + } static getTypeOf() { return this; - ~~~~ [`this` should be replaced with `StaticThis`.] + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] } private _value = 5; @@ -26,24 +38,38 @@ class StaticThis { class Child extends StaticThis { static word = 'Hello'; static staticValue = this.word; - ~~~~ [`this` should be replaced with `Child`.] + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] static getTypeOf() { return this; - ~~~~ [`this` should be replaced with `Child`.] + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] } } const AnonymClass = class { static getTypeOf() { return this; - ~~~~ [`this` usage in static context could be a potential reason of runtime errors.] + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + } +} + +class AnonymClassChild extends AnonymClass { + static getTypeOf() { + return this; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] } } const NamedClass = class Name { static getTypeOf() { return this; - ~~~~ [`this` should be replaced with `Name`.] + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + } +} + +class NamedClassChild extends NamedClass { + static getTypeOf() { + return this; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] } } From fe266ec2f1291142b1ba845dd0059c76d624bcac Mon Sep 17 00:00:00 2001 From: Maxim Belsky Date: Sun, 24 Feb 2019 17:59:34 +0300 Subject: [PATCH 11/11] [static-this] Fix lint errors --- src/rules/staticThisRule.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/rules/staticThisRule.ts b/src/rules/staticThisRule.ts index aade4779e71..b18a5cbcb74 100644 --- a/src/rules/staticThisRule.ts +++ b/src/rules/staticThisRule.ts @@ -19,6 +19,8 @@ import * as utils from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; +import { Replacement } from "../language/rule/rule"; + import { codeExamples } from "./code-examples/staticThis.examples"; export class Rule extends Lint.Rules.AbstractRule { @@ -42,7 +44,8 @@ export class Rule extends Lint.Rules.AbstractRule { }; /* tslint:enable:object-literal-sort-keys */ - public static FAILURE_STRING = 'Use the parent class name instead of `this` when in a `static` context.'; + public static FAILURE_STRING = + "Use the parent class name instead of `this` when in a `static` context."; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { return this.applyWithFunction(sourceFile, walk); @@ -64,16 +67,12 @@ function walk(ctx: Lint.WalkContext) { currentParentClass = originalParentClass; } - if (node.kind === ts.SyntaxKind.ThisKeyword && currentParentClass) { - let className: string | undefined; + if (node.kind === ts.SyntaxKind.ThisKeyword && currentParentClass !== undefined) { + let fix: Replacement | undefined; if (currentParentClass.name !== undefined) { - className = currentParentClass.name.text; + fix = Lint.Replacement.replaceNode(node, currentParentClass.name.text); } - const fix = className - ? Lint.Replacement.replaceNode(node, className) - : undefined; - ctx.addFailureAtNode(node, Rule.FAILURE_STRING, fix); return; }