fix: some rules not considering options correctly (#617)

Author: rafaelss95

src/maxInlineDeclarationsRule.ts

@@ -1,53 +1,82 @@
-import * as Lint from 'tslint';
-import * as ts from 'typescript';
-import { ComponentMetadata } from './angular/metadata';
+import { sprintf } from 'sprintf-js';
+import { IOptions, IRuleMetadata, RuleFailure, Rules, Utils } from 'tslint/lib';
+import { SourceFile } from 'typescript/lib/typescript';
+import { CodeWithSourceMap, ComponentMetadata } from './angular/metadata';
 import { NgWalker } from './angular/ngWalker';
 
-export class Rule extends Lint.Rules.AbstractRule {
-  public static metadata: Lint.IRuleMetadata = {
-    ruleName: 'max-inline-declarations',
-    type: 'maintainability',
-    description: 'Disallows having too many lines in inline template or styles. Forces separate template or styles file creation.',
-    descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-04',
+const DEFAULT_STYLES_LIMIT: number = 3;
+const DEFAULT_TEMPLATE_LIMIT: number = 3;
+const OPTION_STYLES = 'styles';
+const OPTION_TEMPLATE = 'template';
+
+export class Rule extends Rules.AbstractRule {
+  static readonly metadata: IRuleMetadata = {
+    description: 'Disallows having too many lines in inline template and styles. Forces separate template or styles file creation.',
+    descriptionDetails: 'See more at https://angular.io/guide/styleguide#style-05-04.',
+    optionExamples: [true, [true, { [OPTION_STYLES]: 8, [OPTION_TEMPLATE]: 5 }]],
     options: {
-      type: 'array',
       items: {
+        properties: {
+          [OPTION_STYLES]: {
+            type: 'number'
+          },
+          [OPTION_TEMPLATE]: {
+            type: 'number'
+          }
+        },
         type: 'object'
-      }
+      },
+      maxLength: 1,
+      minLength: 0,
+      type: 'array'
     },
-    optionsDescription: 'Define inline template and styles lines limit.',
-    optionExamples: ['[{template: 5, styles: 8}]'],
+    optionsDescription: Utils.dedent`
+      It can take an optional object with the properties '${OPTION_STYLES}' and '${OPTION_TEMPLATE}':
+      * \`${OPTION_STYLES}\` - number > 0 defining the maximum allowed inline lines for styles. Defaults to ${DEFAULT_STYLES_LIMIT}.
+      * \`${OPTION_TEMPLATE}\` - number > 0 defining the maximum allowed inline lines for template. Defaults to ${DEFAULT_TEMPLATE_LIMIT}.
+    `,
+    rationale:
+      "Large, inline templates and styles obscure the component's purpose and implementation, reducing readability and maintainability.",
+    ruleName: 'max-inline-declarations',
+    type: 'maintainability',
     typescriptOnly: true
   };
 
-  private readonly templateLinesLimit: number = 3;
-  private readonly stylesLinesLimit: number = 3;
+  static readonly FAILURE_STRING = 'Exceeds the maximum allowed inline lines for %s. Defined limit: %s / total lines: %s';
 
-  constructor(options: Lint.IOptions) {
-    super(options);
-    if (options.ruleArguments.length > 1) {
-      const args = options.ruleArguments[1];
-      if (args.template > -1) {
-        this.templateLinesLimit = args.template;
-      }
-      if (args.styles > -1) {
-        this.stylesLinesLimit = args.styles;
-      }
-    }
-  }
-
-  public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-    return this.applyWithWalker(
-      new MaxInlineDeclarationsValidator(sourceFile, this.getOptions(), this.templateLinesLimit, this.stylesLinesLimit)
-    );
+  apply(sourceFile: SourceFile): RuleFailure[] {
+    return this.applyWithWalker(new MaxInlineDeclarationsValidator(sourceFile, this.getOptions()));
   }
 }
 
+type PropertyType = 'styles' | 'template';
+
+export type PropertyPair = { [key in PropertyType]?: number };
+
+const generateFailure = (type: PropertyType, limit: number, value: number): string => {
+  return sprintf(Rule.FAILURE_STRING, type, limit, value);
+};
+
+export const getStylesFailure = (value: number, limit = DEFAULT_STYLES_LIMIT): string => {
+  return generateFailure(OPTION_STYLES, limit, value);
+};
+
+export const getTemplateFailure = (value: number, limit = DEFAULT_TEMPLATE_LIMIT): string => {
+  return generateFailure(OPTION_TEMPLATE, limit, value);
+};
+
 export class MaxInlineDeclarationsValidator extends NgWalker {
-  private newLineRegExp = /\r\n|\r|\n/;
+  private readonly stylesLinesLimit = DEFAULT_STYLES_LIMIT;
+  private readonly templateLinesLimit = DEFAULT_TEMPLATE_LIMIT;
+  private readonly newLineRegExp = /\r\n|\r|\n/;
 
-  constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, private templateLinesLimit: number, private stylesLinesLimit: number) {
+  constructor(sourceFile: SourceFile, options: IOptions) {
     super(sourceFile, options);
+
+    const { styles, template } = (options.ruleArguments[0] || []) as PropertyPair;
+
+    this.stylesLinesLimit = styles > -1 ? styles : this.stylesLinesLimit;
+    this.templateLinesLimit = template > -1 ? template : this.templateLinesLimit;
   }
 
   protected visitNgComponent(metadata: ComponentMetadata): void {
@@ -56,30 +85,44 @@ export class MaxInlineDeclarationsValidator extends NgWalker {
     super.visitNgComponent(metadata);
   }
 
-  private validateInlineTemplate(metadata: ComponentMetadata): void {
-    if (this.hasInlineTemplate(metadata) && this.getTemplateLinesCount(metadata) > this.templateLinesLimit) {
-      const templateLinesCount = this.getTemplateLinesCount(metadata);
-      const msg = `Inline template lines limit exceeded. Defined limit: ${this.templateLinesLimit} / template lines: ${templateLinesCount}`;
-      this.addFailureAtNode(metadata.template.node, msg);
-    }
+  private getTotalLines(source: CodeWithSourceMap['source']): number {
+    return source.trim().split(this.newLineRegExp).length;
+  }
+
+  private getTemplateLinesCount(metadata: ComponentMetadata): number {
+    return this.hasInlineTemplate(metadata) ? this.getTotalLines(metadata.template.template.source) : 0;
   }
 
   private hasInlineTemplate(metadata: ComponentMetadata): boolean {
-    return !!metadata.template && !metadata.template.url && !!metadata.template.template && !!metadata.template.template.source;
+    return !!(metadata.template && !metadata.template.url && metadata.template.template && metadata.template.template.source);
   }
 
-  private getTemplateLinesCount(metadata: ComponentMetadata): number {
-    return metadata.template.template.source.split(this.newLineRegExp).length;
+  private validateInlineTemplate(metadata: ComponentMetadata): void {
+    const templateLinesCount = this.getTemplateLinesCount(metadata);
+
+    if (templateLinesCount <= this.templateLinesLimit) {
+      return;
+    }
+
+    const failureMessage = getTemplateFailure(templateLinesCount, this.templateLinesLimit);
+
+    this.addFailureAtNode(metadata.template.node, failureMessage);
   }
 
-  private validateInlineStyles(metadata: ComponentMetadata): void {
-    if (this.hasInlineStyles(metadata) && this.getInlineStylesLinesCount(metadata) > this.stylesLinesLimit) {
-      const stylesLinesCount = this.getInlineStylesLinesCount(metadata);
-      const msg = `Inline styles lines limit exceeded. Defined limit: ${this.stylesLinesLimit} / styles lines: ${stylesLinesCount}`;
-      for (let i = 0; i < metadata.styles.length; i++) {
-        this.addFailureAtNode(metadata.styles[i].node, msg);
+  private getInlineStylesLinesCount(metadata: ComponentMetadata): number {
+    let result = 0;
+
+    if (!this.hasInlineStyles(metadata)) {
+      return result;
+    }
+
+    for (let i = 0; i < metadata.styles.length; i++) {
+      if (!metadata.styles[i].url) {
+        result += this.getTotalLines(metadata.styles[i].style.source);
       }
     }
+
+    return result;
   }
 
   private hasInlineStyles(metadata: ComponentMetadata): boolean {
@@ -89,6 +132,7 @@ export class MaxInlineDeclarationsValidator extends NgWalker {
 
     for (let i = 0; i < metadata.styles.length; i++) {
       const style = metadata.styles[i];
+
       if (!style.url && style.style && style.style.source) {
         return true;
       }
@@ -97,14 +141,17 @@ export class MaxInlineDeclarationsValidator extends NgWalker {
     return false;
   }
 
-  private getInlineStylesLinesCount(metadata: ComponentMetadata) {
-    let result = 0;
-    for (let i = 0; i < metadata.styles.length; i++) {
-      if (!metadata.styles[i].url) {
-        result += metadata.styles[i].style.source.split(this.newLineRegExp).length;
-      }
+  private validateInlineStyles(metadata: ComponentMetadata): void {
+    const stylesLinesCount = this.getInlineStylesLinesCount(metadata);
+
+    if (stylesLinesCount <= this.stylesLinesLimit) {
+      return;
     }
 
-    return result;
+    const failureMessage = getStylesFailure(stylesLinesCount, this.stylesLinesLimit);
+
+    for (let i = 0; i < metadata.styles.length; i++) {
+      this.addFailureAtNode(metadata.styles[i].node, failureMessage);
+    }
   }
 }

src/noInputPrefixRule.ts

@@ -50,7 +50,7 @@ class NoInputPrefixWalker extends NgWalker {
 
   constructor(source: SourceFile, options: IOptions) {
     super(source, options);
-    this.blacklistedPrefixes = options.ruleArguments.slice(1);
+    this.blacklistedPrefixes = options.ruleArguments;
   }
 
   protected visitNgInput(property: PropertyDeclaration, input: Decorator, args: string[]) {

src/preferInlineDecoratorRule.ts

@@ -41,7 +41,7 @@ type DecoratorKeys =
   | 'ViewChild'
   | 'ViewChildren';
 
-export const decoratorKeys = new Set<DecoratorKeys>([
+export const decoratorKeys: ReadonlySet<DecoratorKeys> = new Set<DecoratorKeys>([
   'ContentChild',
   'ContentChildren',
   'HostBinding',
@@ -52,12 +52,16 @@ export const decoratorKeys = new Set<DecoratorKeys>([
   'ViewChildren'
 ]);
 
+export const getFailureMessage = (): string => {
+  return Rule.FAILURE_STRING;
+};
+
 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));
+    this.blacklistedDecorators = new Set<DecoratorKeys>(options.ruleArguments);
   }
 
   protected visitMethodDecorator(decorator: Decorator) {
@@ -86,6 +90,6 @@ export class PreferInlineDecoratorWalker extends NgWalker {
     }
 
     const fix = Replacement.deleteFromTo(decorator.getEnd(), propertyStartPos - 1);
-    this.addFailureAt(decoratorStartPos, property.getWidth(), Rule.FAILURE_STRING, fix);
+    this.addFailureAt(decoratorStartPos, property.getWidth(), getFailureMessage(), fix);
   }
 }