fix(rule): no-input-prefix not being able to check for multiple concu…

…rrent prefixes (#590)

LGTM

src/noInputPrefixRule.ts

@@ -1,51 +1,75 @@
-import * as Lint from 'tslint';
-import * as ts from 'typescript';
 import { sprintf } from 'sprintf-js';
+import { IOptions, IRuleMetadata, RuleFailure, Rules } from 'tslint/lib';
+import { arrayify } from 'tslint/lib/utils';
+import { Decorator, Node, PropertyAccessExpression, PropertyDeclaration, SourceFile } from 'typescript';
+
 import { NgWalker } from './angular/ngWalker';
 
-export class Rule extends Lint.Rules.AbstractRule {
-  public static metadata: Lint.IRuleMetadata = {
-    ruleName: 'no-input-prefix',
-    type: 'maintainability',
-    description: 'Input names should not be prefixed with the configured disallowed prefixes.',
-    rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inputs.
-    * Example: 'enabled' is prefered over 'isEnabled'.
-    `,
+export class Rule extends Rules.AbstractRule {
+  static readonly metadata: IRuleMetadata = {
+    description: 'Input names should not be prefixed by the configured disallowed prefixes.',
+    optionExamples: ['[true, "can", "is", "should"]'],
     options: {
-      type: 'array',
-      items: [{ type: 'string' }]
+      items: [{ type: 'string' }],
+      type: 'array'
     },
-    optionExamples: ['["is", "can", "should"]'],
     optionsDescription: 'Options accept a string array of disallowed input prefixes.',
+    rationale: `HTML attributes are not prefixed. It's considered best not to prefix Inpu
+    * Example: 'enabled' is prefered over 'isEnabled'.
+    `,
+    ruleName: 'no-input-prefix',
+    type: 'maintainability',
     typescriptOnly: true
   };
 
-  static FAILURE_STRING: string = 'In the class "%s", the input property "%s" should not be prefixed with %s';
+  static readonly FAILURE_STRING = 'In the class "%s", the input property "%s" should not be prefixed by %s';
 
-  public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-    return this.applyWithWalker(new InputWalker(sourceFile, this.getOptions()));
+  apply(sourceFile: SourceFile): RuleFailure[] {
+    return this.applyWithWalker(new NoInputPrefixWalker(sourceFile, this.getOptions()));
   }
 }
 
-class InputWalker extends NgWalker {
-  visitNgInput(property: ts.PropertyDeclaration, input: ts.Decorator, args: string[]) {
-    const className = (<any>property).parent.name.text;
-    const memberName = (<any>property.name).text as string;
-    const options = this.getOptions() as string[];
-    let prefixLength: number;
+const getReadablePrefixes = (prefixes: string[]): string => {
+  const prefixesLength = prefixes.length;
 
-    if (memberName) {
-      const foundInvalid = options.find(x => memberName.startsWith(x));
-      prefixLength = foundInvalid ? foundInvalid.length : 0;
-    }
+  if (prefixesLength === 1) {
+    return `"${prefixes[0]}"`;
+  }
 
-    if (
-      prefixLength > 0 &&
-      !(memberName.length >= prefixLength + 1 && memberName[prefixLength] !== memberName[prefixLength].toUpperCase())
-    ) {
-      const failureConfig: string[] = [Rule.FAILURE_STRING, className, memberName, options.join(', ')];
-      const errorMessage = sprintf.apply(null, failureConfig);
-      this.addFailure(this.createFailure(property.getStart(), property.getWidth(), errorMessage));
+  return `${prefixes
+    .map(x => `"${x}"`)
+    .slice(0, prefixesLength - 1)
+    .join(', ')} or "${[...prefixes].pop()}"`;
+};
+
+export const getFailureMessage = (className: string, propertyName: string, prefixes: string[]): string => {
+  return sprintf(Rule.FAILURE_STRING, className, propertyName, getReadablePrefixes(prefixes));
+};
+
+class NoInputPrefixWalker extends NgWalker {
+  private readonly blacklistedPrefixes: string[];
+
+  constructor(source: SourceFile, options: IOptions) {
+    super(source, options);
+    this.blacklistedPrefixes = arrayify<string>(options.ruleArguments).slice(1);
+  }
+
+  protected visitNgInput(property: PropertyDeclaration, input: Decorator, args: string[]) {
+    this.validatePrefix(property, input, args);
+    super.visitNgInput(property, input, args);
+  }
+
+  private validatePrefix(property: PropertyDeclaration, input: Decorator, args: string[]) {
+    const memberName = property.name.getText();
+    const isBlackListedPrefix = this.blacklistedPrefixes.some(x => new RegExp(`^${x}[^a-z]`).test(memberName));
+
+    if (!isBlackListedPrefix) {
+      return;
     }
+
+    const className = (property.parent as PropertyAccessExpression).name.getText();
+    const failure = getFailureMessage(className, memberName, this.blacklistedPrefixes);
+
+    this.addFailureAtNode(property, failure);
   }
 }

test/noInputPrefixRule.spec.ts

@@ -1,117 +1,121 @@
-import { assertSuccess, assertAnnotated } from './testHelper';
+import { getFailureMessage, Rule } from '../src/noInputPrefixRule';
+import { assertAnnotated, assertSuccess } from './testHelper';
 
-describe('no-input-prefix', () => {
-  describe('invalid directive input property', () => {
-    it('should fail, when a component input property is named with is prefix', () => {
-      const source = `
-        @Component()
-        class ButtonComponent {
-          @Input() isDisabled: boolean;
-          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-        }
-      `;
-      assertAnnotated({
-        ruleName: 'no-input-prefix',
-        options: ['is'],
-        message: 'In the class "ButtonComponent", the input property "isDisabled" should not be prefixed with is',
-        source
-      });
-    });
+const {
+  FAILURE_STRING,
+  metadata: { ruleName }
+} = Rule;
+const className = 'Test';
+
+const getFailureAnnotations = (num: number): string => {
+  return '~'.repeat(num);
+};
+
+const getComposedOptions = (prefixes: string[]): (boolean | string)[] => {
+  return [true, ...prefixes];
+};
 
-    it('should fail, when a directive input property is named with is prefix', () => {
+describe(ruleName, () => {
+  describe('failure', () => {
+    it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by one prefix', () => {
+      const prefixes = ['is'];
+      const propertyName = `${prefixes[0]}Disabled`;
+      const inputExpression = `@Input() ${propertyName}: boolean;`;
       const source = `
         @Directive()
-        class ButtonDirective {
-          @Input() isDisabled: boolean;
-          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+        class ${className} {
+          ${inputExpression}
+          ${getFailureAnnotations(inputExpression.length)}
         }
       `;
       assertAnnotated({
-        ruleName: 'no-input-prefix',
-        options: ['is'],
-        message: 'In the class "ButtonDirective", the input property "isDisabled" should not be prefixed with is',
+        message: getFailureMessage(className, propertyName, prefixes),
+        options: getComposedOptions(prefixes),
+        ruleName,
         source
       });
     });
 
-    it('should fail, when a directive input property is named with is prefix', () => {
+    it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two prefixes', () => {
+      const prefixes = ['can', 'is'];
+      const propertyName = `${prefixes[0]}Enable`;
+      const inputExpression = `@Input() ${propertyName}: boolean;`;
       const source = `
-        @Directive()
-        class ButtonDirective {
-          @Input() mustDisable: string;
-          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+        @Component()
+        class ${className} {
+          ${inputExpression}
+          ${getFailureAnnotations(inputExpression.length)}
         }
       `;
       assertAnnotated({
-        ruleName: 'no-input-prefix',
-        options: ['must'],
-        message: 'In the class "ButtonDirective", the input property "mustDisable" should not be prefixed with must',
+        message: getFailureMessage(className, propertyName, prefixes),
+        options: getComposedOptions(prefixes),
+        ruleName,
         source
       });
     });
 
-    it('should fail, when a directive input property is named with is prefix', () => {
+    it('should fail when an input property is prefixed by a blacklisted prefix and blacklist is composed by two concurrent prefixes', () => {
+      const prefixes = ['is', 'isc'];
+      const propertyName = `${prefixes[1]}Hange`;
+      const inputExpression = `@Input() ${propertyName}: boolean;`;
       const source = `
-        @Directive()
-        class ButtonDirective {
-          @Input() is = true;
-          ~~~~~~~~~~~~~~~~~~~
+        @Component()
+        class ${className} {
+          ${inputExpression}
+          ${getFailureAnnotations(inputExpression.length)}
         }
       `;
       assertAnnotated({
-        ruleName: 'no-input-prefix',
-        options: ['is'],
-        message: 'In the class "ButtonDirective", the input property "is" should not be prefixed with is',
+        message: getFailureMessage(className, propertyName, prefixes),
+        options: getComposedOptions(prefixes),
+        ruleName,
         source
       });
     });
 
-    it('should fail, when a directive input property is named with can prefix', () => {
+    it('should fail when an input property is snakecased and contains a blacklisted prefix', () => {
+      const prefixes = ['do'];
+      const propertyName = `${prefixes[0]}_it`;
+      const inputExpression = `@Input() ${propertyName}: number;`;
       const source = `
         @Directive()
-        class ButtonDirective {
-          @Input() canEnable = true;
-          ~~~~~~~~~~~~~~~~~~~~~~~~~~
+        class ${className} {
+          ${inputExpression}
+          ${getFailureAnnotations(inputExpression.length)}
         }
       `;
       assertAnnotated({
-        ruleName: 'no-input-prefix',
-        options: ['can', 'is'],
-        message: 'In the class "ButtonDirective", the input property "canEnable" should not be prefixed with can, is',
+        message: getFailureMessage(className, propertyName, prefixes),
+        options: getComposedOptions(prefixes),
+        ruleName,
         source
       });
     });
   });
 
-  describe('valid directive input property', () => {
-    it('should succeed, when a directive input property is properly named', () => {
-      const source = `
-        @Directive()
-        class ButtonComponent {
-          @Input() disabled = true;
-        }
-      `;
-      assertSuccess('no-input-prefix', source);
-    });
-
-    it('should succeed, when a directive input property is properly named', () => {
+  describe('success', () => {
+    it('should succeed when an input property is not prefixed', () => {
       const source = `
         @Directive()
-        class ButtonComponent {
-          @Input() disabled = "yes";
+        class ${className} {
+          @Input() mustmust = true;
         }
       `;
-      assertSuccess('no-input-prefix', source);
+      assertSuccess(ruleName, source, getComposedOptions(['must']));
     });
 
-    it('should succeed, when a component input property is properly named with is', () => {
+    it('should succeed when multiple input properties are prefixed by something not present in the blacklist', () => {
       const source = `
         @Component()
-        class ButtonComponent {
-          @Input() isometric: boolean;
+        class ${className} {
+          @Input() cana: string;
+          @Input() disabledThing: boolean;
+          @Input() isFoo = 'yes';
+          @Input() shoulddoit: boolean;
         }
       `;
-      assertSuccess('no-input-prefix', source);
+      assertSuccess(ruleName, source, getComposedOptions(['can', 'should', 'dis', 'disable']));
     });
   });
 });