honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler-cli): pass real source spans where they are empty (#31805) 

Some consumers of functions that take `ParseSourceSpan`s currently pass
empty and incorrect source spans. This fixes those cases.

PR Close #31805
This commit is contained in:
Ayaz Hafiz 2019-07-23 12:32:14 -07:00 committed by Kara Erickson
parent 8be8466a00
commit e893c5a330
5 changed files with 153 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ConstantPool, EMPTY_SOURCE_SPAN, Expression, Identifiers, ParseError, ParsedHostBindings, R3DependencyMetadata, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler';
import {ConstantPool, Expression, Identifiers, ParseError, ParsedHostBindings, R3DependencyMetadata, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
@ -21,7 +21,7 @@ import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFl
import {getDirectiveDiagnostics, getProviderDiagnostics} from './diagnostics';
import {compileNgFactoryDefField} from './factory';
import {generateSetClassMetadataCall} from './metadata';
import {findAngularDecorator, getConstructorDependencies, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from './util';
import {createSourceSpan, findAngularDecorator, getConstructorDependencies, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from './util';
const EMPTY_OBJECT: {[key: string]: string} = {};
const FIELD_DECORATORS = [
@ -320,7 +320,7 @@ export function extractDirectiveMetadata(
outputs: {...outputsFromMeta, ...outputsFromFields}, queries, viewQueries, selector,
fullInheritance: !!(flags & HandlerFlags.FULL_INHERITANCE), type, internalType,
typeArgumentCount: reflector.getGenericArityOfClass(clazz) || 0,
typeSourceSpan: EMPTY_SOURCE_SPAN, usesInheritance, exportAs, providers
typeSourceSpan: createSourceSpan(clazz.name), usesInheritance, exportAs, providers
};
return {decorator: directive, metadata};
}
@ -580,54 +580,61 @@ type StringMap<T> = {
[key: string]: T;
};
export function extractHostBindings(
members: ClassMember[], evaluator: PartialEvaluator, coreModule: string | undefined,
metadata?: Map<string, ts.Expression>): ParsedHostBindings {
let hostMetadata: StringMap<string|Expression> = {};
if (metadata && metadata.has('host')) {
const expr = metadata.get('host') !;
const hostMetaMap = evaluator.evaluate(expr);
if (!(hostMetaMap instanceof Map)) {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, expr, `Decorator host metadata must be an object`);
}
hostMetaMap.forEach((value, key) => {
// Resolve Enum references to their declared value.
if (value instanceof EnumValue) {
value = value.resolved;
}
if (typeof key !== 'string') {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, expr,
`Decorator host metadata must be a string -> string object, but found unparseable key`);
}
if (typeof value == 'string') {
hostMetadata[key] = value;
} else if (value instanceof DynamicValue) {
hostMetadata[key] = new WrappedNodeExpr(value.node as ts.Expression);
} else {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, expr,
`Decorator host metadata must be a string -> string object, but found unparseable value`);
}
});
function evaluateHostExpressionBindings(
hostExpr: ts.Expression, evaluator: PartialEvaluator): ParsedHostBindings {
const hostMetaMap = evaluator.evaluate(hostExpr);
if (!(hostMetaMap instanceof Map)) {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, hostExpr, `Decorator host metadata must be an object`);
}
const hostMetadata: StringMap<string|Expression> = {};
hostMetaMap.forEach((value, key) => {
// Resolve Enum references to their declared value.
if (value instanceof EnumValue) {
value = value.resolved;
}
if (typeof key !== 'string') {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, hostExpr,
`Decorator host metadata must be a string -> string object, but found unparseable key`);
}
if (typeof value == 'string') {
hostMetadata[key] = value;
} else if (value instanceof DynamicValue) {
hostMetadata[key] = new WrappedNodeExpr(value.node as ts.Expression);
} else {
throw new FatalDiagnosticError(
ErrorCode.VALUE_HAS_WRONG_TYPE, hostExpr,
`Decorator host metadata must be a string -> string object, but found unparseable value`);
}
});
const bindings = parseHostBindings(hostMetadata);
// TODO: create and provide proper sourceSpan to make error message more descriptive (FW-995)
// For now, pass an incorrect (empty) but valid sourceSpan.
const errors = verifyHostBindings(bindings, EMPTY_SOURCE_SPAN);
const errors = verifyHostBindings(bindings, createSourceSpan(hostExpr));
if (errors.length > 0) {
throw new FatalDiagnosticError(
// TODO: provide more granular diagnostic and output specific host expression that triggered
// an error instead of the whole host object
ErrorCode.HOST_BINDING_PARSE_ERROR, metadata !.get('host') !,
// TODO: provide more granular diagnostic and output specific host expression that
// triggered an error instead of the whole host object.
ErrorCode.HOST_BINDING_PARSE_ERROR, hostExpr,
errors.map((error: ParseError) => error.msg).join('\n'));
}
return bindings;
}
export function extractHostBindings(
members: ClassMember[], evaluator: PartialEvaluator, coreModule: string | undefined,
metadata?: Map<string, ts.Expression>): ParsedHostBindings {
let bindings: ParsedHostBindings;
if (metadata && metadata.has('host')) {
bindings = evaluateHostExpressionBindings(metadata.get('host') !, evaluator);
} else {
bindings = parseHostBindings({});
}
filterToMembersWithDecorator(members, 'HostBinding', coreModule).forEach(({member, decorators}) => {
decorators.forEach(decorator => {
let hostPropertyName: string = member.name;

16
packages/compiler-cli/src/ngtsc/annotations/src/util.ts
View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Expression, ExternalExpr, LiteralExpr, R3DependencyMetadata, R3Reference, R3ResolvedDependencyType, WrappedNodeExpr} from '@angular/compiler';
import {Expression, ExternalExpr, LiteralExpr, ParseLocation, ParseSourceFile, ParseSourceSpan, R3DependencyMetadata, R3Reference, R3ResolvedDependencyType, WrappedNodeExpr} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError, makeDiagnostic} from '../../diagnostics';
@ -475,3 +475,17 @@ export function wrapTypeReference(reflector: ReflectionHost, clazz: ClassDeclara
value;
return {value, type};
}
/** Creates a ParseSourceSpan for a TypeScript node. */
export function createSourceSpan(node: ts.Node): ParseSourceSpan {
const sf = node.getSourceFile();
const [startOffset, endOffset] = [node.getStart(), node.getEnd()];
const {line: startLine, character: startCol} = sf.getLineAndCharacterOfPosition(startOffset);
const {line: endLine, character: endCol} = sf.getLineAndCharacterOfPosition(endOffset);
const parseSf = new ParseSourceFile(sf.getFullText(), sf.fileName);
// +1 because values are zero-indexed.
return new ParseSourceSpan(
new ParseLocation(parseSf, startOffset, startLine + 1, startCol + 1),
new ParseLocation(parseSf, endOffset, endLine + 1, endCol + 1));
}

107
packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts
View File

@ -5,6 +5,7 @@
* 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 {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports';
@ -16,10 +17,10 @@ import {getDeclaration, makeProgram} from '../../testing';
import {DirectiveDecoratorHandler} from '../src/directive';
runInEachFileSystem(() => {
describe('DirectiveDecoratorHandler', () => {
let _: typeof absoluteFrom;
beforeEach(() => _ = absoluteFrom);
let _: typeof absoluteFrom;
beforeEach(() => _ = absoluteFrom);
describe('DirectiveDecoratorHandler', () => {
it('should use the `ReflectionHost` to detect class inheritance', () => {
const {program} = makeProgram([
{
@ -40,53 +41,73 @@ runInEachFileSystem(() => {
},
]);
const checker = program.getTypeChecker();
const reflectionHost = new TestReflectionHost(checker);
const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null);
const metaReader = new LocalMetadataRegistry();
const dtsReader = new DtsMetadataReader(checker, reflectionHost);
const scopeRegistry = new LocalModuleScopeRegistry(
metaReader, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]),
null);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new DirectiveDecoratorHandler(
reflectionHost, evaluator, scopeRegistry, scopeRegistry, metaReader,
NOOP_DEFAULT_IMPORT_RECORDER, injectableRegistry,
/* isCore */ false, /* annotateForClosureCompiler */ false);
const analyzeDirective = (dirName: string) => {
const DirNode = getDeclaration(program, _('/entry.ts'), dirName, isNamedClassDeclaration);
const detected =
handler.detect(DirNode, reflectionHost.getDecoratorsOfDeclaration(DirNode));
if (detected === undefined) {
throw new Error(`Failed to recognize @Directive (${dirName}).`);
}
const {analysis} = handler.analyze(DirNode, detected.metadata);
if (analysis === undefined) {
throw new Error(`Failed to analyze @Directive (${dirName}).`);
}
return analysis;
};
// By default, `TestReflectionHost#hasBaseClass()` returns `false`.
const analysis1 = analyzeDirective('TestDir1');
const analysis1 = analyzeDirective(program, 'TestDir1', /*hasBaseClass*/ false);
expect(analysis1.meta.usesInheritance).toBe(false);
// Tweak `TestReflectionHost#hasBaseClass()` to return true.
reflectionHost.hasBaseClassReturnValue = true;
const analysis2 = analyzeDirective('TestDir2');
const analysis2 = analyzeDirective(program, 'TestDir2', /*hasBaseClass*/ true);
expect(analysis2.meta.usesInheritance).toBe(true);
});
it('should record the source span of a Directive class type', () => {
const src = `
import {Directive} from '@angular/core';
@Directive({selector: 'test-dir'})
export class TestDir {}
`;
const {program} = makeProgram([
{
name: _('/node_modules/@angular/core/index.d.ts'),
contents: 'export const Directive: any;',
},
{
name: _('/entry.ts'),
contents: src,
},
]);
const analysis = analyzeDirective(program, 'TestDir');
const span = analysis.meta.typeSourceSpan;
expect(span.toString()).toBe('TestDir');
expect(span.start.toString()).toContain('/entry.ts@5:22');
expect(span.end.toString()).toContain('/entry.ts@5:29');
});
});
// Helpers
class TestReflectionHost extends TypeScriptReflectionHost {
hasBaseClassReturnValue = false;
function analyzeDirective(program: ts.Program, dirName: string, hasBaseClass: boolean = false) {
class TestReflectionHost extends TypeScriptReflectionHost {
constructor(checker: ts.TypeChecker) { super(checker); }
hasBaseClass(clazz: ClassDeclaration): boolean { return this.hasBaseClassReturnValue; }
hasBaseClass(_class: ClassDeclaration): boolean { return hasBaseClass; }
}
const checker = program.getTypeChecker();
const reflectionHost = new TestReflectionHost(checker);
const evaluator = new PartialEvaluator(reflectionHost, checker, /*dependencyTracker*/ null);
const metaReader = new LocalMetadataRegistry();
const dtsReader = new DtsMetadataReader(checker, reflectionHost);
const scopeRegistry = new LocalModuleScopeRegistry(
metaReader, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]),
null);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new DirectiveDecoratorHandler(
reflectionHost, evaluator, scopeRegistry, scopeRegistry, metaReader,
NOOP_DEFAULT_IMPORT_RECORDER, injectableRegistry, /*isCore*/ false,
/*annotateForClosureCompiler*/ false);
const DirNode = getDeclaration(program, _('/entry.ts'), dirName, isNamedClassDeclaration);
const detected = handler.detect(DirNode, reflectionHost.getDecoratorsOfDeclaration(DirNode));
if (detected === undefined) {
throw new Error(`Failed to recognize @Directive (${dirName}).`);
}
const {analysis} = handler.analyze(DirNode, detected.metadata);
if (analysis === undefined) {
throw new Error(`Failed to analyze @Directive (${dirName}).`);
}
return analysis;
}
});

25
packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View File

@ -37,6 +37,10 @@ const setClassMetadataRegExp = (expectedType: string): RegExp =>
const testFiles = loadStandardTestFiles();
function getDiagnosticSourceCode(diag: ts.Diagnostic): string {
return diag.file !.text.substr(diag.start !, diag.length !);
}
runInEachFileSystem(os => {
describe('ngtsc behavioral tests', () => {
let env !: NgtscTestEnvironment;
@ -2897,6 +2901,27 @@ runInEachFileSystem(os => {
`Unexpected global target 'UnknownTarget' defined for 'click' event. Supported list of global targets: window,document,body.`);
});
it('should provide error location for invalid host properties', () => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test',
template: '...',
host: {
'(click)': 'act() | pipe',
}
})
class FooCmp {}
`);
const errors = env.driveDiagnostics();
expect(getDiagnosticSourceCode(errors[0])).toBe(`{
'(click)': 'act() | pipe',
}`);
expect(errors[0].messageText).toContain('/test.ts@7:17');
});
it('should throw in case pipes are used in host listeners', () => {
env.write(`test.ts`, `
import {Component} from '@angular/core';

4
packages/compiler/src/parse_util.ts
View File

@ -7,7 +7,6 @@
*/
import * as chars from './chars';
import {CompileIdentifierMetadata, identifierModuleUrl, identifierName} from './compile_metadata';
import {error} from './util';
export class ParseLocation {
constructor(
@ -109,9 +108,6 @@ export class ParseSourceSpan {
}
}
export const EMPTY_PARSE_LOCATION = new ParseLocation(new ParseSourceFile('', ''), 0, 0, 0);
export const EMPTY_SOURCE_SPAN = new ParseSourceSpan(EMPTY_PARSE_LOCATION, EMPTY_PARSE_LOCATION);
export enum ParseErrorLevel {
WARNING,
ERROR,