diff --git a/README.md b/README.md index 637b771..c5b2063 100644 --- a/README.md +++ b/README.md @@ -64,16 +64,16 @@ If your project is a multi-package monorepo, you can follow the instructions ## List of supported rules -| Rule | Description | Fixable | Requires type-checking | -| -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ | :-----: | :--------------------: | -| [fp-ts/no-lib-imports](docs/rules/no-lib-imports.md) | Disallow imports from `fp-ts/lib/` | 🔧 | | -| [fp-ts/no-pipeable](docs/rules/no-pipeable.md) | Disallow imports from the `pipeable` module | 🔧 | | -| [fp-ts/no-module-imports](docs/rules/no-module-imports.md) | Disallow imports from fp-ts modules | 🔧 | | -| [fp-ts/no-redundant-flow](docs/rules/no-redundant-flow.md) | Remove redundant uses of `flow` | 🔧 | | -| [fp-ts/prefer-traverse](docs/rules/prefer-traverse.md) | Replace `map` + `sequence` with `traverse` | 💡 | | -| [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 | 💡 | 🦄 | +| Rule | Description | Fixable | Requires type-checking | +| -------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | :-----: | :--------------------: | +| [fp-ts/no-lib-imports](docs/rules/no-lib-imports.md) | Disallow imports from `fp-ts/lib/` | 🔧 | | +| [fp-ts/no-pipeable](docs/rules/no-pipeable.md) | Disallow imports from the `pipeable` module | 🔧 | | +| [fp-ts/no-module-imports](docs/rules/no-module-imports.md) | Disallow imports from fp-ts modules | 🔧 | | +| [fp-ts/no-redundant-flow](docs/rules/no-redundant-flow.md) | Remove redundant uses of `flow` | 🔧 | | +| [fp-ts/prefer-traverse](docs/rules/prefer-traverse.md) | Replace `map` + `sequence` with `traverse` | 💡 | | +| [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`, `IO`, or `Reader`) in statement position | 💡 | 🦄 | ### Fixable legend: diff --git a/docs/rules/no-discarded-pure-expression.md b/docs/rules/no-discarded-pure-expression.md index 28e6aae..34f4e09 100644 --- a/docs/rules/no-discarded-pure-expression.md +++ b/docs/rules/no-discarded-pure-expression.md @@ -1,10 +1,11 @@ -# Disallow expressions returning pure data types (like `Task` or `IO`) where `void` or `unknown` is expected or in statement position (fp-ts/no-discarded-pure-expression) +# Disallow expressions returning pure data types (like `Task`, `IO`, or `Reader`) where `void` or `unknown` is expected or in statement position (fp-ts/no-discarded-pure-expression) -Expressions which return a pure data type, such as `IO`, `Task` and their -variants, should normally be passed as an argument, returned, or run. +Expressions which return a pure data type, such as `IO`, `Task`, 'Reader' and +their variants, should normally be passed as an argument, returned, or run. -Failing to do so causes the program represented by `IO` or `Task` to never be -run, leading to surprising behavior which is normally difficult to debug. +Failing to do so causes the program represented by `IO`, `Task`, or `Reader` to +never be run, leading to surprising behavior which is normally difficult to +debug. This rule covers two common scenarios that are common programming errors: diff --git a/src/rules/no-discarded-pure-expression.ts b/src/rules/no-discarded-pure-expression.ts index dbe7b2c..4295ef4 100644 --- a/src/rules/no-discarded-pure-expression.ts +++ b/src/rules/no-discarded-pure-expression.ts @@ -1,5 +1,6 @@ import { ParserServices, + TSESLint, AST_NODE_TYPES, } from "@typescript-eslint/experimental-utils"; import { array, option, readonlyArray } from "fp-ts"; @@ -34,7 +35,8 @@ export default createRule({ create(context) { const { isFromFpTs, typeOfNode, parserServices } = contextUtils(context); - const pureDataPrefixes = ["Task", "IO"]; + const pureDataPrefixes = ["Task", "IO", "Reader"]; + const trivialRunPrefixes = ["Task", "IO"]; function isPureDataType(t: ts.Type): boolean { return ( @@ -48,6 +50,15 @@ export default createRule({ ); } + function canRunWithoutArgs(t: ts.Type): boolean { + if (t.isUnion()) { + return pipe(t.types, array.every(canRunWithoutArgs)); + } + return trivialRunPrefixes.some( + prefix => t.symbol.escapedName.toString().startsWith(prefix) + ) + } + function pureDataReturnType(t: ts.Type): Option { if (t.isUnion()) { return pipe(t.types, readonlyArray.findFirstMap(pureDataReturnType)); @@ -92,6 +103,11 @@ export default createRule({ return isPureDataType(t); }), option.fold(constVoid, (t) => { + const addReturn = (fixer: TSESLint.RuleFixer) => + fixer.insertTextBefore(node.expression, "return "); + const runExpression = (fixer: TSESLint.RuleFixer) => + fixer.insertTextAfter(node.expression, "()"); + context.report({ node: node.expression, messageId: "pureExpressionInStatementPosition", @@ -100,20 +116,14 @@ export default createRule({ ? t.types[0]!.symbol.escapedName : t.symbol.escapedName, }, - suggest: [ - { - messageId: "addReturn", - fix(fixer) { - return fixer.insertTextBefore(node.expression, "return "); - }, - }, - { - messageId: "runExpression", - fix(fixer) { - return fixer.insertTextAfter(node.expression, "()"); - }, - }, - ], + suggest: canRunWithoutArgs(t) + ? [ + { messageId: "addReturn", fix: addReturn, }, + { messageId: "runExpression", fix: runExpression }, + ] + : [ + { messageId: "addReturn", fix: addReturn }, + ], }); }) ); diff --git a/tests/rules/no-discarded-pure-expression.test.ts b/tests/rules/no-discarded-pure-expression.test.ts index c01318a..69eb8a9 100644 --- a/tests/rules/no-discarded-pure-expression.test.ts +++ b/tests/rules/no-discarded-pure-expression.test.ts @@ -45,6 +45,22 @@ ruleTester.run("no-discarded-pure-expression", rule, { } `, }, + { + code: stripIndent` + import { reader } from "fp-ts" + function ok() { + return reader.of(42) + } + `, + }, + { + code: stripIndent` + import { reader } from "fp-ts" + function ok() { + reader.of(42)(null) + } + `, + }, { parserOptions: { ecmaFeatures: { jsx: true } }, code: stripIndent` @@ -250,6 +266,35 @@ ruleTester.run("no-discarded-pure-expression", rule, { }, ], }, + { + code: stripIndent` + import { reader } from "fp-ts" + + function woops() { + reader.of(42) + } + `, + errors: [ + { + messageId: "pureExpressionInStatementPosition", + data: { + dataType: "Reader", + }, + suggestions: [ + { + messageId: "addReturn", + output: stripIndent` + import { reader } from "fp-ts" + + function woops() { + return reader.of(42) + } + `, + }, + ], + }, + ], + }, { code: stripIndent` import { task, taskEither } from "fp-ts" @@ -310,6 +355,49 @@ ruleTester.run("no-discarded-pure-expression", rule, { }, ], }, + { + code: stripIndent` + import { task, reader } from "fp-ts" + + function f(n: number) { + if (n > 1) { + return reader.of("foo") + } + return task.of(42) + } + + function woops() { + f(2) + } + `, + errors: [ + { + messageId: "pureExpressionInStatementPosition", + data: { + dataType: "Reader", + }, + suggestions: [ + { + messageId: "addReturn", + output: stripIndent` + import { task, reader } from "fp-ts" + + function f(n: number) { + if (n > 1) { + return reader.of("foo") + } + return task.of(42) + } + + function woops() { + return f(2) + } + `, + } + ], + }, + ], + }, { parserOptions: { ecmaFeatures: { jsx: true } }, code: stripIndent`