member-access: add fixer (palantir#2969)

HyphnKnight · Apr 9, 2018 · a98d492 · a98d492
1 parent 3bb6fb1
commit a98d492
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 10 deletions.
diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-import { getChildOfKind, isClassLikeDeclaration } from "tsutils";
+import { getChildOfKind, getModifier, getNextToken, isClassLikeDeclaration } from "tsutils";
 import * as ts from "typescript";
 
 import { showWarningOnce } from "../error";
@@ -55,6 +55,7 @@ export class Rule extends Lint.Rules.AbstractRule {
         optionExamples: [true, [true, OPTION_NO_PUBLIC], [true, OPTION_CHECK_ACCESSOR]],
         type: "typescript",
         typescriptOnly: true,
+        hasFix: true,
     };
     /* tslint:enable:object-literal-sort-keys */
 
@@ -117,19 +118,26 @@ function walk(ctx: Lint.WalkContext<Options>) {
         if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.ProtectedKeyword, ts.SyntaxKind.PrivateKeyword)) {
             return;
         }
-
-        const isPublic = Lint.hasModifier(node.modifiers, ts.SyntaxKind.PublicKeyword);
-
-        if (noPublic && isPublic) {
-            const publicKeyword = node.modifiers!.find((m) => m.kind === ts.SyntaxKind.PublicKeyword)!;
-            ctx.addFailureAtNode(publicKeyword, Rule.FAILURE_STRING_NO_PUBLIC);
+        const publicKeyword = getModifier(node, ts.SyntaxKind.PublicKeyword);
+        if (noPublic && publicKeyword !== undefined) {
+            const start = publicKeyword.end - "public".length;
+            ctx.addFailure(
+                start,
+                publicKeyword.end,
+                Rule.FAILURE_STRING_NO_PUBLIC,
+                Lint.Replacement.deleteFromTo(start, getNextToken(publicKeyword, ctx.sourceFile)!.getStart(ctx.sourceFile)),
+            );
         }
-        if (!noPublic && !isPublic) {
+        if (!noPublic && publicKeyword === undefined) {
             const nameNode = node.kind === ts.SyntaxKind.Constructor
                 ? getChildOfKind(node, ts.SyntaxKind.ConstructorKeyword, ctx.sourceFile)!
                 : node.name !== undefined ? node.name : node;
             const memberName = node.name !== undefined && node.name.kind === ts.SyntaxKind.Identifier ? node.name.text : undefined;
-            ctx.addFailureAtNode(nameNode, Rule.FAILURE_STRING_FACTORY(typeToString(node), memberName));
+            ctx.addFailureAtNode(
+                nameNode,
+                Rule.FAILURE_STRING_FACTORY(typeToString(node), memberName),
+                Lint.Replacement.appendText(node.getStart(ctx.sourceFile), "public "),
+            );
         }
     }
 }

diff --git a/test/rules/member-access/accessor/test.ts.fix b/test/rules/member-access/accessor/test.ts.fix
@@ -0,0 +1,17 @@
+class Members {
+    public get g() {
+        return 1;
+    }
+    public set s(o: any) {}
+
+    public get publicG() {
+        return 1;
+    }
+    public set publicS(o: any) {}
+}
+
+const obj = {
+    get g() {
+        return 1;
+    }
+};
diff --git a/test/rules/member-access/constructor/test.ts.fix b/test/rules/member-access/constructor/test.ts.fix
@@ -0,0 +1,10 @@
+class ContructorsNoAccess {
+    public constructor(i: number);
+    public constructor(o: any) {}
+}
+
+class ContructorsAccess {
+    public constructor(i: number);
+    public constructor(o: any) {}
+}
+
diff --git a/test/rules/member-access/default/test.ts.fix b/test/rules/member-access/default/test.ts.fix
@@ -0,0 +1,48 @@
+declare class AmbientNoAccess {
+    public a(): number;
+
+    public static b(): number;
+}
+
+declare class AmbientAccess {
+    public a(): number;
+    public static b(): number;
+}
+
+class Members {
+    [x: string]: number;
+    public i: number;
+    public static j: number;
+
+    public nPublic: number;
+    protected nProtected: number;
+    private nPrivate: number;
+
+    public static nsPublic: number;
+    protected static nsProtected: number;
+    private static nsPrivate: number;
+
+    public noAccess(x: number): number;
+    public noAccess(o: any): any {}
+
+    public static noAccess(x: number): number;
+    public static noAccess(o: any): any {}
+
+    public access(x: number): number;
+    public access(o: any): any {}
+
+    public static access(x: number): number;
+    public static access(o: any): any {}
+}
+
+const obj = {
+    func() {}
+};
+
+function main() {
+    class A {
+        public i: number;
+        public static j: number;
+        public n: number;
+    }
+}
diff --git a/test/rules/member-access/no-public/test.ts.fix b/test/rules/member-access/no-public/test.ts.fix
@@ -0,0 +1,7 @@
+class {
+    x() {}
+    constructor() {}
+    get y() {}
+    public() {}
+}
+
diff --git a/test/rules/member-access/no-public/test.ts.lint b/test/rules/member-access/no-public/test.ts.lint
@@ -3,8 +3,9 @@ class {
     ~~~~~~ [0]
     public constructor() {}
     ~~~~~~ [0]
-    public get y() {}
+    public/*some comment*/ get y() {}
     ~~~~~~ [0]
+    public() {}
 }
 
 [0]: 'public' is implicit.