Add arrow-return-shorthand rule
#1972
Conversation
| /* tslint:enable:object-literal-sort-keys */ | ||
|
|
||
| public static FAILURE_STRING = | ||
| "Arrow function body may be converted just this expression, with no `{ return ... }`."; |
There was a problem hiding this comment.
How about This arrow function body can be simplified by omitting the curly braces and the keyword 'return' where the function body is underlined. Not sure that people will get what "this expression" means and that excluding { return ... } still includes the ...
| @@ -0,0 +1,29 @@ | |||
| // Invalid: | |||
There was a problem hiding this comment.
can you add a test case for returning an object literal?
| super.visitArrowFunction(node); | ||
| } | ||
|
|
||
| private fix(arrowFunction: ts.FunctionLikeDeclaration, body: ts.Block, expr: ts.Expression): Lint.Fix | undefined { |
There was a problem hiding this comment.
rename to createArrowFunctionFix
| } | ||
|
|
||
| class Walker extends Lint.RuleWalker { | ||
| public visitArrowFunction(node: ts.FunctionLikeDeclaration) { |
There was a problem hiding this comment.
You are only handling ts.ArrowFunctions here, so you can use it as your parameter's type. Same in the fix method below.
You can then omit the check for node.body && in the condition below, as it will always be defined.
| export class Rule extends Lint.Rules.AbstractRule { | ||
| /* tslint:disable:object-literal-sort-keys */ | ||
| public static metadata: Lint.IRuleMetadata = { | ||
| ruleName: "expression-valued-functions", |
There was a problem hiding this comment.
I don't really like this name. To me it's not clear what this rule does without looking at the description. I'm not a native speaker though.
I'd rename it to prefer-arrow-shorthand-return or similar ...
There was a problem hiding this comment.
yeah, I like prefer-arrow-shorthand-return as well
| public static metadata: Lint.IRuleMetadata = { | ||
| ruleName: "expression-valued-functions", | ||
| description: "Suggests to convert `() => { return x; }` to `() => x`.", | ||
| optionsDescription: "Not configurable.", |
There was a problem hiding this comment.
Can you add an option to exclude multiline arrow-functions.
Otherwise it would conflict with https://www.npmjs.com/package/vrsource-tslint-rules#multiline-arrow
It's not a core rule, but I like to use it to increase readability.
There was a problem hiding this comment.
I think this should skip multiline arrow functions by default; if you want to lint those too, you can add that as an option
| ]); | ||
|
|
||
| function hasComments(start: number, end: number): boolean { | ||
| return !isAllWhitespace(text, start, end); |
There was a problem hiding this comment.
Can you use ts.get{Leading,Trailing}CommentRanges like in https://github.com/palantir/tslint/blob/master/src/rules/noEmptyRule.ts#L51-L52 instead of implementing it on your own?
If you feel like it, you could add a function to utils.js, e.g. hasCommentBetween(sourceFile, fromNode, toNode)
andy-hanson
changed the title
expression-valued-functions ruleprefer-arrow-shorthand-return rule
|
|
||
| export class Rule extends Lint.Rules.AbstractRule { | ||
| /* tslint:disable:object-literal-sort-keys */ | ||
| public static metadata: Lint.IRuleMetadata = { |
There was a problem hiding this comment.
can you please add hasFix: true to the metadata?
| export class Rule extends Lint.Rules.AbstractRule { | ||
| /* tslint:disable:object-literal-sort-keys */ | ||
| public static metadata: Lint.IRuleMetadata = { | ||
| ruleName: "prefer-arrow-shorthand-return", |
There was a problem hiding this comment.
sorry for changing this up again -- I actually think we can simplify the name further and put "shorthand" at the end to be consistent with object-literal-shorthand. how about "arrow-return-shorthand"?
andy-hanson
force-pushed
the
expression_valued_functions
branch
from
00b1d2e to
ff252e2
Compare
|
@andy-hanson thanks! |
nchen63
changed the title
prefer-arrow-shorthand-return rulearrow-shorthand-return rule
nchen63
changed the title
arrow-shorthand-return rulearrow-return-shorthand rule
This is missing in the rules documentation. |
PR checklist
What changes did you make?
Added the
expression-valued-functionsrule, which suggests to change() => { return x; }to() => x.