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

Implement static-this rule (#4428) #4475

Merged
merged 11 commits into from
Feb 25, 2019
1 change: 1 addition & 0 deletions src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/language/rule/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]) {
Expand All @@ -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) {
Expand Down
50 changes: 50 additions & 0 deletions src/rules/code-examples/staticThis.examples.ts
Original file line number Diff line number Diff line change
@@ -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()}\`
}
}
`,
},
];
84 changes: 84 additions & 0 deletions src/rules/staticThisRule.ts
Original file line number Diff line number Diff line change
@@ -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.",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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<void>) {
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);
}
6 changes: 3 additions & 3 deletions src/rules/unifiedSignaturesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
65 changes: 65 additions & 0 deletions test/rules/static-this/test.ts.fix
Original file line number Diff line number Diff line change
@@ -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;
}
}
75 changes: 75 additions & 0 deletions test/rules/static-this/test.ts.lint
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
class StaticThis {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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() {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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 {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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 {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
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.]
}
}
5 changes: 5 additions & 0 deletions test/rules/static-this/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"static-this": true
}
}