Merge pull request #362 from mgechev/fix-345

fix(rules): support for multiple interpolations in boundtext
mgechev · Jul 2, 2017 · 0be8563 · 0be8563
2 parents ecfde3d + 4ac0334
commit 0be8563
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 33 deletions.
diff --git a/src/angularWhitespaceRule.ts b/src/angularWhitespaceRule.ts
@@ -10,27 +10,13 @@ import {RecursiveAngularExpressionVisitor} from './angular/templates/recursiveAn
 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}`);
+  `\\s(.*?)\\S${InterpolationClose}|${InterpolationOpen}\\S(.*?)\\s${InterpolationClose}`, 'g');
 const InterpolationExtraWhitespaceRe =
-  new RegExp(`${InterpolationOpen}\\s\\s(.*?)\\s${InterpolationClose}|${InterpolationOpen}\\s(.*?)\\s\\s${InterpolationClose}`);
+  new RegExp(`${InterpolationOpen}\\s\\s(.*?)\\s${InterpolationClose}|${InterpolationOpen}\\s(.*?)\\s\\s${InterpolationClose}`, 'g');
 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;
-  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}`)
-  ];
-};
-
-
 const getSemicolonReplacements = (text: ast.BoundDirectivePropertyAst, absolutePosition: number) => {
 
   return [
@@ -45,29 +31,41 @@ interface ConfigurableVisitor {
   getOption(): Option;
 }
 
-/* Inrerpolation visitors */
+/* Interpolation 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)));
-      }
+      const applyRegex = (regex: RegExp, failure: string) => {
+        let match: RegExpExecArray | null;
+        while (match = regex.exec(expr)) {
+          const start = text.sourceSpan.start.offset + match.index;
+          const absolutePosition = context.getSourcePosition(start);
+          const length = match[0].length;
+          context.addFailure(
+            context.createFailure(
+            start, length, failure, [
+            new Lint.Replacement(
+              absolutePosition,
+              length,
+              `${InterpolationOpen} ${match[0].replace(InterpolationOpen, '').replace(InterpolationClose, '').trim()} ${InterpolationClose}`
+            )
+          ]));
+        }
+      };
+      InterpolationNoWhitespaceRe.lastIndex = 0;
+      applyRegex(
+        InterpolationNoWhitespaceRe,
+        `Missing whitespace in interpolation; expecting ${InterpolationOpen} expr ${InterpolationClose}`
+      );
+      InterpolationExtraWhitespaceRe.lastIndex = 0;
+      applyRegex(
+        InterpolationExtraWhitespaceRe,
+        `Extra whitespace in interpolation; expecting ${InterpolationOpen} expr ${InterpolationClose}`
+      );
     }
     super.visitBoundText(text, context);
     return null;

diff --git a/test/angularWhitespaceRule.spec.ts b/test/angularWhitespaceRule.spec.ts
@@ -1,4 +1,4 @@
-import {assertSuccess, assertAnnotated, assertMultipleAnnotated} from './testHelper';
+import { assertSuccess, assertAnnotated, assertMultipleAnnotated } from './testHelper';
 import {Replacement} from 'tslint';
 import {expect} from 'chai';
 import {FsFileResolver} from '../src/angular/fileResolver/fsFileResolver';
@@ -258,6 +258,81 @@ describe('failure', () => {
           class Bar {}`);
       });
 
+      it('should fail and apply proper replacements when style is incorrect', () => {
+        let source = `
+          @Component({
+            template: \`
+              <div>
+              some additional text
+                {{foo}}
+                ~~~~~~~
+              </div>
+            \`
+          })
+          class Bar {}`;
+        const failures = assertAnnotated({
+          ruleName: 'angular-whitespace',
+          message: 'Missing whitespace in interpolation; expecting {{ expr }}',
+          source,
+          options: ['check-interpolation']
+        });
+
+        const res = Replacement.applyAll(source, failures[0].getFix());
+        expect(res).to.eq(`
+          @Component({
+            template: \`
+              <div>
+              some additional text
+                {{ foo }}
+                ~~~~~~~
+              </div>
+            \`
+          })
+          class Bar {}`);
+      });
+
+      it('should fail and apply proper replacements when style is incorrect with multiple failures', () => {
+        let source = `
+          @Component({
+            template: \`
+              <div>
+              some additional text
+                {{foo}}
+                ~~~~~~~
+                some other text
+                {{  bar  }}
+                ^^^^^^^^^^^
+              </div>
+            \`
+          })
+          class Bar {}`;
+        const failures = assertMultipleAnnotated({
+          ruleName: 'angular-whitespace',
+          failures: [
+            {char: '~', msg: 'Missing whitespace in interpolation; expecting {{ expr }}', },
+            {char: '^', msg: 'Extra whitespace in interpolation; expecting {{ expr }}', },
+          ],
+          source,
+          options: ['check-interpolation']
+        });
+
+        const res = Replacement.applyAll(source, [].concat.apply([], failures.map(f => f.getFix())));
+        expect(res).to.eq(`
+          @Component({
+            template: \`
+              <div>
+              some additional text
+                {{ foo }}
+                ~~~~~~~
+                some other text
+                {{ bar }}
+                ^^^^^^^^^^^
+              </div>
+            \`
+          })
+          class Bar {}`);
+      });
+
       it('should fail and apply proper replacements when style is incorrect', () => {
         let source = `
           @Component({