Merge pull request #321 from mgechev/whitespace

feat: whitespace in interpolation

src/angular/ngWalker.ts

@@ -18,9 +18,7 @@ import {ngWalkerFactoryUtils} from './ngWalkerFactoryUtils';
 import {Config} from './config';
 
 import {logger} from '../util/logger';
-import {getDecoratorName, isSimpleTemplateString, getDecoratorPropertyInitializer} from '../util/utils';
-
-import SyntaxKind = require('../util/syntaxKind');
+import {getDecoratorName} from '../util/utils';
 
 const getDecoratorStringArgs = (decorator: ts.Decorator) => {
   let baseExpr = <any>decorator.expression || {};

src/angular/ngWalkerFactoryUtils.ts

@@ -1,11 +1,8 @@
-import * as ts from 'typescript';
-import * as Lint from 'tslint';
-import {NgWalker, NgWalkerConfig} from './ngWalker';
+import {NgWalkerConfig} from './ngWalker';
 import {MetadataReader} from './metadataReader';
 import {UrlResolver} from './urlResolvers/urlResolver';
 import {FsFileResolver} from './fileResolver/fsFileResolver';
-import {BasicCssAstVisitor, CssAstVisitorCtrl} from './styles/basicCssAstVisitor';
-import {Config} from './config';
+import {BasicCssAstVisitor} from './styles/basicCssAstVisitor';
 
 import { RecursiveAngularExpressionVisitor } from './templates/recursiveAngularExpressionVisitor';
 import {BasicTemplateAstVisitor} from './templates/basicTemplateAstVisitor';

src/angular/sourceMappingVisitor.ts

@@ -1,6 +1,6 @@
 import * as ts from 'typescript';
 import {RuleWalker, RuleFailure, IOptions, Fix, Replacement} from 'tslint';
-import {ComponentMetadata, CodeWithSourceMap} from './metadata';
+import {CodeWithSourceMap} from './metadata';
 import {SourceMapConsumer} from 'source-map';
 
 const LineFeed = 0x0A;

src/angular/styles/cssParser.ts

@@ -443,7 +443,6 @@ export class CssParser {
 
   /** @internal */
   _parseKeyframeDefinition(delimiters: number): CssKeyframeDefinitionAst {
-    const start = this._getScannerIndex();
     var stepTokens: CssToken[] = [];
     delimiters |= LBRACE_DELIM_FLAG;
     while (!characterContainsDelimiter(this._scanner.peek, delimiters)) {

src/angular/templates/basicTemplateAstVisitor.ts

@@ -157,7 +157,6 @@ export class BasicTemplateAstVisitor extends SourceMappingVisitor implements ast
 
   visitDirectiveProperty(prop: ast.BoundDirectivePropertyAst, context: any): any {
     if (ExpTypes.ASTWithSource(prop.value)) {
-      const ast: any = (<e.ASTWithSource>prop.value).ast;
       this.visitNgTemplateAST(prop.value, this.templateStart + getExpressionDisplacement(prop));
     }
   }

src/angular/templates/templateParser.ts

@@ -1,4 +1,4 @@
-import { NO_ERRORS_SCHEMA, ViewEncapsulation, ChangeDetectionStrategy } from '@angular/core';
+import { NO_ERRORS_SCHEMA, ChangeDetectionStrategy } from '@angular/core';
 import * as compiler from '@angular/compiler';
 
 import { Config, DirectiveDeclaration } from '../config';

src/angularWhitespaceRule.ts

@@ -0,0 +1,190 @@
+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';
+
+const InterpolationOpen = Config.interpolation[0];
+const InterpolationClose = Config.interpolation[1];
+const InterpolationNoWhitespaceRe = new RegExp(`${InterpolationOpen}\\S(.*?)\\S${InterpolationClose}`);
+const InterpolationExtraWhitespaceRe =
+  new RegExp(`${InterpolationOpen}\\s\\s(.*?)\\s${InterpolationClose}|${InterpolationOpen}\\s(.*?)\\s\\s${InterpolationClose}`);
+
+const getReplacements = (text: ast.BoundTextAst, absolutePosition: number) => {
+  const expr: string = (text.value as any).source;
+  const internalStart = expr.indexOf(InterpolationOpen);
+  const internalEnd = expr.lastIndexOf(InterpolationClose);
+  const len = expr.trim().length - InterpolationOpen.length - InterpolationClose.length;
+  const trimmed = expr.substr(internalStart + InterpolationOpen.length, len).trim();
+  return [
+    new Lint.Replacement(absolutePosition,
+      internalEnd - internalStart + InterpolationClose.length,
+      `${InterpolationOpen} ${trimmed} ${InterpolationClose}`)
+  ];
+};
+
+type Option = 'check-interpolation' | 'check-pipe';
+
+interface ConfigurableVisitor {
+  getOption(): Option;
+}
+
+/* Inrerpolation visitors */
+
+class InterpolationWhitespaceVisitor extends BasicTemplateAstVisitor implements ConfigurableVisitor {
+  visitBoundText(text: ast.BoundTextAst, context: BasicTemplateAstVisitor): any {
+    if (ExpTypes.ASTWithSource(text.value)) {
+      // Note that will not be reliable for different interpolation symbols
+      let error = null;
+      const expr: any = (<any>text.value).source;
+      if (InterpolationNoWhitespaceRe.test(expr)) {
+        error = `Missing whitespace in interpolation; expecting ${InterpolationOpen} expr ${InterpolationClose}`;
+      }
+      if (InterpolationExtraWhitespaceRe.test(expr)) {
+        error = `Extra whitespace in interpolation; expecting ${InterpolationOpen} expr ${InterpolationClose}`;
+      }
+      if (error) {
+        const internalStart = expr.indexOf(InterpolationOpen);
+        const start = text.sourceSpan.start.offset + internalStart;
+        const absolutePosition = context.getSourcePosition(start);
+        return context.addFailure(
+          context.createFailure(start,
+            expr.trim().length,
+            error, getReplacements(text, absolutePosition)));
+      }
+    }
+    super.visitBoundText(text, context);
+    return null;
+  }
+
+  getOption(): Option {
+    return 'check-interpolation';
+  }
+}
+
+class WhitespaceTemplateVisitor extends BasicTemplateAstVisitor {
+  private visitors: (BasicTemplateAstVisitor & ConfigurableVisitor)[] = [
+    new InterpolationWhitespaceVisitor(this.getSourceFile(), this.getOptions(), this.context, this.templateStart)
+  ];
+
+  visitBoundText(text: ast.BoundTextAst, context: any): any {
+    const options = this.getOptions();
+    this.visitors
+      .filter(v => options.indexOf(v.getOption()) >= 0)
+      .map(v => v.visitBoundText(text, this))
+      .filter(f => !!f)
+      .forEach(f => this.addFailure(f));
+    super.visitBoundText(text, context);
+  }
+}
+
+/* Expression visitors */
+
+class PipeWhitespaceVisitor extends RecursiveAngularExpressionVisitor implements ConfigurableVisitor {
+  visitPipe(ast: ast.BindingPipe, context: RecursiveAngularExpressionVisitor): any {
+    const start = ast.span.start;
+    const exprStart = context.getSourcePosition(ast.exp.span.start);
+    const exprEnd = context.getSourcePosition(ast.exp.span.end);
+    const sf = context.getSourceFile().getFullText();
+    const exprText = sf.substring(exprStart, exprEnd);
+
+    const replacements = [];
+    // Handling the right side of the pipe
+    let leftBeginning = exprEnd + 1; // exprEnd === '|'
+    if (sf[leftBeginning] === ' ') {
+      let ignoreSpace = 1;
+      while (sf[leftBeginning + ignoreSpace] === ' ') {
+        ignoreSpace += 1;
+      }
+      if (ignoreSpace > 1) {
+        replacements.push(new Lint.Replacement(exprEnd + 1, ignoreSpace, ' '));
+      }
+    } else {
+      replacements.push(new Lint.Replacement(exprEnd + 1, 0, ' '));
+    }
+
+    // Handling the left side of the pipe
+    if (exprText[exprText.length - 1] === ' ') {
+      let ignoreSpace = 1;
+      while (exprText[exprText.length - 1 - ignoreSpace] === ' ') {
+        ignoreSpace += 1;
+      }
+      if (ignoreSpace > 1) {
+        replacements.push(new Lint.Replacement(exprEnd - ignoreSpace, ignoreSpace, ' '));
+      }
+    } else {
+      replacements.push(new Lint.Replacement(exprEnd, 0, ' '));
+    }
+
+    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)
+      );
+    }
+    super.visitPipe(ast, context);
+    return null;
+  }
+
+  protected isAsyncBinding(expr: any) {
+    return expr instanceof ast.BindingPipe && expr.name === 'async';
+  }
+
+  getOption(): Option {
+    return 'check-pipe';
+  }
+}
+
+
+class TemplateExpressionVisitor extends RecursiveAngularExpressionVisitor {
+  private visitors: (RecursiveAngularExpressionVisitor & ConfigurableVisitor)[] = [
+    new PipeWhitespaceVisitor(this.getSourceFile(), this.getOptions(), this.context, this.basePosition)
+  ];
+
+  visitPipe(expr: ast.BindingPipe, context: any): any {
+    const options = this.getOptions();
+    this.visitors
+      .filter(v => options.indexOf(v.getOption()) >= 0)
+      .map(v => v.visitPipe(expr, this))
+      .filter(f => !!f)
+      .forEach(f => this.addFailure(f));
+  }
+}
+
+export class Rule extends Lint.Rules.AbstractRule {
+  public static metadata: Lint.IRuleMetadata = {
+    ruleName: 'angular-whitespace',
+    type: 'style',
+    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`,
+    options: {
+      type: 'array',
+      items: {
+        type: 'string',
+        enum: ['check-interpolation'],
+      },
+      minLength: 0,
+      maxLength: 1,
+    },
+    optionExamples: ['[true, "check-interpolation"]'],
+    typescriptOnly: true,
+  };
+
+  static FAILURE: string = 'The %s "%s" that you\'re trying to access does not exist in the class declaration.';
+
+  public apply(sourceFile:ts.SourceFile): Lint.RuleFailure[] {
+    return this.applyWithWalker(
+        new NgWalker(sourceFile,
+            this.getOptions(), {
+              templateVisitorCtrl: WhitespaceTemplateVisitor,
+              expressionVisitorCtrl: TemplateExpressionVisitor
+            }));
+  }
+}

src/index.ts

@@ -22,5 +22,6 @@ export { Rule as TemplateToNgTemplateRule } from './templateToNgTemplateRule';
 export { Rule as UsePipeDecoratorRule } from './usePipeDecoratorRule';
 export { Rule as UseViewEncapsulationRule } from './useViewEncapsulationRule';
 export { Rule as BananaInBoxRule } from './bananaInBoxRule';
+export { Rule as AngularWhitespaceRule } from './angularWhitespaceRule';
 export * from './angular/config';
 

src/pipeImpureRule.ts

@@ -26,7 +26,7 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 export class ClassMetadataWalker extends NgWalker {
 
-  constructor(sourceFile:ts.SourceFile, private rule:Rule) {
+  constructor(sourceFile: ts.SourceFile, private rule: Rule) {
     super(sourceFile, rule.getOptions());
   }
 

src/propertyDecoratorBase.ts

@@ -26,16 +26,13 @@ export class UsePropertyDecorator extends Lint.Rules.AbstractRule {
   }
 
   public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-    let documentRegistry = ts.createDocumentRegistry();
     return this.applyWithWalker(
       new DirectiveMetadataWalker(sourceFile,
         this.getOptions(),  this.config));
   }
 }
 
 class DirectiveMetadataWalker extends Lint.RuleWalker {
-  private languageService : ts.LanguageService;
-
   constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, private config: IUsePropertyDecoratorConfig) {
       super(sourceFile, options);
   }

src/templateToNgTemplateRule.ts

@@ -1,22 +1,14 @@
 import * as Lint from 'tslint';
 import * as ts from 'typescript';
-import {stringDistance} from './util/utils';
-import {getDeclaredProperties, getDeclaredMethods} from './util/classDeclarationUtils';
 import {NgWalker} from './angular/ngWalker';
-import {RecursiveAngularExpressionVisitor} from './angular/templates/recursiveAngularExpressionVisitor';
-import * as e from '@angular/compiler/src/expression_parser/ast';
-import { EmbeddedTemplateAst, ElementAst } from '@angular/compiler';
+import { EmbeddedTemplateAst } from '@angular/compiler';
 import { BasicTemplateAstVisitor } from './angular/templates/basicTemplateAstVisitor';
-import { Fix } from 'tslint';
-import SyntaxKind = require('./util/syntaxKind');
 
 const ErrorMessage = 'You should use <ng-template> instead of <template>';
 const TemplateStart = '<template';
 const TemplateEnd = '</template>';
 const TemplateEndRe = /<\s*\/\s*template\s*>/i;
 
-const set = new Set<EmbeddedTemplateAst>();
-
 class TemplateToNgTemplateVisitor extends BasicTemplateAstVisitor {
   private _prevClosing: number = 0;
   private _visitedElements = new Set<EmbeddedTemplateAst>();

src/useViewEncapsulationRule.ts

@@ -1,5 +1,4 @@
 import { getComponentDecorator, getDecoratorPropertyInitializer } from './util/utils';
-import { ComponentMetadata } from './angular/metadata';
 import * as Lint from 'tslint';
 import * as ts from 'typescript';
 

test/angular/metadataReader.spec.ts

@@ -4,7 +4,7 @@ import chai = require('chai');
 import {DummyFileResolver} from '../../src/angular/fileResolver/dummyFileResolver';
 import {FsFileResolver} from '../../src/angular/fileResolver/fsFileResolver';
 import {MetadataReader} from '../../src/angular/metadataReader';
-import {ComponentMetadata, DirectiveMetadata} from '../../src/angular/metadata';
+import {ComponentMetadata} from '../../src/angular/metadata';
 import {Config} from '../../src/angular/config';
 
 import {join, normalize} from 'path';

test/angular/sourceMappingVisitor.spec.ts

@@ -3,7 +3,6 @@ import chai = require('chai');
 
 import {getDecoratorPropertyInitializer} from '../../src/util/utils';
 import {SourceMappingVisitor} from '../../src/angular/sourceMappingVisitor';
-import {join, normalize} from 'path';
 import {renderSync} from 'node-sass';
 
 const getAst = (code: string, file = 'file.ts') => {