object-literal-shorthand: Refactor, fix and add fixer for methods (#2558

) [new-fixer] `object-literal-shorthand` can fix longhand methods [bugfix] `object-literal-shorthand`: fix suggestion for generator functions [enhancement] `object-literal-shorthand` handles async functions correctly
palantir · May 24, 2017 · 47b20d9 · 47b20d9
1 parent d52d587
commit 47b20d9
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 32 deletions.
diff --git a/src/rules/objectLiteralShorthandRule.ts b/src/rules/objectLiteralShorthandRule.ts
@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import { getChildOfKind, hasModifier, isFunctionExpression, isIdentifier, isPropertyAssignment } from "tsutils";
 import * as ts from "typescript";
 import * as Lint from "../index";
 
@@ -36,35 +37,44 @@ export class Rule extends Lint.Rules.AbstractRule {
     public static LONGHAND_METHOD = "Expected method shorthand in object literal ";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        const objectLiteralShorthandWalker = new ObjectLiteralShorthandWalker(sourceFile, this.getOptions());
-        return this.applyWithWalker(objectLiteralShorthandWalker);
+        return this.applyWithFunction(sourceFile, walk);
     }
 }
 
-class ObjectLiteralShorthandWalker extends Lint.RuleWalker {
-
-    public visitPropertyAssignment(node: ts.PropertyAssignment) {
-        const name = node.name;
-        const value = node.initializer;
-
-        if (name.kind === ts.SyntaxKind.Identifier &&
-            value.kind === ts.SyntaxKind.Identifier &&
-            name.getText() === value.getText()) {
-                // Delete from name start up to value to include the ':'.
-                const lengthToValueStart = value.getStart() - name.getStart();
-                const fix = this.deleteText(name.getStart(), lengthToValueStart);
-                this.addFailureAtNode(node, `${Rule.LONGHAND_PROPERTY}('{${name.getText()}}').`, fix);
-        }
-
-        if (value.kind === ts.SyntaxKind.FunctionExpression) {
-            const fnNode = value as ts.FunctionExpression;
-            if (fnNode.name !== undefined) {
-                return;  // named function expressions are OK.
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node): void {
+        if (isPropertyAssignment(node)) {
+            if (node.name.kind === ts.SyntaxKind.Identifier &&
+                isIdentifier(node.initializer) &&
+                node.name.text === node.initializer.text) {
+                ctx.addFailureAtNode(
+                    node,
+                    `${Rule.LONGHAND_PROPERTY}('{${node.name.text}}').`,
+                    Lint.Replacement.deleteFromTo(node.name.end, node.end),
+                );
+            } else if (isFunctionExpression(node.initializer) &&
+                       // allow named function expressions
+                       node.initializer.name === undefined) {
+                const [name, fix] = handleLonghandMethod(node.name, node.initializer, ctx.sourceFile);
+                ctx.addFailureAtNode(node, `${Rule.LONGHAND_METHOD}('{${name}() {...}}').`, fix);
             }
-            const star = fnNode.asteriskToken !== undefined ? fnNode.asteriskToken.getText() : "";
-            this.addFailureAtNode(node, `${Rule.LONGHAND_METHOD}('{${name.getText()}${star}() {...}}').`);
         }
+        return ts.forEachChild(node, cb);
+    });
+}
 
-        super.visitPropertyAssignment(node);
+function handleLonghandMethod(name: ts.PropertyName, initializer: ts.FunctionExpression, sourceFile: ts.SourceFile): [string, Lint.Fix] {
+    const nameStart = name.getStart(sourceFile);
+    let fix: Lint.Fix = Lint.Replacement.deleteFromTo(name.end, getChildOfKind(initializer, ts.SyntaxKind.OpenParenToken)!.pos);
+    let prefix = "";
+    if (initializer.asteriskToken !== undefined) {
+        prefix = "*";
+    }
+    if (hasModifier(initializer.modifiers, ts.SyntaxKind.AsyncKeyword)) {
+        prefix = `async ${prefix}`;
+    }
+    if (prefix !== "") {
+        fix = [fix, Lint.Replacement.appendText(nameStart, prefix)];
     }
+    return [prefix + sourceFile.text.substring(nameStart, name.end), fix];
 }
diff --git a/test/rules/object-literal-shorthand/test.ts.fix b/test/rules/object-literal-shorthand/test.ts.fix
@@ -1,7 +1,7 @@
 const bad = {
-  w: function() {},
-  x: function *() {},
-  [y]: function() {},
+  w() {},
+  *x() {},
+  [y]() {},
   z
 };
 
@@ -21,7 +21,7 @@ const namedFunctions = {
 };
 
 const quotes = {
-  "foo-bar": function() {},
+  "foo-bar"() {},
   "foo-bar"() {}
 };
 
@@ -35,3 +35,7 @@ const extraCases = {
   }
 };
 
+const asyncFn = {
+  async foo() {},
+  async *bar() {}
+}
diff --git a/test/rules/object-literal-shorthand/test.ts.lint b/test/rules/object-literal-shorthand/test.ts.lint
@@ -2,7 +2,7 @@ const bad = {
   w: function() {},
   ~~~~~~~~~~~~~~~~  [Expected method shorthand in object literal ('{w() {...}}').]
   x: function *() {},
-  ~~~~~~~~~~~~~~~~~~  [Expected method shorthand in object literal ('{x*() {...}}').]
+  ~~~~~~~~~~~~~~~~~~  [Expected method shorthand in object literal ('{*x() {...}}').]
   [y]: function() {},
   ~~~~~~~~~~~~~~~~~~  [Expected method shorthand in object literal ('{[y]() {...}}').]
   z: z
@@ -41,6 +41,9 @@ const extraCases = {
   }
 };
 
-
-[property]: Expected property shorthand in object literal ('{foo, bar}').
-[method]: Expected method shorthand in object literal ('{foo() {...}}').
+const asyncFn = {
+  foo: async function() {},
+  ~~~~~~~~~~~~~~~~~~~~~~~~  [Expected method shorthand in object literal ('{async foo() {...}}').]
+  bar: async function*() {}
+  ~~~~~~~~~~~~~~~~~~~~~~~~~  [Expected method shorthand in object literal ('{async *bar() {...}}').]
+}