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

src/preferInlineDecoratorRule.ts

@@ -0,0 +1,91 @@
+import { IOptions, IRuleMetadata, Replacement, RuleFailure, Rules } from 'tslint/lib';
+import { isSameLine } from 'tsutils';
+import { Decorator, Node, PropertyAccessExpression, SourceFile } from 'typescript';
+
+import { NgWalker } from './angular/ngWalker';
+import { getDecoratorName } from './util/utils';
+
+export class Rule extends Rules.AbstractRule {
+  static readonly metadata: IRuleMetadata = {
+    description: 'Ensures that decorators are on the same line as the property/method it decorates.',
+    descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-12.',
+    hasFix: true,
+    optionExamples: ['true', '[true, "HostListener"]'],
+    options: {
+      items: {
+        type: 'string'
+      },
+      type: 'array'
+    },
+    optionsDescription: 'A list of blacklisted decorators.',
+    rationale: 'Placing the decorator on the same line usually makes for shorter code and still easily identifies the property/method.',
+    ruleName: 'prefer-inline-decorator',
+    type: 'style',
+    typescriptOnly: true
+  };
+
+  static readonly FAILURE_STRING = 'Consider placing decorators on the same line as the property/method it decorates';
+
+  apply(sourceFile: SourceFile): RuleFailure[] {
+    return this.applyWithWalker(new PreferInlineDecoratorWalker(sourceFile, this.getOptions()));
+  }
+}
+
+type DecoratorKeys =
+  | 'ContentChild'
+  | 'ContentChildren'
+  | 'HostBinding'
+  | 'HostListener'
+  | 'Input'
+  | 'Output'
+  | 'ViewChild'
+  | 'ViewChildren';
+
+export const decoratorKeys = new Set<DecoratorKeys>([
+  'ContentChild',
+  'ContentChildren',
+  'HostBinding',
+  'HostListener',
+  'Input',
+  'Output',
+  'ViewChild',
+  'ViewChildren'
+]);
+
+export class PreferInlineDecoratorWalker extends NgWalker {
+  private readonly blacklistedDecorators: typeof decoratorKeys;
+
+  constructor(source: SourceFile, options: IOptions) {
+    super(source, options);
+    this.blacklistedDecorators = new Set<DecoratorKeys>(options.ruleArguments.slice(1));
+  }
+
+  protected visitMethodDecorator(decorator: Decorator) {
+    this.validateDecorator(decorator, decorator.parent);
+    super.visitMethodDecorator(decorator);
+  }
+
+  protected visitPropertyDecorator(decorator: Decorator) {
+    this.validateDecorator(decorator, decorator.parent);
+    super.visitPropertyDecorator(decorator);
+  }
+
+  private validateDecorator(decorator: Decorator, property: Node) {
+    const decoratorName: DecoratorKeys = getDecoratorName(decorator);
+    const isDecoratorBlacklisted = this.blacklistedDecorators.has(decoratorName);
+
+    if (isDecoratorBlacklisted) {
+      return;
+    }
+
+    const decoratorStartPos = decorator.getStart();
+    const propertyStartPos = (property as PropertyAccessExpression).name.getStart();
+
+    if (isSameLine(this.getSourceFile(), decoratorStartPos, propertyStartPos)) {
+      return;
+    }
+
+    const fix = Replacement.deleteFromTo(decorator.getEnd(), propertyStartPos - 1);
+    this.addFailureAt(decoratorStartPos, property.getWidth(), Rule.FAILURE_STRING, fix);
+  }
+}

test/preferInlineDecoratorRule.spec.ts

