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/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/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 new file mode 100644 index 00000000000..b18a5cbcb74 --- /dev/null +++ b/src/rules/staticThisRule.ts @@ -0,0 +1,84 @@ +/** + * @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"; +import { Replacement } from "../language/rule/rule"; + +import { codeExamples } from "./code-examples/staticThis.examples"; + +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.", + hasFix: true, + 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 */ + + 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); + } +} + +function walk(ctx: Lint.WalkContext) { + let currentParentClass: ts.ClassLikeDeclaration | undefined; + + const cb = (node: ts.Node): void => { + const originalParentClass = currentParentClass; + + if ( + utils.isClassLikeDeclaration(node.parent) && + utils.hasModifier(node.modifiers, ts.SyntaxKind.StaticKeyword) + ) { + currentParentClass = node.parent; + ts.forEachChild(node, cb); + currentParentClass = originalParentClass; + } + + if (node.kind === ts.SyntaxKind.ThisKeyword && currentParentClass !== undefined) { + let fix: Replacement | undefined; + if (currentParentClass.name !== undefined) { + fix = Lint.Replacement.replaceNode(node, currentParentClass.name.text); + } + + ctx.addFailureAtNode(node, Rule.FAILURE_STRING, fix); + return; + } + + ts.forEachChild(node, cb); + }; + + ts.forEachChild(ctx.sourceFile, cb); +} 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. diff --git a/test/rules/static-this/test.ts.fix b/test/rules/static-this/test.ts.fix new file mode 100644 index 00000000000..98510a2d587 --- /dev/null +++ b/test/rules/static-this/test.ts.fix @@ -0,0 +1,65 @@ +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; + } + + private _value = 5; + + getValue() { + 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; + } +} + +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 new file mode 100644 index 00000000000..fd5a7a71d0b --- /dev/null +++ b/test/rules/static-this/test.ts.lint @@ -0,0 +1,75 @@ +class StaticThis { + static value = 0; + static staticValue = this.value; + ~~~~ [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; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + } + + private _value = 5; + + getValue() { + 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; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + + static getTypeOf() { + return this; + ~~~~ [Use the parent class name instead of `this` when in a `static` context.] + } +} + +const AnonymClass = class { + static getTypeOf() { + return this; + ~~~~ [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; + ~~~~ [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.] + } +} diff --git a/test/rules/static-this/tslint.json b/test/rules/static-this/tslint.json new file mode 100644 index 00000000000..76bded512b3 --- /dev/null +++ b/test/rules/static-this/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "static-this": true + } +}