refactor(core): static-query schematic should handle abstract classes (#29688)

Queries can not only be accessed within derived classes, but also in
the super class through abstract methods. e.g.

```
abstract class BaseClass {

  abstract getEmbeddedForm(): NgForm {}

  ngOnInit() {
     this.getEmbeddedForm().doSomething();
  }
}

class Subclass extends BaseClass {
  @ViewChild(NgForm) form: NgForm;

  getEmbeddedForm() { return this.form; }

}
```

Same applies for abstract properties which are implemented in the base class
through accessors. This case is also now handled by the schematic.

Resolves FW-1213

PR Close #29688
This commit is contained in:
Paul Gschwendtner 2019-04-08 16:01:25 +02:00 committed by Igor Minar
parent ef85336719
commit 4e8c2c3422
6 changed files with 368 additions and 56 deletions

View File

@ -8,9 +8,11 @@
import * as ts from 'typescript';
import {hasPropertyNameText} from '../../../utils/typescript/property_name';
import {DeclarationUsageVisitor} from './declaration_usage_visitor';
import {DeclarationUsageVisitor, FunctionContext} from './declaration_usage_visitor';
import {ClassMetadataMap} from './ng_query_visitor';
import {NgQueryDefinition, QueryTiming, QueryType} from './query-definition';
import {updateSuperClassAbstractMembersContext} from './super_class';
/**
* Object that maps a given type of query to a list of lifecycle hooks that
@ -34,11 +36,15 @@ export function analyzeNgQueryUsage(
QueryTiming.DYNAMIC;
}
/** Checks whether a given class or it's derived classes use the specified query statically. */
/**
* Checks whether a given query is used statically within the given class, its super
* class or derived classes.
*/
function isQueryUsedStatically(
classDecl: ts.ClassDeclaration, query: NgQueryDefinition, classMetadataMap: ClassMetadataMap,
typeChecker: ts.TypeChecker, knownInputNames: string[]): boolean {
const usageVisitor = new DeclarationUsageVisitor(query.property, typeChecker);
typeChecker: ts.TypeChecker, knownInputNames: string[],
functionCtx: FunctionContext = new Map(), visitInheritedClasses = true): boolean {
const usageVisitor = new DeclarationUsageVisitor(query.property, typeChecker, functionCtx);
const classMetadata = classMetadataMap.get(classDecl);
// In case there is metadata for the current class, we collect all resolved Angular input
@ -48,24 +54,9 @@ function isQueryUsedStatically(
knownInputNames.push(...classMetadata.ngInputNames);
}
// List of TypeScript nodes which can contain usages of the given query in order to
// access it statically. e.g.
// (1) queries used in the "ngOnInit" lifecycle hook are static.
// (2) inputs with setters can access queries statically.
const possibleStaticQueryNodes: ts.Node[] =
classDecl.members
.filter(m => {
if (ts.isMethodDeclaration(m) && m.body && hasPropertyNameText(m.name) &&
STATIC_QUERY_LIFECYCLE_HOOKS[query.type].indexOf(m.name.text) !== -1) {
return true;
} else if (
knownInputNames && ts.isSetAccessor(m) && m.body && hasPropertyNameText(m.name) &&
knownInputNames.indexOf(m.name.text) !== -1) {
return true;
}
return false;
})
.map((member: ts.SetAccessorDeclaration | ts.MethodDeclaration) => member.body !);
// Array of TypeScript nodes which can contain usages of the given query in
// order to access it statically.
const possibleStaticQueryNodes = filterQueryClassMemberNodes(classDecl, query, knownInputNames);
// In case nodes that can possibly access a query statically have been found, check
// if the query declaration is synchronously used within any of these nodes.
@ -74,13 +65,66 @@ function isQueryUsedStatically(
return true;
}
// In case there are classes that derive from the current class, visit each
// derived class as inherited queries could be used statically.
if (classMetadata) {
return classMetadata.derivedClasses.some(
derivedClass => isQueryUsedStatically(
derivedClass, query, classMetadataMap, typeChecker, knownInputNames));
if (!classMetadata) {
return false;
}
// In case derived classes should also be analyzed, we determine the classes that derive
// from the current class and check if these have input setters or lifecycle hooks that
// use the query statically.
if (visitInheritedClasses) {
if (classMetadata.derivedClasses.some(
derivedClass => isQueryUsedStatically(
derivedClass, query, classMetadataMap, typeChecker, knownInputNames))) {
return true;
}
}
// In case the current class has a super class, we determine declared abstract function-like
// declarations in the super-class that are implemented in the current class. The super class
// will then be analyzed with the abstract declarations mapped to the implemented TypeScript
// nodes. This allows us to handle queries which are used in super classes through derived
// abstract method declarations.
if (classMetadata.superClass) {
const superClassDecl = classMetadata.superClass;
// Update the function context to map abstract declaration nodes to their implementation
// node in the base class. This ensures that the declaration usage visitor can analyze
// abstract class member declarations.
updateSuperClassAbstractMembersContext(classDecl, functionCtx, classMetadataMap);
if (isQueryUsedStatically(
superClassDecl, query, classMetadataMap, typeChecker, [], functionCtx, false)) {
return true;
}
}
return false;
}
/**
* Filters all class members from the class declaration that can access the
* given query statically (e.g. ngOnInit lifecycle hook or @Input setters)
*/
function filterQueryClassMemberNodes(
classDecl: ts.ClassDeclaration, query: NgQueryDefinition,
knownInputNames: string[]): ts.Block[] {
// Returns an array of TypeScript nodes which can contain usages of the given query
// in order to access it statically. e.g.
// (1) queries used in the "ngOnInit" lifecycle hook are static.
// (2) inputs with setters can access queries statically.
return classDecl.members
.filter(m => {
if (ts.isMethodDeclaration(m) && m.body && hasPropertyNameText(m.name) &&
STATIC_QUERY_LIFECYCLE_HOOKS[query.type].indexOf(m.name.text) !== -1) {
return true;
} else if (
knownInputNames && ts.isSetAccessor(m) && m.body && hasPropertyNameText(m.name) &&
knownInputNames.indexOf(m.name.text) !== -1) {
return true;
}
return false;
})
.map((member: ts.SetAccessorDeclaration | ts.MethodDeclaration) => member.body !);
}

View File

@ -9,7 +9,7 @@
import * as ts from 'typescript';
import {isFunctionLikeDeclaration, unwrapExpression} from '../../../utils/typescript/functions';
type FunctionContext = Map<ts.ParameterDeclaration, ts.Node>;
export type FunctionContext = Map<ts.Node, ts.Node>;
/**
* List of TypeScript syntax tokens that can be used within a binary expression as
@ -45,7 +45,9 @@ export class DeclarationUsageVisitor {
*/
private context: FunctionContext = new Map();
constructor(private declaration: ts.Node, private typeChecker: ts.TypeChecker) {}
constructor(
private declaration: ts.Node, private typeChecker: ts.TypeChecker,
private baseContext: FunctionContext = new Map()) {}
private isReferringToSymbol(node: ts.Node): boolean {
const symbol = this.typeChecker.getSymbolAtLocation(node);
@ -114,7 +116,7 @@ export class DeclarationUsageVisitor {
private visitPropertyAccessors(
node: ts.PropertyAccessExpression, checkSetter: boolean, checkGetter: boolean) {
const propertySymbol = this.typeChecker.getSymbolAtLocation(node.name);
const propertySymbol = this._getPropertyAccessSymbol(node);
if (!propertySymbol || !propertySymbol.declarations.length ||
(propertySymbol.getFlags() & ts.SymbolFlags.Accessor) === 0) {
@ -142,13 +144,6 @@ export class DeclarationUsageVisitor {
return false;
}
const symbol = this.typeChecker.getSymbolAtLocation(leftExpr.name);
if (!symbol || !symbol.declarations.length ||
(symbol.getFlags() & ts.SymbolFlags.Accessor) === 0) {
return false;
}
if (BINARY_COMPOUND_TOKENS.indexOf(node.operatorToken.kind) !== -1) {
// Compound assignments always cause the getter and setter to be called.
// Therefore we need to check the setter and getter of the property access.
@ -166,9 +161,15 @@ export class DeclarationUsageVisitor {
}
isSynchronouslyUsedInNode(searchNode: ts.Node): boolean {
this.nodeQueue = [searchNode];
this.visitedJumpExprNodes.clear();
this.context.clear();
this.nodeQueue = [searchNode];
// Copy base context values into the current function block context. The
// base context is useful if nodes need to be mapped to other nodes. e.g.
// abstract super class methods are mapped to their implementation node of
// the derived class.
this.baseContext.forEach((value, key) => this.context.set(key, value));
while (this.nodeQueue.length) {
const node = this.nodeQueue.shift() !;
@ -225,7 +226,7 @@ export class DeclarationUsageVisitor {
* the context, the original node is returned.
*/
private _resolveNodeFromContext(node: ts.Node): ts.Node {
if (ts.isParameter(node) && this.context.has(node)) {
if (this.context.has(node)) {
return this.context.get(node) !;
}
return node;
@ -279,4 +280,31 @@ export class DeclarationUsageVisitor {
return symbol;
}
/** Gets the symbol of the given property access expression. */
private _getPropertyAccessSymbol(node: ts.PropertyAccessExpression): ts.Symbol|null {
let propertySymbol = this._getDeclarationSymbolOfNode(node.name);
if (!propertySymbol) {
return null;
}
if (!this.context.has(propertySymbol.valueDeclaration)) {
return propertySymbol;
}
// In case the context has the value declaration of the given property access
// name identifier, we need to replace the "propertySymbol" with the symbol
// referring to the resolved symbol based on the context. e.g. abstract properties
// can ultimately resolve into an accessor declaration based on the implementation.
const contextNode = this._resolveNodeFromContext(propertySymbol.valueDeclaration);
if (!ts.isAccessor(contextNode)) {
return null;
}
// Resolve the symbol referring to the "accessor" using the name identifier
// of the accessor declaration.
return this._getDeclarationSymbolOfNode(contextNode.name);
}
}

View File

@ -16,6 +16,8 @@ import {NgQueryDefinition, QueryType} from './query-definition';
export interface ClassMetadata {
/** List of class declarations that derive from the given class. */
derivedClasses: ts.ClassDeclaration[];
/** Super class of the given class. */
superClass: ts.ClassDeclaration|null;
/** List of property names that declare an Angular input within the given class. */
ngInputNames: string[];
}
@ -101,29 +103,34 @@ export class NgQueryResolveVisitor {
private _recordClassInheritances(node: ts.ClassDeclaration) {
const baseTypes = getBaseTypeIdentifiers(node);
if (!baseTypes || !baseTypes.length) {
if (!baseTypes || baseTypes.length !== 1) {
return;
}
baseTypes.forEach(baseTypeIdentifier => {
// We need to resolve the value declaration through the resolved type as the base
// class could be declared in different source files and the local symbol won't
// contain a value declaration as the value is not declared locally.
const symbol = this.typeChecker.getTypeAtLocation(baseTypeIdentifier).getSymbol();
const superClass = baseTypes[0];
const baseClassMetadata = this._getClassMetadata(node);
if (symbol && symbol.valueDeclaration && ts.isClassDeclaration(symbol.valueDeclaration)) {
const extendedClass = symbol.valueDeclaration;
const classMetadata = this._getClassMetadata(extendedClass);
// We need to resolve the value declaration through the resolved type as the base
// class could be declared in different source files and the local symbol won't
// contain a value declaration as the value is not declared locally.
const symbol = this.typeChecker.getTypeAtLocation(superClass).getSymbol();
// Record all classes that derive from the given class. This makes it easy to
// determine all classes that could potentially use inherited queries statically.
classMetadata.derivedClasses.push(node);
this.classMetadata.set(extendedClass, classMetadata);
}
});
if (symbol && symbol.valueDeclaration && ts.isClassDeclaration(symbol.valueDeclaration)) {
const extendedClass = symbol.valueDeclaration;
const classMetadataExtended = this._getClassMetadata(extendedClass);
// Record all classes that derive from the given class. This makes it easy to
// determine all classes that could potentially use inherited queries statically.
classMetadataExtended.derivedClasses.push(node);
this.classMetadata.set(extendedClass, classMetadataExtended);
// Record the super class of the current class.
baseClassMetadata.superClass = extendedClass;
this.classMetadata.set(node, baseClassMetadata);
}
}
private _getClassMetadata(node: ts.ClassDeclaration): ClassMetadata {
return this.classMetadata.get(node) || {derivedClasses: [], ngInputNames: []};
return this.classMetadata.get(node) || {derivedClasses: [], superClass: null, ngInputNames: []};
}
}

View File

@ -0,0 +1,62 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 * as ts from 'typescript';
import {isFunctionLikeDeclaration} from '../../../utils/typescript/functions';
import {hasModifier} from '../../../utils/typescript/nodes';
import {getPropertyNameText} from '../../../utils/typescript/property_name';
import {FunctionContext} from './declaration_usage_visitor';
import {ClassMetadataMap} from './ng_query_visitor';
/**
* Updates the specified function context to map abstract super-class class members
* to their implementation TypeScript nodes. This allows us to run the declaration visitor
* for the super class with the context of the "baseClass" (e.g. with implemented abstract
* class members)
*/
export function updateSuperClassAbstractMembersContext(
baseClass: ts.ClassDeclaration, context: FunctionContext, classMetadataMap: ClassMetadataMap) {
getSuperClassDeclarations(baseClass, classMetadataMap).forEach(superClassDecl => {
superClassDecl.members.forEach(superClassMember => {
if (!superClassMember.name || !hasModifier(superClassMember, ts.SyntaxKind.AbstractKeyword)) {
return;
}
// Find the matching implementation of the abstract declaration from the super class.
const baseClassImpl = baseClass.members.find(
baseClassMethod => !!baseClassMethod.name &&
getPropertyNameText(baseClassMethod.name) ===
getPropertyNameText(superClassMember.name !));
if (!baseClassImpl || !isFunctionLikeDeclaration(baseClassImpl) || !baseClassImpl.body) {
return;
}
if (!context.has(superClassMember)) {
context.set(superClassMember, baseClassImpl);
}
});
});
}
/** Gets all super-class TypeScript declarations for the given class. */
function getSuperClassDeclarations(
classDecl: ts.ClassDeclaration, classMetadataMap: ClassMetadataMap) {
const declarations: ts.ClassDeclaration[] = [];
let current = classMetadataMap.get(classDecl);
while (current && current.superClass) {
declarations.push(current.superClass);
current = classMetadataMap.get(current.superClass);
}
return declarations;
}

View File

@ -1032,6 +1032,163 @@ describe('static-queries migration', () => {
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});
it('should check derived abstract class methods', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
export abstract class RootBaseClass {
abstract getQuery(): any;
ngOnInit() {
this.getQuery().doSomething();
}
}
export abstract class BaseClass extends RootBaseClass {
abstract getQuery2(): any;
getQuery() {
this.getQuery2();
}
}
@Component({template: '<span #test></span>'})
export class Subclass extends BaseClass {
@${queryType}('test') query: any;
getQuery2(): any {
return this.query;
}
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});
it('should detect queries accessed through deep abstract class method', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
export abstract class RootBaseClass {
abstract getQuery(): any;
ngOnInit() {
this.getQuery().doSomething();
}
}
export abstract class BaseClass extends RootBaseClass {
/* additional layer of indirection */
}
@Component({template: '<span #test></span>'})
export class Subclass extends BaseClass {
@${queryType}('test') query: any;
getQuery(): any {
return this.query;
}
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});
it('should detect queries accessed through abstract property getter', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
export abstract class BaseClass {
abstract myQuery: any;
ngOnInit() {
this.myQuery.doSomething();
}
}
@Component({template: '<span #test></span>'})
export class Subclass extends BaseClass {
@${queryType}('test') query: any;
get myQuery() { return this.query; }
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});
it('should detect queries accessed through abstract property setter', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
export abstract class BaseClass {
abstract myQuery: any;
ngOnInit() {
this.myQuery = "trigger";
}
}
@Component({template: '<span #test></span>'})
export class Subclass extends BaseClass {
@${queryType}('test') query: any;
set myQuery(val: any) { this.query.doSomething() }
get myQuery() { /* noop */ }
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});
it('should detect query usage in abstract class methods accessing inherited query', () => {
writeFile('/index.ts', `
import {Component, ${queryType}} from '@angular/core';
export abstract class RootBaseClass {
abstract getQuery(): any;
ngOnInit() {
this.getQuery().doSomething();
}
}
export abstract class BaseClass extends RootBaseClass {
@${queryType}('test') query: any;
abstract getQuery2(): any;
getQuery() {
this.getQuery2();
}
}
@Component({template: '<span #test></span>'})
export class Subclass extends BaseClass {
getQuery2(): any {
return this.query;
}
}
`);
runMigration();
expect(tree.readContent('/index.ts'))
.toContain(`@${queryType}('test', { static: true }) query: any;`);
});
it('should properly handle multiple tsconfig files', () => {
writeFile('/src/index.ts', `
import {Component, ${queryType}} from '@angular/core';

View File

@ -0,0 +1,14 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 * as ts from 'typescript';
/** Checks whether the given TypeScript node has the specified modifier set. */
export function hasModifier(node: ts.Node, modifierKind: ts.SyntaxKind) {
return !!node.modifiers && node.modifiers.some(m => m.kind === modifierKind);
}