-
Notifications
You must be signed in to change notification settings - Fork 3
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
(Issue #309) Introduced functional-parameters rules that allows for fp-ts Lazy functions #310
base: master
Are you sure you want to change the base?
Conversation
…xSelector conditions to allow Lazy types
"error", | ||
{ | ||
"ignorePrefixSelector": [ | ||
"CallExpression[callee.object.name='TE'][callee.property.name='tryCatch']", |
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.
We can't rely on this import naming convention. For example, this is from the official fp-ts docs:
import { left, right } from 'fp-ts/Either'
import { tryCatch } from 'fp-ts/TaskEither'
tryCatch(() => Promise.resolve(1), String)().then((result) => {
assert.deepStrictEqual(result, right(1))
})
tryCatch(() => Promise.reject('error'), String)().then((result) => {
assert.deepStrictEqual(result, left('error'))
})
You can see that these tryCatch
instances wouldn't be picked up by our selector here.
Let's relax this to permit any invocation of any function named tryCatch
(or the others). This risks ignoring other, unrelated functions that just happen to have the same name (false negatives) but that's a risk worth taking, at least for now. To do better than that we would need access to the TypeScript type information that isn't present in the plain Esprima AST (which is what these ESLint selectors give us access to).
If possible, let's narrow it down by the number of function arguments too. For example we know that tryCatch
takes two arguments, so if possible, let's write our selector in such a way that it won't pick up calls to functions named tryCatch
that take more or less arguments than that.
"ignorePrefixSelector": [ | ||
"CallExpression[callee.object.name='TE'][callee.property.name='tryCatch']", | ||
"CallExpression[callee.object.name='O'][callee.property.name='fold']", | ||
"CallExpression[callee.object.name='E'][callee.property.name='fromOption']" |
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.
Can you think of other functions that are worth adding to this list?
@@ -20,7 +20,7 @@ | |||
"eslint": "^8.17.0", | |||
"eslint-config-prettier": "^8.5.0", | |||
"eslint-config-typed-fp": "^3.0.0", | |||
"eslint-plugin-functional": "^4.2.1", | |||
"eslint-plugin-functional": "^4.4.1", |
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.
🚀
No description provided.