-
Notifications
You must be signed in to change notification settings - Fork 8
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
Turn folding an either into an option into an option.fromEither #64
Open
thewilkybarkid
wants to merge
32
commits into
buildo:main
Choose a base branch
from
thewilkybarkid:either-fold-to-option
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
f6bb3fe
Start turning folding an either into an option into an option.fromEither
thewilkybarkid 415aa5b
Only work on option.fold
thewilkybarkid a864859
Match project style more
thewilkybarkid dcecac3
Extract some functions
thewilkybarkid d7c8729
Make use of functions
thewilkybarkid 89ea792
Use a pipe
thewilkybarkid 7ec2f9f
More pipes
thewilkybarkid a3346ab
Filter out undefined
thewilkybarkid c696acf
Separate as a filter
thewilkybarkid f5c0bcf
Refactoring
thewilkybarkid 345146b
Use Do notation
thewilkybarkid 484b51e
Use a simpler pipe
thewilkybarkid c0175cc
Extract some functions
thewilkybarkid b388177
Extract a pipe
thewilkybarkid 5a48836
Improve scoping and use short returns
thewilkybarkid ff96c47
Use types to check Either
thewilkybarkid 3d5a844
Safely determine Option and function modules
thewilkybarkid 2847883
Separate finding the member expression from whether it's option.none
thewilkybarkid d30b8a4
Set the right namespace
thewilkybarkid 93332ee
Extract isCall and isValue
thewilkybarkid fa53426
Extract isLazyValue and isConstantCall
thewilkybarkid 8683809
Use isCall everywhere
thewilkybarkid 121bd6f
Rewrite isOptionSomeValue
thewilkybarkid e81e8e3
Really use isCall everywhere
thewilkybarkid c22df28
Simplify ensuring arguments
thewilkybarkid fc4e4ef
Move utilities out of the module
thewilkybarkid 9cf00de
Add to documentation and configuration
thewilkybarkid ebf3ad7
Move context-aware utilities into contextUtils
thewilkybarkid a4a384e
Simplify utility changes
thewilkybarkid 3fbaedb
Merge branch 'main' into either-fold-to-option
thewilkybarkid cb92266
CS tweaks
thewilkybarkid 8b5d086
Merge branch 'main' into either-fold-to-option
thewilkybarkid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
import { | ||
AST_NODE_TYPES, | ||
TSESTree, | ||
} from "@typescript-eslint/experimental-utils"; | ||
import { constant, constVoid, flow, pipe } from 'fp-ts/function'; | ||
import { boolean, option, readonlyNonEmptyArray } from "fp-ts"; | ||
import { | ||
calleeIdentifier, | ||
contextUtils, | ||
createRule | ||
} from '../utils'; | ||
|
||
export default createRule({ | ||
name: "prefer-constructor", | ||
meta: { | ||
type: "suggestion", | ||
fixable: "code", | ||
schema: [], | ||
docs: { | ||
category: "Best Practices", | ||
description: "afsafaf", | ||
recommended: "warn", | ||
}, | ||
messages: { | ||
eitherFoldIsOptionFromEither: "cacsaffg", | ||
replaceEitherFoldWithOptionFromEither: "dsagdgdsg", | ||
}, | ||
}, | ||
defaultOptions: [], | ||
create(context) { | ||
const { isIdentifierImportedFrom } = contextUtils(context); | ||
return { | ||
CallExpression(node) { | ||
const isEitherFold = (node: TSESTree.CallExpression) => { | ||
return pipe( | ||
node, | ||
calleeIdentifier, | ||
option.filter((callee) => callee.name === "fold"), | ||
option.chain(flow((callee) => callee.parent, option.fromNullable)), | ||
option.filter((parent): parent is TSESTree.MemberExpression => parent.type === AST_NODE_TYPES.MemberExpression), | ||
option.map((parent) => parent.object), | ||
option.filter((object): object is TSESTree.Identifier => object.type === AST_NODE_TYPES.Identifier), | ||
option.exists((object) => object.name === 'either') | ||
); | ||
} | ||
|
||
const isOptionNoneArrowFunctionExpression = (node: TSESTree.ArrowFunctionExpression) => { | ||
return pipe( | ||
node.body, | ||
option.of, | ||
option.filter((arg): arg is TSESTree.MemberExpression => arg.type === AST_NODE_TYPES.MemberExpression), | ||
option.exists(isOptionNoneMemberExpression) | ||
) | ||
} | ||
|
||
const isOptionNoneMemberExpression = (node: TSESTree.MemberExpression) => { | ||
return pipe( | ||
node, | ||
option.of, | ||
option.filter(() => node.object?.type === AST_NODE_TYPES.Identifier && node.object.name === 'option'), | ||
option.filter(() => node.property?.type === AST_NODE_TYPES.Identifier && node.property.name === 'none'), | ||
option.isSome | ||
) | ||
} | ||
|
||
const isOptionNoneCallExpression = (node: TSESTree.CallExpression) => { | ||
return pipe( | ||
node, | ||
calleeIdentifier, | ||
option.filter( | ||
(callee) => | ||
callee.name === "constant" && | ||
isIdentifierImportedFrom(callee, /fp-ts\//, context) | ||
), | ||
option.map(constant(node.arguments[0])), | ||
option.chain(option.fromNullable), | ||
option.filter((arg): arg is TSESTree.MemberExpression => arg.type === AST_NODE_TYPES.MemberExpression), | ||
option.exists(isOptionNoneMemberExpression) | ||
) | ||
} | ||
|
||
const isOptionNone = (node: TSESTree.Expression) => { | ||
switch (node.type) { | ||
case AST_NODE_TYPES.ArrowFunctionExpression: | ||
return isOptionNoneArrowFunctionExpression(node); | ||
case AST_NODE_TYPES.CallExpression: | ||
return isOptionNoneCallExpression(node); | ||
case AST_NODE_TYPES.MemberExpression: | ||
return isOptionNoneMemberExpression(node); | ||
default: | ||
return false; | ||
} | ||
}; | ||
|
||
const isOptionSomeValue = (node: TSESTree.Expression) => { | ||
return pipe( | ||
node, | ||
option.of, | ||
option.map((n) => n.type === AST_NODE_TYPES.ArrowFunctionExpression && n.body.type === AST_NODE_TYPES.CallExpression ? n.body.callee : n), | ||
option.filter((n): n is TSESTree.MemberExpression => n.type === AST_NODE_TYPES.MemberExpression), | ||
option.exists( | ||
(body) => | ||
body.object?.type === AST_NODE_TYPES.Identifier && body.object.name === 'option' | ||
&& | ||
body.property?.type === AST_NODE_TYPES.Identifier && body.property.name === 'some' | ||
&& | ||
(body.parent?.type !== AST_NODE_TYPES.CallExpression || body.parent.parent?.type !== AST_NODE_TYPES.ArrowFunctionExpression | ||
|| | ||
(body.parent.arguments.length === 1 && body.parent.parent.params.length === 1 && body.parent.arguments[0]?.type === AST_NODE_TYPES.Identifier && body.parent.parent.params[0]?.type === AST_NODE_TYPES.Identifier && body.parent.arguments[0].name === body.parent.parent.params[0].name)) | ||
), | ||
); | ||
} | ||
pipe( | ||
node, | ||
isEitherFold, | ||
boolean.fold(constVoid, () => | ||
pipe( | ||
readonlyNonEmptyArray.fromArray(node.arguments), | ||
option.filter((args) => args.length === 2), | ||
option.filter(flow(readonlyNonEmptyArray.head, isOptionNone)), | ||
option.filter(flow(readonlyNonEmptyArray.last, isOptionSomeValue)), | ||
option.map(() => { | ||
context.report({ | ||
loc: { | ||
start: node.loc.start, | ||
end: node.loc.end, | ||
}, | ||
messageId: "eitherFoldIsOptionFromEither", | ||
suggest: [ | ||
{ | ||
messageId: "replaceEitherFoldWithOptionFromEither", | ||
fix(fixer) { | ||
return [ | ||
fixer.replaceTextRange( | ||
node.range, | ||
`option.fromEither` | ||
), | ||
]; | ||
}, | ||
}, | ||
], | ||
}); | ||
}) | ||
) | ||
) | ||
); | ||
}, | ||
}; | ||
}, | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
import { stripIndent } from 'common-tags'; | ||
import rule from "../../src/rules/prefer-constructor"; | ||
import { ESLintUtils } from "@typescript-eslint/experimental-utils"; | ||
|
||
const ruleTester = new ESLintUtils.RuleTester({ | ||
parser: "@typescript-eslint/parser", | ||
}); | ||
|
||
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 { 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 | ||
) | ||
`, | ||
}, | ||
], | ||
}, | ||
], | ||
}, | ||
], | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently looking for a way to determine if it's
either.fold
, even if it'sE.fold
or whatever.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two ways:
fp-ts
(accurate enough, and faster because it's only a syntactic lookup)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a closer look as soon as I have minute