Skip to content

Commit

Permalink
feat(compiler-cli): improve error messages produced during structural…
Browse files Browse the repository at this point in the history 
… errors

The errors produced when error were encountered while interpreting the
content of a directive was often incomprehencible. With this change
these kind of error messages should be easier to understand and diagnose.
  • Loading branch information
@chuckjaz
chuckjaz committed Nov 20, 2017
1 parent 69c53c3 commit d87a51c
Show file tree
Hide file tree
Showing 25 changed files with 1,219 additions and 295 deletions.
1 change: 0 additions & 1 deletion packages/compiler-cli/src/main.ts
View file
Expand Up @@ -20,7 +20,6 @@ import {GENERATED_FILES} from './transformers/util';

import {exitCodeFromResult, performCompilation, readConfiguration, formatDiagnostics, Diagnostics, ParsedConfiguration, PerformCompilationResult, filterErrorsAndWarnings} from './perform_compile';
import {performWatchCompilation, createPerformWatchHost} from './perform_watch';
import {isSyntaxError} from '@angular/compiler';

export function main(
args: string[], consoleError: (s: string) => void = console.error,
Expand Down
7 changes: 3 additions & 4 deletions packages/compiler-cli/src/metadata/collector.ts
View file
Expand Up @@ -8,8 +8,8 @@

import * as ts from 'typescript';

import {Evaluator, errorSymbol} from './evaluator';
import {ClassMetadata, ConstructorMetadata, FunctionMetadata, InterfaceMetadata, METADATA_VERSION, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression, isMethodMetadata} from './schema';
import {Evaluator, errorSymbol, recordMapEntry, sourceInfo} from './evaluator';
import {ClassMetadata, ConstructorMetadata, FunctionMetadata, InterfaceMetadata, METADATA_VERSION, MemberMetadata, MetadataEntry, MetadataError, MetadataMap, MetadataSymbolicBinaryExpression, MetadataSymbolicCallExpression, MetadataSymbolicExpression, MetadataSymbolicIfExpression, MetadataSymbolicIndexExpression, MetadataSymbolicPrefixExpression, MetadataSymbolicReferenceExpression, MetadataSymbolicSelectExpression, MetadataSymbolicSpreadExpression, MetadataValue, MethodMetadata, ModuleExportMetadata, ModuleMetadata, isClassMetadata, isConstructorMetadata, isFunctionMetadata, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportDefaultReference, isMetadataImportedSymbolReferenceExpression, isMetadataSymbolicExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSelectExpression, isMethodMetadata} from './schema';
import {Symbols} from './symbols';

const isStatic = (node: ts.Node) => ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Static;
Expand Down Expand Up @@ -76,8 +76,7 @@ export class MetadataCollector {
}

function recordEntry<T extends MetadataEntry>(entry: T, node: ts.Node): T {
nodeMap.set(entry, node);
return entry;
return recordMapEntry(entry, node, nodeMap, sourceFile);
}

function errorSym(
Expand Down
50 changes: 36 additions & 14 deletions packages/compiler-cli/src/metadata/evaluator.ts
View file
Expand Up @@ -9,10 +9,11 @@
import * as ts from 'typescript';

import {CollectorOptions} from './collector';
import {MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataSymbolicCallExpression, MetadataValue, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSpreadExpression} from './schema';
import {ClassMetadata, FunctionMetadata, InterfaceMetadata, MetadataEntry, MetadataError, MetadataImportedSymbolReferenceExpression, MetadataSourceLocationInfo, MetadataSymbolicCallExpression, MetadataValue, isMetadataError, isMetadataGlobalReferenceExpression, isMetadataImportDefaultReference, isMetadataImportedSymbolReferenceExpression, isMetadataModuleReferenceExpression, isMetadataSymbolicReferenceExpression, isMetadataSymbolicSpreadExpression} from './schema';
import {Symbols} from './symbols';



// In TypeScript 2.1 the spread element kind was renamed.
const spreadElementSyntaxKind: ts.SyntaxKind =
(ts.SyntaxKind as any).SpreadElement || (ts.SyntaxKind as any).SpreadElementExpression;
Expand All @@ -38,6 +39,24 @@ function isCallOf(callExpression: ts.CallExpression, ident: string): boolean {
return false;
}

/* @internal */
export function recordMapEntry<T extends MetadataEntry>(
entry: T, node: ts.Node,
nodeMap: Map<MetadataValue|ClassMetadata|InterfaceMetadata|FunctionMetadata, ts.Node>,
sourceFile?: ts.SourceFile) {
if (!nodeMap.has(entry)) {
nodeMap.set(entry, node);
if (node && (isMetadataImportedSymbolReferenceExpression(entry) ||
isMetadataImportDefaultReference(entry)) &&
entry.line == null) {
const info = sourceInfo(node, sourceFile);
if (info.line != null) entry.line = info.line;
if (info.character != null) entry.character = info.character;
}
}
return entry;
}

/**
* ts.forEachChild stops iterating children when the callback return a truthy value.
* This method inverts this to implement an `every` style iterator. It will return
Expand Down Expand Up @@ -77,21 +96,22 @@ function getSourceFileOfNode(node: ts.Node | undefined): ts.SourceFile {
}

/* @internal */
export function errorSymbol(
message: string, node?: ts.Node, context?: {[name: string]: string},
sourceFile?: ts.SourceFile): MetadataError {
let result: MetadataError|undefined = undefined;
export function sourceInfo(
node: ts.Node | undefined, sourceFile: ts.SourceFile | undefined): MetadataSourceLocationInfo {
if (node) {
sourceFile = sourceFile || getSourceFileOfNode(node);
if (sourceFile) {
const {line, character} =
ts.getLineAndCharacterOfPosition(sourceFile, node.getStart(sourceFile));
result = {__symbolic: 'error', message, line, character};
return ts.getLineAndCharacterOfPosition(sourceFile, node.getStart(sourceFile));
}
}
if (!result) {
result = {__symbolic: 'error', message};
}
return {};
}

/* @internal */
export function errorSymbol(
message: string, node?: ts.Node, context?: {[name: string]: string},
sourceFile?: ts.SourceFile): MetadataError {
const result: MetadataError = {__symbolic: 'error', message, ...sourceInfo(node, sourceFile)};
if (context) {
result.context = context;
}
Expand Down Expand Up @@ -242,8 +262,7 @@ export class Evaluator {
}
entry = newEntry;
}
t.nodeMap.set(entry, node);
return entry;
return recordMapEntry(entry, node, t.nodeMap);
}

function isFoldableError(value: any): value is MetadataError {
Expand All @@ -256,6 +275,9 @@ export class Evaluator {
// Encode as a global reference. StaticReflector will check the reference.
return recordEntry({__symbolic: 'reference', name}, node);
}
if (reference && isMetadataSymbolicReferenceExpression(reference)) {
return recordEntry({...reference}, node);
}
return reference;
};

Expand Down Expand Up @@ -628,7 +650,7 @@ export class Evaluator {
return recordEntry({__symbolic: 'if', condition, thenExpression, elseExpression}, node);
case ts.SyntaxKind.FunctionExpression:
case ts.SyntaxKind.ArrowFunction:
return recordEntry(errorSymbol('Function call not supported', node), node);
return recordEntry(errorSymbol('Lambda not supported', node), node);
case ts.SyntaxKind.TaggedTemplateExpression:
return recordEntry(
errorSymbol('Tagged template expressions are not supported in metadata', node), node);
Expand Down
39 changes: 23 additions & 16 deletions packages/compiler-cli/src/metadata/schema.ts
View file
Expand Up @@ -178,7 +178,20 @@ export function isMetadataSymbolicIfExpression(value: any): value is MetadataSym
return value && value.__symbolic === 'if';
}

export interface MetadataGlobalReferenceExpression extends MetadataSymbolicExpression {
export interface MetadataSourceLocationInfo {
/**
* The line number of the error in the .ts file the metadata was created for.
*/
line?: number;

/**
* The number of utf8 code-units from the beginning of the file of the error.
*/
character?: number;
}

export interface MetadataGlobalReferenceExpression extends MetadataSymbolicExpression,
MetadataSourceLocationInfo {
__symbolic: 'reference';
name: string;
arguments?: MetadataValue[];
Expand All @@ -188,7 +201,8 @@ export function isMetadataGlobalReferenceExpression(value: any):
return value && value.name && !value.module && isMetadataSymbolicReferenceExpression(value);
}

export interface MetadataModuleReferenceExpression extends MetadataSymbolicExpression {
export interface MetadataModuleReferenceExpression extends MetadataSymbolicExpression,
MetadataSourceLocationInfo {
__symbolic: 'reference';
module: string;
}
Expand All @@ -198,7 +212,8 @@ export function isMetadataModuleReferenceExpression(value: any):
isMetadataSymbolicReferenceExpression(value);
}

export interface MetadataImportedSymbolReferenceExpression extends MetadataSymbolicExpression {
export interface MetadataImportedSymbolReferenceExpression extends MetadataSymbolicExpression,
MetadataSourceLocationInfo {
__symbolic: 'reference';
module: string;
name: string;
Expand All @@ -209,7 +224,8 @@ export function isMetadataImportedSymbolReferenceExpression(value: any):
return value && value.module && !!value.name && isMetadataSymbolicReferenceExpression(value);
}

export interface MetadataImportedDefaultReferenceExpression extends MetadataSymbolicExpression {
export interface MetadataImportedDefaultReferenceExpression extends MetadataSymbolicExpression,
MetadataSourceLocationInfo {
__symbolic: 'reference';
module: string;
default:
Expand All @@ -218,7 +234,7 @@ export interface MetadataImportedDefaultReferenceExpression extends MetadataSymb
}
export function isMetadataImportDefaultReference(value: any):
value is MetadataImportedDefaultReferenceExpression {
return value.module && value.default && isMetadataSymbolicReferenceExpression(value);
return value && value.module && value.default && isMetadataSymbolicReferenceExpression(value);
}

export type MetadataSymbolicReferenceExpression = MetadataGlobalReferenceExpression |
Expand Down Expand Up @@ -248,7 +264,7 @@ export function isMetadataSymbolicSpreadExpression(value: any):
return value && value.__symbolic === 'spread';
}

export interface MetadataError {
export interface MetadataError extends MetadataSourceLocationInfo {
__symbolic: 'error';

/**
Expand All @@ -259,16 +275,6 @@ export interface MetadataError {
*/
message: string;

/**
* The line number of the error in the .ts file the metadata was created for.
*/
line?: number;

/**
* The number of utf8 code-units from the beginning of the file of the error.
*/
character?: number;

/**
* The module of the error (only used in bundled metadata)
*/
Expand All @@ -280,6 +286,7 @@ export interface MetadataError {
*/
context?: {[name: string]: string};
}

export function isMetadataError(value: any): value is MetadataError {
return value && value.__symbolic === 'error';
}
80 changes: 63 additions & 17 deletions packages/compiler-cli/src/perform_compile.ts
View file
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {isSyntaxError, syntaxError} from '@angular/compiler';
import {Position, isSyntaxError, syntaxError} from '@angular/compiler';
import * as fs from 'fs';
import * as path from 'path';
import * as ts from 'typescript';
Expand All @@ -29,30 +29,76 @@ const defaultFormatHost: ts.FormatDiagnosticsHost = {
getNewLine: () => ts.sys.newLine
};

function displayFileName(fileName: string, host: ts.FormatDiagnosticsHost): string {
return path.relative(host.getCurrentDirectory(), host.getCanonicalFileName(fileName));
}

export function formatDiagnosticPosition(
position: Position, host: ts.FormatDiagnosticsHost = defaultFormatHost): string {
return `${displayFileName(position.fileName, host)}(${position.line + 1},${position.column+1})`;
}

export function flattenDiagnosticMessageChain(
chain: api.DiagnosticMessageChain, host: ts.FormatDiagnosticsHost = defaultFormatHost): string {
let result = chain.messageText;
let indent = 1;
let current = chain.next;
const newLine = host.getNewLine();
while (current) {
result += newLine;
for (let i = 0; i < indent; i++) {
result += ' ';
}
result += current.messageText;
const position = current.position;
if (position) {
result += ` at ${formatDiagnosticPosition(position, host)}`;
}
current = current.next;
indent++;
}
return result;
}

export function formatDiagnostic(
diagnostic: api.Diagnostic, host: ts.FormatDiagnosticsHost = defaultFormatHost) {
let result = '';
const newLine = host.getNewLine();
const span = diagnostic.span;
if (span) {
result += `${formatDiagnosticPosition({
fileName: span.start.file.url,
line: span.start.line,
column: span.start.col
}, host)}: `;
} else if (diagnostic.position) {
result += `${formatDiagnosticPosition(diagnostic.position, host)}: `;
}
if (diagnostic.span && diagnostic.span.details) {
result += `: ${diagnostic.span.details}, ${diagnostic.messageText}${newLine}`;
} else if (diagnostic.chain) {
result += `${flattenDiagnosticMessageChain(diagnostic.chain, host)}.${newLine}`;
} else {
result += `: ${diagnostic.messageText}${newLine}`;
}
return result;
}

export function formatDiagnostics(
diags: Diagnostics, tsFormatHost: ts.FormatDiagnosticsHost = defaultFormatHost): string {
diags: Diagnostics, host: ts.FormatDiagnosticsHost = defaultFormatHost): string {
if (diags && diags.length) {
return diags
.map(d => {
if (api.isTsDiagnostic(d)) {
return ts.formatDiagnostics([d], tsFormatHost);
.map(diagnostic => {
if (api.isTsDiagnostic(diagnostic)) {
return ts.formatDiagnostics([diagnostic], host);
} else {
let res = ts.DiagnosticCategory[d.category];
if (d.span) {
res +=
` at ${d.span.start.file.url}(${d.span.start.line + 1},${d.span.start.col + 1})`;
}
if (d.span && d.span.details) {
res += `: ${d.span.details}, ${d.messageText}\n`;
} else {
res += `: ${d.messageText}\n`;
}
return res;
return formatDiagnostic(diagnostic, host);
}
})
.join('');
} else
} else {
return '';
}
}

export interface ParsedConfiguration {
Expand Down
10 changes: 9 additions & 1 deletion packages/compiler-cli/src/transformers/api.ts
View file
Expand Up @@ -6,16 +6,24 @@
* found in the LICENSE file at https://angular.io/license
*/

import {GeneratedFile, ParseSourceSpan} from '@angular/compiler';
import {GeneratedFile, ParseSourceSpan, Position} from '@angular/compiler';
import * as ts from 'typescript';

export const DEFAULT_ERROR_CODE = 100;
export const UNKNOWN_ERROR_CODE = 500;
export const SOURCE = 'angular' as 'angular';

export interface DiagnosticMessageChain {
messageText: string;
position?: Position;
next?: DiagnosticMessageChain;
}

export interface Diagnostic {
messageText: string;
span?: ParseSourceSpan;
position?: Position;
chain?: DiagnosticMessageChain;
category: ts.DiagnosticCategory;
code: number;
source: 'angular';
Expand Down

0 comments on commit d87a51c

Please sign in to comment.