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(rule): add prefer-inline-decorator rule #586
feat(rule): add prefer-inline-decorator rule #586
Conversation
About the name: not sure if it's the best, you can suggest another one. I implemented it for all "property" decorators, does it makes sense to add it for "method" decorators, as It would fail in this case:
... but not for this:
If so, what would be the best way to setup?
vs
I've tried to verify the case I described in #549 (comment), however it seems that this case does not trigger either the |
sure, implement this for method make sense. @HostListener is good candidate. |
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.
Good job! Left two minor comments.
src/preferDecoratorInlineRule.ts
Outdated
optionsDescription: 'Not configurable.', | ||
rationale: | ||
'Placing the decorator on the same line usually makes for shorter code and still easily identifies the property as an input or output.', | ||
ruleName: 'prefer-decorator-inline', |
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'd call it prefer-inline-decorator
.
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.
Done.
src/preferDecoratorInlineRule.ts
Outdated
import { Decorator, Node, PropertyAccessExpression, SourceFile } from 'typescript'; | ||
import { NgWalker } from './angular/ngWalker'; | ||
|
||
export class Rule extends Rules.AbstractRule { |
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.
It makes sense to allow rule configuration (i.e.blacklist decorators for example).
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.
Done.
This ensures that decorators are on the same line as the property it decorates.
Failure example:
Success example:
Closes #549.
Note that I've tried to implement a fix for this. Let me know if I did something wrong.