Merge pull request #356 from wKoza/whitespaceAfterSemiColon

Ensure whitespaces after semicolon in structural dir
mgechev · Jun 27, 2017 · 25667f9 · 25667f9
2 parents 2f52d77 + 3abd245
commit 25667f9
Show file tree

Hide file tree

Showing 3 changed files with 368 additions and 156 deletions.
diff --git a/src/angular/ngWalker.ts b/src/angular/ngWalker.ts
@@ -47,12 +47,6 @@ export class NgWalker extends Lint.RuleWalker {
       cssVisitorCtrl: BasicCssAstVisitor
     }, this._config || {});
 
-    this._config = Object.assign({
-      templateVisitorCtrl: BasicTemplateAstVisitor,
-      expressionVisitorCtrl: RecursiveAngularExpressionVisitor,
-      cssVisitorCtrl: BasicCssAstVisitor
-    }, this._config || {});
-    // this._config = ngWalkerFactoryUtils.normalizeConfig(this._config);
   }
 
   visitClassDeclaration(declaration: ts.ClassDeclaration) {

diff --git a/src/angularWhitespaceRule.ts b/src/angularWhitespaceRule.ts
@@ -2,17 +2,20 @@ import * as Lint from 'tslint';
 import * as ts from 'typescript';
 import {NgWalker} from './angular/ngWalker';
 import * as ast from '@angular/compiler';
-import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor';
-import { ExpTypes } from './angular/expressionTypes';
-import { Config } from './angular/config';
-import { RecursiveAngularExpressionVisitor } from './angular/templates/recursiveAngularExpressionVisitor';
+import {BasicTemplateAstVisitor} from './angular/templates/basicTemplateAstVisitor';
+import {ExpTypes} from './angular/expressionTypes';
+import {Config} from './angular/config';
+import {RecursiveAngularExpressionVisitor} from './angular/templates/recursiveAngularExpressionVisitor';
 
 const InterpolationOpen = Config.interpolation[0];
 const InterpolationClose = Config.interpolation[1];
 const InterpolationNoWhitespaceRe = new RegExp(`${InterpolationOpen}\\S(.*?)\\S${InterpolationClose}|${InterpolationOpen}` +
   `\\s(.*?)\\S${InterpolationClose}|${InterpolationOpen}\\S(.*?)\\s${InterpolationClose}`);
 const InterpolationExtraWhitespaceRe =
   new RegExp(`${InterpolationOpen}\\s\\s(.*?)\\s${InterpolationClose}|${InterpolationOpen}\\s(.*?)\\s\\s${InterpolationClose}`);
+const SemicolonNoWhitespaceNotInSimpleQuoteRe = new RegExp(/;\S(?![^']*')/);
+const SemicolonNoWhitespaceNotInDoubleQuoteRe = new RegExp(/;\S(?![^"]*")/);
+
 
 const getReplacements = (text: ast.BoundTextAst, absolutePosition: number) => {
   const expr: string = (text.value as any).source;
@@ -27,7 +30,16 @@ const getReplacements = (text: ast.BoundTextAst, absolutePosition: number) => {
   ];
 };
 
-type Option = 'check-interpolation' | 'check-pipe';
+
+const getSemicolonReplacements = (text: ast.BoundDirectivePropertyAst, absolutePosition: number) => {
+
+  return [
+    new Lint.Replacement(absolutePosition, 1, '; ')
+  ];
+
+};
+
+type Option = 'check-interpolation' | 'check-pipe' | 'check-semicolon';
 
 interface ConfigurableVisitor {
   getOption(): Option;
@@ -66,9 +78,52 @@ class InterpolationWhitespaceVisitor extends BasicTemplateAstVisitor implements
   }
 }
 
+class SemicolonTemplateVisitor extends BasicTemplateAstVisitor implements ConfigurableVisitor {
+
+  visitDirectiveProperty(prop: ast.BoundDirectivePropertyAst, context: BasicTemplateAstVisitor): any {
+
+
+    if (prop.sourceSpan) {
+      const directive = (<any>prop.sourceSpan).toString();
+      const rawExpression = directive.split('=')[1].trim();
+      const expr = rawExpression.substring(1, rawExpression.length - 1).trim();
+
+      const doubleQuote = rawExpression.substring(0, 1).indexOf('\"') === 0;
+
+      // Note that will not be reliable for different interpolation symbols
+      let error = null;
+
+      if (doubleQuote && SemicolonNoWhitespaceNotInSimpleQuoteRe.test(expr)) {
+        error = 'Missing whitespace after semicolon; expecting \'; expr\'';
+        const internalStart = expr.search(SemicolonNoWhitespaceNotInSimpleQuoteRe) + 1;
+        const start = prop.sourceSpan.start.offset + internalStart + directive.length - directive.split('=')[1].trim().length + 1;
+        const absolutePosition = context.getSourcePosition(start - 1);
+        return context.addFailure(context.createFailure(start, 2,
+          error, getSemicolonReplacements(prop, absolutePosition))
+        );
+      } else if (!doubleQuote && SemicolonNoWhitespaceNotInDoubleQuoteRe.test(expr)) {
+        error = 'Missing whitespace after semicolon; expecting \'; expr\'';
+        const internalStart = expr.search(SemicolonNoWhitespaceNotInDoubleQuoteRe) + 1;
+        const start = prop.sourceSpan.start.offset + internalStart + directive.length - directive.split('=')[1].trim().length + 1;
+        const absolutePosition = context.getSourcePosition(start - 1);
+        return context.addFailure(context.createFailure(start, 2,
+          error, getSemicolonReplacements(prop, absolutePosition))
+        );
+      }
+    }
+  }
+
+  getOption(): Option {
+    return 'check-semicolon';
+  }
+
+}
+
+
 class WhitespaceTemplateVisitor extends BasicTemplateAstVisitor {
   private visitors: (BasicTemplateAstVisitor & ConfigurableVisitor)[] = [
-    new InterpolationWhitespaceVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart)
+    new InterpolationWhitespaceVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart),
+    new SemicolonTemplateVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart)
   ];
 
   visitBoundText(text: ast.BoundTextAst, context: any): any {
@@ -80,8 +135,21 @@ class WhitespaceTemplateVisitor extends BasicTemplateAstVisitor {
       .forEach(f => this.addFailure(f));
     super.visitBoundText(text, context);
   }
+
+  visitDirectiveProperty(prop: ast.BoundDirectivePropertyAst, context: any): any {
+    const options = this.getOptions();
+    this.visitors
+      .filter(v => options.indexOf(v.getOption()) >= 0)
+      .map(v => v.visitDirectiveProperty(prop, this))
+      .filter(f => !!f)
+      .forEach(f => this.addFailure(f));
+    super.visitDirectiveProperty(prop, context);
+  }
+
+
 }
 
