fix(eslint-plugin): [default-param-last] handle param props (#1650)

typescript-eslint · Feb 28, 2020 · 3534c6e · 3534c6e
1 parent 4eedd7f
commit 3534c6e
Show file tree

Hide file tree

Showing 3 changed files with 153 additions and 2 deletions.
diff --git a/packages/eslint-plugin/docs/rules/default-param-last.md b/packages/eslint-plugin/docs/rules/default-param-last.md
@@ -12,6 +12,12 @@ Examples of **incorrect** code for this rule:
 function f(a = 0, b: number) {}
 function f(a: number, b = 0, c: number) {}
 function f(a: number, b?: number, c: number) {}
+class Foo {
+  constructor(public a = 10, private b: number) {}
+}
+class Foo {
+  constructor(public a?: number, private b: number) {}
+}
 ```
 
 Examples of **correct** code for this rule:
@@ -24,6 +30,12 @@ function f(a: number, b = 0) {}
 function f(a: number, b?: number) {}
 function f(a: number, b?: number, c = 0) {}
 function f(a: number, b = 0, c?: number) {}
+class Foo {
+  constructor(public a, private b = 0) {}
+}
+class Foo {
+  constructor(public a, private b?: number) {}
+}
 ```
 
 <sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/default-param-last.md)</sup>
diff --git a/packages/eslint-plugin/src/rules/default-param-last.ts b/packages/eslint-plugin/src/rules/default-param-last.ts
@@ -51,7 +51,11 @@ export default createRule({
     ): void {
       let hasSeenPlainParam = false;
       for (let i = node.params.length - 1; i >= 0; i--) {
-        const param = node.params[i];
+        const current = node.params[i];
+        const param =
+          current.type === AST_NODE_TYPES.TSParameterProperty
+            ? current.parameter
+            : current;
 
         if (isPlainParam(param)) {
           hasSeenPlainParam = true;
@@ -63,7 +67,7 @@ export default createRule({
           (isOptionalParam(param) ||
             param.type === AST_NODE_TYPES.AssignmentPattern)
         ) {
-          context.report({ node: param, messageId: 'shouldBeLast' });
+          context.report({ node: current, messageId: 'shouldBeLast' });
         }
       }
     }

diff --git a/packages/eslint-plugin/tests/rules/default-param-last.test.ts b/packages/eslint-plugin/tests/rules/default-param-last.test.ts
@@ -42,6 +42,51 @@ ruleTester.run('default-param-last', rule, {
     'const foo = (a: number, b = 1, c?: number) => {}',
     'const foo = (a: number, b?: number, c = 1) => {}',
     'const foo = (a: number, b = 1, ...c) => {}',
+    `
+class Foo {
+  constructor(a: number, b: number, c: number) {}
+}
+    `,
+    `
+class Foo {
+  constructor(a: number, b?: number, c = 1) {}
+}
+    `,
+    `
+class Foo {
+  constructor(a: number, b = 1, c?: number) {}
+}
+    `,
+    `
+class Foo {
+  constructor(public a: number, protected b: number, private c: number) {}
+}
+    `,
+    `
+class Foo {
+  constructor(public a: number, protected b?: number, private c = 10) {}
+}
+    `,
+    `
+class Foo {
+  constructor(public a: number, protected b = 10, private c?: number) {}
+}
+    `,
+    `
+class Foo {
+  constructor(a: number, protected b?: number, private c = 0) {}
+}
+    `,
+    `
+class Foo {
+  constructor(a: number, b?: number, private c = 0) {}
+}
+    `,
+    `
+class Foo {
+  constructor(a: number, private b?: number, c = 0) {}
+}
+    `,
   ],
   invalid: [
     {
@@ -527,5 +572,95 @@ ruleTester.run('default-param-last', rule, {
         },
       ],
     },
+    {
+      code: `
+class Foo {
+  constructor(public a: number, protected b?: number, private c: number) {}
+}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeLast',
+          line: 3,
+          column: 33,
+          endColumn: 53,
+        },
+      ],
+    },
+    {
+      code: `
+class Foo {
+  constructor(public a: number, protected b = 0, private c: number) {}
+}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeLast',
+          line: 3,
+          column: 33,
+          endColumn: 48,
+        },
+      ],
+    },
+    {
+      code: `
+class Foo {
+  constructor(public a?: number, private b: number) {}
+}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeLast',
+          line: 3,
+          column: 15,
+          endColumn: 32,
+        },
+      ],
+    },
+    {
+      code: `
+class Foo {
+  constructor(public a = 0, private b: number) {}
+}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeLast',
+          line: 3,
+          column: 15,
+          endColumn: 27,
+        },
+      ],
+    },
+    {
+      code: `
+class Foo {
+  constructor(a = 0, b: number) {}
+}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeLast',
+          line: 3,
+          column: 15,
+          endColumn: 20,
+        },
+      ],
+    },
+    {
+      code: `
+class Foo {
+  constructor(a?: number, b: number) {}
+}
+      `,
+      errors: [
+        {
+          messageId: 'shouldBeLast',
+          line: 3,
+          column: 15,
+          endColumn: 25,
+        },
+      ],
+    },
   ],
 });