Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Reader and its variants for no-discarded-pure-expression #103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
11 changes: 6 additions & 5 deletions docs/rules/no-discarded-pure-expression.md
Original file line number Diff line number Diff line change
@@ -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:

Expand Down
40 changes: 25 additions & 15 deletions src/rules/no-discarded-pure-expression.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
ParserServices,
TSESLint,
AST_NODE_TYPES,
} from "@typescript-eslint/experimental-utils";
import { array, option, readonlyArray } from "fp-ts";
Expand Down Expand Up @@ -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 (
Expand All @@ -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<ts.Type> {
if (t.isUnion()) {
return pipe(t.types, readonlyArray.findFirstMap(pureDataReturnType));
Expand Down Expand Up @@ -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",
Expand All @@ -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 },
],
});
})
);
Expand Down
88 changes: 88 additions & 0 deletions tests/rules/no-discarded-pure-expression.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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`
Expand Down