From be952938130bd3892656473252cd0444f240e117 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 18 Jul 2023 19:12:49 +0930 Subject: [PATCH 1/3] feat(eslint-plugin): sync getFunctionHeadLoc implementation with upstream --- .../src/util/getFunctionHeadLoc.ts | 230 ++++++++++++++---- .../explicit-function-return-type.test.ts | 170 ++++++------- .../explicit-module-boundary-types.test.ts | 148 +++++------ 3 files changed, 340 insertions(+), 208 deletions(-) diff --git a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts index ddc004d0397..a8f6bc40afb 100644 --- a/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts +++ b/packages/eslint-plugin/src/util/getFunctionHeadLoc.ts @@ -1,5 +1,9 @@ +// adapted from https://github.com/eslint/eslint/blob/5bdaae205c3a0089ea338b382df59e21d5b06436/lib/rules/utils/ast-utils.js#L1668-L1787 + import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils'; + +import { isArrowToken, isOpeningParenToken } from './astUtils'; type FunctionNode = | TSESTree.ArrowFunctionExpression @@ -7,65 +11,193 @@ type FunctionNode = | TSESTree.FunctionExpression; /** - * Creates a report location for the given function. - * The location only encompasses the "start" of the function, and not the body - * - * eg. - * - * ``` - * function foo(args) {} - * ^^^^^^^^^^^^^^^^^^ - * - * get y(args) {} - * ^^^^^^^^^^^ - * - * const x = (args) => {} - * ^^^^^^^^^ - * ``` + * Gets the `(` token of the given function node. + * @param node The function node to get. + * @param sourceCode The source code object to get tokens. + * @returns `(` token. */ -export function getFunctionHeadLoc( +function getOpeningParenOfParams( node: FunctionNode, sourceCode: TSESLint.SourceCode, -): TSESTree.SourceLocation { - function getLocStart(): TSESTree.Position { - if (node.parent.type === AST_NODE_TYPES.MethodDefinition) { - // return the start location for class method - - if (node.parent.decorators && node.parent.decorators.length > 0) { - // exclude decorators - return sourceCode.getTokenAfter( - node.parent.decorators[node.parent.decorators.length - 1], - )!.loc.start; - } +): TSESTree.Token { + // If the node is an arrow function and doesn't have parens, this returns the identifier of the first param. + if ( + node.type === AST_NODE_TYPES.ArrowFunctionExpression && + node.params.length === 1 + ) { + const argToken = ESLintUtils.nullThrows( + sourceCode.getFirstToken(node.params[0]), + ESLintUtils.NullThrowsReasons.MissingToken('parameter', 'arrow function'), + ); + const maybeParenToken = sourceCode.getTokenBefore(argToken); - return node.parent.loc.start; - } + return maybeParenToken && isOpeningParenToken(maybeParenToken) + ? maybeParenToken + : argToken; + } - if (node.parent.type === AST_NODE_TYPES.Property && node.parent.method) { - // return the start location for object method shorthand - return node.parent.loc.start; - } + // Otherwise, returns paren. + return node.id != null + ? ESLintUtils.nullThrows( + sourceCode.getTokenAfter(node.id, isOpeningParenToken), + ESLintUtils.NullThrowsReasons.MissingToken('id', 'function'), + ) + : ESLintUtils.nullThrows( + sourceCode.getFirstToken(node, isOpeningParenToken), + ESLintUtils.NullThrowsReasons.MissingToken( + 'opening parenthesis', + 'function', + ), + ); +} - // return the start location for a regular function - return node.loc.start; - } +/** + * Gets the location of the given function node for reporting. + * + * - `function foo() {}` + * ^^^^^^^^^^^^ + * - `(function foo() {})` + * ^^^^^^^^^^^^ + * - `(function() {})` + * ^^^^^^^^ + * - `function* foo() {}` + * ^^^^^^^^^^^^^ + * - `(function* foo() {})` + * ^^^^^^^^^^^^^ + * - `(function*() {})` + * ^^^^^^^^^ + * - `() => {}` + * ^^ + * - `async () => {}` + * ^^ + * - `({ foo: function foo() {} })` + * ^^^^^^^^^^^^^^^^^ + * - `({ foo: function() {} })` + * ^^^^^^^^^^^^^ + * - `({ ['foo']: function() {} })` + * ^^^^^^^^^^^^^^^^^ + * - `({ [foo]: function() {} })` + * ^^^^^^^^^^^^^^^ + * - `({ foo() {} })` + * ^^^ + * - `({ foo: function* foo() {} })` + * ^^^^^^^^^^^^^^^^^^ + * - `({ foo: function*() {} })` + * ^^^^^^^^^^^^^^ + * - `({ ['foo']: function*() {} })` + * ^^^^^^^^^^^^^^^^^^ + * - `({ [foo]: function*() {} })` + * ^^^^^^^^^^^^^^^^ + * - `({ *foo() {} })` + * ^^^^ + * - `({ foo: async function foo() {} })` + * ^^^^^^^^^^^^^^^^^^^^^^^ + * - `({ foo: async function() {} })` + * ^^^^^^^^^^^^^^^^^^^ + * - `({ ['foo']: async function() {} })` + * ^^^^^^^^^^^^^^^^^^^^^^^ + * - `({ [foo]: async function() {} })` + * ^^^^^^^^^^^^^^^^^^^^^ + * - `({ async foo() {} })` + * ^^^^^^^^^ + * - `({ get foo() {} })` + * ^^^^^^^ + * - `({ set foo(a) {} })` + * ^^^^^^^ + * - `class A { constructor() {} }` + * ^^^^^^^^^^^ + * - `class A { foo() {} }` + * ^^^ + * - `class A { *foo() {} }` + * ^^^^ + * - `class A { async foo() {} }` + * ^^^^^^^^^ + * - `class A { ['foo']() {} }` + * ^^^^^^^ + * - `class A { *['foo']() {} }` + * ^^^^^^^^ + * - `class A { async ['foo']() {} }` + * ^^^^^^^^^^^^^ + * - `class A { [foo]() {} }` + * ^^^^^ + * - `class A { *[foo]() {} }` + * ^^^^^^ + * - `class A { async [foo]() {} }` + * ^^^^^^^^^^^ + * - `class A { get foo() {} }` + * ^^^^^^^ + * - `class A { set foo(a) {} }` + * ^^^^^^^ + * - `class A { static foo() {} }` + * ^^^^^^^^^^ + * - `class A { static *foo() {} }` + * ^^^^^^^^^^^ + * - `class A { static async foo() {} }` + * ^^^^^^^^^^^^^^^^ + * - `class A { static get foo() {} }` + * ^^^^^^^^^^^^^^ + * - `class A { static set foo(a) {} }` + * ^^^^^^^^^^^^^^ + * - `class A { foo = function() {} }` + * ^^^^^^^^^^^^^^ + * - `class A { static foo = function() {} }` + * ^^^^^^^^^^^^^^^^^^^^^ + * - `class A { foo = (a, b) => {} }` + * ^^^^^^ + * @param node The function node to get. + * @param sourceCode The source code object to get tokens. + * @returns The location of the function node for reporting. + */ +export function getFunctionHeadLoc( + node: FunctionNode, + sourceCode: TSESLint.SourceCode, +): TSESTree.SourceLocation { + const parent = node.parent; + let start = null; + let end = null; - function getLocEnd(): TSESTree.Position { - if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { - // find the end location for arrow function expression - return sourceCode.getTokenBefore( - node.body, - token => - token.type === AST_TOKEN_TYPES.Punctuator && token.value === '=>', - )!.loc.end; + if ( + parent.type === AST_NODE_TYPES.MethodDefinition || + parent.type === AST_NODE_TYPES.PropertyDefinition + ) { + // the decorator's range is included within the member + // however it's usually irrelevant to the member itself - so we don't want + // to highlight it ever. + if (parent.decorators.length > 0) { + const lastDecorator = parent.decorators[parent.decorators.length - 1]; + const firstTokenAfterDecorator = ESLintUtils.nullThrows( + sourceCode.getTokenAfter(lastDecorator), + ESLintUtils.NullThrowsReasons.MissingToken( + 'modifier or member name', + 'class member', + ), + ); + start = firstTokenAfterDecorator.loc.start; + } else { + start = parent.loc.start; } + end = getOpeningParenOfParams(node, sourceCode).loc.start; + } else if (parent.type === AST_NODE_TYPES.Property) { + start = parent.loc.start; + end = getOpeningParenOfParams(node, sourceCode).loc.start; + } else if (node.type === AST_NODE_TYPES.ArrowFunctionExpression) { + const arrowToken = ESLintUtils.nullThrows( + sourceCode.getTokenBefore(node.body, isArrowToken), + ESLintUtils.NullThrowsReasons.MissingToken( + 'arrow token', + 'arrow function', + ), + ); - // return the end location for a regular function - return sourceCode.getTokenBefore(node.body)!.loc.end; + start = arrowToken.loc.start; + end = arrowToken.loc.end; + } else { + start = node.loc.start; + end = getOpeningParenOfParams(node, sourceCode).loc.start; } return { - start: getLocStart(), - end: getLocEnd(), + start: Object.assign({}, start), + end: Object.assign({}, end), }; } diff --git a/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts b/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts index a17a27dec17..d65263a9c02 100644 --- a/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-function-return-type.test.ts @@ -680,7 +680,7 @@ function test(a: number, b: number) { line: 2, endLine: 2, column: 1, - endColumn: 36, + endColumn: 14, }, ], }, @@ -696,7 +696,7 @@ function test() { line: 2, endLine: 2, column: 1, - endColumn: 16, + endColumn: 14, }, ], }, @@ -712,7 +712,7 @@ var fn = function () { line: 2, endLine: 2, column: 10, - endColumn: 21, + endColumn: 19, }, ], }, @@ -725,7 +725,7 @@ var arrowFn = () => 'test'; messageId: 'missingReturnType', line: 2, endLine: 2, - column: 15, + column: 18, endColumn: 20, }, ], @@ -751,30 +751,30 @@ class Test { { messageId: 'missingReturnType', line: 4, - endLine: 4, column: 3, - endColumn: 13, + endLine: 4, + endColumn: 11, }, { messageId: 'missingReturnType', line: 8, - endLine: 8, column: 3, - endColumn: 11, + endLine: 8, + endColumn: 9, }, { messageId: 'missingReturnType', line: 11, + column: 3, endLine: 11, - column: 11, - endColumn: 16, + endColumn: 11, }, { messageId: 'missingReturnType', line: 12, - endLine: 12, column: 3, - endColumn: 19, + endLine: 12, + endColumn: 17, }, ], }, @@ -791,7 +791,7 @@ function test() { line: 2, endLine: 2, column: 1, - endColumn: 16, + endColumn: 14, }, ], }, @@ -803,7 +803,7 @@ function test() { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 13, + column: 16, endColumn: 18, }, ], @@ -817,7 +817,7 @@ function test() { line: 1, endLine: 1, column: 13, - endColumn: 24, + endColumn: 22, }, ], }, @@ -829,7 +829,7 @@ function test() { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 16, + column: 19, endColumn: 21, }, ], @@ -843,7 +843,7 @@ function test() { line: 1, endLine: 1, column: 16, - endColumn: 27, + endColumn: 25, }, ], }, @@ -863,37 +863,37 @@ class Foo { { messageId: 'missingReturnType', line: 3, + column: 3, endLine: 3, - column: 14, - endColumn: 19, + endColumn: 14, }, { messageId: 'missingReturnType', line: 4, + column: 3, endLine: 4, - column: 14, - endColumn: 25, + endColumn: 23, }, { messageId: 'missingReturnType', line: 5, + column: 3, endLine: 5, - column: 14, - endColumn: 29, + endColumn: 27, }, { messageId: 'missingReturnType', line: 7, + column: 3, endLine: 7, - column: 14, - endColumn: 19, + endColumn: 14, }, { messageId: 'missingReturnType', line: 8, + column: 3, endLine: 8, - column: 14, - endColumn: 25, + endColumn: 23, }, ], }, @@ -905,7 +905,7 @@ class Foo { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 15, + column: 18, endColumn: 20, }, ], @@ -923,7 +923,7 @@ var funcExpr = function () { line: 2, endLine: 2, column: 16, - endColumn: 27, + endColumn: 25, }, ], }, @@ -936,7 +936,7 @@ var funcExpr = function () { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 12, + column: 15, endColumn: 17, }, ], @@ -954,8 +954,8 @@ const x = { messageId: 'missingReturnType', line: 4, endLine: 4, - column: 8, - endColumn: 13, + column: 3, + endColumn: 8, }, ], }, @@ -972,8 +972,8 @@ const x: Foo = { messageId: 'missingReturnType', line: 4, endLine: 4, - column: 8, - endColumn: 13, + column: 3, + endColumn: 8, }, ], }, @@ -985,7 +985,7 @@ const x: Foo = { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 7, + column: 10, endColumn: 12, }, ], @@ -999,7 +999,7 @@ const x: Foo = { line: 1, endLine: 1, column: 7, - endColumn: 18, + endColumn: 16, }, ], }, @@ -1015,7 +1015,7 @@ const x: Foo = { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1033,7 +1033,7 @@ const x: Foo = { line: 3, endLine: 3, column: 10, - endColumn: 21, + endColumn: 19, }, ], }, @@ -1049,7 +1049,7 @@ function fn() { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1067,7 +1067,7 @@ function fn() { line: 3, endLine: 3, column: 10, - endColumn: 21, + endColumn: 19, }, ], }, @@ -1093,7 +1093,7 @@ function FunctionDeclaration() { messageId: 'missingReturnType', line: 9, endLine: 9, - column: 11, + column: 14, endColumn: 16, }, ], @@ -1112,7 +1112,7 @@ function FunctionDeclaration() { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1136,36 +1136,36 @@ foo(() => ''); { messageId: 'missingReturnType', line: 3, + column: 8, endLine: 3, - column: 5, endColumn: 10, }, { messageId: 'missingReturnType', line: 4, + column: 8, endLine: 4, - column: 5, endColumn: 10, }, { messageId: 'missingReturnType', line: 5, + column: 8, endLine: 5, - column: 5, endColumn: 10, }, { messageId: 'missingReturnType', line: 6, + column: 8, endLine: 6, - column: 5, endColumn: 10, }, { messageId: 'missingReturnType', line: 7, + column: 8, endLine: 7, - column: 5, endColumn: 10, }, ], @@ -1192,7 +1192,7 @@ new Accumulator().accumulate(() => 1); messageId: 'missingReturnType', line: 10, endLine: 10, - column: 30, + column: 33, endColumn: 35, }, ], @@ -1209,7 +1209,7 @@ new Accumulator().accumulate(() => 1); messageId: 'missingReturnType', line: 1, endLine: 1, - column: 2, + column: 5, endColumn: 7, }, ], @@ -1242,23 +1242,23 @@ foo({ { messageId: 'missingReturnType', line: 4, - endLine: 4, column: 3, - endColumn: 9, + endLine: 4, + endColumn: 7, }, { messageId: 'missingReturnType', line: 9, + column: 3, endLine: 9, - column: 9, - endColumn: 20, + endColumn: 18, }, { messageId: 'missingReturnType', line: 14, + column: 3, endLine: 14, - column: 9, - endColumn: 14, + endColumn: 9, }, ], }, @@ -1278,7 +1278,7 @@ const x: HigherOrderType = () => arg1 => arg2 => 'foo'; messageId: 'missingReturnType', line: 3, endLine: 3, - column: 42, + column: 47, endColumn: 49, }, ], @@ -1299,21 +1299,21 @@ const x: HigherOrderType = () => arg1 => arg2 => 'foo'; messageId: 'missingReturnType', line: 3, endLine: 3, - column: 28, + column: 31, endColumn: 33, }, { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 34, + column: 39, endColumn: 41, }, { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 42, + column: 47, endColumn: 49, }, ], @@ -1333,14 +1333,14 @@ const func = (value: number) => ({ type: 'X', value } as Action); messageId: 'missingReturnType', line: 2, endLine: 2, - column: 14, + column: 30, endColumn: 32, }, { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 14, + column: 30, endColumn: 32, }, ], @@ -1359,7 +1359,7 @@ const func = (value: number) => ({ type: 'X', value } as const); messageId: 'missingReturnType', line: 2, endLine: 2, - column: 14, + column: 30, endColumn: 32, }, ], @@ -1374,7 +1374,7 @@ const func = (value: number) => ({ type: 'X', value } as const); messageId: 'missingReturnType', line: 1, endLine: 1, - column: 13, + column: 31, endColumn: 33, }, ], @@ -1391,7 +1391,7 @@ const func = (value: number) => ({ type: 'X', value } as const); messageId: 'missingReturnType', line: 2, endLine: 2, - column: 21, + column: 39, endColumn: 41, }, ], @@ -1451,46 +1451,46 @@ const x = { `, errors: [ { - messageId: 'missingReturnType', line: 2, - endLine: 2, column: 1, - endColumn: 16, + messageId: 'missingReturnType', + endLine: 2, + endColumn: 14, }, { - messageId: 'missingReturnType', line: 5, + column: 16, + messageId: 'missingReturnType', endLine: 5, - column: 13, endColumn: 18, }, { - messageId: 'missingReturnType', line: 8, - endLine: 8, column: 13, - endColumn: 24, + messageId: 'missingReturnType', + endLine: 8, + endColumn: 22, }, { - messageId: 'missingReturnType', line: 11, - endLine: 11, column: 20, - endColumn: 31, + messageId: 'missingReturnType', + endLine: 11, + endColumn: 29, }, { line: 15, - column: 12, + column: 3, messageId: 'missingReturnType', endLine: 15, - endColumn: 23, + endColumn: 21, }, { - messageId: 'missingReturnType', line: 20, + column: 3, + messageId: 'missingReturnType', endLine: 20, - column: 6, - endColumn: 17, + endColumn: 15, }, ], }, @@ -1508,7 +1508,7 @@ class Foo { line: 4, endLine: 4, column: 3, - endColumn: 18, + endColumn: 16, }, ], }, @@ -1529,7 +1529,7 @@ const foo = (function () { line: 2, endLine: 2, column: 14, - endColumn: 25, + endColumn: 23, }, ], }, @@ -1551,7 +1551,7 @@ const foo = (function () { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1573,7 +1573,7 @@ let foo = function () { line: 2, endLine: 2, column: 11, - endColumn: 22, + endColumn: 20, }, ], }, @@ -1591,7 +1591,7 @@ let foo = (() => () => {})()(); messageId: 'missingReturnType', line: 2, endLine: 2, - column: 18, + column: 21, endColumn: 23, }, ], diff --git a/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts b/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts index 854aa1bfe34..ce9176c136b 100644 --- a/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-module-boundary-types.test.ts @@ -753,7 +753,7 @@ export function test(a: number, b: number) { line: 2, endLine: 2, column: 8, - endColumn: 43, + endColumn: 21, }, ], }, @@ -769,7 +769,7 @@ export function test() { line: 2, endLine: 2, column: 8, - endColumn: 23, + endColumn: 21, }, ], }, @@ -785,7 +785,7 @@ export var fn = function () { line: 2, endLine: 2, column: 17, - endColumn: 28, + endColumn: 26, }, ], }, @@ -798,7 +798,7 @@ export var arrowFn = () => 'test'; messageId: 'missingReturnType', line: 2, endLine: 2, - column: 22, + column: 25, endColumn: 27, }, ], @@ -825,15 +825,15 @@ export class Test { { messageId: 'missingReturnType', line: 4, - endLine: 4, column: 3, - endColumn: 13, + endLine: 4, + endColumn: 11, }, { messageId: 'missingArgType', line: 7, - endLine: 7, column: 12, + endLine: 7, endColumn: 17, data: { name: 'value', @@ -842,22 +842,22 @@ export class Test { { messageId: 'missingReturnType', line: 8, - endLine: 8, column: 3, - endColumn: 11, + endLine: 8, + endColumn: 9, }, { messageId: 'missingReturnType', line: 11, + column: 3, endLine: 11, - column: 11, - endColumn: 17, + endColumn: 11, }, { messageId: 'missingArgType', line: 11, - endLine: 11, column: 11, + endLine: 11, endColumn: 14, data: { name: 'arg', @@ -897,37 +897,37 @@ export class Foo { { messageId: 'missingReturnType', line: 3, + column: 3, endLine: 3, - column: 14, - endColumn: 19, + endColumn: 14, }, { messageId: 'missingReturnType', line: 4, + column: 3, endLine: 4, - column: 14, - endColumn: 25, + endColumn: 23, }, { messageId: 'missingReturnType', line: 5, + column: 3, endLine: 5, - column: 14, - endColumn: 29, + endColumn: 27, }, { messageId: 'missingReturnType', line: 7, + column: 3, endLine: 7, - column: 14, - endColumn: 19, + endColumn: 14, }, { messageId: 'missingReturnType', line: 8, + column: 3, endLine: 8, - column: 14, - endColumn: 25, + endColumn: 23, }, ], }, @@ -938,7 +938,7 @@ export class Foo { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 16, + column: 19, endColumn: 21, }, ], @@ -951,7 +951,7 @@ export class Foo { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 22, + column: 25, endColumn: 27, }, ], @@ -969,7 +969,7 @@ export var funcExpr = function () { line: 2, endLine: 2, column: 23, - endColumn: 34, + endColumn: 32, }, ], }, @@ -986,8 +986,8 @@ export const x: Foo = { messageId: 'missingReturnType', line: 4, endLine: 4, - column: 8, - endColumn: 13, + column: 3, + endColumn: 8, }, ], }, @@ -999,7 +999,7 @@ export const x: Foo = { messageId: 'missingReturnType', line: 1, endLine: 1, - column: 22, + column: 25, endColumn: 27, }, ], @@ -1013,7 +1013,7 @@ export const x: Foo = { line: 1, endLine: 1, column: 22, - endColumn: 33, + endColumn: 31, }, ], }, @@ -1029,7 +1029,7 @@ export default () => { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1047,7 +1047,7 @@ export default () => { line: 3, endLine: 3, column: 10, - endColumn: 21, + endColumn: 19, }, ], }, @@ -1063,7 +1063,7 @@ export function fn() { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1081,7 +1081,7 @@ export function fn() { line: 3, endLine: 3, column: 10, - endColumn: 21, + endColumn: 19, }, ], }, @@ -1107,7 +1107,7 @@ export function FunctionDeclaration() { messageId: 'missingReturnType', line: 9, endLine: 9, - column: 11, + column: 14, endColumn: 16, }, ], @@ -1126,7 +1126,7 @@ export default () => () => { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 10, + column: 13, endColumn: 15, }, ], @@ -1146,14 +1146,14 @@ export const func2 = (value: number) => ({ type: 'X', value } as Action); messageId: 'missingReturnType', line: 2, endLine: 2, - column: 22, + column: 38, endColumn: 40, }, { messageId: 'missingReturnType', line: 3, endLine: 3, - column: 22, + column: 38, endColumn: 40, }, ], @@ -1172,7 +1172,7 @@ export const func = (value: number) => ({ type: 'X', value } as const); messageId: 'missingReturnType', line: 2, endLine: 2, - column: 21, + column: 37, endColumn: 39, }, ], @@ -1203,14 +1203,14 @@ export class Test { line: 8, endLine: 8, column: 3, - endColumn: 11, + endColumn: 9, }, { messageId: 'missingReturnType', line: 12, endLine: 12, - column: 9, - endColumn: 14, + column: 3, + endColumn: 9, }, ], }, @@ -1254,7 +1254,7 @@ export const func2 = (value: number) => value; messageId: 'missingReturnType', line: 2, endLine: 2, - column: 22, + column: 38, endColumn: 40, }, ], @@ -1342,10 +1342,6 @@ const foo = arg => arg; export default foo; `, errors: [ - { - messageId: 'missingReturnType', - line: 2, - }, { messageId: 'missingArgType', line: 2, @@ -1353,6 +1349,10 @@ export default foo; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 2, + }, ], }, { @@ -1361,10 +1361,6 @@ const foo = arg => arg; export = foo; `, errors: [ - { - messageId: 'missingReturnType', - line: 2, - }, { messageId: 'missingArgType', line: 2, @@ -1372,6 +1368,10 @@ export = foo; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 2, + }, ], }, { @@ -1381,10 +1381,6 @@ foo = arg => arg; export default foo; `, errors: [ - { - messageId: 'missingReturnType', - line: 3, - }, { messageId: 'missingArgType', line: 3, @@ -1392,6 +1388,10 @@ export default foo; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 3, + }, ], }, { @@ -1400,10 +1400,6 @@ const foo = arg => arg; export default [foo]; `, errors: [ - { - messageId: 'missingReturnType', - line: 2, - }, { messageId: 'missingArgType', line: 2, @@ -1411,6 +1407,10 @@ export default [foo]; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 2, + }, ], }, { @@ -1419,10 +1419,6 @@ const foo = arg => arg; export default { foo }; `, errors: [ - { - messageId: 'missingReturnType', - line: 2, - }, { messageId: 'missingArgType', line: 2, @@ -1430,6 +1426,10 @@ export default { foo }; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 2, + }, ], }, { @@ -1617,10 +1617,6 @@ test = (): void => { export default test; `, errors: [ - { - messageId: 'missingReturnType', - line: 2, - }, { messageId: 'missingArgType', line: 2, @@ -1628,6 +1624,10 @@ export default test; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 2, + }, ], }, { @@ -1639,10 +1639,6 @@ test = (): void => { export { test }; `, errors: [ - { - messageId: 'missingReturnType', - line: 2, - }, { messageId: 'missingArgType', line: 2, @@ -1650,6 +1646,10 @@ export { test }; name: 'arg', }, }, + { + messageId: 'missingReturnType', + line: 2, + }, ], }, { @@ -1667,7 +1667,7 @@ export const foo = { messageId: 'missingReturnType', line: 3, - column: 3, + column: 6, }, ], }, @@ -1680,7 +1680,7 @@ export var arrowFn = () => () => {}; { messageId: 'missingReturnType', line: 2, - column: 28, + column: 31, }, ], }, @@ -1889,14 +1889,14 @@ export const foo = { line: 2, endLine: 2, column: 8, - endColumn: 24, + endColumn: 22, }, { messageId: 'missingReturnType', line: 6, endLine: 6, column: 3, - endColumn: 10, + endColumn: 8, }, ], }, From 4527fb7afe47759de6397c5f607646a7c84156d9 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 13 Feb 2023 12:06:50 +1030 Subject: [PATCH 2/3] feat(eslint-plugin): [class-methods-use-this] add extension rule --- .../eslint-plugin/TSLINT_RULE_ALTERNATIVES.md | 4 +- packages/eslint-plugin/docs/rules/TEMPLATE.md | 6 +- .../docs/rules/class-methods-use-this.md | 51 +++ packages/eslint-plugin/src/configs/all.ts | 2 + .../src/rules/class-methods-use-this.ts | 265 +++++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + .../src/util/getStaticStringValue.ts | 49 ++ packages/eslint-plugin/src/util/index.ts | 1 + .../eslint-plugin/src/util/isNullLiteral.ts | 2 +- .../class-methods-use-this-core.test.ts | 431 ++++++++++++++++++ .../class-methods-use-this.test.ts | 186 ++++++++ .../class-methods-use-this.shot | 52 +++ 12 files changed, 1047 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/class-methods-use-this.md create mode 100644 packages/eslint-plugin/src/rules/class-methods-use-this.ts create mode 100644 packages/eslint-plugin/src/util/getStaticStringValue.ts create mode 100644 packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this-core.test.ts create mode 100644 packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts create mode 100644 packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot diff --git a/packages/eslint-plugin/TSLINT_RULE_ALTERNATIVES.md b/packages/eslint-plugin/TSLINT_RULE_ALTERNATIVES.md index 097de893804..8276fabd922 100644 --- a/packages/eslint-plugin/TSLINT_RULE_ALTERNATIVES.md +++ b/packages/eslint-plugin/TSLINT_RULE_ALTERNATIVES.md @@ -185,7 +185,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`one-line`] | 🌟 | [`brace-style`][brace-style] or [Prettier] | | [`one-variable-per-declaration`] | 🌟 | [`one-var`][one-var] | | [`ordered-imports`] | 🌓 | [`import/order`] | -| [`prefer-function-over-method`] | 🌟 | [`class-methods-use-this`][class-methods-use-this] | +| [`prefer-function-over-method`] | 🌟 | [`@typescript-eslint/class-methods-use-this`] | | [`prefer-method-signature`] | ✅ | [`@typescript-eslint/method-signature-style`] | | [`prefer-switch`] | 🛑 | N/A | | [`prefer-template`] | 🌟 | [`prefer-template`][prefer-template] | @@ -566,7 +566,6 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [object-shorthand]: https://eslint.org/docs/rules/object-shorthand [brace-style]: https://eslint.org/docs/rules/brace-style [one-var]: https://eslint.org/docs/rules/one-var -[class-methods-use-this]: https://eslint.org/docs/rules/class-methods-use-this [prefer-template]: https://eslint.org/docs/rules/prefer-template [quotes]: https://eslint.org/docs/rules/quotes [semi]: https://eslint.org/docs/rules/semi @@ -598,6 +597,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/await-thenable`]: https://typescript-eslint.io/rules/await-thenable [`@typescript-eslint/ban-types`]: https://typescript-eslint.io/rules/ban-types [`@typescript-eslint/ban-ts-comment`]: https://typescript-eslint.io/rules/ban-ts-comment +[`@typescript-eslint/class-methods-use-this`]: https://typescript-eslint.io/rules/class-methods-use-this [`@typescript-eslint/consistent-type-assertions`]: https://typescript-eslint.io/rules/consistent-type-assertions [`@typescript-eslint/consistent-type-definitions`]: https://typescript-eslint.io/rules/consistent-type-definitions [`@typescript-eslint/explicit-member-accessibility`]: https://typescript-eslint.io/rules/explicit-member-accessibility diff --git a/packages/eslint-plugin/docs/rules/TEMPLATE.md b/packages/eslint-plugin/docs/rules/TEMPLATE.md index ddc6def34a5..3fb311ad2a1 100644 --- a/packages/eslint-plugin/docs/rules/TEMPLATE.md +++ b/packages/eslint-plugin/docs/rules/TEMPLATE.md @@ -1,6 +1,10 @@ +--- +description: '' +--- + > 🛑 This file is source code, not the primary documentation location! 🛑 > -> See **https://typescript-eslint.io/rules/your-rule-name** for documentation. +> See **https://typescript-eslint.io/rules/RULE_NAME_REPLACEME** for documentation. ## Examples diff --git a/packages/eslint-plugin/docs/rules/class-methods-use-this.md b/packages/eslint-plugin/docs/rules/class-methods-use-this.md new file mode 100644 index 00000000000..3195333cbbe --- /dev/null +++ b/packages/eslint-plugin/docs/rules/class-methods-use-this.md @@ -0,0 +1,51 @@ +--- +description: 'Enforce that class methods utilize `this`.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/class-methods-use-this** for documentation. + +## Examples + +This rule extends the base [`eslint/class-methods-use-this`](https://eslint.org/docs/rules/class-methods-use-this) rule. +It adds support for ignoring `override` methods or methods on classes that implement an interface. + +## Options + +This rule adds the following options: + +```ts +interface Options extends BaseClassMethodsUseThisOptions { + ignoreOverrideMethods?: boolean; + ignoreClassesThatImplementAnInterface?: boolean; +} + +const defaultOptions: Options = { + ...baseClassMethodsUseThisOptions, + ignoreOverrideMethods: false, + ignoreClassesThatImplementAnInterface: false, +}; +``` + +### `ignoreOverrideMethods` + +Example of a correct code when `ignoreOverrideMethods` is set to `true`: + +```ts +class X { + override method() {} + override property = () => {}; +} +``` + +### `ignoreClassesThatImplementAnInterface` + +Example of a correct code when `ignoreClassesThatImplementAnInterface` is set to `true`: + +```ts +class X implements Y { + method() {} + property = () => {}; +} +``` diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 184a241a66f..d0bd265b099 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -19,6 +19,8 @@ export = { 'brace-style': 'off', '@typescript-eslint/brace-style': 'error', '@typescript-eslint/class-literal-property-style': 'error', + 'class-methods-use-this': 'off', + '@typescript-eslint/class-methods-use-this': 'error', 'comma-dangle': 'off', '@typescript-eslint/comma-dangle': 'error', 'comma-spacing': 'off', diff --git a/packages/eslint-plugin/src/rules/class-methods-use-this.ts b/packages/eslint-plugin/src/rules/class-methods-use-this.ts new file mode 100644 index 00000000000..562429ef4c2 --- /dev/null +++ b/packages/eslint-plugin/src/rules/class-methods-use-this.ts @@ -0,0 +1,265 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import * as util from '../util'; + +type Options = [ + { + exceptMethods?: string[]; + enforceForClassFields?: boolean; + ignoreOverrideMethods?: boolean; + ignoreClassesThatImplementAnInterface?: boolean; + }, +]; +type MessageIds = 'missingThis'; + +export default util.createRule({ + name: 'class-methods-use-this', + meta: { + type: 'suggestion', + docs: { + description: 'Enforce that class methods utilize `this`', + extendsBaseRule: true, + requiresTypeChecking: false, + }, + fixable: 'code', + hasSuggestions: false, + schema: [ + { + type: 'object', + properties: { + exceptMethods: { + type: 'array', + description: + 'Allows specified method names to be ignored with this rule', + items: { + type: 'string', + }, + }, + enforceForClassFields: { + type: 'boolean', + description: + 'Enforces that functions used as instance field initializers utilize `this`', + default: true, + }, + ignoreOverrideMethods: { + type: 'boolean', + description: 'Ingore members marked with the `override` modifier', + }, + ignoreClassesThatImplementAnInterface: { + type: 'boolean', + description: + 'Ignore classes that specifically implement some interface', + }, + }, + additionalProperties: false, + }, + ], + messages: { + missingThis: "Expected 'this' to be used by class {{name}}.", + }, + }, + defaultOptions: [ + { + enforceForClassFields: true, + exceptMethods: [], + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + create( + context, + [ + { + enforceForClassFields, + exceptMethods: exceptMethodsRaw, + ignoreClassesThatImplementAnInterface, + ignoreOverrideMethods, + }, + ], + ) { + const exceptMethods = new Set(exceptMethodsRaw); + type Stack = + | { + member: null; + class: null; + parent: Stack | undefined; + usesThis: boolean; + } + | { + member: TSESTree.MethodDefinition | TSESTree.PropertyDefinition; + class: TSESTree.ClassDeclaration | TSESTree.ClassExpression; + parent: Stack | undefined; + usesThis: boolean; + }; + let stack: Stack | undefined; + + const sourceCode = context.getSourceCode(); + + function pushContext( + member?: TSESTree.MethodDefinition | TSESTree.PropertyDefinition, + ): void { + if (member && member.parent?.type === AST_NODE_TYPES.ClassBody) { + stack = { + member, + class: member.parent.parent as + | TSESTree.ClassDeclaration + | TSESTree.ClassExpression, + usesThis: false, + parent: stack, + }; + } else { + stack = { + member: null, + class: null, + usesThis: false, + parent: stack, + }; + } + } + + function enterFunction( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + ): void { + if ( + node.parent?.type === AST_NODE_TYPES.MethodDefinition || + node.parent?.type === AST_NODE_TYPES.PropertyDefinition + ) { + pushContext(node.parent); + } else { + pushContext(); + } + } + + /** + * Pop `this` used flag from the stack. + */ + function popContext(): Stack | undefined { + const oldStack = stack; + stack = stack?.parent; + return oldStack; + } + + /** + * Check if the node is an instance method not excluded by config + */ + function isIncludedInstanceMethod( + node: NonNullable, + ): node is NonNullable { + if ( + node.static || + (node.type === AST_NODE_TYPES.MethodDefinition && + node.kind === 'constructor') || + (node.type === AST_NODE_TYPES.PropertyDefinition && + !enforceForClassFields) + ) { + return false; + } + + if (node.computed || exceptMethods.size === 0) { + return true; + } + + const hashIfNeeded = + node.key.type === AST_NODE_TYPES.PrivateIdentifier ? '#' : ''; + const name = + node.key.type === AST_NODE_TYPES.Literal + ? util.getStaticStringValue(node.key) + : node.key.name || ''; + + return !exceptMethods.has(hashIfNeeded + (name ?? '')); + } + + /** + * Checks if we are leaving a function that is a method, and reports if 'this' has not been used. + * Static methods and the constructor are exempt. + * Then pops the context off the stack. + */ + function exitFunction( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, + ): void { + const stackContext = popContext(); + if ( + stackContext?.member == null || + stackContext.class == null || + stackContext.usesThis || + (ignoreOverrideMethods && stackContext.member.override) || + (ignoreClassesThatImplementAnInterface && + stackContext.class.implements != null) + ) { + return; + } + + if (isIncludedInstanceMethod(stackContext.member)) { + context.report({ + node, + loc: util.getFunctionHeadLoc(node, sourceCode), + messageId: 'missingThis', + data: { + name: util.getFunctionNameWithKind(node), + }, + }); + } + } + + return { + // function declarations have their own `this` context + FunctionDeclaration(): void { + pushContext(); + }, + 'FunctionDeclaration:exit'(): void { + popContext(); + }, + + FunctionExpression(node): void { + enterFunction(node); + }, + 'FunctionExpression:exit'(node): void { + exitFunction(node); + }, + ...(enforceForClassFields + ? { + 'PropertyDefinition > ArrowFunctionExpression.value'( + node: TSESTree.ArrowFunctionExpression, + ): void { + enterFunction(node); + }, + 'PropertyDefinition > ArrowFunctionExpression.value:exit'( + node: TSESTree.ArrowFunctionExpression, + ): void { + exitFunction(node); + }, + } + : {}), + + /* + * Class field value are implicit functions. + */ + 'PropertyDefinition > *.key:exit'(): void { + pushContext(); + }, + 'PropertyDefinition:exit'(): void { + popContext(); + }, + + /* + * Class static blocks are implicit functions. They aren't required to use `this`, + * but we have to push context so that it captures any use of `this` in the static block + * separately from enclosing contexts, because static blocks have their own `this` and it + * shouldn't count as used `this` in enclosing contexts. + */ + StaticBlock(): void { + pushContext(); + }, + 'StaticBlock:exit'(): void { + popContext(); + }, + + 'ThisExpression, Super'(): void { + if (stack) { + stack.usesThis = true; + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index f5cf92c28d1..44aedd6198e 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -7,6 +7,7 @@ import banTypes from './ban-types'; import blockSpacing from './block-spacing'; import braceStyle from './brace-style'; import classLiteralPropertyStyle from './class-literal-property-style'; +import classMethodsUseThis from './class-methods-use-this'; import commaDangle from './comma-dangle'; import commaSpacing from './comma-spacing'; import consistentGenericConstructors from './consistent-generic-constructors'; @@ -141,6 +142,7 @@ export default { 'block-spacing': blockSpacing, 'brace-style': braceStyle, 'class-literal-property-style': classLiteralPropertyStyle, + 'class-methods-use-this': classMethodsUseThis, 'comma-dangle': commaDangle, 'comma-spacing': commaSpacing, 'consistent-generic-constructors': consistentGenericConstructors, diff --git a/packages/eslint-plugin/src/util/getStaticStringValue.ts b/packages/eslint-plugin/src/util/getStaticStringValue.ts new file mode 100644 index 00000000000..6eeaf9af8ce --- /dev/null +++ b/packages/eslint-plugin/src/util/getStaticStringValue.ts @@ -0,0 +1,49 @@ +// adapted from https://github.com/eslint/eslint/blob/5bdaae205c3a0089ea338b382df59e21d5b06436/lib/rules/utils/ast-utils.js#L191-L230 + +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import { isNullLiteral } from './isNullLiteral'; + +/** + * Returns the result of the string conversion applied to the evaluated value of the given expression node, + * if it can be determined statically. + * + * This function returns a `string` value for all `Literal` nodes and simple `TemplateLiteral` nodes only. + * In all other cases, this function returns `null`. + * @param node Expression node. + * @returns String value if it can be determined. Otherwise, `null`. + */ +export function getStaticStringValue(node: TSESTree.Node): string | null { + switch (node.type) { + case AST_NODE_TYPES.Literal: + // eslint-disable-next-line eqeqeq -- intentional strict comparison for literal value + if (node.value === null) { + if (isNullLiteral(node)) { + return String(node.value); // "null" + } + if ('regex' in node) { + return `/${node.regex.pattern}/${node.regex.flags}`; + } + + if ('bigint' in node) { + return node.bigint; + } + + // Otherwise, this is an unknown literal. The function will return null. + } else { + return String(node.value); + } + break; + + case AST_NODE_TYPES.TemplateLiteral: + if (node.expressions.length === 0 && node.quasis.length === 1) { + return node.quasis[0].value.cooked; + } + break; + + // no default + } + + return null; +} diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index 53a19a96d36..5e9994a136d 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -5,6 +5,7 @@ export * from './collectUnusedVariables'; export * from './createRule'; export * from './getFunctionHeadLoc'; export * from './getOperatorPrecedence'; +export * from './getStaticStringValue'; export * from './getStringLength'; export * from './getThisExpression'; export * from './getWrappingFixer'; diff --git a/packages/eslint-plugin/src/util/isNullLiteral.ts b/packages/eslint-plugin/src/util/isNullLiteral.ts index 85bf4588212..d59a926c5aa 100644 --- a/packages/eslint-plugin/src/util/isNullLiteral.ts +++ b/packages/eslint-plugin/src/util/isNullLiteral.ts @@ -1,6 +1,6 @@ import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -export function isNullLiteral(i: TSESTree.Node): boolean { +export function isNullLiteral(i: TSESTree.Node): i is TSESTree.NullLiteral { return i.type === AST_NODE_TYPES.Literal && i.value == null; } diff --git a/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this-core.test.ts b/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this-core.test.ts new file mode 100644 index 00000000000..4fba4d557f1 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this-core.test.ts @@ -0,0 +1,431 @@ +/* eslint-disable @typescript-eslint/internal/plugin-test-formatting -- +keeping eslint core formatting on purpose to make upstream diffing easier and so we don't need to edit line/cols */ +import { RuleTester } from '@typescript-eslint/rule-tester'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import rule from '../../../src/rules/class-methods-use-this'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('class-methods-use-this', rule, { + valid: [ + { code: 'class A { constructor() {} }', parserOptions: { ecmaVersion: 6 } }, + { code: 'class A { foo() {this} }', parserOptions: { ecmaVersion: 6 } }, + { + code: "class A { foo() {this.bar = 'bar';} }", + parserOptions: { ecmaVersion: 6 }, + }, + { + code: 'class A { foo() {bar(this);} }', + parserOptions: { ecmaVersion: 6 }, + }, + { + code: 'class A extends B { foo() {super.foo();} }', + parserOptions: { ecmaVersion: 6 }, + }, + { + code: 'class A { foo() { if(true) { return this; } } }', + parserOptions: { ecmaVersion: 6 }, + }, + { code: 'class A { static foo() {} }', parserOptions: { ecmaVersion: 6 } }, + { code: '({ a(){} });', parserOptions: { ecmaVersion: 6 } }, + { + code: 'class A { foo() { () => this; } }', + parserOptions: { ecmaVersion: 6 }, + }, + { code: '({ a: function () {} });', parserOptions: { ecmaVersion: 6 } }, + { + code: 'class A { foo() {this} bar() {} }', + options: [{ exceptMethods: ['bar'] }], + parserOptions: { ecmaVersion: 6 }, + }, + { + code: 'class A { "foo"() { } }', + options: [{ exceptMethods: ['foo'] }], + parserOptions: { ecmaVersion: 6 }, + }, + { + code: 'class A { 42() { } }', + options: [{ exceptMethods: ['42'] }], + parserOptions: { ecmaVersion: 6 }, + }, + { + code: 'class A { foo = function() {this} }', + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { foo = () => {this} }', + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { foo = () => {super.toString} }', + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { static foo = function() {} }', + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { static foo = () => {} }', + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { #bar() {} }', + options: [{ exceptMethods: ['#bar'] }], + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { foo = function () {} }', + options: [{ enforceForClassFields: false }], + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { foo = () => {} }', + options: [{ enforceForClassFields: false }], + parserOptions: { ecmaVersion: 2022 }, + }, + { + code: 'class A { foo() { return class { [this.foo] = 1 }; } }', + parserOptions: { ecmaVersion: 2022 }, + }, + { code: 'class A { static {} }', parserOptions: { ecmaVersion: 2022 } }, + ], + invalid: [ + { + code: 'class A { foo() {} }', + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() {/**this**/} }', + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() {var a = function () {this};} }', + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() {var a = function () {var b = function(){this}};} }', + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() {window.this} }', + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: "class A { foo() {that.this = 'this';} }", + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() { () => undefined; } }', + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() {} bar() {} }', + options: [{ exceptMethods: ['bar'] }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + ], + }, + { + code: 'class A { foo() {} hasOwnProperty() {} }', + options: [{ exceptMethods: ['foo'] }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 20, + messageId: 'missingThis', + data: { name: "method 'hasOwnProperty'" }, + }, + ], + }, + { + code: 'class A { [foo]() {} }', + options: [{ exceptMethods: ['foo'] }], + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 11, + messageId: 'missingThis', + data: { name: 'method' }, + }, + ], + }, + { + code: 'class A { #foo() { } foo() {} #bar() {} }', + options: [{ exceptMethods: ['#foo'] }], + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 22, + messageId: 'missingThis', + data: { name: "method 'foo'" }, + }, + { + type: AST_NODE_TYPES.FunctionExpression, + line: 1, + column: 31, + messageId: 'missingThis', + data: { name: 'private method #bar' }, + }, + ], + }, + { + code: "class A { foo(){} 'bar'(){} 123(){} [`baz`](){} [a](){} [f(a)](){} get quux(){} set[a](b){} *quuux(){} }", + parserOptions: { ecmaVersion: 6 }, + errors: [ + { + messageId: 'missingThis', + data: { name: "method 'foo'" }, + type: AST_NODE_TYPES.FunctionExpression, + column: 11, + }, + { + messageId: 'missingThis', + data: { name: "method 'bar'" }, + type: AST_NODE_TYPES.FunctionExpression, + column: 19, + }, + { + messageId: 'missingThis', + data: { name: "method '123'" }, + type: AST_NODE_TYPES.FunctionExpression, + column: 29, + }, + { + messageId: 'missingThis', + data: { name: "method 'baz'" }, + type: AST_NODE_TYPES.FunctionExpression, + column: 37, + }, + { + messageId: 'missingThis', + data: { name: 'method' }, + type: AST_NODE_TYPES.FunctionExpression, + column: 49, + }, + { + messageId: 'missingThis', + data: { name: 'method' }, + type: AST_NODE_TYPES.FunctionExpression, + column: 57, + }, + { + messageId: 'missingThis', + data: { name: "getter 'quux'" }, + type: AST_NODE_TYPES.FunctionExpression, + column: 68, + }, + { + messageId: 'missingThis', + data: { name: 'setter' }, + type: AST_NODE_TYPES.FunctionExpression, + column: 81, + }, + { + messageId: 'missingThis', + data: { name: "generator method 'quuux'" }, + type: AST_NODE_TYPES.FunctionExpression, + column: 93, + }, + ], + }, + { + code: 'class A { foo = function() {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: "method 'foo'" }, + column: 11, + endColumn: 25, + }, + ], + }, + { + code: 'class A { foo = () => {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: "method 'foo'" }, + column: 11, + endColumn: 17, + }, + ], + }, + { + code: 'class A { #foo = function() {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: 'private method #foo' }, + column: 11, + endColumn: 26, + }, + ], + }, + { + code: 'class A { #foo = () => {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: 'private method #foo' }, + column: 11, + endColumn: 18, + }, + ], + }, + { + code: 'class A { #foo() {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: 'private method #foo' }, + column: 11, + endColumn: 15, + }, + ], + }, + { + code: 'class A { get #foo() {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: 'private getter #foo' }, + column: 11, + endColumn: 19, + }, + ], + }, + { + code: 'class A { set #foo(x) {} }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: 'private setter #foo' }, + column: 11, + endColumn: 19, + }, + ], + }, + { + code: 'class A { foo () { return class { foo = this }; } }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: "method 'foo'" }, + column: 11, + endColumn: 15, + }, + ], + }, + { + code: 'class A { foo () { return function () { foo = this }; } }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: "method 'foo'" }, + column: 11, + endColumn: 15, + }, + ], + }, + { + code: 'class A { foo () { return class { static { this; } } } }', + parserOptions: { ecmaVersion: 2022 }, + errors: [ + { + messageId: 'missingThis', + data: { name: "method 'foo'" }, + column: 11, + endColumn: 15, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts b/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts new file mode 100644 index 00000000000..5f2532b7b3d --- /dev/null +++ b/packages/eslint-plugin/tests/rules/class-methods-use-this/class-methods-use-this.test.ts @@ -0,0 +1,186 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../../src/rules/class-methods-use-this'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('class-methods-use-this', rule, { + valid: [ + { + code: ` +class Foo implements Bar { + method() {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: true }], + }, + { + code: ` +class Foo { + override method() {} +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo implements Bar { + override method() {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: true, + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + property = () => {}; +} + `, + options: [{ ignoreClassesThatImplementAnInterface: true }], + }, + { + code: ` +class Foo { + override property = () => {}; +} + `, + options: [{ ignoreOverrideMethods: true }], + }, + { + code: ` +class Foo implements Bar { + override property = () => {}; +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: true, + ignoreOverrideMethods: true, + }, + ], + }, + { + code: ` +class Foo implements Bar { + property = () => {}; +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + enforceForClassFields: false, + }, + ], + }, + { + code: ` +class Foo { + override property = () => {}; +} + `, + options: [ + { + ignoreOverrideMethods: false, + enforceForClassFields: false, + }, + ], + }, + ], + invalid: [ + { + code: ` +class Foo implements Bar { + method() {} +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + override method() {} +} + `, + options: [{ ignoreOverrideMethods: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + override method() {} +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + property = () => {}; +} + `, + options: [{ ignoreClassesThatImplementAnInterface: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo { + override property = () => {}; +} + `, + options: [{ ignoreOverrideMethods: false }], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + { + code: ` +class Foo implements Bar { + override property = () => {}; +} + `, + options: [ + { + ignoreClassesThatImplementAnInterface: false, + ignoreOverrideMethods: false, + }, + ], + errors: [ + { + messageId: 'missingThis', + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot b/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot new file mode 100644 index 00000000000..14858cd2617 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/class-methods-use-this.shot @@ -0,0 +1,52 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes class-methods-use-this 1`] = ` +" +# SCHEMA: + +[ + { + "additionalProperties": false, + "properties": { + "enforceForClassFields": { + "default": true, + "description": "Enforces that functions used as instance field initializers utilize \`this\`", + "type": "boolean" + }, + "exceptMethods": { + "description": "Allows specified method names to be ignored with this rule", + "items": { + "type": "string" + }, + "type": "array" + }, + "ignoreClassesThatImplementAnInterface": { + "description": "Ignore classes that specifically implement some interface", + "type": "boolean" + }, + "ignoreOverrideMethods": { + "description": "Ingore members marked with the \`override\` modifier", + "type": "boolean" + } + }, + "type": "object" + } +] + + +# TYPES: + +type Options = [ + { + /** Enforces that functions used as instance field initializers utilize \`this\` */ + enforceForClassFields?: boolean; + /** Allows specified method names to be ignored with this rule */ + exceptMethods?: string[]; + /** Ignore classes that specifically implement some interface */ + ignoreClassesThatImplementAnInterface?: boolean; + /** Ingore members marked with the \`override\` modifier */ + ignoreOverrideMethods?: boolean; + }, +]; +" +`; From 41da7964b63e87fb103bb4d37d09739cdee97626 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 18 Jul 2023 20:28:23 +0930 Subject: [PATCH 3/3] updoots --- packages/eslint-plugin/docs/rules/class-methods-use-this.md | 6 ++++++ packages/eslint-plugin/src/rules/class-methods-use-this.ts | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/class-methods-use-this.md b/packages/eslint-plugin/docs/rules/class-methods-use-this.md index 3195333cbbe..4ee075552d2 100644 --- a/packages/eslint-plugin/docs/rules/class-methods-use-this.md +++ b/packages/eslint-plugin/docs/rules/class-methods-use-this.md @@ -30,6 +30,8 @@ const defaultOptions: Options = { ### `ignoreOverrideMethods` +Makes the rule to ignores any class member explicitly marked with `override`. + Example of a correct code when `ignoreOverrideMethods` is set to `true`: ```ts @@ -41,6 +43,10 @@ class X { ### `ignoreClassesThatImplementAnInterface` +Makes the rule ignore all class members that are defined within a class that `implements` a type. + +It's important to note that this option does not only apply to members defined in the interface as that would require type information. + Example of a correct code when `ignoreClassesThatImplementAnInterface` is set to `true`: ```ts diff --git a/packages/eslint-plugin/src/rules/class-methods-use-this.ts b/packages/eslint-plugin/src/rules/class-methods-use-this.ts index 562429ef4c2..948aa4def5b 100644 --- a/packages/eslint-plugin/src/rules/class-methods-use-this.ts +++ b/packages/eslint-plugin/src/rules/class-methods-use-this.ts @@ -99,7 +99,7 @@ export default util.createRule({ function pushContext( member?: TSESTree.MethodDefinition | TSESTree.PropertyDefinition, ): void { - if (member && member.parent?.type === AST_NODE_TYPES.ClassBody) { + if (member?.parent.type === AST_NODE_TYPES.ClassBody) { stack = { member, class: member.parent.parent as @@ -122,8 +122,8 @@ export default util.createRule({ node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression, ): void { if ( - node.parent?.type === AST_NODE_TYPES.MethodDefinition || - node.parent?.type === AST_NODE_TYPES.PropertyDefinition + node.parent.type === AST_NODE_TYPES.MethodDefinition || + node.parent.type === AST_NODE_TYPES.PropertyDefinition ) { pushContext(node.parent); } else {