From fd6b86def37d78a8032e2bff19b2e19f2a4f58e4 Mon Sep 17 00:00:00 2001 From: Oliver Joseph Ash Date: Sat, 11 Dec 2021 11:02:29 +0000 Subject: [PATCH 1/3] Add --- src/rules/no-redundant-pipe.ts | 90 ++++++++++++++ src/utils.ts | 15 ++- tests/rules/no-redundant-pipe.test.ts | 169 ++++++++++++++++++++++++++ 3 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 src/rules/no-redundant-pipe.ts create mode 100644 tests/rules/no-redundant-pipe.test.ts diff --git a/src/rules/no-redundant-pipe.ts b/src/rules/no-redundant-pipe.ts new file mode 100644 index 0000000..1206ef6 --- /dev/null +++ b/src/rules/no-redundant-pipe.ts @@ -0,0 +1,90 @@ +import * as NonEmptyArray from "fp-ts/NonEmptyArray"; +import { AST_NODE_TYPES } from "@typescript-eslint/experimental-utils"; +import { flow, pipe } from "fp-ts/function"; +import * as O from "fp-ts/Option"; +import { + contextUtils, + createRule, + createSequenceExpressionFromCallExpressionWithExpressionArgs, + getCallExpressionWithExpressionArgs, + prettyPrint, + CallExpressionWithExpressionArgs, +} from "../utils"; + +export default createRule({ + name: "no-redundant-pipe", + meta: { + type: "suggestion", + fixable: "code", + hasSuggestions: true, + schema: [], + docs: { + description: "Remove redundant uses of pipe", + recommended: "warn", + }, + messages: { + redundantPipe: + "pipe can be removed because it takes only one argument or it is used as the first argument inside another pipe", + removePipe: "remove pipe", + }, + }, + defaultOptions: [], + create(context) { + const { isPipeExpression } = contextUtils(context); + + const getPipeCallExpressionWithExpressionArgs = flow( + O.fromPredicate(isPipeExpression), + /** + * We ignore Pipe calls which contain a spread argument because these are never invalid. + */ + O.chain(getCallExpressionWithExpressionArgs) + ); + + const getRedundantPipeCall = ( + pipeCall: CallExpressionWithExpressionArgs + ): O.Option => { + const firstArg = pipe(pipeCall.args, NonEmptyArray.head); + + if (pipeCall.args.length === 1) { + return O.some(pipeCall); + } else if (firstArg.type === AST_NODE_TYPES.CallExpression) { + return getPipeCallExpressionWithExpressionArgs(firstArg); + } else { + return O.none; + } + }; + + return { + CallExpression(node) { + pipe( + node, + getPipeCallExpressionWithExpressionArgs, + O.chain(getRedundantPipeCall), + O.map((redundantPipeCall) => { + context.report({ + node: redundantPipeCall.node, + messageId: "redundantPipe", + suggest: [ + { + messageId: "removePipe", + fix(fixer) { + const sequenceExpression = + createSequenceExpressionFromCallExpressionWithExpressionArgs( + redundantPipeCall + ); + return [ + fixer.replaceText( + redundantPipeCall.node, + prettyPrint(sequenceExpression) + ), + ]; + }, + }, + ], + }); + }) + ); + }, + }; + }, +}); diff --git a/src/utils.ts b/src/utils.ts index ad821a2..1c9b072 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -362,6 +362,18 @@ export const contextUtils = < ); } + function isPipeExpression(node: TSESTree.CallExpression): boolean { + return pipe( + node, + calleeIdentifier, + option.exists( + (callee) => + callee.name === "pipe" && + isIdentifierImportedFrom(callee, /fp-ts\//, context) + ) + ); + } + function isPipeOrFlowExpression(node: TSESTree.CallExpression): boolean { return pipe( node, @@ -464,6 +476,7 @@ export const contextUtils = < return { addNamedImportIfNeeded, removeImportDeclaration, + isPipeExpression, isFlowExpression, isPipeOrFlowExpression, isIdentifierImportedFrom, @@ -488,7 +501,7 @@ const getArgumentExpression = ( const checkIsArgumentExpression = O.getRefinement(getArgumentExpression); -type CallExpressionWithExpressionArgs = { +export type CallExpressionWithExpressionArgs = { node: TSESTree.CallExpression; args: NonEmptyArray.NonEmptyArray; }; diff --git a/tests/rules/no-redundant-pipe.test.ts b/tests/rules/no-redundant-pipe.test.ts new file mode 100644 index 0000000..b663348 --- /dev/null +++ b/tests/rules/no-redundant-pipe.test.ts @@ -0,0 +1,169 @@ +import rule from "../../src/rules/no-redundant-pipe"; +import { ESLintUtils } from "@typescript-eslint/experimental-utils"; + +const ruleTester = new ESLintUtils.RuleTester({ + parser: "@typescript-eslint/parser", + parserOptions: { + sourceType: "module", + }, +}); + +ruleTester.run("no-redundant-pipe", rule, { + valid: [ + `import { pipe } from "fp-ts/function" + pipe(...x); + `, + `import { pipe } from "fp-ts/function" + pipe(value, fn); + `, + `import { pipe } from "fp-ts/function" + pipe(value, pipe(value2, fn)); + `, + ], + invalid: [ + { + code: ` +import { pipe } from "fp-ts/function" +pipe(value); +`, + errors: [ + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + output: ` +import { pipe } from "fp-ts/function" +value; +`, + }, + ], + }, + ], + }, + { + code: ` +import { pipe } from "fp-ts/function" +pipe(pipe(pipe(value, fn1), fn2), fn3); +`, + errors: [ + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + output: ` +import { pipe } from "fp-ts/function" +pipe(pipe(value, fn1), fn2, fn3); +`, + }, + ], + }, + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + output: ` +import { pipe } from "fp-ts/function" +pipe(pipe(value, fn1, fn2), fn3); +`, + }, + ], + }, + ], + }, + { + code: ` +import { pipe } from "fp-ts/function" +pipe(pipe(value, fn1), fn2, fn3); +`, + errors: [ + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + output: ` +import { pipe } from "fp-ts/function" +pipe(value, fn1, fn2, fn3); +`, + }, + ], + }, + ], + }, + { + code: ` +import { pipe } from "fp-ts/function" +pipe( + // foo + pipe(value, fn1), + fn2 +);`, + errors: [ + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + output: ` +import { pipe } from "fp-ts/function" +pipe( + // foo + value, fn1, + fn2 +);`, + }, + ], + }, + ], + }, + { + code: ` +import { pipe } from "fp-ts/function" +pipe( + value, +); +`, + errors: [ + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + output: ` +import { pipe } from "fp-ts/function" +value; +`, + }, + ], + }, + ], + }, + { + code: ` +import { pipe } from "fp-ts/function" +pipe( + // foo + value, +); +`, + errors: [ + { + messageId: "redundantPipe", + suggestions: [ + { + messageId: "removePipe", + // TODO: ideally we would preserve the comment here but I'm not sure how + output: ` +import { pipe } from "fp-ts/function" +value; +`, + }, + ], + }, + ], + }, + ], +}); From 2a88e762d82e5a332b06036ebbd761824274ea75 Mon Sep 17 00:00:00 2001 From: Oliver Joseph Ash Date: Sat, 11 Dec 2021 11:12:06 +0000 Subject: [PATCH 2/3] Distinguish error types --- src/rules/no-redundant-pipe.ts | 37 ++++++++++++++++++++++----- tests/rules/no-redundant-pipe.test.ts | 14 +++++----- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/rules/no-redundant-pipe.ts b/src/rules/no-redundant-pipe.ts index 1206ef6..c7305d3 100644 --- a/src/rules/no-redundant-pipe.ts +++ b/src/rules/no-redundant-pipe.ts @@ -11,6 +11,13 @@ import { CallExpressionWithExpressionArgs, } from "../utils"; +const errorMessages = { + redundantPipeWithSingleArg: + "pipe can be removed because it takes only one argument", + redundantPipeWithSingleArgInsidePipe: + "pipe can be removed because it is used as the first argument inside another pipe", +}; + export default createRule({ name: "no-redundant-pipe", meta: { @@ -23,8 +30,7 @@ export default createRule({ recommended: "warn", }, messages: { - redundantPipe: - "pipe can be removed because it takes only one argument or it is used as the first argument inside another pipe", + ...errorMessages, removePipe: "remove pipe", }, }, @@ -40,15 +46,32 @@ export default createRule({ O.chain(getCallExpressionWithExpressionArgs) ); + type RedundantPipeCallAndMessage = { + redundantPipeCall: CallExpressionWithExpressionArgs; + errorMessageId: keyof typeof errorMessages; + }; + const getRedundantPipeCall = ( pipeCall: CallExpressionWithExpressionArgs - ): O.Option => { + ): O.Option => { const firstArg = pipe(pipeCall.args, NonEmptyArray.head); if (pipeCall.args.length === 1) { - return O.some(pipeCall); + const result: RedundantPipeCallAndMessage = { + redundantPipeCall: pipeCall, + errorMessageId: "redundantPipeWithSingleArg", + }; + return O.some(result); } else if (firstArg.type === AST_NODE_TYPES.CallExpression) { - return getPipeCallExpressionWithExpressionArgs(firstArg); + return pipe( + getPipeCallExpressionWithExpressionArgs(firstArg), + O.map( + (redundantPipeCall): RedundantPipeCallAndMessage => ({ + redundantPipeCall, + errorMessageId: "redundantPipeWithSingleArgInsidePipe", + }) + ) + ); } else { return O.none; } @@ -60,10 +83,10 @@ export default createRule({ node, getPipeCallExpressionWithExpressionArgs, O.chain(getRedundantPipeCall), - O.map((redundantPipeCall) => { + O.map(({ redundantPipeCall, errorMessageId }) => { context.report({ node: redundantPipeCall.node, - messageId: "redundantPipe", + messageId: errorMessageId, suggest: [ { messageId: "removePipe", diff --git a/tests/rules/no-redundant-pipe.test.ts b/tests/rules/no-redundant-pipe.test.ts index b663348..b56114c 100644 --- a/tests/rules/no-redundant-pipe.test.ts +++ b/tests/rules/no-redundant-pipe.test.ts @@ -28,7 +28,7 @@ pipe(value); `, errors: [ { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArg", suggestions: [ { messageId: "removePipe", @@ -48,7 +48,7 @@ pipe(pipe(pipe(value, fn1), fn2), fn3); `, errors: [ { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArgInsidePipe", suggestions: [ { messageId: "removePipe", @@ -60,7 +60,7 @@ pipe(pipe(value, fn1), fn2, fn3); ], }, { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArgInsidePipe", suggestions: [ { messageId: "removePipe", @@ -80,7 +80,7 @@ pipe(pipe(value, fn1), fn2, fn3); `, errors: [ { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArgInsidePipe", suggestions: [ { messageId: "removePipe", @@ -103,7 +103,7 @@ pipe( );`, errors: [ { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArgInsidePipe", suggestions: [ { messageId: "removePipe", @@ -128,7 +128,7 @@ pipe( `, errors: [ { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArg", suggestions: [ { messageId: "removePipe", @@ -151,7 +151,7 @@ pipe( `, errors: [ { - messageId: "redundantPipe", + messageId: "redundantPipeWithSingleArg", suggestions: [ { messageId: "removePipe", From f5d0612f6cf14ee0c2ce9a36d1ff9d40106f75e3 Mon Sep 17 00:00:00 2001 From: Oliver Joseph Ash Date: Sun, 12 Dec 2021 10:32:12 +0000 Subject: [PATCH 3/3] Format --- src/rules/no-redundant-pipe.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/no-redundant-pipe.ts b/src/rules/no-redundant-pipe.ts index c7305d3..a24bf7a 100644 --- a/src/rules/no-redundant-pipe.ts +++ b/src/rules/no-redundant-pipe.ts @@ -41,7 +41,7 @@ export default createRule({ const getPipeCallExpressionWithExpressionArgs = flow( O.fromPredicate(isPipeExpression), /** - * We ignore Pipe calls which contain a spread argument because these are never invalid. + * We ignore pipe calls which contain a spread argument because these are never invalid. */ O.chain(getCallExpressionWithExpressionArgs) );