fix(eslint-plugin): [expl-member-a11y] fix parameter properties (#912)

Author: ankeetmaini

packages/eslint-plugin/docs/rules/explicit-member-accessibility.md

@@ -214,6 +214,46 @@ class Animal {
 }
 ```
 
+e.g. `[ { accessibility: 'off', overrides: { parameterProperties: 'explicit' } } ]`
+
+The following code is considered incorrect with the example override
+
+```ts
+class Animal {
+  constructor(readonly animalName: string) {}
+}
+```
+
+The following code patterns are considered correct with the example override
+
+```ts
+class Animal {
+  constructor(public readonly animalName: string) {}
+}
+
+class Animal {
+  constructor(public animalName: string) {}
+}
+```
+
+e.g. `[ { accessibility: 'off', overrides: { parameterProperties: 'no-public' } } ]`
+
+The following code is considered incorrect with the example override
+
+```ts
+class Animal {
+  constructor(public readonly animalName: string) {}
+}
+```
+
+The following code is considered correct with the example override
+
+```ts
+class Animal {
+  constructor(public animalName: string) {}
+}
+```
+
 #### Disable any checks on given member type
 
 e.g. `[{ overrides: { accessors : 'off' } } ]`

packages/eslint-plugin/src/rules/explicit-member-accessibility.ts

@@ -196,8 +196,24 @@ export default util.createRule<Options, MessageIds>({
           : // has to be an Identifier or TSC will throw an error
             (node.parameter.left as TSESTree.Identifier).name;
 
-      if (paramPropCheck === 'no-public' && node.accessibility === 'public') {
-        reportIssue('unwantedPublicAccessibility', nodeType, node, nodeName);
+      switch (paramPropCheck) {
+        case 'explicit': {
+          if (!node.accessibility) {
+            reportIssue('missingAccessibility', nodeType, node, nodeName);
+          }
+          break;
+        }
+        case 'no-public': {
+          if (node.accessibility === 'public' && node.readonly) {
+            reportIssue(
+              'unwantedPublicAccessibility',
+              nodeType,
+              node,
+              nodeName,
+            );
+          }
+          break;
+        }
       }
     }
 

packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts

@@ -10,6 +10,104 @@ ruleTester.run('explicit-member-accessibility', rule, {
     {
       filename: 'test.ts',
       code: `
+class Test {
+  public constructor(private foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'explicit' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(private readonly foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'explicit' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(private foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'off' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(protected foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'off' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(public foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'off' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(readonly foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'off' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(private readonly foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'off' },
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
 class Test {
   protected name: string
   private x: number
@@ -147,11 +245,90 @@ class Test {
         },
       ],
     },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  constructor(public foo: number){}
+}
+      `,
+      options: [
+        {
+          accessibility: 'no-public',
+        },
+      ],
+    },
   ],
   invalid: [
     {
       filename: 'test.ts',
       code: `
+export class XXXX {
+  public constructor(readonly value: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'off',
+          overrides: {
+            parameterProperties: 'explicit',
+          },
+        },
+      ],
+      errors: [
+        {
+          messageId: 'missingAccessibility',
+          column: 22,
+          line: 3,
+        },
+      ],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+export class WithParameterProperty {
+  public constructor(readonly value: string) {}
+}
+      `,
+      options: [{ accessibility: 'explicit' }],
+      errors: [{ messageId: 'missingAccessibility' }],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+export class XXXX {
+  public constructor(readonly samosa: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'off',
+          overrides: {
+            constructors: 'explicit',
+            parameterProperties: 'explicit',
+          },
+        },
+      ],
+      errors: [{ messageId: 'missingAccessibility' }],
+    },
+    {
+      filename: 'test.ts',
+      code: `
+class Test {
+  public constructor(readonly foo: string) {}
+}
+      `,
+      options: [
+        {
+          accessibility: 'explicit',
+          overrides: { parameterProperties: 'explicit' },
+        },
+      ],
+      errors: [{ messageId: 'missingAccessibility' }],
+    },
+    {
+      filename: 'test.ts',
+      code: `
 class Test {
   x: number
   public getX () {
@@ -365,18 +542,21 @@ class Test {
       code: `
 class Test {
   constructor(public x: number){}
+  public foo(): string {
+    return 'foo';
+  }
 }
       `,
       errors: [
         {
-          messageId: 'unwantedPublicAccessibility',
+          messageId: 'missingAccessibility',
           line: 3,
-          column: 15,
+          column: 3,
         },
       ],
       options: [
         {
-          accessibility: 'no-public',
+          overrides: { parameterProperties: 'no-public' },
         },
       ],
     },
@@ -385,9 +565,6 @@ class Test {
       code: `
 class Test {
   constructor(public x: number){}
-  public foo(): string {
-    return 'foo';
-  }
 }
       `,
       errors: [
@@ -396,30 +573,26 @@ class Test {
           line: 3,
           column: 3,
         },
-        {
-          messageId: 'unwantedPublicAccessibility',
-          line: 3,
-          column: 15,
-        },
-      ],
-      options: [
-        {
-          overrides: { parameterProperties: 'no-public' },
-        },
       ],
     },
     {
       filename: 'test.ts',
       code: `
 class Test {
-  constructor(public x: number){}
+  constructor(public readonly x: number){}
 }
       `,
+      options: [
+        {
+          accessibility: 'off',
+          overrides: { parameterProperties: 'no-public' },
+        },
+      ],
       errors: [
         {
-          messageId: 'missingAccessibility',
+          messageId: 'unwantedPublicAccessibility',
           line: 3,
-          column: 3,
+          column: 15,
         },
       ],
     },