| 
									
										
											  
											
												refactor(compiler): Remove NullAstVisitor and visitAstChildren (#35619)
This commit removes the `NullAstVisitor` and `visitAstChildren` exported
from `packages/compiler/src/expression_parser/ast.ts` because they
contain duplicate and buggy implementation, and their use cases could be
sufficiently covered by `RecursiveAstVisitor` if the latter implements the
`visit` method. This use case is only needed in the language service.
With this change, any visitor that extends `RecursiveAstVisitor` could
just define their own `visit` function and the parent class will behave
correctly.
A bit of historical context:
In language service, we need a way to tranverse the expression AST in a
selective manner based on where the user's cursor is. This means we need a
"filtering" function to decide which node to visit and which node to not
visit. Instead of refactoring `RecursiveAstVisitor` to support this,
`visitAstChildren` was created. `visitAstChildren` duplicates the
implementation of `RecursiveAstVisitor`, but introduced some bugs along
the way. For example, in `visitKeyedWrite`, it visits
```
obj -> key -> obj
```
instead of
```
obj -> key -> value
```
Moreover, because of the following line
```
visitor.visit && visitor.visit(ast, context) || ast.visit(visitor, context);
```
`visitAstChildren` visits every node *twice*.
PR Close #35619
											
										 
											2020-02-21 10:20:52 -08:00
										 |  |  | /** | 
					
						
							|  |  |  |  * @license | 
					
						
							| 
									
										
										
										
											2020-05-19 12:08:49 -07:00
										 |  |  |  * Copyright Google LLC All Rights Reserved. | 
					
						
							| 
									
										
											  
											
												refactor(compiler): Remove NullAstVisitor and visitAstChildren (#35619)
This commit removes the `NullAstVisitor` and `visitAstChildren` exported
from `packages/compiler/src/expression_parser/ast.ts` because they
contain duplicate and buggy implementation, and their use cases could be
sufficiently covered by `RecursiveAstVisitor` if the latter implements the
`visit` method. This use case is only needed in the language service.
With this change, any visitor that extends `RecursiveAstVisitor` could
just define their own `visit` function and the parent class will behave
correctly.
A bit of historical context:
In language service, we need a way to tranverse the expression AST in a
selective manner based on where the user's cursor is. This means we need a
"filtering" function to decide which node to visit and which node to not
visit. Instead of refactoring `RecursiveAstVisitor` to support this,
`visitAstChildren` was created. `visitAstChildren` duplicates the
implementation of `RecursiveAstVisitor`, but introduced some bugs along
the way. For example, in `visitKeyedWrite`, it visits
```
obj -> key -> obj
```
instead of
```
obj -> key -> value
```
Moreover, because of the following line
```
visitor.visit && visitor.visit(ast, context) || ast.visit(visitor, context);
```
`visitAstChildren` visits every node *twice*.
PR Close #35619
											
										 
											2020-02-21 10:20:52 -08:00
										 |  |  |  * | 
					
						
							|  |  |  |  * Use of this source code is governed by an MIT-style license that can be | 
					
						
							|  |  |  |  * found in the LICENSE file at https://angular.io/license
 | 
					
						
							|  |  |  |  */ | 
					
						
							|  |  |  | 
 | 
					
						
							|  |  |  | import {AST, Lexer, Parser, RecursiveAstVisitor} from '@angular/compiler'; | 
					
						
							|  |  |  | import {ImplicitReceiver, MethodCall, PropertyRead} from '@angular/compiler/src/compiler'; | 
					
						
							|  |  |  | 
 | 
					
						
							|  |  |  | describe('RecursiveAstVisitor', () => { | 
					
						
							|  |  |  |   it('should visit every node', () => { | 
					
						
							|  |  |  |     const parser = new Parser(new Lexer()); | 
					
						
							| 
									
										
										
										
											2020-11-17 10:07:54 +00:00
										 |  |  |     const ast = parser.parseBinding('x.y()', '', 0 /* absoluteOffset */); | 
					
						
							| 
									
										
											  
											
												refactor(compiler): Remove NullAstVisitor and visitAstChildren (#35619)
This commit removes the `NullAstVisitor` and `visitAstChildren` exported
from `packages/compiler/src/expression_parser/ast.ts` because they
contain duplicate and buggy implementation, and their use cases could be
sufficiently covered by `RecursiveAstVisitor` if the latter implements the
`visit` method. This use case is only needed in the language service.
With this change, any visitor that extends `RecursiveAstVisitor` could
just define their own `visit` function and the parent class will behave
correctly.
A bit of historical context:
In language service, we need a way to tranverse the expression AST in a
selective manner based on where the user's cursor is. This means we need a
"filtering" function to decide which node to visit and which node to not
visit. Instead of refactoring `RecursiveAstVisitor` to support this,
`visitAstChildren` was created. `visitAstChildren` duplicates the
implementation of `RecursiveAstVisitor`, but introduced some bugs along
the way. For example, in `visitKeyedWrite`, it visits
```
obj -> key -> obj
```
instead of
```
obj -> key -> value
```
Moreover, because of the following line
```
visitor.visit && visitor.visit(ast, context) || ast.visit(visitor, context);
```
`visitAstChildren` visits every node *twice*.
PR Close #35619
											
										 
											2020-02-21 10:20:52 -08:00
										 |  |  |     const visitor = new Visitor(); | 
					
						
							|  |  |  |     const path: AST[] = []; | 
					
						
							|  |  |  |     visitor.visit(ast.ast, path); | 
					
						
							|  |  |  |     // If the visitor method of RecursiveAstVisitor is implemented correctly,
 | 
					
						
							|  |  |  |     // then we should have collected the full path from root to leaf.
 | 
					
						
							|  |  |  |     expect(path.length).toBe(3); | 
					
						
							|  |  |  |     const [methodCall, propertyRead, implicitReceiver] = path; | 
					
						
							|  |  |  |     expectType(methodCall, MethodCall); | 
					
						
							|  |  |  |     expectType(propertyRead, PropertyRead); | 
					
						
							|  |  |  |     expectType(implicitReceiver, ImplicitReceiver); | 
					
						
							|  |  |  |     expect(methodCall.name).toBe('y'); | 
					
						
							|  |  |  |     expect(methodCall.args).toEqual([]); | 
					
						
							|  |  |  |     expect(propertyRead.name).toBe('x'); | 
					
						
							|  |  |  |   }); | 
					
						
							|  |  |  | }); | 
					
						
							|  |  |  | 
 | 
					
						
							|  |  |  | class Visitor extends RecursiveAstVisitor { | 
					
						
							|  |  |  |   visit(node: AST, path: AST[]) { | 
					
						
							|  |  |  |     path.push(node); | 
					
						
							|  |  |  |     node.visit(this, path); | 
					
						
							|  |  |  |   } | 
					
						
							|  |  |  | } | 
					
						
							|  |  |  | 
 | 
					
						
							|  |  |  | type Newable = new (...args: any) => any; | 
					
						
							|  |  |  | function expectType<T extends Newable>(val: any, t: T): asserts val is InstanceType<T> { | 
					
						
							|  |  |  |   expect(val instanceof t).toBe(true, `expect ${val.constructor.name} to be ${t.name}`); | 
					
						
							|  |  |  | } |