From 6aaca21c27f0e220e2ec59b7ea46d20e1a70a597 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Fri, 28 Jun 2019 15:59:21 -0700 Subject: [PATCH] fix(compiler): give ASTWithSource its own visit method (#31347) ASTWithSource contains more information that AST and should have its own visit method, if desired. This implements that. PR Close #31347 --- .../src/ngtsc/indexer/test/template_spec.ts | 175 +++++++++--------- .../compiler/src/expression_parser/ast.ts | 8 +- 2 files changed, 96 insertions(+), 87 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts index 15f6b521cd..6e016116cd 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts @@ -7,6 +7,7 @@ */ import {AbsoluteSourceSpan, IdentifierKind} from '..'; +import {runInEachFileSystem} from '../../file_system/testing'; import {getTemplateIdentifiers} from '../src/template'; import * as util from './util'; @@ -17,112 +18,114 @@ function bind(template: string) { }); } -describe('getTemplateIdentifiers', () => { - it('should generate nothing in HTML-only template', () => { - const refs = getTemplateIdentifiers(bind('
')); +runInEachFileSystem(() => { + describe('getTemplateIdentifiers', () => { + it('should generate nothing in HTML-only template', () => { + const refs = getTemplateIdentifiers(bind('
')); - expect(refs.size).toBe(0); - }); - - it('should ignore comments', () => { - const refs = getTemplateIdentifiers(bind('')); - - expect(refs.size).toBe(0); - }); - - it('should handle arbitrary whitespace', () => { - const template = '
\n\n {{foo}}
'; - const refs = getTemplateIdentifiers(bind(template)); - - const [ref] = Array.from(refs); - expect(ref).toEqual({ - name: 'foo', - kind: IdentifierKind.Property, - span: new AbsoluteSourceSpan(12, 15), + expect(refs.size).toBe(0); }); - }); - it('should ignore identifiers defined in the template', () => { - const template = ` + it('should ignore comments', () => { + const refs = getTemplateIdentifiers(bind('')); + + expect(refs.size).toBe(0); + }); + + it('should handle arbitrary whitespace', () => { + const template = '
\n\n {{foo}}
'; + const refs = getTemplateIdentifiers(bind(template)); + + const [ref] = Array.from(refs); + expect(ref).toEqual({ + name: 'foo', + kind: IdentifierKind.Property, + span: new AbsoluteSourceSpan(12, 15), + }); + }); + + it('should ignore identifiers defined in the template', () => { + const template = ` {{model.valid}} `; - const refs = getTemplateIdentifiers(bind(template)); - - const refArr = Array.from(refs); - const modelId = refArr.find(ref => ref.name === 'model'); - expect(modelId).toBeUndefined(); - }); - - describe('generates identifiers for PropertyReads', () => { - it('should discover component properties', () => { - const template = '{{foo}}'; - const refs = getTemplateIdentifiers(bind(template)); - expect(refs.size).toBe(1); - - const [ref] = Array.from(refs); - expect(ref).toEqual({ - name: 'foo', - kind: IdentifierKind.Property, - span: new AbsoluteSourceSpan(2, 5), - }); - }); - - it('should discover nested properties', () => { - const template = '
{{foo}}
'; const refs = getTemplateIdentifiers(bind(template)); const refArr = Array.from(refs); - expect(refArr).toEqual(jasmine.arrayContaining([{ - name: 'foo', - kind: IdentifierKind.Property, - span: new AbsoluteSourceSpan(13, 16), - }])); + const modelId = refArr.find(ref => ref.name === 'model'); + expect(modelId).toBeUndefined(); }); - it('should ignore identifiers that are not implicitly received by the template', () => { - const template = '{{foo.bar.baz}}'; - const refs = getTemplateIdentifiers(bind(template)); - expect(refs.size).toBe(1); + describe('generates identifiers for PropertyReads', () => { + it('should discover component properties', () => { + const template = '{{foo}}'; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); - const [ref] = Array.from(refs); - expect(ref.name).toBe('foo'); - }); - }); + const [ref] = Array.from(refs); + expect(ref).toEqual({ + name: 'foo', + kind: IdentifierKind.Property, + span: new AbsoluteSourceSpan(2, 5), + }); + }); - describe('generates identifiers for MethodCalls', () => { - it('should discover component method calls', () => { - const template = '{{foo()}}'; - const refs = getTemplateIdentifiers(bind(template)); - expect(refs.size).toBe(1); + it('should discover nested properties', () => { + const template = '
{{foo}}
'; + const refs = getTemplateIdentifiers(bind(template)); - const [ref] = Array.from(refs); - expect(ref).toEqual({ - name: 'foo', - kind: IdentifierKind.Method, - span: new AbsoluteSourceSpan(2, 5), + const refArr = Array.from(refs); + expect(refArr).toEqual(jasmine.arrayContaining([{ + name: 'foo', + kind: IdentifierKind.Property, + span: new AbsoluteSourceSpan(13, 16), + }])); + }); + + it('should ignore identifiers that are not implicitly received by the template', () => { + const template = '{{foo.bar.baz}}'; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref.name).toBe('foo'); }); }); - it('should discover nested properties', () => { - const template = '
{{foo()}}
'; - const refs = getTemplateIdentifiers(bind(template)); + describe('generates identifiers for MethodCalls', () => { + it('should discover component method calls', () => { + const template = '{{foo()}}'; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); - const refArr = Array.from(refs); - expect(refArr).toEqual(jasmine.arrayContaining([{ - name: 'foo', - kind: IdentifierKind.Method, - span: new AbsoluteSourceSpan(13, 16), - }])); - }); + const [ref] = Array.from(refs); + expect(ref).toEqual({ + name: 'foo', + kind: IdentifierKind.Method, + span: new AbsoluteSourceSpan(2, 5), + }); + }); - it('should ignore identifiers that are not implicitly received by the template', () => { - const template = '{{foo().bar().baz()}}'; - const refs = getTemplateIdentifiers(bind(template)); - expect(refs.size).toBe(1); + it('should discover nested properties', () => { + const template = '
{{foo()}}
'; + const refs = getTemplateIdentifiers(bind(template)); - const [ref] = Array.from(refs); - expect(ref.name).toBe('foo'); + const refArr = Array.from(refs); + expect(refArr).toEqual(jasmine.arrayContaining([{ + name: 'foo', + kind: IdentifierKind.Method, + span: new AbsoluteSourceSpan(13, 16), + }])); + }); + + it('should ignore identifiers that are not implicitly received by the template', () => { + const template = '{{foo().bar().baz()}}'; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref.name).toBe('foo'); + }); }); }); }); diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 89b8d4669e..7756fde607 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -209,7 +209,12 @@ export class ASTWithSource extends AST { public errors: ParserError[]) { super(new ParseSpan(0, source == null ? 0 : source.length)); } - visit(visitor: AstVisitor, context: any = null): any { return this.ast.visit(visitor, context); } + visit(visitor: AstVisitor, context: any = null): any { + if (visitor.visitASTWithSource) { + return visitor.visitASTWithSource(this, context); + } + return this.ast.visit(visitor, context); + } toString(): string { return `${this.source} in ${this.location}`; } } @@ -240,6 +245,7 @@ export interface AstVisitor { visitQuote(ast: Quote, context: any): any; visitSafeMethodCall(ast: SafeMethodCall, context: any): any; visitSafePropertyRead(ast: SafePropertyRead, context: any): any; + visitASTWithSource?(ast: ASTWithSource, context: any): any; visit?(ast: AST, context?: any): any; }