+
 /* Expression visitors */
 
 class PipeWhitespaceVisitor extends RecursiveAngularExpressionVisitor implements ConfigurableVisitor {
@@ -123,8 +191,8 @@ class PipeWhitespaceVisitor extends RecursiveAngularExpressionVisitor implements
     if (replacements.length) {
       context.addFailure(
         context.createFailure(ast.exp.span.end - 1, 3,
-        'The pipe operator should be surrounded by one space on each side, i.e. " | ".',
-        replacements)
+          'The pipe operator should be surrounded by one space on each side, i.e. " | ".',
+          replacements)
       );
     }
     super.visitPipe(ast, context);
@@ -163,27 +231,30 @@ export class Rule extends Lint.Rules.AbstractRule {
     description: `Ensures the proper formatting of Angular expressions.`,
     rationale: `Having whitespace in the right places in an Angular expression makes the template more readable.`,
     optionsDescription: Lint.Utils.dedent`
-      One argument may be optionally provided:
-      * \`"check-interpolation"\` checks for whitespace before and after the interpolation characters`,
+      Arguments may be optionally provided:
+      * \`"check-interpolation"\` checks for whitespace before and after the interpolation characters
+      * \`"check-pipe"\` checks for whitespace before and after a pipe
+      * \`"check-semicolon"\` checks for whitespace after semicolon`,
+
     options: {
       type: 'array',
       items: {
         type: 'string',
-        enum: ['check-interpolation'],
+        enum: ['check-interpolation', 'check-pipe', 'check-semicolon'],
       },
       minLength: 0,
-      maxLength: 1,
+      maxLength: 3,
     },
     optionExamples: ['[true, "check-interpolation"]'],
     typescriptOnly: true,
   };
 
   public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
     return this.applyWithWalker(
-        new NgWalker(sourceFile,
-            this.getOptions(), {
-              templateVisitorCtrl: WhitespaceTemplateVisitor,
-              expressionVisitorCtrl: TemplateExpressionVisitor
-            }));
+      new NgWalker(sourceFile,
+        this.getOptions(), {
+          templateVisitorCtrl: WhitespaceTemplateVisitor,
+          expressionVisitorCtrl: TemplateExpressionVisitor,
+        }));
   }
 }