Fix4057 Correct the broken auto-fix code of prefer-method-signature…

… rule. (palantir#4066) * Add test to show the current fix of multiline signature is incomplete. * Correct the replace function of prefer-method-signature rule.
grantwwu · Jul 25, 2018 · 9280754 · 9280754
1 parent 9a31504
commit 9280754
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 6 deletions.
diff --git a/src/rules/preferMethodSignatureRule.ts b/src/rules/preferMethodSignatureRule.ts
@@ -30,11 +30,31 @@ export class Rule extends Lint.Rules.AbstractRule {
         options: null,
         optionExamples: [true],
         type: "style",
-        typescriptOnly: false,
+        typescriptOnly: false
     };
     /* tslint:enable:object-literal-sort-keys */
 
-    public static FAILURE_STRING = "Use a method signature instead of a property signature of function type.";
+    public static FAILURE_STRING =
+        "Use a method signature instead of a property signature of function type.";
+
+    public static METH_SIGN_STRING(ps: ts.PropertySignature): string {
+        const { type, questionToken } = ps;
+
+        let result = ps.name.getText();
+        if (questionToken !== undefined) {
+            result += "?";
+        }
+        if (type !== undefined && isFunctionTypeNode(type) && type.type !== undefined) {
+            if (type.typeParameters !== undefined) {
+                const tps = type.typeParameters.map(tp => tp.getText()).join(", ");
+                result += `<${tps}>`;
+            }
+            const args = type.parameters.map(v => v.getText()).join(", ");
+            result += `(${args}): ${type.type.getText()};`;
+        }
+
+        return result;
+    }
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
         return this.applyWithFunction(sourceFile, walk);
@@ -46,10 +66,13 @@ function walk(ctx: Lint.WalkContext<void>): void {
         if (isPropertySignature(node)) {
             const { type } = node;
             if (type !== undefined && isFunctionTypeNode(type)) {
-                ctx.addFailureAtNode(node.name, Rule.FAILURE_STRING, type.type === undefined ? undefined : [
-                    Lint.Replacement.deleteFromTo(type.pos - 1, type.getStart()), // delete colon in 'foo: () => void'
-                    Lint.Replacement.replaceFromTo(type.parameters.end + 1, type.type.pos, ":"), // replace => with colon
-                ]);
+                ctx.addFailureAtNode(
+                    node.name,
+                    Rule.FAILURE_STRING,
+                    type.type === undefined
+                        ? undefined
+                        : [Lint.Replacement.replaceNode(node, Rule.METH_SIGN_STRING(node))]
+                );
             }
         }
         return ts.forEachChild(node, cb);

diff --git a/test/rules/prefer-method-signature/test.ts.fix b/test/rules/prefer-method-signature/test.ts.fix
@@ -12,3 +12,7 @@ class C {
     foo: () => void;
 }
 
+export interface Bar {
+  someMethod(someArg: Pick<HTMLAllCollection, "item">): string;
+}
+
diff --git a/test/rules/prefer-method-signature/test.ts.lint b/test/rules/prefer-method-signature/test.ts.lint
@@ -15,4 +15,11 @@ class C {
     foo: () => void;
 }
 
+export interface Bar {
+  someMethod: (
+  ~~~~~~~~~~ [0]
+    someArg: Pick<HTMLAllCollection, "item">
+  ) => string;
+}
+
 [0]: Use a method signature instead of a property signature of function type.