From a37ff9f3af97ccd83727b7377ae535f61d2651f4 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Fri, 10 Jan 2020 22:02:46 +0200 Subject: [PATCH] feat(eslint-plugin): add default-param-last rule (#1418) --- packages/eslint-plugin/README.md | 1 + .../docs/rules/default-param-last.md | 29 + packages/eslint-plugin/src/configs/all.json | 1 + .../src/rules/default-param-last.ts | 76 +++ packages/eslint-plugin/src/rules/index.ts | 2 + .../tests/rules/default-param-last.test.ts | 531 ++++++++++++++++++ 6 files changed, 640 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/default-param-last.md create mode 100644 packages/eslint-plugin/src/rules/default-param-last.ts create mode 100644 packages/eslint-plugin/tests/rules/default-param-last.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index a332bafec80..cdf79f97f3f 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -105,6 +105,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/class-name-casing`](./docs/rules/class-name-casing.md) | Require PascalCased class and interface names | :heavy_check_mark: | | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | +| [`@typescript-eslint/default-param-last`](./docs/rules/default-param-last.md) | Enforce default parameters to be last | | | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | | [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | | | | [`@typescript-eslint/func-call-spacing`](./docs/rules/func-call-spacing.md) | Require or disallow spacing between function identifiers and their invocations | | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/default-param-last.md b/packages/eslint-plugin/docs/rules/default-param-last.md new file mode 100644 index 00000000000..8f2518c50ae --- /dev/null +++ b/packages/eslint-plugin/docs/rules/default-param-last.md @@ -0,0 +1,29 @@ +# Enforce default parameters to be last (`default-param-last`) + +## Rule Details + +This rule enforces default or optional parameters to be the last of parameters. + +Examples of **incorrect** code for this rule: + +```ts +/* eslint @typescript-eslint/default-param-last: ["error"] */ + +function f(a = 0, b: number) {} +function f(a: number, b = 0, c?: number) {} +function f(a: number, b = 0, c: number) {} +function f(a: number, b?: number, c: number) {} +``` + +Examples of **correct** code for this rule: + +```ts +/* eslint @typescript-eslint/default-param-last: ["error"] */ + +function f(a = 0) {} +function f(a: number, b = 0) {} +function f(a: number, b?: number) {} +function f(a: number, b?: number, c = 0) {} +``` + +Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/default-param-last.md) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index f9eed1fca9c..d046a2c9c0b 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -13,6 +13,7 @@ "@typescript-eslint/class-name-casing": "error", "@typescript-eslint/consistent-type-assertions": "error", "@typescript-eslint/consistent-type-definitions": "error", + "@typescript-eslint/default-param-last": "error", "@typescript-eslint/explicit-function-return-type": "error", "@typescript-eslint/explicit-member-accessibility": "error", "func-call-spacing": "off", diff --git a/packages/eslint-plugin/src/rules/default-param-last.ts b/packages/eslint-plugin/src/rules/default-param-last.ts new file mode 100644 index 00000000000..0f0191155a4 --- /dev/null +++ b/packages/eslint-plugin/src/rules/default-param-last.ts @@ -0,0 +1,76 @@ +import { createRule } from '../util'; +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; + +export default createRule({ + name: 'default-param-last', + meta: { + type: 'suggestion', + docs: { + description: 'Enforce default parameters to be last', + category: 'Best Practices', + recommended: false, + }, + schema: [], + messages: { + shouldBeLast: 'Default parameters should be last.', + }, + }, + defaultOptions: [], + create(context) { + /** + * checks if node is optional parameter + * @param node the node to be evaluated + * @private + */ + function isOptionalParam(node: TSESTree.Parameter): boolean { + return 'optional' in node && node.optional === true; + } + + /** + * checks if node is plain parameter + * @param node the node to be evaluated + * @private + */ + function isPlainParam(node: TSESTree.Parameter): boolean { + return !( + node.type === AST_NODE_TYPES.AssignmentPattern || + node.type === AST_NODE_TYPES.RestElement || + isOptionalParam(node) + ); + } + + function checkDefaultParamLast( + node: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression, + ): void { + let hasSeenPlainParam = false; + for (let i = node.params.length - 1; i >= 0; i--) { + const param = node.params[i]; + + if (isPlainParam(param)) { + hasSeenPlainParam = true; + continue; + } + + if ( + hasSeenPlainParam && + (isOptionalParam(param) || + param.type === AST_NODE_TYPES.AssignmentPattern) + ) { + context.report({ node: param, messageId: 'shouldBeLast' }); + } + } + } + + return { + ArrowFunctionExpression: checkDefaultParamLast, + FunctionDeclaration: checkDefaultParamLast, + FunctionExpression: checkDefaultParamLast, + }; + }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 5d2f53cce25..bb5589a6220 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -8,6 +8,7 @@ import camelcase from './camelcase'; import classNameCasing from './class-name-casing'; import consistentTypeAssertions from './consistent-type-assertions'; import consistentTypeDefinitions from './consistent-type-definitions'; +import defaultParamLast from './default-param-last'; import explicitFunctionReturnType from './explicit-function-return-type'; import explicitMemberAccessibility from './explicit-member-accessibility'; import funcCallSpacing from './func-call-spacing'; @@ -87,6 +88,7 @@ export default { 'class-name-casing': classNameCasing, 'consistent-type-assertions': consistentTypeAssertions, 'consistent-type-definitions': consistentTypeDefinitions, + 'default-param-last': defaultParamLast, 'explicit-function-return-type': explicitFunctionReturnType, 'explicit-member-accessibility': explicitMemberAccessibility, 'func-call-spacing': funcCallSpacing, diff --git a/packages/eslint-plugin/tests/rules/default-param-last.test.ts b/packages/eslint-plugin/tests/rules/default-param-last.test.ts new file mode 100644 index 00000000000..dd70b3b05cd --- /dev/null +++ b/packages/eslint-plugin/tests/rules/default-param-last.test.ts @@ -0,0 +1,531 @@ +import rule from '../../src/rules/default-param-last'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('default-param-last', rule, { + valid: [ + 'function foo() {}', + 'function foo(a: number) {}', + 'function foo(a = 1) {}', + 'function foo(a?: number) {}', + 'function foo(a: number, b: number) {}', + 'function foo(a: number, b: number, c?: number) {}', + 'function foo(a: number, b = 1) {}', + 'function foo(a: number, b = 1, c = 1) {}', + 'function foo(a: number, b = 1, c?: number) {}', + 'function foo(a: number, b?: number, c = 1) {}', + 'function foo(a: number, b = 1, ...c) {}', + + 'const foo = function () {}', + 'const foo = function (a: number) {}', + 'const foo = function (a = 1) {}', + 'const foo = function (a?: number) {}', + 'const foo = function (a: number, b: number) {}', + 'const foo = function (a: number, b: number, c?: number) {}', + 'const foo = function (a: number, b = 1) {}', + 'const foo = function (a: number, b = 1, c = 1) {}', + 'const foo = function (a: number, b = 1, c?: number) {}', + 'const foo = function (a: number, b?: number, c = 1) {}', + 'const foo = function (a: number, b = 1, ...c) {}', + + 'const foo = () => {}', + 'const foo = (a: number) => {}', + 'const foo = (a = 1) => {}', + 'const foo = (a?: number) => {}', + 'const foo = (a: number, b: number) => {}', + 'const foo = (a: number, b: number, c?: number) => {}', + 'const foo = (a: number, b = 1) => {}', + 'const foo = (a: number, b = 1, c = 1) => {}', + 'const foo = (a: number, b = 1, c?: number) => {}', + 'const foo = (a: number, b?: number, c = 1) => {}', + 'const foo = (a: number, b = 1, ...c) => {}', + ], + invalid: [ + { + code: 'function foo(a = 1, b: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'function foo(a = 1, b = 2, c: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 21, + endColumn: 26, + }, + ], + }, + { + code: 'function foo(a = 1, b: number, c = 2, d: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 32, + endColumn: 37, + }, + ], + }, + { + code: 'function foo(a = 1, b: number, c = 2) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'function foo(a = 1, b: number, ...c) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'function foo(a?: number, b: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 24, + }, + ], + }, + { + code: 'function foo(a: number, b?: number, c: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 25, + endColumn: 35, + }, + ], + }, + { + code: 'function foo(a = 1, b?: number, c: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 21, + endColumn: 31, + }, + ], + }, + { + code: 'function foo(a = 1, { b }) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'function foo({ a } = {}, b) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 24, + }, + ], + }, + { + code: 'function foo({ a, b } = { a: 1, b: 2 }, c) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 39, + }, + ], + }, + { + code: 'function foo([a] = [], b) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 22, + }, + ], + }, + { + code: 'function foo([a, b] = [1, 2], c) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 29, + }, + ], + }, + { + code: 'const foo = function(a = 1, b: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + ], + }, + { + code: 'const foo = function(a = 1, b = 2, c: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 29, + endColumn: 34, + }, + ], + }, + { + code: 'const foo = function(a = 1, b: number, c = 2, d: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 40, + endColumn: 45, + }, + ], + }, + { + code: 'const foo = function(a = 1, b: number, c = 2) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + ], + }, + { + code: 'const foo = function(a = 1, b: number, ...c) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + ], + }, + { + code: 'const foo = function(a?: number, b: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 32, + }, + ], + }, + { + code: 'const foo = function(a: number, b?: number, c: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 33, + endColumn: 43, + }, + ], + }, + { + code: 'const foo = function(a = 1, b?: number, c: number) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 29, + endColumn: 39, + }, + ], + }, + { + code: 'const foo = function(a = 1, { b }) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 27, + }, + ], + }, + { + code: 'const foo = function({ a } = {}, b) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 32, + }, + ], + }, + { + code: 'const foo = function({ a, b } = { a: 1, b: 2 }, c) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 47, + }, + ], + }, + { + code: 'const foo = function([a] = [], b) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 30, + }, + ], + }, + { + code: 'const foo = function([a, b] = [1, 2], c) {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 22, + endColumn: 37, + }, + ], + }, + { + code: 'const foo = (a = 1, b: number) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'const foo = (a = 1, b = 2, c: number) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 21, + endColumn: 26, + }, + ], + }, + { + code: 'const foo = (a = 1, b: number, c = 2, d: number) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 32, + endColumn: 37, + }, + ], + }, + { + code: 'const foo = (a = 1, b: number, c = 2) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'const foo = (a = 1, b: number, ...c) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'const foo = (a?: number, b: number) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 24, + }, + ], + }, + { + code: 'const foo = (a: number, b?: number, c: number) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 25, + endColumn: 35, + }, + ], + }, + { + code: 'const foo = (a = 1, b?: number, c: number) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + { + messageId: 'shouldBeLast', + line: 1, + column: 21, + endColumn: 31, + }, + ], + }, + { + code: 'const foo = (a = 1, { b }) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 19, + }, + ], + }, + { + code: 'const foo = ({ a } = {}, b) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 24, + }, + ], + }, + { + code: 'const foo = ({ a, b } = { a: 1, b: 2 }, c) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 39, + }, + ], + }, + { + code: 'const foo = ([a] = [], b) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 22, + }, + ], + }, + { + code: 'const foo = ([a, b] = [1, 2], c) => {}', + errors: [ + { + messageId: 'shouldBeLast', + line: 1, + column: 14, + endColumn: 29, + }, + ], + }, + ], +});