diff --git a/README.md b/README.md index 637b771..43d4047 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,7 @@ If your project is a multi-package monorepo, you can follow the instructions | [fp-ts/prefer-chain](docs/rules/prefer-chain.md) | Replace `map` + `flatten` with `chain` | 💡 | | | [fp-ts/prefer-bimap](docs/rules/prefer-bimap.md) | Replace `map` + `mapLeft` with `bimap` | 💡 | | | [fp-ts/no-discarded-pure-expression](docs/rules/no-discarded-pure-expression.md) | Disallow expressions returning pure data types (like `Task` or `IO`) in statement position | 💡 | 🦄 | +| [fp-ts/prefer-constructor](docs/rules/prefer-constructor.md) | Replace destructors with constructors | 💡 | 🦄 | ### Fixable legend: diff --git a/docs/rules/prefer-constructor.md b/docs/rules/prefer-constructor.md new file mode 100644 index 0000000..5b86f8f --- /dev/null +++ b/docs/rules/prefer-constructor.md @@ -0,0 +1,35 @@ +# Replace destructors with constructors (fp-ts/prefer-constructor) + +Suggest replacing the combination of a destructor and constructors with a constructor when changing types. + +This rule covers: + +- `Option.fromEither` + +**💡 Fixable**: This rule provides in-editor suggested fixes. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```ts +import { either, option } from "fp-ts" +import { pipe } from "fp-ts/function" + +pipe( + either.of(1), + either.fold(() => option.none, option.some) +) +``` + +Examples of **correct** code for this rule: + +```ts +import { either, option } from "fp-ts" +import { pipe } from "fp-ts/function" + +pipe( + either.of(1), + option.fromEither +) +``` diff --git a/src/index.ts b/src/index.ts index c1642a4..b1346bc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ const suggestions = { "no-redundant-flow": require("./rules/no-redundant-flow").default, "prefer-chain": require("./rules/prefer-chain").default, "prefer-bimap": require("./rules/prefer-bimap").default, + "prefer-constructor": require("./rules/prefer-constructor").default }; export const rules = { diff --git a/src/rules/prefer-constructor.ts b/src/rules/prefer-constructor.ts new file mode 100644 index 0000000..e413a95 --- /dev/null +++ b/src/rules/prefer-constructor.ts @@ -0,0 +1,61 @@ +import { option, readonlyNonEmptyArray } from "fp-ts" +import { flow, pipe } from "fp-ts/function" +import { contextUtils, createRule, ensureArguments } from "../utils" + +export default createRule({ + name: "prefer-constructor", + meta: { + type: "suggestion", + fixable: "code", + hasSuggestions: true, + schema: [], + docs: { + description: "Replace destructor + constructors with a constructor", + recommended: "warn" + }, + messages: { + eitherFoldIsOptionFromEither: "Either.fold can be replaced with Option.fromEither", + replaceEitherFoldWithOptionFromEither: "replace Either.fold with Option.fromEither" + } + }, + defaultOptions: [], + create(context) { + const { findNamespace, isCall, isLazyValue } = contextUtils(context) + return { + CallExpression(node) { + pipe( + node, + option.of, + option.filter(isCall("Either", "fold")), + option.chain(ensureArguments([ + isLazyValue("Option", "none"), + isCall("Option", "some") + ])), + option.bind("namespace", flow(readonlyNonEmptyArray.head, findNamespace)), + option.map(({ namespace }) => { + context.report({ + loc: { + start: node.loc.start, + end: node.loc.end + }, + messageId: "eitherFoldIsOptionFromEither", + suggest: [ + { + messageId: "replaceEitherFoldWithOptionFromEither", + fix(fixer) { + return [ + fixer.replaceTextRange( + node.range, + `${namespace}.fromEither` + ) + ] + } + } + ] + }) + }) + ) + } + } + } +}) diff --git a/src/utils.ts b/src/utils.ts index ad821a2..06566b3 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -8,8 +8,8 @@ import { } from "@typescript-eslint/experimental-utils"; import * as recast from "recast"; import { visitorKeys as tsVisitorKeys } from "@typescript-eslint/typescript-estree"; -import { array, option, apply } from "fp-ts"; -import { pipe } from "fp-ts/function"; +import { array, option, apply, readonlyArray, readonlyNonEmptyArray } from "fp-ts"; +import { constant, flow, pipe } from "fp-ts/function"; import estraverse from "estraverse"; import { @@ -34,6 +34,12 @@ declare module "typescript" { function toFileNameLowerCase(x: string): string; } +const modules = ["Either", "Option", "function"] as const + +type Module = typeof modules[number] + +const isModule = (name: string): name is Module => modules.includes(name as Module) + const version = require("../package.json").version; export const createRule = ESLintUtils.RuleCreator( @@ -41,12 +47,12 @@ export const createRule = ESLintUtils.RuleCreator( `https://github.com/buildo/eslint-plugin-fp-ts/blob/v${version}/docs/rules/${name}.md` ); -export function calleeIdentifier( - node: - | TSESTree.CallExpression - | TSESTree.MemberExpression - | TSESTree.Identifier -): option.Option { +type Callee = + | TSESTree.CallExpression + | TSESTree.MemberExpression + | TSESTree.Identifier + +export function calleeIdentifier(node: Callee): option.Option { switch (node.type) { case AST_NODE_TYPES.MemberExpression: if (node.property.type === AST_NODE_TYPES.Identifier) { @@ -71,12 +77,17 @@ export function calleeIdentifier( } } -function isWithinTypes( - n: TSESTree.Node | undefined, - types: N["type"][] -): n is N { - return !!n && types.includes(n.type); -} +const isWithinTypes = (types: T["type"][]) => (node: TSESTree.Node): node is T => types.includes(node.type) + +const isArrowFunctionExpression = isWithinTypes([AST_NODE_TYPES.ArrowFunctionExpression]) + +const isCallExpression = isWithinTypes([AST_NODE_TYPES.CallExpression]) + +const isCallee = isWithinTypes([AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.Identifier]) + +const isIdentifier = isWithinTypes([AST_NODE_TYPES.Identifier]) + +const isMemberExpression = isWithinTypes([AST_NODE_TYPES.MemberExpression]) type CombinatorNode = | TSESTree.CallExpression @@ -106,11 +117,11 @@ export function getAdjacentCombinators< const firstCombinatorIndex = pipeOrFlowExpression.arguments.findIndex( (a, index) => { if ( - isWithinTypes(a, combinator1.types) && + isWithinTypes(combinator1.types)(a) && index < pipeOrFlowExpression.arguments.length - 1 ) { const b = pipeOrFlowExpression.arguments[index + 1]; - if (isWithinTypes(b, combinator2.types)) { + if (b && isWithinTypes(combinator2.types)(b)) { return pipe( apply.sequenceS(option.option)({ idA: calleeIdentifier(a), @@ -191,6 +202,91 @@ export function inferQuote(node: TSESTree.Literal): Quote { return node.raw[0] === "'" ? "'" : '"'; } +const getDeclarationFileName = (declaration: ts.Declaration) => declaration.getSourceFile().fileName + +const getDeclarations = flow( + (symbol: ts.Symbol) => symbol.getDeclarations(), + option.fromNullable, + option.chain(readonlyNonEmptyArray.fromArray) +) + +const getFileName = flow( + (type: ts.Type) => type.aliasSymbol ?? type.symbol, + option.fromNullable, + option.chain(getDeclarations), + option.map(readonlyNonEmptyArray.head), + option.map(getDeclarationFileName) +) + +const getFpTsModule = flow( + (fileName: string) => /\/fp-ts\/lib\/(.+?)\.d\.ts$/.exec(fileName), + option.fromNullable, + option.chain(array.lookup(1)), + option.filter(isModule) +) + +const getModule = flow( + getFileName, + option.chain(getFpTsModule) +) + +const isFromModule = (expected: Module) => flow( + getModule, + option.exists(module => module === expected) +) + +const getArguments = (call: TSESTree.CallExpression) => pipe( + call.arguments, + readonlyNonEmptyArray.fromArray +) + +const getFirstArgument = flow( + getArguments, + option.map(readonlyNonEmptyArray.head) +) + +const getParams = (call: TSESTree.FunctionLike) => pipe( + call.params, + readonlyNonEmptyArray.fromArray +) + +const getFirstParam = flow( + getParams, + option.map(readonlyNonEmptyArray.head) +) + +const isIdentifierWithName = (expected: string) => flow( + option.fromPredicate(isIdentifier), + option.exists(({ name }) => name === expected) +) + +const getWrappedCall = (node: TSESTree.ArrowFunctionExpression) => pipe( + node.body, + option.fromPredicate(isCallExpression), + option.filter(flow( + option.of, + option.bindTo("call"), + option.bind("param", () => pipe(node, getFirstParam, option.filter(isIdentifier))), + option.exists(({ call, param }) => pipe(call, ensureArguments([isIdentifierWithName(param.name)]), option.isSome)) + )) +) + +const findMemberExpressionFromArrowFunctionExpression = (node: TSESTree.ArrowFunctionExpression) => pipe( + node.body, + option.fromPredicate(isMemberExpression) +) + +export const ensureArguments = (args: ReadonlyArray<(node: TSESTree.Expression) => boolean>) => flow( + getArguments, + option.filter((array) => array.length === args.length), + option.chain(flow( + readonlyNonEmptyArray.map(value => value.type === AST_NODE_TYPES.SpreadElement ? value.argument : value), + readonlyNonEmptyArray.traverseWithIndex(option.option)( + (i, value) => pipe(args, readonlyArray.lookup(i), option.filter(test => test(value)), option.map(constant(value))) + )) + ) +) + export const contextUtils = < TMessageIds extends string, TOptions extends readonly unknown[] @@ -461,6 +557,64 @@ export const contextUtils = < ); } + const isCall = (module: Module, name: string) => (node: T) => pipe( + node, + option.fromPredicate(isArrowFunctionExpression), + option.chain(getWrappedCall), + option.altW(constant(option.some(node))), + option.filter(isCallee), + option.exists(isCallTo(module, name)) + ) + + const isCallTo = (module: Module, expected: string) => flow( + calleeIdentifier, + option.filter(({name}) => name === expected), + option.chain(typeOfNode), + option.exists(isFromModule(module)) + ) + + const isLazyValue = (module: Module, name: string) => flow( + findMemberExpression, + option.exists(isValue(module, name)) + ) + + const isValue = (module: Module, name: string) => flow( + option.fromPredicate((node: TSESTree.MemberExpression) => pipe(node.property, isIdentifierWithName(name))), + option.chain(typeOfNode), + option.exists(isFromModule(module)) + ) + + const isConstantCall = flow( + (node: TSESTree.CallExpression) => pipe(node, calleeIdentifier), + option.filter(({ name }) => name === "constant"), + option.chain(typeOfNode), + option.exists(isFromModule("function")) + ) + + const findMemberExpressionFromCallExpression = flow( + option.fromPredicate(isConstantCall), + option.chain(getFirstArgument), + option.filter(isMemberExpression) + ) + + const findMemberExpression = (node: TSESTree.Expression) => { + switch (node.type) { + case AST_NODE_TYPES.ArrowFunctionExpression: + return findMemberExpressionFromArrowFunctionExpression(node) + case AST_NODE_TYPES.CallExpression: + return findMemberExpressionFromCallExpression(node) + default: + return option.none + } + } + + const findNamespace = flow( + findMemberExpression, + option.map((node) => node.object), + option.filter(isIdentifier), + option.map((identifier) => identifier.name) + ) + return { addNamedImportIfNeeded, removeImportDeclaration, @@ -471,6 +625,9 @@ export const contextUtils = < typeOfNode, isFromFpTs, parserServices, + isCall, + isLazyValue, + findNamespace, }; }; diff --git a/tests/rules/prefer-constructor.test.ts b/tests/rules/prefer-constructor.test.ts new file mode 100644 index 0000000..0f3663b --- /dev/null +++ b/tests/rules/prefer-constructor.test.ts @@ -0,0 +1,182 @@ +import { ESLintUtils } from "@typescript-eslint/experimental-utils" +import { stripIndent } from "common-tags" +import path from "path" +import rule from "../../src/rules/prefer-constructor" + +const fixtureProjectPath = path.join( + __dirname, + "..", + "fixtures", + "fp-ts-project" +) + +const ruleTester = new ESLintUtils.RuleTester({ + parser: "@typescript-eslint/parser", + parserOptions: { + sourceType: "module", + tsconfigRootDir: fixtureProjectPath, + project: "./tsconfig.json" + } +}) + +ruleTester.run("prefer-constructor", rule, { + valid: [ + { + code: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + either.fold(() => option.none, (value) => option.some(otherValue)) + ) + ` + }, + { + code: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + either.fold(() => option.some(otherValue), (value) => option.some(value)) + ) + ` + }, + { + code: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + either.fold(() => option.some(otherValue), () => option.none) + ) + ` + } + ], + invalid: [ + { + code: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + either.fold(() => option.none, (value) => option.some(value)) + ) + `, + errors: [ + { + messageId: "eitherFoldIsOptionFromEither", + suggestions: [ + { + messageId: "replaceEitherFoldWithOptionFromEither", + output: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + option.fromEither + ) + ` + } + ] + } + ] + }, + { + code: stripIndent` + import * as E from "fp-ts/Either" + import * as O from "fp-ts/Option" + import { pipe } from "fp-ts/function" + + pipe( + E.of(1), + E.fold(() => O.none, (value) => O.some(value)) + ) + `, + errors: [ + { + messageId: "eitherFoldIsOptionFromEither", + suggestions: [ + { + messageId: "replaceEitherFoldWithOptionFromEither", + output: stripIndent` + import * as E from "fp-ts/Either" + import * as O from "fp-ts/Option" + import { pipe } from "fp-ts/function" + + pipe( + E.of(1), + O.fromEither + ) + ` + } + ] + } + ] + }, + { + code: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + either.fold(() => option.none, option.some) + ) + `, + errors: [ + { + messageId: "eitherFoldIsOptionFromEither", + suggestions: [ + { + messageId: "replaceEitherFoldWithOptionFromEither", + output: stripIndent` + import { either, option } from "fp-ts" + import { pipe } from "fp-ts/function" + + pipe( + either.of(1), + option.fromEither + ) + ` + } + ] + } + ] + }, + { + code: stripIndent` + import { either, option } from "fp-ts" + import { constant, pipe } from "fp-ts/function" + + pipe( + either.of(1), + either.fold(constant(option.none), option.some) + ) + `, + errors: [ + { + messageId: "eitherFoldIsOptionFromEither", + suggestions: [ + { + messageId: "replaceEitherFoldWithOptionFromEither", + output: stripIndent` + import { either, option } from "fp-ts" + import { constant, pipe } from "fp-ts/function" + + pipe( + either.of(1), + option.fromEither + ) + ` + } + ] + } + ] + } + ] +})