Skip to content
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

Merged
merged 1 commit into from Apr 30, 2018
Merged

feat(rule): add prefer-inline-decorator rule #586

merged 1 commit into from Apr 30, 2018

Conversation

rafaelss95
Copy link
Collaborator

This ensures that decorators are on the same line as the property it decorates.

Failure example:

@Input()
x: string;

Success example:

@Input() x: string;

Closes #549.

Note that I've tried to implement a fix for this. Let me know if I did something wrong.

@rafaelss95
Copy link
Collaborator Author

rafaelss95 commented Apr 28, 2018

@mgechev:

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 @HostListener, for example?

It would fail in this case:

@HostListener('keydown')
newColor() {
  // something nice here
}

... but not for this:

@HostListener('keydown') newColor() {
  // something nice here
}

If so, what would be the best way to setup?

"prefer-decorator-inline": true // no difference for methods or properties decorators

vs

"prefer-decorator-inline": [true, "properties", "methods"] // you can disable one of them based on your needs

I've tried to verify the case I described in #549 (comment), however it seems that this case does not trigger either the visitPropertyDecorator method or the visitNgInput method, so I'm stucked.

@wKoza
Copy link
Collaborator

wKoza commented Apr 29, 2018

sure, implement this for method make sense. @HostListener is good candidate.
Concerning the configuration of the rule, the global approach seems the best way since we want to identify easily the role of an input / method.

Copy link
Owner

@mgechev mgechev left a 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.

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',
Copy link
Owner

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

import { Decorator, Node, PropertyAccessExpression, SourceFile } from 'typescript';
import { NgWalker } from './angular/ngWalker';

export class Rule extends Rules.AbstractRule {
Copy link
Owner

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).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@rafaelss95 rafaelss95 changed the title feat(rule): add prefer-decorator-inline rule feat(rule): add prefer-inline-decorator rule Apr 29, 2018
@mgechev mgechev merged commit 5d5e21d into mgechev:master Apr 30, 2018
@rafaelss95 rafaelss95 deleted the feat-prefer-decorator-inline branch May 1, 2018 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Enforce single line @Input decorator
3 participants