@@ -0,0 +1,201 @@
+import { expect } from 'chai';
+import { Replacement } from 'tslint/lib';
+
+import { decoratorKeys, Rule } from '../src/preferInlineDecoratorRule';
+import { assertFailure, assertFailures, assertSuccess, IExpectedFailure } from './testHelper';
+
+const {
+  FAILURE_STRING,
+  metadata: { ruleName }
+} = Rule;
+const className = 'Test';
+
+describe(ruleName, () => {
+  describe('failure', () => {
+    const expectedFailure: IExpectedFailure = {
+      endPosition: {
+        character: 35,
+        line: 3
+      },
+      message: FAILURE_STRING,
+      startPosition: {
+        character: 14,
+        line: 2
+      }
+    };
+
+    decoratorKeys.forEach(decoratorKey => {
+      describe(decoratorKey, () => {
+        it('should fail when a property does not start on the same line as the decoratorKey', () => {
+          const source = `
+            class ${className} {
+              @${decoratorKey}('childTest')
+              childTest: ChildTest;
+            }
+          `;
+          assertFailure(ruleName, source, expectedFailure);
+        });
+
+        it('should fail and apply proper replacements when a property does not start on the same line as the decoratorKey', () => {
+          const source = `
+            class ${className} {
+              @${decoratorKey}('childTest')
+              childTest: ChildTest;
+            }
+          `;
+          const failures = assertFailure(ruleName, source, expectedFailure);
+          const replacement = Replacement.applyFixes(source, failures.map(f => f.getFix()));
+
+          expect(replacement).to.eq(`
+            class ${className} {
+              @${decoratorKey}('childTest') childTest: ChildTest;
+            }
+          `);
+        });
+      });
+    });
+
+    describe('blacklist', () => {
+      it('should fail when a property does not start on the same line as the decoratorKey and is not present on blacklist options', () => {
+        const [firstDecorator, ...restDecorators] = Array.from(decoratorKeys);
+        const source = `
+          class ${className} {
+            @${firstDecorator}()
+            test = new EventEmitter<void>();
+          }
+        `;
+        assertFailure(
+          ruleName,
+          source,
+          {
+            endPosition: {
+              character: 44,
+              line: 3
+            },
+            message: FAILURE_STRING,
+            startPosition: {
+              character: 12,
+              line: 2
+            }
+          },
+          [true, restDecorators]
+        );
+      });
+    });
+
+    it('should fail when there are multiple properties that does not start on the same line as the decoratorKey', () => {
+      const source = `
+        class ${className} {
+          @Input('childTest')
+          childTest: ChildTest;
+          @Input('foo')
+          foo: Foo;
+        }
+      `;
+      assertFailures(ruleName, source, [
+        {
+          endPosition: {
+            character: 31,
+            line: 3
+          },
+          message: FAILURE_STRING,
+          startPosition: {
+            character: 10,
+            line: 2
+          }
+        },
+        {
+          endPosition: {
+            character: 19,
+            line: 5
+          },
+          message: FAILURE_STRING,
+          startPosition: {
+            character: 10,
+            line: 4
+          }
+        }
+      ]);
+    });
+  });
+
+  describe('success', () => {
+    decoratorKeys.forEach(decoratorKey => {
+      describe(decoratorKey, () => {
+        it('should succeed when a property starts and ends on the same line as the decoratorKey', () => {
+          const source = `
+            class ${className} {
+              @${decoratorKey}('childTest') childTest123: ChildTest;
+            }
+          `;
+          assertSuccess(ruleName, source);
+        });
+
+        it('should succeed when a property starts on the same line as the decoratorKey and ends on another line', () => {
+          const source = `
+            class ${className} {
+              @${decoratorKey}('childTest') childTest123: ChildTest =
+                veryVeryVeryVeryVeryVeryVeryLongDefaultVariable;
+            }
+          `;
+          assertSuccess(ruleName, source);
+        });
+      });
+    });
+
+    describe('blacklist', () => {
+      it('should succeed when a property starts on another line and is present on blacklist options', () => {
+        const [firstDecorator] = Array.from(decoratorKeys);
+        const source = `
+          class ${className} {
+            @${firstDecorator}()
+            test = new EventEmitter<void>();
+          }
+        `;
+        assertSuccess(ruleName, source, [true, firstDecorator]);
+      });
+    });
+
+    describe('special cases', () => {
+      it('should succeed when getters starts on the same line as the decoratorKey and ends on another line', () => {
+        const source = `
+          class ${className} {
+            @Input() get test(): string {
+              return this._test;
+            }
+            private _test: string;
+          }
+        `;
+        assertSuccess(ruleName, source);
+      });
+
+      it('should succeed when setters starts on the same line as the decoratorKey and ends on another line', () => {
+        const source = `
+          class ${className} {
+            @Input() set test(value: string) {
+              this._test = value;
+            }
+            private _test: string;
+          }
+        `;
+        assertSuccess(ruleName, source);
+      });
+
+      it('should succeed for getters and setters on another line', () => {
+        const source = `
+          class ${className} {
+            @Input()
+            get test(): string {
+              return this._test;
+            }
+            set test(value: string) {
+              this._test = value;
+            }
+            private _test: string;
+          }
+        `;
+        assertSuccess(ruleName, source);
+      });
+    });
+  });
+});