fix(rules) : fix after review

mgechev · Sep 6, 2017 · c71fac8 · c71fac8
1 parent dcfaf28
commit c71fac8
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 23 deletions.
diff --git a/src/angular/ngWalker.ts b/src/angular/ngWalker.ts
@@ -38,8 +38,6 @@ export interface NgWalkerConfig {
 
 export class NgWalker extends Lint.RuleWalker {
 
-  static prop: any;
-
   constructor(sourceFile: ts.SourceFile,
               protected _originalOptions: Lint.IOptions,
               private _config?: NgWalkerConfig,

diff --git a/src/angular/sourceMappingVisitor.ts b/src/angular/sourceMappingVisitor.ts
@@ -97,6 +97,8 @@ function computeLineStarts(text: string): number[] {
 }
 
 export class SourceMappingVisitor extends RuleWalker {
+
+  parentAST: any;
   private consumer: SourceMapConsumer;
 
   constructor(sourceFile: ts.SourceFile, options: IOptions, protected codeWithMap: CodeWithSourceMap, protected basePosition: number) {
@@ -127,9 +129,18 @@ export class SourceMappingVisitor extends RuleWalker {
         console.log(e);
       }
     }
+
+    if (this.parentAST && this.parentAST.templateName) {
+      pos = pos - this.parentAST.value.ast.span.start;
+    }
     return pos + this.basePosition;
   }
 
+  addParentAST(parentAST: any): any {
+    this.parentAST = parentAST;
+    return this;
+  }
+
   private getMappedInterval(start: number, length: number) {
     let end = start + length;
     start = this.getSourcePosition(start);

diff --git a/src/angular/templates/basicTemplateAstVisitor.ts b/src/angular/templates/basicTemplateAstVisitor.ts
@@ -174,7 +174,7 @@ export class BasicTemplateAstVisitor extends SourceMappingVisitor implements ast
     const templateVisitor =
       new this.expressionVisitorCtrl(this.getSourceFile(), this._originalOptions, this.context, templateStart);
     templateVisitor.preDefinedVariables = this._variables;
-    NgWalker.prop = prop;
+    templateVisitor.parentAST = prop;
     templateVisitor.visit(ast);
     templateVisitor.getFailures().forEach(f => this.addFailure(f));
   }

diff --git a/src/angularWhitespaceRule.ts b/src/angularWhitespaceRule.ts
@@ -154,18 +154,12 @@ class PipeWhitespaceVisitor extends RecursiveAngularExpressionVisitor implements
   visitPipe(ast: ast.BindingPipe, context: RecursiveAngularExpressionVisitor): any {
 
     let exprStart, exprEnd, exprText, sf;
-    if (NgWalker.prop && ( NgWalker.prop.templateName === 'ngForOf' || NgWalker.prop.templateName === 'ngIf')) {
-      exprStart = context.getSourcePosition(ast.exp.span.start) - (ast.span.start); // t pony of ponies |
-      exprEnd = context.getSourcePosition(ast.exp.span.end) - (ast.span.start); // - (ast.span.end - ast.span.start);
-      sf = context.getSourceFile().getFullText();
-      exprText = sf.substring(exprStart, exprEnd);
-      NgWalker.prop = null;
-    } else {
-       exprStart = context.getSourcePosition(ast.exp.span.start);
-       exprEnd = context.getSourcePosition(ast.exp.span.end);
-       sf = context.getSourceFile().getFullText();
-       exprText = sf.substring(exprStart, exprEnd);
-    }
+
+    exprStart = context.getSourcePosition(ast.exp.span.start);
+    exprEnd = context.getSourcePosition(ast.exp.span.end);
+    sf = context.getSourceFile().getFullText();
+    exprText = sf.substring(exprStart, exprEnd);
+
 
     const replacements = [];
     // Handling the right side of the pipe
@@ -224,6 +218,7 @@ class TemplateExpressionVisitor extends RecursiveAngularExpressionVisitor {
   visitPipe(expr: ast.BindingPipe, context: any): any {
     const options = this.getOptions();
     this.visitors
+      .map(v => v.addParentAST(this.parentAST))
       .filter(v => options.indexOf(v.getOption()) >= 0)
       .map(v => v.visitPipe(expr, this))
       .filter(f => !!f)

diff --git a/test/angularWhitespaceRule.spec.ts b/test/angularWhitespaceRule.spec.ts
@@ -785,8 +785,8 @@ describe('pipes', () => {
       @Component({
         selector: 'foo',
         template: \`
-           <div *ngFor="let pony of ponies |slice:0:4">dummy
-                                                      ~~~
+           <div *ngFor="let pony of ponies |slice:0:4">
+                                          ~~~
              <h2>{{ pony.name }}</h2>
            </div>
         \`
@@ -807,8 +807,8 @@ describe('pipes', () => {
       @Component({
         selector: 'foo',
         template: \`
-           <div *ngFor="let pony of ponies | slice:0:4">dummy
-                                                      ~~~
+           <div *ngFor="let pony of ponies | slice:0:4">
+                                          ~~~
              <h2>{{ pony.name }}</h2>
            </div>
         \`

diff --git a/test/templatesUsePublicRule.spec.ts b/test/templatesUsePublicRule.spec.ts
@@ -215,7 +215,7 @@ describe('templates-use-public', () => {
           selector: 'foobar',
           template: \`
             <div *ngFor="let smile of smile2"><smile-cmp [smile]="smile"></smile-cmp></div>
-                                                   ~~~~~~
+                                      ~~~~~~
 
           \`
         })
@@ -230,13 +230,13 @@ describe('templates-use-public', () => {
     });
 
     // Angular doesn't provide the correct source span of the property.
-    it('should fail when private property used in *ngFor', () => {
+    it('should fail when private property used in *ngFor 2', () => {
       let source = `
         @Component({
           selector: 'foobar',
           template: \`
-            <div *ngFor="let smile of smile"><smile-cmp [smile]="smile"></smile-cmp></div>dummy
-                                                                                       ~~~~~
+            <div *ngFor="let smile of smile"><smile-cmp [smile]="smile"></smile-cmp></div>
+                                                                          ~~~~~
           \`
         })
         class Test {