-
-
Notifications
You must be signed in to change notification settings - Fork 503
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
feat: add option to skip arrow function for explicit function return … #4233
base: main
Are you sure you want to change the base?
Conversation
@nazarhussain thank you for your contribution. We would appreciate it if you'd get involved first with the relative issue: #2017 The issue was assigned to another contributor, which already implemented your use case: #4036 |
We recently renamed the rule to |
221337e
to
ea270a5
Compare
CodSpeed Performance ReportMerging #4233 will improve performances by 7.11%Comparing Summary
Benchmarks breakdown
|
const x = { prop: () => {} } | ||
const x = { bar: { prop: () => {} } } |
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.
These examples should be still invalid.
/// interface Behavior { | ||
/// attribute: string; | ||
/// func: () => string; | ||
/// arrowFunc: () => string; | ||
/// } |
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.
This example seems not related to what we check. We could remove it.
/// function getObjectWithFunction(): Behavior { | ||
/// return { | ||
/// attribute: 'value', | ||
/// func: function myFunc(): string { return "value" }, | ||
/// arrowFunc: () => {}, | ||
/// } | ||
/// } |
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.
Code example should be in code block.
/// function getObjectWithFunction(): Behavior { | |
/// return { | |
/// attribute: 'value', | |
/// func: function myFunc(): string { return "value" }, | |
/// arrowFunc: () => {}, | |
/// } | |
/// } | |
/// ```ts | |
/// function getObjectWithFunction(): Behavior { | |
/// return { | |
/// attribute: 'value', | |
/// func: function myFunc(): string { return "value" }, | |
/// arrowFunc: () => {}, | |
/// } | |
/// } | |
/// ``` |
matches!( | ||
func.syntax().parent().kind(), | ||
Some(JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER) | ||
) |
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.
This is not enough because this will accept cases we want to reject.
For example, the following case should be rejected:
const x = {
prop: () => {}
}
In contrast, the following could be accepted:
const X: Type = {
prop: () => {}
};
f({ prop: () => {} })
/// Check if a function is an arrow function | ||
fn is_arrow_func(func: &AnyJsFunction) -> bool { | ||
func.as_js_arrow_function_expression().is_some() | ||
} |
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.
This check is to permissive. This accepts too much code.
Summary
This PR introduce new option
allowArrowFunctions
to thelint/nursary/useExplicitFunctionReturnType
rule.In larger code base there are a lot of arrow functions used for a lot of inline functionality, mostly to overcome need of passing a function instead of a value. e.g.
() => object.attr
. Specifying explicit return type for such arrow functions is tedious to do and don't provide a of help, as the TS can infer the return type easily.This option is similar to
allowExpressions
option for@typescript-eslint/explicit-function-return-type
but not identical.Test Plan
Tested manually by building binary and testing against a sample code.
Could not find a way to set custom option in the test-docs for specific rule, so could not add new test cases.