feat: allow multiple selector types

mgechev · mgechev · commit 4fa35f6c0b8d · 2017-04-20T19:40:07.000-07:00
Allow multiple selector types (fix #290) and refactor the selector-related rules to depend on less state.
diff --git a/src/componentSelectorRule.ts b/src/componentSelectorRule.ts
@@ -60,8 +60,13 @@ export class Rule extends SelectorRule {
   };
 
   public handleType = 'Component';
-  public getTypeFailure():any { return 'The selector of the component "%s" should be used as %s ($$05-03$$)'; }
-  public getNameFailure():any { return 'The selector of the component "%s" should be named %s ($$05-02$$)'; }
-  getSinglePrefixFailure():any { return 'The selector of the component "%s" should have prefix "%s" ($$02-07$$)'; }
-  getManyPrefixFailure():any { return 'The selector of the component "%s" should have one of the prefixes: %s ($$02-07$$)'; }
+  public getTypeFailure(): any { return 'The selector of the component "%s" should be used as %s ($$05-03$$)'; }
+  public getStyleFailure(): any { return 'The selector of the component "%s" should be named %s ($$05-02$$)'; }
+  getPrefixFailure(prefixes: string[]): any {
+    if (prefixes.length === 1) {
+      return 'The selector of the component "%s" should have prefix "%s" ($$02-07$$)';
+    } else {
+      return 'The selector of the component "%s" should have one of the prefixes "%s" ($$02-07$$)';
+    }
+  }
 }
diff --git a/src/directiveSelectorRule.ts b/src/directiveSelectorRule.ts
@@ -57,10 +57,15 @@ export class Rule extends SelectorRule {
   };
 
   public handleType = 'Directive';
-  public getTypeFailure():any { return 'The selector of the directive "%s" should be used as %s ($$02-06$$)'; }
-  public getNameFailure():any { return 'The selector of the directive "%s" should be named %s ($$02-06$$)'; }
-  getSinglePrefixFailure():any { return 'The selector of the directive "%s" should have prefix "%s" ($$02-08$$)'; }
-  getManyPrefixFailure():any { return 'The selector of the directive "%s" should have one of the prefixes: %s ($$02-08$$)'; }
+  public getTypeFailure(): string { return 'The selector of the directive "%s" should be used as %s ($$02-06$$)'; }
+  public getStyleFailure(): string { return 'The selector of the directive "%s" should be named %s ($$02-06$$)'; }
+  getPrefixFailure(prefixes: string[]): string {
+    if (prefixes.length === 1) {
+      return 'The selector of the directive "%s" should have prefix "%s" ($$02-08$$)';
+    } else {
+      return 'The selector of the directive "%s" should have one of the prefixes "%s" ($$02-08$$)';
+    }
+  }
 
 }
 
diff --git a/src/selectorNameBase.ts b/src/selectorNameBase.ts
@@ -1,115 +1,91 @@
 import * as Lint from 'tslint';
-import {SelectorValidator} from './util/selectorValidator';
+import { SelectorValidator } from './util/selectorValidator';
 import * as ts from 'typescript';
 import {sprintf} from 'sprintf-js';
 import * as compiler from '@angular/compiler';
 import { IOptions } from 'tslint';
 import SyntaxKind = require('./util/syntaxKind');
 
