Skip to content

Commit

Permalink
feat: no-this-in-static (#73)
Browse files Browse the repository at this point in the history
New rule that forbid the use of the keywords `this` and `super` in
static methods.

Useful to prevent bugs like aws/aws-cdk#32454.
  • Loading branch information
otaviomacedo authored Dec 20, 2024
1 parent 21a2d6b commit ac2f54d
Show file tree
Hide file tree
Showing 12 changed files with 255 additions and 0 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ Eslint plugin for the CDK repository. Contains rules that need to be applied spe

* `no-throw-default-error`: Forbid throwing the default JavaScript error type. Instead a custom typed error should be thrown.

* `no-this-in-static`: Forbid the use of the keywords `this` and `super` in
static methods.

## How to use these rules

Import the plugin and declare rules with the `@cdklabs` prefix:
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ export const rules = {
'no-invalid-path': require('./rules/no-invalid-path'),
'no-throw-default-error': require('./rules/no-throw-default-error'),
'promiseall-no-unbounded-parallelism': require('./rules/promiseall-no-unbounded-parallelism'),
'no-this-in-static': require('./rules/no-this-in-static'),
};
151 changes: 151 additions & 0 deletions src/rules/no-this-in-static.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/**
* Adapted from https://github.com/mysticatea/eslint-plugin/blob/v13.0.0/tests/lib/rules/no-this-in-static.js
* @author Toru Nagashima
* @copyright 2016 Toru Nagashima. All rights reserved.
* MIT License
*/

import { Rule } from 'eslint';
import type { ClassDeclaration, FunctionDeclaration, FunctionExpression, Super, ThisExpression } from 'estree';
import NodeParentExtension = Rule.NodeParentExtension;

export const meta = {
fixable: true,
};

export function create(context: Rule.RuleContext): Rule.NodeListener {
const sourceCode = context.sourceCode;
const staticFunctionStack: boolean[] = [];
const classDeclarations: Map<string, ClassDeclaration> = new Map();

function isStaticMethod(node: any): boolean {
return (
node.type === 'FunctionExpression' &&
node.parent.type === 'MethodDefinition' &&
node.parent.static === true
);
}

function enterFunction(node: (FunctionDeclaration | FunctionExpression) & NodeParentExtension) {
staticFunctionStack.push(isStaticMethod(node));
}

function exitFunction() {
staticFunctionStack.pop();
}

function reportIfStatic(node: (ThisExpression | Super) & NodeParentExtension) {
const currentMethodIsStatic = staticFunctionStack.length > 0 && staticFunctionStack[staticFunctionStack.length - 1];

if (currentMethodIsStatic) {
context.report({
node,
loc: node.loc ?? { line: 0, column: 0 },
message: "'{{type}}' keyword found in a static method. Replace it with the class name.",
data: { type: sourceCode.getText(node) },
fix(fixer: Rule.RuleFixer) {
if (node.type === 'Super') {
const hierarchy = superClasses(findClassDeclaration(node));
const memberName = findMemberName(node);

if (memberName == null) {
return null;
}

const superDecl = hierarchy.find(hasStaticMemberCalled(memberName));
return superDecl ? fixer.replaceText(node, superDecl.id.name) : null;
}

if (node.type === 'ThisExpression') {
const decl = findClassDeclaration(node);
return decl ? fixer.replaceText(node, decl.id.name) : null;
}

return null;
},
});
}
}

function superClasses(node: ClassDeclaration | null): ClassDeclaration[] {
if (node == null) {
return [];
}

const result: ClassDeclaration[] = [];

let superClass = node.superClass;

while (superClass != null) {
if (superClass.type === 'Identifier') {
const className = superClass.name;
const decl = classDeclarations.get(className)!;
result.push(decl);
superClass = classDeclarations.get(className)?.superClass;
}
}

return result;
}

function enterClass(node: ClassDeclaration & NodeParentExtension) {
classDeclarations.set(node.id.name, node);
}

function findClassDeclaration(node: NodeParentExtension): ClassDeclaration | null {
let parent = node.parent;

while (parent != null) {
if (parent.type === 'ClassDeclaration') {
return parent;
}

parent = parent.parent;
}

return null;
}

function findMemberName(node: NodeParentExtension): string | null {
let parent = node.parent;

while (parent != null) {
if (
parent.type == 'MemberExpression'
&& parent.property.type === 'Identifier'
) {
return parent.property.name;
}

if (
parent.type === 'CallExpression'
&& parent.callee.type === 'MemberExpression'
&& parent.callee.property.type === 'Identifier'
) {
return parent.callee.property.name;
}

parent = parent.parent;
}

return null;
}

const hasStaticMemberCalled = (name: string) => (decl: ClassDeclaration) => {
return decl.body.body.some((node) => {
return (node.type === 'MethodDefinition' || node.type === 'PropertyDefinition')
&& node.static && node.key.type === 'Identifier'
&& node.key.name === name;
});
};

return {
'FunctionDeclaration': enterFunction,
'FunctionExpression': enterFunction,
'FunctionDeclaration:exit': exitFunction,
'FunctionExpression:exit': exitFunction,
'ThisExpression': reportIfStatic,
'Super': reportIfStatic,
'ClassDeclaration': enterClass,
};
}
6 changes: 6 additions & 0 deletions test/rules/fixtures/no-this-in-static/eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
plugins: ['local'],
rules: {
'local/no-this-in-static': [ 'error' ],
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class Super3 {
static propOneHop = 'abc';
static propThreeHops = 'abc';
}

class Super2 extends Super3 {
static oneHop() {
}

static twoHops() {
}
}

class Super extends Super2 {
static propOneHop = 'abc';
static oneHop() {
}
}

class Foo extends Super {
static baz() {
Super.oneHop();
Super2.twoHops();
return Super.propOneHop + Super3.propThreeHops;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class Super3 {
static propOneHop = 'abc';
static propThreeHops = 'abc';
}

class Super2 extends Super3 {
static oneHop() {
}

static twoHops() {
}
}

class Super extends Super2 {
static propOneHop = 'abc';
static oneHop() {
}
}

class Foo extends Super {
static baz() {
super.oneHop();
super.twoHops();
return super.propOneHop + super.propThreeHops;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Foo {
static prop = 'abc';

static bar() {
}

static baz() {
Foo.bar();
return Foo.prop;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Foo {
static prop = 'abc';

static bar() {
}

static baz() {
this.bar();
return this.prop;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class A {
x: string;
constructor(x: string) {
this.x = x;
}
}
6 changes: 6 additions & 0 deletions test/rules/fixtures/no-this-in-static/this-in-constructor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class A {
x: string;
constructor(x: string) {
this.x = x;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class A {
private x = 'foo';
foo() { return this.x; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class A {
private x = 'foo';
foo() { return this.x; }
}

0 comments on commit ac2f54d

Please sign in to comment.