From d5429633ba23c3d8e0c3c285b0cd6cb7c3ce9979 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 27 Sep 2023 15:28:38 +0300 Subject: [PATCH 1/8] feat(eslint-plugin): [max-params] don't count `this: void` parameter Closes #7538 --- .../eslint-plugin/docs/rules/max-params.md | 10 ++ packages/eslint-plugin/src/configs/all.ts | 2 + packages/eslint-plugin/src/rules/index.ts | 2 + .../eslint-plugin/src/rules/max-params.ts | 99 +++++++++++++++++ .../src/util/getESLintCoreRule.ts | 1 + .../tests/rules/max-params.test.ts | 104 ++++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 18 +++ 7 files changed, 236 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/max-params.md create mode 100644 packages/eslint-plugin/src/rules/max-params.ts create mode 100644 packages/eslint-plugin/tests/rules/max-params.test.ts diff --git a/packages/eslint-plugin/docs/rules/max-params.md b/packages/eslint-plugin/docs/rules/max-params.md new file mode 100644 index 000000000000..e472c83cc50c --- /dev/null +++ b/packages/eslint-plugin/docs/rules/max-params.md @@ -0,0 +1,10 @@ +--- +description: 'Enforce a maximum number of parameters in function definitions' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/max-params** for documentation. + +This rule extends the base [`eslint/max-params`](https://eslint.org/docs/rules/max-params) rule. +This version adds support for TypeScript `this` parameter so it won't be counted as a parameter. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index d0bd265b0996..7a4a49423693 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -52,6 +52,8 @@ export = { '@typescript-eslint/lines-around-comment': 'error', 'lines-between-class-members': 'off', '@typescript-eslint/lines-between-class-members': 'error', + 'max-params': 'off', + '@typescript-eslint/max-params': 'error', '@typescript-eslint/member-delimiter-style': 'error', '@typescript-eslint/member-ordering': 'error', '@typescript-eslint/method-signature-style': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 44aedd6198e1..b90267b3a447 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -28,6 +28,7 @@ import keySpacing from './key-spacing'; import keywordSpacing from './keyword-spacing'; import linesAroundComment from './lines-around-comment'; import linesBetweenClassMembers from './lines-between-class-members'; +import maxParams from './max-params'; import memberDelimiterStyle from './member-delimiter-style'; import memberOrdering from './member-ordering'; import methodSignatureStyle from './method-signature-style'; @@ -163,6 +164,7 @@ export default { 'keyword-spacing': keywordSpacing, 'lines-around-comment': linesAroundComment, 'lines-between-class-members': linesBetweenClassMembers, + 'max-params': maxParams, 'member-delimiter-style': memberDelimiterStyle, 'member-ordering': memberOrdering, 'method-signature-style': methodSignatureStyle, diff --git a/packages/eslint-plugin/src/rules/max-params.ts b/packages/eslint-plugin/src/rules/max-params.ts new file mode 100644 index 000000000000..5ffe96c2fee8 --- /dev/null +++ b/packages/eslint-plugin/src/rules/max-params.ts @@ -0,0 +1,99 @@ +import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; +import type { JSONSchema4 } from '@typescript-eslint/utils/json-schema'; + +import * as util from '../util'; +import { getESLintCoreRule } from '../util/getESLintCoreRule'; + +type FunctionLike = + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.ArrowFunctionExpression; + +type FunctionRuleListener = (node: T) => void; + +const baseRule = getESLintCoreRule('max-params'); + +export type Options = util.InferOptionsTypeFromRule; +export type MessageIds = util.InferMessageIdsTypeFromRule; + +const schema = Object.values( + util.deepMerge( + { ...baseRule.meta.schema }, + { + 0: { + oneOf: [ + baseRule.meta.schema[0].oneOf[0], + { + ...baseRule.meta.schema[0].oneOf[1], + properties: { + ...baseRule.meta.schema[0].oneOf[1].properties, + countVoidThis: { + type: 'boolean', + }, + }, + }, + ], + }, + }, + ), +) as JSONSchema4[]; + +export default util.createRule({ + name: 'max-params', + meta: { + type: 'suggestion', + docs: { + description: + 'Enforce a maximum number of parameters in function definitions', + extendsBaseRule: true, + }, + schema, + messages: baseRule.meta.messages, + }, + defaultOptions: [{ max: 3, countVoidThis: false }], + + create(context, [{ countVoidThis }]) { + const baseRules = baseRule.create(context); + + if (countVoidThis === true) { + return baseRules; + } + + const removeVoidThisParam = (node: T): T => { + if (node.params.length === 0) { + return node; + } + + const params = [...node.params]; + + if ( + params[0] && + params[0].type === AST_NODE_TYPES.Identifier && + params[0].name === 'this' && + params[0].typeAnnotation?.typeAnnotation.type === + AST_NODE_TYPES.TSVoidKeyword + ) { + params.shift(); + } + + return { + ...node, + params, + }; + }; + + const wrapListener = ( + listener: FunctionRuleListener, + ): FunctionRuleListener => { + return (node: T): void => { + listener(removeVoidThisParam(node)); + }; + }; + + return { + ArrowFunctionExpression: wrapListener(baseRules.ArrowFunctionExpression), + FunctionDeclaration: wrapListener(baseRules.FunctionDeclaration), + FunctionExpression: wrapListener(baseRules.FunctionExpression), + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/getESLintCoreRule.ts b/packages/eslint-plugin/src/util/getESLintCoreRule.ts index 30de8347c0dd..8d754451bd3f 100644 --- a/packages/eslint-plugin/src/util/getESLintCoreRule.ts +++ b/packages/eslint-plugin/src/util/getESLintCoreRule.ts @@ -17,6 +17,7 @@ interface RuleMap { 'keyword-spacing': typeof import('eslint/lib/rules/keyword-spacing'); 'lines-around-comment': typeof import('eslint/lib/rules/lines-around-comment'); 'lines-between-class-members': typeof import('eslint/lib/rules/lines-between-class-members'); + 'max-params': typeof import('eslint/lib/rules/max-params'); 'no-dupe-args': typeof import('eslint/lib/rules/no-dupe-args'); 'no-dupe-class-members': typeof import('eslint/lib/rules/no-dupe-class-members'); 'no-empty-function': typeof import('eslint/lib/rules/no-empty-function'); diff --git a/packages/eslint-plugin/tests/rules/max-params.test.ts b/packages/eslint-plugin/tests/rules/max-params.test.ts new file mode 100644 index 000000000000..054c57363a39 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/max-params.test.ts @@ -0,0 +1,104 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/max-params'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('max-params', rule, { + valid: [ + 'function foo() {}', + 'const foo = function () {};', + 'const foo = () => {};', + 'function foo(a) {}', + ` +class Foo { + constructor(a) {} +} + `, + ` +class Foo { + method(this: void, a, b, c) {} +} + `, + ` +class Foo { + method(this: Foo, a, b) {} +} + `, + { + code: 'function foo(a, b, c, d) {}', + options: [{ max: 4 }], + }, + { + code: 'function foo(a, b, c, d) {}', + options: [{ maximum: 4 }], + }, + { + code: ` +class Foo { + method(this: void) {} +} + `, + options: [{ max: 0 }], + }, + { + code: ` +class Foo { + method(this: void, a) {} +} + `, + options: [{ max: 1 }], + }, + { + code: ` +class Foo { + method(this: void, a) {} +} + `, + options: [{ max: 2, countVoidThis: true }], + }, + ], + invalid: [ + { code: 'function foo(a, b, c, d) {}', errors: [{ messageId: 'exceed' }] }, + { + code: 'const foo = function (a, b, c, d) {};', + errors: [{ messageId: 'exceed' }], + }, + { + code: 'const foo = (a, b, c, d) => {};', + errors: [{ messageId: 'exceed' }], + }, + { + code: 'const foo = a => {};', + options: [{ max: 0 }], + errors: [{ messageId: 'exceed' }], + }, + { + code: ` +class Foo { + method(this: void, a, b, c, d) {} +} + `, + errors: [{ messageId: 'exceed' }], + }, + { + code: ` +class Foo { + method(this: void, a) {} +} + `, + options: [{ max: 1, countVoidThis: true }], + errors: [{ messageId: 'exceed' }], + }, + { + code: ` +class Foo { + method(this: Foo, a, b, c) {} +} + `, + errors: [{ messageId: 'exceed' }], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index 295dd4d757c5..0e39089fc4b2 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -272,6 +272,24 @@ declare module 'eslint/lib/rules/keyword-spacing' { export = rule; } +declare module 'eslint/lib/rules/max-params' { + import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; + + const rule: TSESLint.RuleModule< + 'exceed', + ( + | { max: number; countVoidThis?: boolean } + | { maximum: number; countVoidThis?: boolean } + )[], + { + FunctionDeclaration(node: TSESTree.FunctionDeclaration): void; + FunctionExpression(node: TSESTree.FunctionExpression): void; + ArrowFunctionExpression(node: TSESTree.ArrowFunctionExpression): void; + } + >; + export = rule; +} + declare module 'eslint/lib/rules/no-dupe-class-members' { import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; From 59f2f37fa73130f8f2565d29f8c6621ae771f1db Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 27 Sep 2023 15:52:42 +0300 Subject: [PATCH 2/8] hard-code schema --- .../eslint-plugin/src/rules/max-params.ts | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/packages/eslint-plugin/src/rules/max-params.ts b/packages/eslint-plugin/src/rules/max-params.ts index 5ffe96c2fee8..c11d724aa307 100644 --- a/packages/eslint-plugin/src/rules/max-params.ts +++ b/packages/eslint-plugin/src/rules/max-params.ts @@ -16,27 +16,25 @@ const baseRule = getESLintCoreRule('max-params'); export type Options = util.InferOptionsTypeFromRule; export type MessageIds = util.InferMessageIdsTypeFromRule; -const schema = Object.values( - util.deepMerge( - { ...baseRule.meta.schema }, - { - 0: { - oneOf: [ - baseRule.meta.schema[0].oneOf[0], - { - ...baseRule.meta.schema[0].oneOf[1], - properties: { - ...baseRule.meta.schema[0].oneOf[1].properties, - countVoidThis: { - type: 'boolean', - }, - }, - }, - ], +const schema = [ + { + type: 'object', + properties: { + maximum: { + type: 'integer', + minimum: 0, + }, + max: { + type: 'integer', + minimum: 0, + }, + countVoidThis: { + type: 'boolean', }, }, - ), -) as JSONSchema4[]; + additionalProperties: false, + }, +] as JSONSchema4[]; export default util.createRule({ name: 'max-params', From f9c59587d4c67861bb401b225eecb4d4fb32fa77 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 27 Sep 2023 15:56:42 +0300 Subject: [PATCH 3/8] refactor --- .../eslint-plugin/src/rules/max-params.ts | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/packages/eslint-plugin/src/rules/max-params.ts b/packages/eslint-plugin/src/rules/max-params.ts index c11d724aa307..73729e367087 100644 --- a/packages/eslint-plugin/src/rules/max-params.ts +++ b/packages/eslint-plugin/src/rules/max-params.ts @@ -1,5 +1,4 @@ import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; -import type { JSONSchema4 } from '@typescript-eslint/utils/json-schema'; import * as util from '../util'; import { getESLintCoreRule } from '../util/getESLintCoreRule'; @@ -16,26 +15,6 @@ const baseRule = getESLintCoreRule('max-params'); export type Options = util.InferOptionsTypeFromRule; export type MessageIds = util.InferMessageIdsTypeFromRule; -const schema = [ - { - type: 'object', - properties: { - maximum: { - type: 'integer', - minimum: 0, - }, - max: { - type: 'integer', - minimum: 0, - }, - countVoidThis: { - type: 'boolean', - }, - }, - additionalProperties: false, - }, -] as JSONSchema4[]; - export default util.createRule({ name: 'max-params', meta: { @@ -45,7 +24,25 @@ export default util.createRule({ 'Enforce a maximum number of parameters in function definitions', extendsBaseRule: true, }, - schema, + schema: [ + { + type: 'object', + properties: { + maximum: { + type: 'integer', + minimum: 0, + }, + max: { + type: 'integer', + minimum: 0, + }, + countVoidThis: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], messages: baseRule.meta.messages, }, defaultOptions: [{ max: 3, countVoidThis: false }], From 23c4ce74091362f603b1ad5990e52b52fc1dbca8 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 27 Sep 2023 19:32:24 +0300 Subject: [PATCH 4/8] lint --- packages/eslint-plugin/docs/rules/max-params.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/max-params.md b/packages/eslint-plugin/docs/rules/max-params.md index e472c83cc50c..3ee62f769b3d 100644 --- a/packages/eslint-plugin/docs/rules/max-params.md +++ b/packages/eslint-plugin/docs/rules/max-params.md @@ -1,5 +1,5 @@ --- -description: 'Enforce a maximum number of parameters in function definitions' +description: 'Enforce a maximum number of parameters in function definitions.' --- > 🛑 This file is source code, not the primary documentation location! 🛑 From f6135304c35fc87b7657d94205064f6c2bbe28c7 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Wed, 27 Sep 2023 20:22:31 +0300 Subject: [PATCH 5/8] snapshot --- .../tests/schema-snapshots/max-params.shot | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 packages/eslint-plugin/tests/schema-snapshots/max-params.shot diff --git a/packages/eslint-plugin/tests/schema-snapshots/max-params.shot b/packages/eslint-plugin/tests/schema-snapshots/max-params.shot new file mode 100644 index 000000000000..646c1287dfae --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/max-params.shot @@ -0,0 +1,38 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes max-params 1`] = ` +" +# SCHEMA: + +[ + { + "additionalProperties": false, + "properties": { + "countVoidThis": { + "type": "boolean" + }, + "max": { + "minimum": 0, + "type": "integer" + }, + "maximum": { + "minimum": 0, + "type": "integer" + } + }, + "type": "object" + } +] + + +# TYPES: + +type Options = [ + { + countVoidThis?: boolean; + max?: number; + maximum?: number; + }, +]; +" +`; From 9b258f7436057d7004792ab367bf8055aed9d651 Mon Sep 17 00:00:00 2001 From: StyleShit Date: Mon, 16 Oct 2023 20:10:07 +0300 Subject: [PATCH 6/8] wip --- packages/eslint-plugin/docs/rules/max-params.md | 2 +- packages/eslint-plugin/src/rules/max-params.ts | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/max-params.md b/packages/eslint-plugin/docs/rules/max-params.md index 3ee62f769b3d..03854473cf36 100644 --- a/packages/eslint-plugin/docs/rules/max-params.md +++ b/packages/eslint-plugin/docs/rules/max-params.md @@ -7,4 +7,4 @@ description: 'Enforce a maximum number of parameters in function definitions.' > See **https://typescript-eslint.io/rules/max-params** for documentation. This rule extends the base [`eslint/max-params`](https://eslint.org/docs/rules/max-params) rule. -This version adds support for TypeScript `this` parameter so it won't be counted as a parameter. +This version adds support for TypeScript `this` parameters so they won't be counted as a parameter. diff --git a/packages/eslint-plugin/src/rules/max-params.ts b/packages/eslint-plugin/src/rules/max-params.ts index 73729e367087..45a912fe721d 100644 --- a/packages/eslint-plugin/src/rules/max-params.ts +++ b/packages/eslint-plugin/src/rules/max-params.ts @@ -59,17 +59,14 @@ export default util.createRule({ return node; } - const params = [...node.params]; - - if ( - params[0] && - params[0].type === AST_NODE_TYPES.Identifier && - params[0].name === 'this' && - params[0].typeAnnotation?.typeAnnotation.type === + const params = + node.params[0] && + node.params[0].type === AST_NODE_TYPES.Identifier && + node.params[0].name === 'this' && + node.params[0].typeAnnotation?.typeAnnotation.type === AST_NODE_TYPES.TSVoidKeyword - ) { - params.shift(); - } + ? node.params.slice(1) + : node.params; return { ...node, From cfc6c29df2fba7dc1fe05bf5e73ecd093ef6978d Mon Sep 17 00:00:00 2001 From: StyleShit <32631382+StyleShit@users.noreply.github.com> Date: Tue, 17 Oct 2023 16:46:06 +0300 Subject: [PATCH 7/8] Update packages/eslint-plugin/src/rules/max-params.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Josh Goldberg ✨ --- packages/eslint-plugin/src/rules/max-params.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/max-params.ts b/packages/eslint-plugin/src/rules/max-params.ts index 45a912fe721d..1dee33dc1150 100644 --- a/packages/eslint-plugin/src/rules/max-params.ts +++ b/packages/eslint-plugin/src/rules/max-params.ts @@ -60,7 +60,6 @@ export default util.createRule({ } const params = - node.params[0] && node.params[0].type === AST_NODE_TYPES.Identifier && node.params[0].name === 'this' && node.params[0].typeAnnotation?.typeAnnotation.type === From b69113a1984822c0fa96c466eeea46733834041b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 17 Oct 2023 09:49:12 -0400 Subject: [PATCH 8/8] Small PR nits, don't mind me, it's great either way --- .../eslint-plugin/src/rules/max-params.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/src/rules/max-params.ts b/packages/eslint-plugin/src/rules/max-params.ts index 1dee33dc1150..29a18358946f 100644 --- a/packages/eslint-plugin/src/rules/max-params.ts +++ b/packages/eslint-plugin/src/rules/max-params.ts @@ -1,6 +1,10 @@ import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; -import * as util from '../util'; +import type { + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule, +} from '../util'; +import { createRule } from '../util'; import { getESLintCoreRule } from '../util/getESLintCoreRule'; type FunctionLike = @@ -12,10 +16,10 @@ type FunctionRuleListener = (node: T) => void; const baseRule = getESLintCoreRule('max-params'); -export type Options = util.InferOptionsTypeFromRule; -export type MessageIds = util.InferMessageIdsTypeFromRule; +export type Options = InferOptionsTypeFromRule; +export type MessageIds = InferMessageIdsTypeFromRule; -export default util.createRule({ +export default createRule({ name: 'max-params', meta: { type: 'suggestion', @@ -55,21 +59,19 @@ export default util.createRule({ } const removeVoidThisParam = (node: T): T => { - if (node.params.length === 0) { + if ( + node.params.length === 0 || + node.params[0].type !== AST_NODE_TYPES.Identifier || + node.params[0].name !== 'this' || + node.params[0].typeAnnotation?.typeAnnotation.type !== + AST_NODE_TYPES.TSVoidKeyword + ) { return node; } - const params = - node.params[0].type === AST_NODE_TYPES.Identifier && - node.params[0].name === 'this' && - node.params[0].typeAnnotation?.typeAnnotation.type === - AST_NODE_TYPES.TSVoidKeyword - ? node.params.slice(1) - : node.params; - return { ...node, - params, + params: node.params.slice(1), }; };