perf: use flat symbol table

mgechev · Apr 22, 2017 · b1b36a3 · b1b36a3
1 parent a7500cb
commit b1b36a3
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 52 deletions.
diff --git a/src/angular/templates/basicTemplateAstVisitor.ts b/src/angular/templates/basicTemplateAstVisitor.ts
@@ -74,7 +74,7 @@ export interface TemplateAstVisitorCtr {
 }
 
 export class BasicTemplateAstVisitor extends SourceMappingVisitor implements ast.TemplateAstVisitor {
-  private _variables = [];
+  private _variables = {};
 
   constructor(sourceFile: ts.SourceFile,
     private _originalOptions: Lint.IOptions,
@@ -119,16 +119,14 @@ export class BasicTemplateAstVisitor extends SourceMappingVisitor implements ast
   visitReference(ast: ast.ReferenceAst, context: any): any {}
 
   visitVariable(ast: ast.VariableAst, context: any): any {
-    this._variables.push(ast.name);
+    this._variables[ast.name] = true;
   }
 
   visitEvent(ast: ast.BoundEventAst, context: any): any {
-    if (this._variables.indexOf('$event') < 0) {
-      this._variables.push('$event');
-    }
+    this._variables['$event'] = true;
     this.visitNgTemplateAST(ast.handler,
         this.templateStart + getExpressionDisplacement(ast));
-    this._variables.splice(this._variables.indexOf('$event'), 1);
+    this._variables['$event'] = false;
   }
 
   visitElementProperty(prop: ast.BoundElementPropertyAst, context: any): any {

diff --git a/src/angular/templates/recursiveAngularExpressionVisitor.ts b/src/angular/templates/recursiveAngularExpressionVisitor.ts
@@ -5,8 +5,12 @@ import * as e from '@angular/compiler/src/expression_parser/ast';
 import {SourceMappingVisitor} from '../sourceMappingVisitor';
 import {ComponentMetadata} from '../metadata';
 
+export interface FlatSymbolTable {
+  [identifier: string]: boolean;
+}
+
 export class RecursiveAngularExpressionVisitor extends SourceMappingVisitor implements e.AstVisitor {
-  public preDefinedVariables = [];
+  public preDefinedVariables: FlatSymbolTable = {};
 
   constructor(sourceFile: ts.SourceFile, options: Lint.IOptions,
       protected context: ComponentMetadata, protected basePosition: number) {

diff --git a/src/angular/templates/referenceCollectorVisitor.ts b/src/angular/templates/referenceCollectorVisitor.ts
@@ -1,9 +1,10 @@
 import * as ast from '@angular/compiler';
+import { FlatSymbolTable } from './recursiveAngularExpressionVisitor';
 
 export class ReferenceCollectorVisitor implements ast.TemplateAstVisitor {
-  private _variables = [];
+  private _variables: FlatSymbolTable = {};
 
-  visit?(node: ast.TemplateAst, context: any): string[] {
+  visit?(node: ast.TemplateAst, context: any): FlatSymbolTable {
     node.visit(this, context);
     return this._variables;
   }
@@ -29,15 +30,13 @@ export class ReferenceCollectorVisitor implements ast.TemplateAstVisitor {
   visitEvent(ast: ast.BoundEventAst, context: any): any {}
 
   visitEmbeddedTemplate(ast: ast.EmbeddedTemplateAst, context: any): any {
-    const references = ast.references.map(r => r.name);
+    ast.references.forEach(r => this._variables[r.name] = true);
     ast.children.forEach(e => this.visit(e, context));
-    this._variables = this._variables.concat(references);
   }
 
   visitElement(element: ast.ElementAst, context: any): any {
-    const references = element.references.map(r => r.name);
+    element.references.forEach(r => this._variables[r.name] = true);
     element.children.forEach(e => this.visit(e, context));
-    this._variables = this._variables.concat(references);
   }
 
   get variables() {

diff --git a/src/noAccessMissingMemberRule.ts b/src/noAccessMissingMemberRule.ts
@@ -3,7 +3,7 @@ import * as ts from 'typescript';
 import {sprintf} from 'sprintf-js';
 import {stringDistance} from './util/utils';
 import {NgWalker} from './angular/ngWalker';
-import {RecursiveAngularExpressionVisitor} from './angular/templates/recursiveAngularExpressionVisitor';
+import { RecursiveAngularExpressionVisitor, FlatSymbolTable } from './angular/templates/recursiveAngularExpressionVisitor';
 import {ExpTypes} from './angular/expressionTypes';
 import {getDeclaredMethodNames, getDeclaredPropertyNames} from './util/classDeclarationUtils';
 import * as e from '@angular/compiler/src/expression_parser/ast';
@@ -37,16 +37,18 @@ class SymbolAccessValidator extends RecursiveAngularExpressionVisitor {
 
   private doCheck(ast: ASTField, type: DeclarationType, context: any): any {
     let symbolType: string;
-    let available: string[];
+    let available: FlatSymbolTable;
     if (type === DeclarationType.Method) {
       symbolType = 'method';
     } else {
       symbolType = 'property';
     }
 
-    available = getDeclaredMethodNames(this.context.controller)
-      .concat(getDeclaredPropertyNames(this.context.controller))
-      .concat(this.preDefinedVariables);
+    available = Object.assign({},
+      getDeclaredMethodNames(this.context.controller),
+      getDeclaredPropertyNames(this.context.controller),
+      this.preDefinedVariables
+    );
 
     // Do not support nested properties yet
     let tmp = ast;
@@ -71,24 +73,22 @@ class SymbolAccessValidator extends RecursiveAngularExpressionVisitor {
       }
     }
 
-    if (available.indexOf(ast.name) < 0) {
+    if (ast.name && !available[ast.name]) {
       let failureString = sprintf.apply(this, [Rule.FAILURE, symbolType, ast.name]);
-      if (ast.name) {
-        const top = this.getTopSuggestion(available, ast.name);
-        const getSuggestion = (list: string[]) => {
-          if (list.length === 1) {
-            return `"${list[0]}"`;
-          }
-          let result = `"${list.shift()}"`;
-          while (list.length > 1) {
-            result += `, "${list.shift()}"`;
-          }
-          result += ` or "${list.shift()}"`;
-          return result;
-        };
-        if (top.length && top[0].distance <= 2) {
-          failureString += ` Probably you mean: ${getSuggestion(top.map(s => s.element))}.`;
+      const top = this.getTopSuggestion(Object.keys(available), ast.name);
+      const getSuggestion = (list: string[]) => {
+        if (list.length === 1) {
+          return `"${list[0]}"`;
+        }
+        let result = `"${list.shift()}"`;
+        while (list.length > 1) {
+          result += `, "${list.shift()}"`;
         }
+        result += ` or "${list.shift()}"`;
+        return result;
+      };
+      if (top.length && top[0].distance <= 2) {
+        failureString += ` Probably you mean: ${getSuggestion(top.map(s => s.element))}.`;
       }
       const width = ast.name.length;
       this.addFailure(this.createFailure(ast.span.start, width, failureString));

diff --git a/src/util/classDeclarationUtils.ts b/src/util/classDeclarationUtils.ts
@@ -1,5 +1,6 @@
 import * as ts from 'typescript';
 import { current } from './syntaxKind';
+import { FlatSymbolTable } from '../angular/templates/recursiveAngularExpressionVisitor';
 
 const SyntaxKind = current();
 
@@ -16,7 +17,12 @@ export const getDeclaredProperties = (declaration: ts.ClassDeclaration) => {
 };
 
 export const getDeclaredPropertyNames = (declaration: ts.ClassDeclaration) => {
-  return getDeclaredProperties(declaration).filter((p: any) => p && p.name).map((p: any) => p.name.text);
+  return getDeclaredProperties(declaration)
+    .filter((p: any) => p && p.name)
+    .reduce((accum: FlatSymbolTable, p: any) => {
+      accum[p.name.text] = true;
+      return accum;
+    }, {});
 };
 
 export const getDeclaredMethods = (declaration: ts.ClassDeclaration) => {
@@ -25,6 +31,10 @@ export const getDeclaredMethods = (declaration: ts.ClassDeclaration) => {
 
 export const getDeclaredMethodNames = (declaration: ts.ClassDeclaration) => {
   return getDeclaredMethods(declaration)
-    .map((d: any) => (<ts.Identifier>d.name).text);
+    .map((d: any) => (<ts.Identifier>d.name).text)
+    .reduce((accum: FlatSymbolTable, m: string) => {
+      accum[m] = true;
+      return accum;
+    }, {});
 };
 
diff --git a/test/angular/referenceCollectorVisitor.spec.ts b/test/angular/referenceCollectorVisitor.spec.ts
@@ -8,40 +8,40 @@ describe('ReferenceCollectorVisitor', () => {
     const template = parseTemplate('');
     const rv = new ReferenceCollectorVisitor();
     templateVisitAll(rv, template, null);
-    expect(rv.variables.length).eq(0);
+    expect(Object.keys(rv.variables).length).eq(0);
   });
 
   it('should work with simple templates', () => {
     const template = parseTemplate('<div></div>');
     const rv = new ReferenceCollectorVisitor();
     templateVisitAll(rv, template, null);
-    expect(rv.variables.length).eq(0);
+    expect(Object.keys(rv.variables).length).eq(0);
   });
 
   it('should work with templates with one reference', () => {
     const template = parseTemplate('<div #foo></div>');
     const rv = new ReferenceCollectorVisitor();
     templateVisitAll(rv, template, null);
-    expect(rv.variables.length).eq(1);
-    expect(rv.variables[0]).eq('foo');
+    expect(Object.keys(rv.variables).length).eq(1);
+    expect(rv.variables['foo']).to.eq(true);
   });
 
   it('should work with templates with nested elements with references', () => {
     const template = parseTemplate('<div #foo><span #bar></span></div>');
     const rv = new ReferenceCollectorVisitor();
     templateVisitAll(rv, template, null);
-    expect(rv.variables.length).eq(2);
-    expect(rv.variables[0]).eq('bar');
-    expect(rv.variables[1]).eq('foo');
+    expect(Object.keys(rv.variables).length).eq(2);
+    expect(rv.variables['bar']).to.eq(true);
+    expect(rv.variables['foo']).to.eq(true);
   });
 
   it('should work with templates with multiple elements with different references', () => {
     const template = parseTemplate('<div #foo><span #bar></span></div><span #qux></span>');
     const rv = new ReferenceCollectorVisitor();
     templateVisitAll(rv, template, null);
-    expect(rv.variables.length).eq(3);
-    expect(rv.variables[0]).eq('bar');
-    expect(rv.variables[1]).eq('foo');
-    expect(rv.variables[2]).eq('qux');
+    expect(Object.keys(rv.variables).length).eq(3);
+    expect(rv.variables['foo']).eq(true);
+    expect(rv.variables['bar']).eq(true);
+    expect(rv.variables['qux']).eq(true);
   });
 });
diff --git a/test/noAccessMissingMemberRule.spec.ts b/test/noAccessMissingMemberRule.spec.ts
@@ -91,7 +91,7 @@ describe('no-access-missing-member', () => {
         @Component({
           selector: 'foobar',
           template: '<div> test {{ bar() }} {{ bar() + baz() }}</div>
-                                                       ~~~                                 
+                                                       ~~~
         })
         class Test {
           bar() {}

diff --git a/test/util/classDeclarationUtils.spec.ts b/test/util/classDeclarationUtils.spec.ts
@@ -3,6 +3,7 @@ import * as tslint from 'tslint';
 
 import {NgWalker} from '../../src/angular/ngWalker';
 import {getDeclaredMethodNames, getDeclaredPropertyNames} from '../../src/util/classDeclarationUtils';
+import { FlatSymbolTable } from '../../src/angular/templates/recursiveAngularExpressionVisitor';
 import chai = require('chai');
 
 describe('ngWalker', () => {
@@ -20,8 +21,8 @@ describe('ngWalker', () => {
       disabledIntervals: null,
       ruleSeverity: 'warning'
     };
-    let properties: string[] = [];
-    let methods: string[] = [];
+    let properties: FlatSymbolTable = {};
+    let methods: FlatSymbolTable = {};
 
     class ClassUtilWalker extends NgWalker {
       visitClassDeclaration(node: ts.ClassDeclaration) {
@@ -33,7 +34,7 @@ describe('ngWalker', () => {
     let sf = ts.createSourceFile('foo', source, null);
     let walker = new ClassUtilWalker(sf, ruleArgs);
     walker.walk(sf);
-    (<any>chai.expect(methods.join())).to.equal(['bar', 'baz'].join());
-    (<any>chai.expect(properties.join())).to.equal(['foo'].join());
+    chai.expect(methods).to.deep.eq({ bar: true, baz: true });
+    chai.expect(properties).to.deep.eq({ foo: true });
   });
 });