Skip to content

Commit cf3548a

Browse files
chuckjazchuckjaz
committed
fix(compiler): Improved error reporting of the static reflector.
StaticReflector provides more context on errors reported by the collector. The metadata collector now records the line and character of the node that caused it to report the error. Includes other minor fixes to error reporting and a wording change. Fixes angular#8978 Closes angular#9011
1 parent c197e2b commit cf3548a

File tree

7 files changed

+248
-39
lines changed

7 files changed

+248
-39
lines changed

‎modules/@angular/compiler-cli/src/static_reflector.ts‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
keyframes
3131
} from "@angular/core";
3232
import {ReflectorReader} from "./core_private";
33-
33+
3434
const SUPPORTED_SCHEMA_VERSION = 1;
3535

3636
/**
@@ -390,7 +390,11 @@ export class StaticReflector implements ReflectorReader {
390390
return context;
391391
}
392392
case "error":
393-
throw new Error(expression['message']);
393+
let message = produceErrorMessage(expression);
394+
if (expression['line']) {
395+
message = `${message} (position ${expression['line']}:${expression['character']} in the original .ts file)`;
396+
}
397+
throw new Error(message);
394398
}
395399
return null;
396400
}
@@ -399,7 +403,11 @@ export class StaticReflector implements ReflectorReader {
399403
return null;
400404
}
401405

402-
return simplify(value);
406+
try {
407+
return simplify(value);
408+
} catch(e) {
409+
throw new Error(`${e.message}, resolving symbol ${context.name} in ${context.filePath}`);
410+
}
403411
}
404412

405413
/**
@@ -431,6 +439,33 @@ export class StaticReflector implements ReflectorReader {
431439
}
432440
return result;
433441
}
442+
443+
}
444+
445+
function expandedMessage(error: any): string {
446+
switch (error.message) {
447+
case 'Reference to non-exported class':
448+
if (error.context && error.context.className) {
449+
return `Reference to a non-exported class ${error.context.className}`;
450+
}
451+
break;
452+
case 'Variable not initialized':
453+
return 'Only initialized variables and constants can be referenced';
454+
case 'Destructuring not supported':
455+
return 'Referencing an exported destructured variable or constant is not supported';
456+
case 'Could not resolve type':
457+
if (error.context && error.context.typeName) {
458+
return `Could not resolve type ${error.context.typeName}`;
459+
}
460+
break;
461+
case 'Function call not supported':
462+
return 'Function calls are not supported. Consider replacing the function or lambda with a reference to an exported function';
463+
}
464+
return error.message;
465+
}
466+
467+
function produceErrorMessage(error: any): string {
468+
return `Error encountered resolving symbol values statically. ${expandedMessage(error)}`;
434469
}
435470

436471
function mapStringMap(input: {[key: string]: any},

‎modules/@angular/compiler-cli/test/static_reflector_spec.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ describe('StaticReflector', () => {
109109
expect(parameters).toEqual([]);
110110
});
111111

112+
it('should provide context for errors reported by the collector', () => {
113+
let SomeClass = host.findDeclaration('src/error-reporting', 'SomeClass');
114+
expect(() => reflector.annotations(SomeClass)).toThrow(new Error('Error encountered resolving symbol values statically. A reasonable error message (position 12:33 in the original .ts file), resolving symbol ErrorSym in /tmp/src/error-references.d.ts, resolving symbol Link2 in /tmp/src/error-references.d.ts, resolving symbol Link1 in /tmp/src/error-references.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts'));
115+
});
116+
112117
it('should simplify primitive into itself', () => {
113118
expect(simplify(noContext, 1)).toBe(1);
114119
expect(simplify(noContext, true)).toBe(true);
@@ -537,6 +542,58 @@ class MockReflectorHost implements StaticReflectorHost {
537542
},
538543
'/src/extern.d.ts': {"__symbolic": "module", "version": 1, metadata: {s: "s"}},
539544
'/tmp/src/version-error.d.ts': {"__symbolic": "module", "version": 100, metadata: {e: "s"}},
545+
'/tmp/src/error-reporting.d.ts': {
546+
__symbolic: "module",
547+
version: 1,
548+
metadata: {
549+
SomeClass: {
550+
__symbolic: "class",
551+
decorators: [
552+
{
553+
__symbolic: "call",
554+
expression: {
555+
__symbolic: "reference",
556+
name: "Component",
557+
module: "angular2/src/core/metadata"
558+
},
559+
arguments: [
560+
{
561+
directives: [
562+
{
563+
__symbolic: "reference",
564+
module: "src/error-references",
565+
name: "Link1",
566+
}
567+
]
568+
}
569+
]
570+
}
571+
],
572+
}
573+
}
574+
},
575+
'/tmp/src/error-references.d.ts': {
576+
__symbolic: "module",
577+
version: 1,
578+
metadata: {
579+
Link1: {
580+
__symbolic: "reference",
581+
module: "src/error-references",
582+
name: "Link2"
583+
},
584+
Link2: {
585+
__symbolic: "reference",
586+
module: "src/error-references",
587+
name: "ErrorSym"
588+
},
589+
ErrorSym: {
590+
__symbolic: "error",
591+
message: "A reasonable error message",
592+
line: 12,
593+
character: 33
594+
}
595+
}
596+
}
540597
};
541598
return data[moduleId];
542599
}

‎tools/@angular/tsc-wrapped/src/collector.ts‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as ts from 'typescript';
22

3-
import {Evaluator, ImportMetadata, ImportSpecifierMetadata, isPrimitive} from './evaluator';
3+
import {Evaluator, ImportMetadata, ImportSpecifierMetadata, errorSymbol, isPrimitive} from './evaluator';
44
import {ClassMetadata, ConstructorMetadata, ModuleMetadata, MemberMetadata, MetadataError, MetadataMap, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataValue, MethodMetadata, isMetadataError, isMetadataSymbolicReferenceExpression, VERSION} from './schema';
55
import {Symbols} from './symbols';
66

@@ -23,6 +23,11 @@ export class MetadataCollector {
2323
return <MetadataSymbolicExpression>evaluator.evaluateNode(decoratorNode.expression);
2424
}
2525

26+
function errorSym(
27+
message: string, node?: ts.Node, context?: {[name: string]: string}): MetadataError {
28+
return errorSymbol(message, node, context, sourceFile);
29+
}
30+
2631
function classMetadataOf(classDeclaration: ts.ClassDeclaration): ClassMetadata {
2732
let result: ClassMetadata = {__symbolic: 'class'};
2833

@@ -37,7 +42,7 @@ export class MetadataCollector {
3742
if (isMetadataError(result) || isMetadataSymbolicReferenceExpression(result)) {
3843
return result;
3944
} else {
40-
return {__symbolic: 'error', message: 'Symbol reference expected'};
45+
return errorSym('Symbol reference expected', node);
4146
}
4247
}
4348

@@ -128,8 +133,7 @@ export class MetadataCollector {
128133
locals.define(className, {__symbolic: 'reference', name: className});
129134
} else {
130135
locals.define(
131-
className,
132-
{__symbolic: 'error', message: `Reference to non-exported class ${className}`});
136+
className, errorSym('Reference to non-exported class', node, {className}));
133137
}
134138
break;
135139
}
@@ -156,10 +160,7 @@ export class MetadataCollector {
156160
if (variableDeclaration.initializer) {
157161
varValue = evaluator.evaluateNode(variableDeclaration.initializer);
158162
} else {
159-
varValue = {
160-
__symbolic: 'error',
161-
message: 'Only intialized variables and constants can be referenced statically'
162-
};
163+
varValue = errorSym('Variable not initialized', nameNode);
163164
}
164165
if (variableStatement.flags & ts.NodeFlags.Export ||
165166
variableDeclaration.flags & ts.NodeFlags.Export) {
@@ -175,14 +176,11 @@ export class MetadataCollector {
175176
// or
176177
// var [<identifier>[, <identifier}+] = <expression>;
177178
// are not supported.
178-
let varValue = {
179-
__symbolc: 'error',
180-
message: 'Destructuring declarations cannot be referenced statically'
181-
};
182179
const report = (nameNode: ts.Node) => {
183180
switch (nameNode.kind) {
184181
case ts.SyntaxKind.Identifier:
185182
const name = <ts.Identifier>nameNode;
183+
const varValue = errorSym('Destructuring not supported', nameNode);
186184
locals.define(name.text, varValue);
187185
if (node.flags & ts.NodeFlags.Export) {
188186
if (!metadata) metadata = {};

‎tools/@angular/tsc-wrapped/src/evaluator.ts‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,28 @@ export interface ImportMetadata {
5454
from: string; // from 'place'
5555
}
5656

57+
58+
function getSourceFileOfNode(node: ts.Node): ts.SourceFile {
59+
while (node && node.kind != ts.SyntaxKind.SourceFile) {
60+
node = node.parent
61+
}
62+
return <ts.SourceFile>node;
63+
}
64+
65+
/* @internal */
66+
export function errorSymbol(
67+
message: string, node?: ts.Node, context?: {[name: string]: string},
68+
sourceFile?: ts.SourceFile): MetadataError {
69+
if (node) {
70+
sourceFile = sourceFile || getSourceFileOfNode(node);
71+
if (sourceFile) {
72+
let {line, character} = ts.getLineAndCharacterOfPosition(sourceFile, node.pos);
73+
return {__symbolic: 'error', message, line, character, context};
74+
};
75+
}
76+
return {__symbolic: 'error', message, context};
77+
}
78+
5779
/**
5880
* Produce a symbolic representation of an expression folding values into their final value when
5981
* possible.
@@ -69,10 +91,7 @@ export class Evaluator {
6991
if (isMetadataError(result) || typeof result === 'string') {
7092
return result;
7193
} else {
72-
return {
73-
__symbolic: 'error',
74-
message: `Name expected a string or an identifier but received "${node.getText()}""`
75-
};
94+
return errorSymbol('Name expected', node, {received: node.getText()});
7695
}
7796
}
7897

@@ -185,7 +204,8 @@ export class Evaluator {
185204
const assignment = <ts.PropertyAssignment>child;
186205
const propertyName = this.nameOf(assignment.name);
187206
if (isMetadataError(propertyName)) {
188-
return propertyName;
207+
error = propertyName;
208+
return true;
189209
}
190210
const propertyValue = this.evaluateNode(assignment.initializer);
191211
if (isMetadataError(propertyValue)) {
@@ -306,13 +326,13 @@ export class Evaluator {
306326
const typeReferenceNode = <ts.TypeReferenceNode>node;
307327
const typeNameNode = typeReferenceNode.typeName;
308328
if (typeNameNode.kind != ts.SyntaxKind.Identifier) {
309-
return { __symbolic: 'error', message: 'Qualified type names not supported' }
329+
return errorSymbol('Qualified type names not supported', node);
310330
}
311331
const typeNameIdentifier = <ts.Identifier>typeReferenceNode.typeName;
312332
const typeName = typeNameIdentifier.text;
313333
const typeReference = this.symbols.resolve(typeName);
314334
if (!typeReference) {
315-
return {__symbolic: 'error', message: `Could not resolve type ${typeName}`};
335+
return errorSymbol('Could not resolve type', node, {typeName});
316336
}
317337
if (typeReferenceNode.typeArguments && typeReferenceNode.typeArguments.length) {
318338
const args = typeReferenceNode.typeArguments.map(element => this.evaluateNode(element));
@@ -452,7 +472,10 @@ export class Evaluator {
452472
};
453473
}
454474
break;
475+
case ts.SyntaxKind.FunctionExpression:
476+
case ts.SyntaxKind.ArrowFunction:
477+
return errorSymbol('Function call not supported', node);
455478
}
456-
return {__symbolic: 'error', message: 'Expression is too complex to resolve statically'};
479+
return errorSymbol('Expression form not supported', node);
457480
}
458481
}

‎tools/@angular/tsc-wrapped/src/schema.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,30 @@ export function isMetadataSymbolicSelectExpression(value: any):
191191

192192
export interface MetadataError {
193193
__symbolic: 'error';
194+
195+
/**
196+
* This message should be short and relatively discriptive and should be fixed once it is created.
197+
* If the reader doesn't recognize the message, it will display the message unmodified. If the
198+
* reader recognizes the error message is it free to use substitute message the is more
199+
* descriptive and/or localized.
200+
*/
194201
message: string;
202+
203+
/**
204+
* The line number of the error in the .ts file the metadata was created for.
205+
*/
206+
line?: number;
207+
208+
/**
209+
* The number of utf8 code-units from the beginning of the file of the error.
210+
*/
211+
character?: number;
212+
213+
/**
214+
* Context information that can be used to generate a more descriptive error message. The content
215+
* of the context is dependent on the error message.
216+
*/
217+
context?: {[name: string]: string};
195218
}
196219
export function isMetadataError(value: any): value is MetadataError {
197220
return value && value.__symbolic === 'error';

0 commit comments

Comments
 (0)