Merge pull request #224 from OliverJAsh/oja/no-redundant-flow/fix-fixes

Author: gabro

src/rules/no-redundant-flow.ts

@@ -1,4 +1,12 @@
-import { contextUtils, createRule } from "../utils";
+import { pipe } from "fp-ts/function";
+import * as O from "fp-ts/Option";
+import {
+  contextUtils,
+  createRule,
+  createSequenceExpressionFromCallExpressionWithExpressionArgs,
+  getCallExpressionWithExpressionArgs,
+  prettyPrint,
+} from "../utils";
 
 export default createRule({
   name: "no-redundant-flow",
@@ -22,26 +30,38 @@ export default createRule({
 
     return {
       CallExpression(node) {
-        if (node.arguments.length === 1 && isFlowExpression(node)) {
-          context.report({
-            node,
-            messageId: "redundantFlow",
-            suggest: [
-              {
-                messageId: "removeFlow",
-                fix(fixer) {
-                  return [
-                    fixer.removeRange([
-                      node.callee.range[0],
-                      node.callee.range[1] + 1,
-                    ]),
-                    fixer.removeRange([node.range[1] - 1, node.range[1]]),
-                  ];
+        pipe(
+          node,
+          O.fromPredicate(isFlowExpression),
+          /**
+           * We ignore flow calls which contain a spread argument because these are never invalid.
+           */
+          O.chain(getCallExpressionWithExpressionArgs),
+          O.filter((flowCall) => flowCall.node.arguments.length === 1),
+          O.map((redundantFlowCall) => {
+            context.report({
+              node: redundantFlowCall.node,
+              messageId: "redundantFlow",
+              suggest: [
+                {
+                  messageId: "removeFlow",
+                  fix(fixer) {
+                    const sequenceExpression =
+                      createSequenceExpressionFromCallExpressionWithExpressionArgs(
+                        redundantFlowCall
+                      );
+                    return [
+                      fixer.replaceText(
+                        redundantFlowCall.node,
+                        prettyPrint(sequenceExpression)
+                      ),
+                    ];
+                  },
                 },
-              },
-            ],
-          });
-        }
+              ],
+            });
+          })
+        );
       },
     };
   },

src/utils.ts

@@ -18,6 +18,8 @@ import {
 } from "@typescript-eslint/experimental-utils/dist/ts-eslint";
 import ts from "typescript";
 import { Option } from "fp-ts/Option";
+import * as NonEmptyArray from "fp-ts/NonEmptyArray";
+import * as O from "fp-ts/Option";
 
 declare module "typescript" {
   interface TypeChecker {
@@ -471,3 +473,51 @@ export const contextUtils = <
     parserServices,
   };
 };
+
+/**
+ * Ideally we could implement this predicate in terms of an existing
+ * `isExpression` predicate but it seems like this doesn't exist anywhere.
+ *
+ * There is an `isExpression` in `tsutils`. However, in the TS AST, spread is
+ * classed as an expression (!).
+ */
+const getArgumentExpression = (
+  x: TSESTree.CallExpressionArgument
+): O.Option<TSESTree.Expression> =>
+  x.type !== AST_NODE_TYPES.SpreadElement ? O.some(x) : O.none;
+
+const checkIsArgumentExpression = O.getRefinement(getArgumentExpression);
+
+type CallExpressionWithExpressionArgs = {
+  node: TSESTree.CallExpression;
+  args: NonEmptyArray.NonEmptyArray<TSESTree.Expression>;
+};
+
+export const getCallExpressionWithExpressionArgs = (
+  node: TSESTree.CallExpression
+): O.Option<CallExpressionWithExpressionArgs> =>
+  node.arguments.every(checkIsArgumentExpression)
+    ? pipe(
+        node.arguments,
+        NonEmptyArray.fromArray,
+        O.map(
+          (args): CallExpressionWithExpressionArgs => ({
+            node,
+            args,
+          })
+        )
+      )
+    : O.none;
+
+export const createSequenceExpressionFromCallExpressionWithExpressionArgs = (
+  call: CallExpressionWithExpressionArgs
+): TSESTree.SequenceExpression => {
+  const firstArg = pipe(call.args, NonEmptyArray.head);
+  const lastArg = pipe(call.args, NonEmptyArray.last);
+  return {
+    loc: call.node.loc,
+    range: [firstArg.range[0], lastArg.range[1]],
+    type: AST_NODE_TYPES.SequenceExpression,
+    expressions: call.args,
+  };
+};

tests/rules/no-redundant-flow.test.ts

@@ -19,6 +19,9 @@ ruleTester.run("no-redundant-flow", rule, {
       bar
     )
     `,
+    `import { flow } from "fp-ts/function"
+    flow(...fns)
+    `,
   ],
   invalid: [
     {
@@ -56,14 +59,34 @@ const a = flow(
               messageId: "removeFlow",
               output: `
 import { flow } from "fp-ts/function"
-const a = ${""}
-  foo
-
+const a = foo
 `,
             },
           ],
         },
       ],
     },
+    {
+      code: `
+import { flow } from "fp-ts/function"
+const a = flow(
+  foo,
+);
+`,
+      errors: [
+        {
+          messageId: "redundantFlow",
+          suggestions: [
+            {
+              messageId: "removeFlow",
+              output: `
+import { flow } from "fp-ts/function"
+const a = foo;
+`,
+            },
+          ],
+        },
+      ],
+    }
   ],
 });