Skip to content

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

Merged
mgechev merged 1 commit intomgechev:masterfrom
rafaelss95:feat-prefer-decorator-inline
Apr 30, 2018
Merged

feat(rule): add prefer-inline-decorator rule#586
mgechev merged 1 commit intomgechev:masterfrom
rafaelss95:feat-prefer-decorator-inline

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