refactor(language-service): Omit typechecking for finding directives (#32156)

Remove unnecessary private method `getDeclarationFromNode` and moved
some logic to utils instead so that it can be tested in isolation of the
Language Service infrastructure.
The use of typechecker to check the directive is also not necessary,
since resolve.getNonNormalizedDirectiveMetadata() will check if the
directive is actually an Angular entity.

PR Close #32156
This commit is contained in:
Keen Yee Liau 2019-08-15 14:09:33 -07:00 committed by Andrew Kushnir
parent abb44f7db0
commit 71ada483bf
4 changed files with 141 additions and 63 deletions

View File

@ -14,8 +14,8 @@ import {AstResult, TemplateInfo} from './common';
import {createLanguageService} from './language_service';
import {ReflectorHost} from './reflector_host';
import {ExternalTemplate, InlineTemplate, getClassDeclFromTemplateNode} from './template';
import {Declaration, DeclarationError, Declarations, Diagnostic, DiagnosticKind, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
import {findTighestNode} from './utils';
import {Declaration, DeclarationError, Diagnostic, DiagnosticKind, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
import {findTightestNode, getDirectiveClassLike} from './utils';
/**
* Create a `LanguageServiceHost`
@ -194,24 +194,50 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
return results;
}
getDeclarations(fileName: string): Declarations {
/**
* Return metadata about all class declarations in the file that are Angular
* directives. Potential matches are `@NgModule`, `@Component`, `@Directive`,
* `@Pipes`, etc. class declarations.
*
* @param fileName TS file
*/
getDeclarations(fileName: string): Declaration[] {
if (!fileName.endsWith('.ts')) {
return [];
}
const result: Declarations = [];
const sourceFile = this.getSourceFile(fileName);
if (sourceFile) {
const visit = (child: ts.Node) => {
const declaration = this.getDeclarationFromNode(sourceFile, child);
if (declaration) {
result.push(declaration);
} else {
ts.forEachChild(child, visit);
}
};
ts.forEachChild(sourceFile, visit);
if (!sourceFile) {
return [];
}
return result;
const results: Declaration[] = [];
const visit = (child: ts.Node) => {
const candidate = getDirectiveClassLike(child);
if (candidate) {
const {decoratorId, classDecl} = candidate;
const declarationSpan = spanOf(decoratorId);
const className = classDecl.name !.text;
const classSymbol = this.reflector.getStaticSymbol(sourceFile.fileName, className);
// Ask the resolver to check if candidate is actually Angular directive
if (!this.resolver.isDirective(classSymbol)) {
return;
}
const data = this.resolver.getNonNormalizedDirectiveMetadata(classSymbol);
if (!data) {
return;
}
results.push({
type: classSymbol,
declarationSpan,
metadata: data.metadata,
errors: this.getCollectedErrors(declarationSpan, sourceFile),
});
} else {
child.forEachChild(visit);
}
};
ts.forEachChild(sourceFile, visit);
return results;
}
getSourceFile(fileName: string): ts.SourceFile|undefined {
@ -277,7 +303,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
* class AppComponent {}
* ^---- class declaration node
*
*
* @param node Potential template node
*/
private getInternalTemplate(node: ts.Node): TemplateSource|undefined {
@ -355,49 +380,6 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
});
}
private getDeclarationFromNode(sourceFile: ts.SourceFile, node: ts.Node): Declaration|undefined {
if (node.kind == ts.SyntaxKind.ClassDeclaration && node.decorators &&
(node as ts.ClassDeclaration).name) {
for (const decorator of node.decorators) {
if (decorator.expression && decorator.expression.kind == ts.SyntaxKind.CallExpression) {
const classDeclaration = node as ts.ClassDeclaration;
if (classDeclaration.name) {
const call = decorator.expression as ts.CallExpression;
const target = call.expression;
const type = this.program.getTypeChecker().getTypeAtLocation(target);
if (type) {
const staticSymbol =
this.reflector.getStaticSymbol(sourceFile.fileName, classDeclaration.name.text);
try {
if (this.resolver.isDirective(staticSymbol as any)) {
const {metadata} =
this.resolver.getNonNormalizedDirectiveMetadata(staticSymbol as any) !;
const declarationSpan = spanOf(target);
return {
type: staticSymbol,
declarationSpan,
metadata,
errors: this.getCollectedErrors(declarationSpan, sourceFile)
};
}
} catch (e) {
if (e.message) {
this.collectError(e, sourceFile.fileName);
const declarationSpan = spanOf(target);
return {
type: staticSymbol,
declarationSpan,
errors: this.getCollectedErrors(declarationSpan, sourceFile)
};
}
}
}
}
}
}
}
}
/**
* Return the parsed template for the template at the specified `position`.
* @param fileName TS or HTML file
@ -411,7 +393,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
return;
}
// Find the node that most closely matches the position
const node = findTighestNode(sourceFile, position);
const node = findTightestNode(sourceFile, position);
if (!node) {
return;
}

View File

@ -183,8 +183,51 @@ export function findTemplateAstAt(
* @param node
* @param position
*/
export function findTighestNode(node: ts.Node, position: number): ts.Node|undefined {
export function findTightestNode(node: ts.Node, position: number): ts.Node|undefined {
if (node.getStart() <= position && position < node.getEnd()) {
return node.forEachChild(c => findTighestNode(c, position)) || node;
return node.forEachChild(c => findTightestNode(c, position)) || node;
}
}
interface DirectiveClassLike {
decoratorId: ts.Identifier; // decorator identifier
classDecl: ts.ClassDeclaration;
}
/**
* Return metadata about `node` if it looks like an Angular directive class.
* In this case, potential matches are `@NgModule`, `@Component`, `@Directive`,
* `@Pipe`, etc.
* These class declarations all share some common attributes, namely their
* decorator takes exactly one parameter and the parameter must be an object
* literal.
*
* For example,
* v---------- `decoratorId`
* @NgModule({
* declarations: [],
* })
* class AppModule {}
* ^----- `classDecl`
*
* @param node Potential node that represents an Angular directive.
*/
export function getDirectiveClassLike(node: ts.Node): DirectiveClassLike|undefined {
if (!ts.isClassDeclaration(node) || !node.name || !node.decorators) {
return;
}
for (const d of node.decorators) {
const expr = d.expression;
if (!ts.isCallExpression(expr) || expr.arguments.length !== 1 ||
!ts.isIdentifier(expr.expression)) {
continue;
}
const arg = expr.arguments[0];
if (ts.isObjectLiteralExpression(arg)) {
return {
decoratorId: expr.expression,
classDecl: node,
};
}
}
}

View File

@ -12,7 +12,26 @@ import {toh} from './test_data';
import {MockTypescriptHost} from './test_utils';
describe('getClassDeclFromTemplateNode', () => {
it('should return class declaration', () => {
it('should find class declaration in syntax-only mode', () => {
const sourceFile = ts.createSourceFile(
'foo.ts', `
@Component({
template: '<div></div>'
})
class MyComponent {}`,
ts.ScriptTarget.ES2015, true /* setParentNodes */);
function visit(node: ts.Node): ts.ClassDeclaration|undefined {
return getClassDeclFromTemplateNode(node) || node.forEachChild(visit);
}
const classDecl = sourceFile.forEachChild(visit);
expect(classDecl).toBeTruthy();
expect(classDecl !.kind).toBe(ts.SyntaxKind.ClassDeclaration);
expect((classDecl as ts.ClassDeclaration).name !.text).toBe('MyComponent');
});
it('should return class declaration for AppComponent', () => {
const host = new MockTypescriptHost(['/app/app.component.ts'], toh);
const tsLS = ts.createLanguageService(host);
const sourceFile = tsLS.getProgram() !.getSourceFile('/app/app.component.ts');

View File

@ -0,0 +1,34 @@
/**
* @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 {getDirectiveClassLike} from '../src/utils';
describe('getDirectiveClassLike()', () => {
it('should return a directive class', () => {
const sourceFile = ts.createSourceFile(
'foo.ts', `
@NgModule({
declarations: [],
})
class AppModule {}
`,
ts.ScriptTarget.ES2015);
const result = sourceFile.forEachChild(c => {
const directive = getDirectiveClassLike(c);
if (directive) {
return directive;
}
});
expect(result).toBeTruthy();
const {decoratorId, classDecl} = result !;
expect(decoratorId.kind).toBe(ts.SyntaxKind.Identifier);
expect((decoratorId as ts.Identifier).text).toBe('NgModule');
expect(classDecl.name !.text).toBe('AppModule');
});
});