-export abstract class SelectorRule extends Lint.Rules.AbstractRule {
-
-  public isMultiPrefix:boolean;
-  public prefixArguments:string;
-  public cssSelectorProperty:string;
+export type SelectorType = 'element' | 'attribute';
+export type SelectorTypeInternal = 'element' | 'attrs';
+export type SelectorStyle = 'kebab-case' | 'camelCase';
 
-  public handleType: string;
-
-  private typeValidator:Function;
-  private prefixValidator:Function;
-  private nameValidator:Function;
-  private FAILURE_PREFIX;
-  private isMultiSelectors:boolean;
+export abstract class SelectorRule extends Lint.Rules.AbstractRule {
+  handleType: string;
+  prefixes: string[];
+  types: SelectorTypeInternal[];
+  style: SelectorStyle[];
 
   constructor(options: IOptions) {
-    const args = options.ruleArguments;
-    let type = args[1];
-    let prefix = args[2] || [];
-    let name = args[3];
     super(options);
-    this.setMultiPrefix(prefix);
-    this.setPrefixArguments(prefix);
-    this.setPrefixValidator(prefix, name);
-    this.setPrefixFailure();
-    this.setTypeValidator(type);
-    this.setNameValidator(name);
-  }
-
-  public getPrefixFailure():string {
-    return this.FAILURE_PREFIX;
-  }
-
-  public validateType(selector:string):boolean {
-    return this.typeValidator(selector);
-  }
+    const args = options.ruleArguments;
 
-  public validateName(selector:any):boolean {
-    if(this.isMultiSelectors) {
-      return selector.some((a) => this.nameValidator(a));
-    } else {
-      return this.nameValidator(selector);
+    let type: SelectorType[] = args[1] || ['element', 'attribute'];
+    if (!(type instanceof Array)) {
+      type = [type];
     }
-  }
-
-  public validatePrefix(selector:any):boolean {
-    if(this.isMultiSelectors) {
-      return selector.some((a) => this.prefixValidator(a));
-    } else {
-      return this.prefixValidator(selector);
+    let internal: SelectorTypeInternal[] = [];
+    if (type.indexOf('element') >= 0) {
+      internal.push('element');
     }
-  }
-
-  public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-    return this.applyWithWalker(
-      new SelectorValidatorWalker(
-        sourceFile,
-        this));
-  }
+    if (type.indexOf('attribute') >= 0) {
+      internal.push('attrs');
+    }
+    this.types = internal;
 
-  public abstract getTypeFailure(): any;
-  public abstract  getNameFailure(): any;
-  protected abstract getSinglePrefixFailure(): any;
-  protected abstract getManyPrefixFailure(): any;
+    let prefix = args[2] || [];
+    if (!(prefix instanceof Array)) {
+      prefix = [prefix];
+    }
+    this.prefixes = prefix;
 
-  private setNameValidator(name:string) {
-    if (name === 'camelCase') {
-      this.nameValidator = SelectorValidator.camelCase;
-    } else if(name === 'kebab-case') {
-      this.nameValidator = SelectorValidator.kebabCase;
+    let style = args[3];
+    if (!(style instanceof Array)) {
+      style = [style];
     }
+    this.style = style;
   }
 
-  private setMultiPrefix(prefix:string) {
-    this.isMultiPrefix = typeof prefix ==='string';
+  public validateType(selectors: compiler.CssSelector[]): boolean {
+    return this.getValidSelectors(selectors).length > 0;
   }
 
-  private setPrefixArguments(prefix:any) {
-    this.prefixArguments = this.isMultiPrefix?prefix:prefix.join(',');
+  public validateStyle(selectors: compiler.CssSelector[]): boolean {
+    return this.getValidSelectors(selectors).some(selector => {
+      return this.style.some(style => {
+        let validator = SelectorValidator.camelCase;
+        if (style === 'kebab-case') {
+          validator = SelectorValidator.kebabCase;
+        }
+        return validator(selector);
+      });
+    });
   }
 
-  private setPrefixValidator(prefix: any, name: string) {
-    if (!this.isMultiPrefix) {
-      prefix = (prefix||[]).sort((a: string, b: string) => {
-        return a.length < b.length ? 1 : -1;
-      });
-    }
-    let prefixExpression: string = this.isMultiPrefix?prefix:(prefix||[]).join('|');
-    this.prefixValidator = SelectorValidator.prefix(prefixExpression, name);
+  public validatePrefix(selectors: compiler.CssSelector[]): boolean {
+    return this.getValidSelectors(selectors)
+      .some(selector => !this.prefixes.length || this.prefixes.some(p =>
+        this.style.some(s => SelectorValidator.prefix(p, s)(selector))));
   }
 
-  private setPrefixFailure() {
-    this.FAILURE_PREFIX = this.isMultiPrefix?this.getSinglePrefixFailure():this.getManyPrefixFailure();
+  public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+    return this.applyWithWalker(new SelectorValidatorWalker(sourceFile, this));
   }
 
-  private setTypeValidator(type:string) {
-    if (type === 'element') {
-      this.typeValidator = SelectorValidator.element;
-      this.isMultiSelectors = false;
-      this.cssSelectorProperty = 'element';
-    } else if (type === 'attribute') {
-      this.typeValidator = SelectorValidator.attribute;
-      this.isMultiSelectors = true;
-      this.cssSelectorProperty = 'attrs';
-    }
+  abstract getTypeFailure(): string;
+  abstract getStyleFailure(): string;
+  abstract getPrefixFailure(prefixes: string[]): string;
+
+  private getValidSelectors(selectors: compiler.CssSelector[]) {
+    return [].concat.apply([], selectors.map(selector => {
+      return [].concat.apply([], this.types.map(t => {
+        let prop = selector[t];
+        if (prop && !(prop instanceof Array)) {
+          prop = [prop];
+        }
+        return prop;
+      }).filter(s => !!s));
+    }));
   }
 }
 
@@ -140,30 +116,21 @@ export class SelectorValidatorWalker extends Lint.RuleWalker {
   private validateSelector(className: string, arg: ts.Node) {
     if (arg.kind === SyntaxKind.current().ObjectLiteralExpression) {
       (<ts.ObjectLiteralExpression>arg).properties.filter(prop => this.validateProperty(prop))
-        .map(prop=>(<any>prop).initializer)
+        .map(prop => (<any>prop).initializer)
         .forEach(i => {
-          const selectors: any = this.extractMainSelector(i);
-          const validateSelectors = (cb: any) => {
-            // If all selectors fail, this will return true which means that the selector is invalid
-            // according to the validation callback.
-            // Since the method is called validateSelector, it's suppose to return true if the
-            // selector is valid, so we're taking the result with "!".
-            return !selectors.every((selector: any) => {
-              return !cb(selector[this.rule.cssSelectorProperty]);
-            });
-          };
-          if (!validateSelectors(this.rule.validateType.bind(this.rule))) {
+          const selectors: compiler.CssSelector[] = this.extractMainSelector(i);
+          if (!this.rule.validateType(selectors)) {
             let error = sprintf(this.rule.getTypeFailure(), className, this.rule.getOptions().ruleArguments[1]);
-            this.addFailure(this.createFailure(i.getStart(), i.getWidth(),error));
-          } else if (!validateSelectors(this.rule.validateName.bind(this.rule))) {
+            this.addFailure(this.createFailure(i.getStart(), i.getWidth(), error));
+          } else if (!this.rule.validateStyle(selectors)) {
             let name = this.rule.getOptions().ruleArguments[3];
             if (name === 'kebab-case') {
               name += ' and include dash';
             }
-            let error = sprintf(this.rule.getNameFailure(), className, name);
+            let error = sprintf(this.rule.getStyleFailure(), className, name);
             this.addFailure(this.createFailure(i.getStart(), i.getWidth(), error));
-          } else if (!validateSelectors(this.rule.validatePrefix.bind(this.rule))) {
-            let error = sprintf(this.rule.getPrefixFailure(),className,this.rule.prefixArguments);
+          } else if (!this.rule.validatePrefix(selectors)) {
+            let error = sprintf(this.rule.getPrefixFailure(this.rule.prefixes), className, this.rule.prefixes.join(', '));
             this.addFailure(this.createFailure(i.getStart(), i.getWidth(), error));
           }
         });
@@ -179,7 +146,7 @@ export class SelectorValidatorWalker extends Lint.RuleWalker {
     return [current.StringLiteral, current.NoSubstitutionTemplateLiteral].some(kindType => kindType === kind);
   }
 
-  private extractMainSelector(i:any) {
+  private extractMainSelector(i: any) {
     return compiler.CssSelector.parse(i.text);
   }
 }
diff --git a/test/componentClassSuffixRule.spec.ts b/test/componentClassSuffixRule.spec.ts
@@ -8,7 +8,7 @@ describe('component-class-suffix', () => {
                 selector: 'sg-foo-bar'
               })
               class Test {}
-                    ~~~~    
+                    ~~~~
               `;
             assertAnnotated({
                 ruleName: 'component-class-suffix',
diff --git a/test/componentSelectorRule.spec.ts b/test/componentSelectorRule.spec.ts
@@ -1,4 +1,4 @@
-import {assertSuccess, assertAnnotated, assertMultipleAnnotated} from './testHelper';
+import { assertSuccess, assertAnnotated, assertMultipleAnnotated } from './testHelper';
 
 describe('component-selector-prefix', () => {
     describe('invalid component selectors', () => {
@@ -26,7 +26,7 @@ describe('component-selector-prefix', () => {
       class Test {}`;
             assertAnnotated({
                 ruleName: 'component-selector',
-                message:  'The selector of the component "Test" should have one of the prefixes: sg,mg,ng ($$02-07$$)',
+                message:  'The selector of the component "Test" should have one of the prefixes "sg, mg, ng" ($$02-07$$)',
                 source,
                 options: ['element', ['sg','mg','ng'], 'kebab-case']
             });
@@ -40,7 +40,7 @@ describe('component-selector-prefix', () => {
       class Test {}`;
             assertAnnotated({
                 ruleName: 'component-selector',
-                message:  'The selector of the component "Test" should have one of the prefixes: sg,mg,ng ($$02-07$$)',
+                message:  'The selector of the component "Test" should have one of the prefixes "sg, mg, ng" ($$02-07$$)',
                 source,
                 options: ['element', ['sg','mg','ng'], 'kebab-case']
             });
@@ -49,16 +49,16 @@ describe('component-selector-prefix', () => {
 
         it('should fail when component used longer prefix', () => {
             let source = `
-          @Component({selector: 'foo-bar'}) class TestOne {} 
+          @Component({selector: 'foo-bar'}) class TestOne {}
                                 ~~~~~~~~~
           @Component({selector: 'ngg-bar'}) class TestTwo {}
                                 ^^^^^^^^^
           `;
             assertMultipleAnnotated({
                 ruleName: 'component-selector',
                 failures: [
-                  { char: '~', msg: 'The selector of the component "TestOne" should have one of the prefixes: fo,mg,ng ($$02-07$$)'},
-                  { char: '^', msg: 'The selector of the component "TestTwo" should have one of the prefixes: fo,mg,ng ($$02-07$$)'},
+                  { char: '~', msg: 'The selector of the component "TestOne" should have one of the prefixes "fo, mg, ng" ($$02-07$$)'},
+                  { char: '^', msg: 'The selector of the component "TestTwo" should have one of the prefixes "fo, mg, ng" ($$02-07$$)'},
                 ],
                 source,
                 options: ['element', ['fo','mg','ng'], 'kebab-case']
@@ -152,6 +152,15 @@ describe('component-selector-type', () => {
                 options: ['element', ['sg','ng'], 'kebab-case']
             });
         });
+
+        it('should accept several selector types', () => {
+          let source = `
+          @Component({
+            selector: \`[fooBar]\`
+          })
+          class Test {}`;
+          assertSuccess('component-selector', source, [['element', 'attribute'], ['foo','ng'], 'camelCase']);
+        });
     });
 
     describe('valid component selector', () => {
diff --git a/test/directiveSelectorRule.spec.ts b/test/directiveSelectorRule.spec.ts
@@ -95,7 +95,7 @@ describe('directive-selector-prefix', () => {
           class Test {}`;
             assertAnnotated({
                 ruleName: 'directive-selector',
-                message: 'The selector of the directive "Test" should have one of the prefixes: sg,ng,mg ($$02-08$$)',
+                message: 'The selector of the directive "Test" should have one of the prefixes "sg, ng, mg" ($$02-08$$)',
                 source,
                 options: ['attribute',['sg', 'ng', 'mg'],'camelCase']
             });
@@ -109,7 +109,7 @@ describe('directive-selector-prefix', () => {
           class Test {}`;
             assertAnnotated({
                 ruleName: 'directive-selector',
-                message: 'The selector of the directive "Test" should have one of the prefixes: sg,ng,mg ($$02-08$$)',
+                message: 'The selector of the directive "Test" should have one of the prefixes "sg, ng, mg" ($$02-08$$)',
                 source,
                 options: ['attribute',['sg', 'ng', 'mg'],'camelCase']
             });
@@ -125,6 +125,15 @@ describe('directive-selector-prefix', () => {
             assertSuccess('directive-selector', source, ['attribute','sg','camelCase']);
         });
 
+        it('should succeed when set valid selector in @Directive', () => {
+            let source = `
+      @Directive({
+        selector: 'sgBarFoo'
+      })
+      class Test {}`;
+            assertSuccess('directive-selector', source, [['attribute', 'element'],'sg','camelCase']);
+        });
+
         it('should succeed when set valid selector in @Directive using multiple prefixes', () => {
             let source = `
       @Directive({
