From 8ecda94899e6751960545a80339f16629e7673bb Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Tue, 14 Nov 2017 17:49:47 -0800 Subject: [PATCH] feat(compiler-cli): improve error messages produced during structural errors (#20459) 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. PR Close #20459 --- packages/compiler-cli/src/main.ts | 1 - .../compiler-cli/src/metadata/collector.ts | 7 +- .../compiler-cli/src/metadata/evaluator.ts | 50 ++- packages/compiler-cli/src/metadata/schema.ts | 39 +- packages/compiler-cli/src/perform_compile.ts | 80 +++- packages/compiler-cli/src/transformers/api.ts | 10 +- .../compiler-cli/src/transformers/program.ts | 114 +++-- .../test/metadata/collector_spec.ts | 208 +++++++-- .../test/metadata/evaluator_spec.ts | 9 +- packages/compiler-cli/test/ngc_spec.ts | 51 ++- .../compiler-cli/test/perform_watch_spec.ts | 2 +- .../test/transformers/program_spec.ts | 7 +- packages/compiler/src/aot/formatted_error.ts | 60 +++ packages/compiler/src/aot/static_reflector.ts | 423 +++++++++++++----- .../src/aot/static_symbol_resolver.ts | 46 +- packages/compiler/src/compiler.ts | 1 + packages/compiler/test/aot/compiler_spec.ts | 30 +- .../test/aot/static_reflector_spec.ts | 259 ++++++++++- .../test/aot/static_symbol_resolver_spec.ts | 20 +- packages/language-service/src/diagnostics.ts | 4 +- packages/language-service/src/ts_plugin.ts | 22 +- packages/language-service/src/types.ts | 33 +- .../language-service/src/typescript_host.ts | 30 +- .../language-service/test/diagnostics_spec.ts | 16 +- packages/language-service/test/test_utils.ts | 33 +- 25 files changed, 1247 insertions(+), 308 deletions(-) create mode 100644 packages/compiler/src/aot/formatted_error.ts diff --git a/packages/compiler-cli/src/main.ts b/packages/compiler-cli/src/main.ts index 593345060a..476f78dd10 100644 --- a/packages/compiler-cli/src/main.ts +++ b/packages/compiler-cli/src/main.ts @@ -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, diff --git a/packages/compiler-cli/src/metadata/collector.ts b/packages/compiler-cli/src/metadata/collector.ts index ddca6a3380..8ba2d307b4 100644 --- a/packages/compiler-cli/src/metadata/collector.ts +++ b/packages/compiler-cli/src/metadata/collector.ts @@ -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} 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; @@ -76,8 +76,7 @@ export class MetadataCollector { } function recordEntry(entry: T, node: ts.Node): T { - nodeMap.set(entry, node); - return entry; + return recordMapEntry(entry, node, nodeMap, sourceFile); } function errorSym( diff --git a/packages/compiler-cli/src/metadata/evaluator.ts b/packages/compiler-cli/src/metadata/evaluator.ts index db02169a2b..273db143c2 100644 --- a/packages/compiler-cli/src/metadata/evaluator.ts +++ b/packages/compiler-cli/src/metadata/evaluator.ts @@ -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; @@ -38,6 +39,24 @@ function isCallOf(callExpression: ts.CallExpression, ident: string): boolean { return false; } +/* @internal */ +export function recordMapEntry( + entry: T, node: ts.Node, + nodeMap: Map, + 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 @@ -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; } @@ -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 { @@ -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; }; @@ -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); diff --git a/packages/compiler-cli/src/metadata/schema.ts b/packages/compiler-cli/src/metadata/schema.ts index 68f193db48..a5ca3fce74 100644 --- a/packages/compiler-cli/src/metadata/schema.ts +++ b/packages/compiler-cli/src/metadata/schema.ts @@ -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[]; @@ -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; } @@ -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; @@ -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: @@ -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 | @@ -248,7 +264,7 @@ export function isMetadataSymbolicSpreadExpression(value: any): return value && value.__symbolic === 'spread'; } -export interface MetadataError { +export interface MetadataError extends MetadataSourceLocationInfo { __symbolic: 'error'; /** @@ -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) */ @@ -280,6 +286,7 @@ export interface MetadataError { */ context?: {[name: string]: string}; } + export function isMetadataError(value: any): value is MetadataError { return value && value.__symbolic === 'error'; } diff --git a/packages/compiler-cli/src/perform_compile.ts b/packages/compiler-cli/src/perform_compile.ts index f3f419eec7..8135298d4f 100644 --- a/packages/compiler-cli/src/perform_compile.ts +++ b/packages/compiler-cli/src/perform_compile.ts @@ -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'; @@ -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 { diff --git a/packages/compiler-cli/src/transformers/api.ts b/packages/compiler-cli/src/transformers/api.ts index 12f757180d..d7e6f8addf 100644 --- a/packages/compiler-cli/src/transformers/api.ts +++ b/packages/compiler-cli/src/transformers/api.ts @@ -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'; diff --git a/packages/compiler-cli/src/transformers/program.ts b/packages/compiler-cli/src/transformers/program.ts index 878dbb6d6f..07188165be 100644 --- a/packages/compiler-cli/src/transformers/program.ts +++ b/packages/compiler-cli/src/transformers/program.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AotCompiler, AotCompilerHost, AotCompilerOptions, EmitterVisitorContext, GeneratedFile, MessageBundle, NgAnalyzedFile, NgAnalyzedModules, ParseSourceSpan, Serializer, TypeScriptEmitter, Xliff, Xliff2, Xmb, core, createAotCompiler, getParseErrors, isSyntaxError} from '@angular/compiler'; +import {AotCompiler, AotCompilerHost, AotCompilerOptions, EmitterVisitorContext, FormattedMessageChain, GeneratedFile, MessageBundle, NgAnalyzedFile, NgAnalyzedModules, ParseSourceSpan, Position, Serializer, TypeScriptEmitter, Xliff, Xliff2, Xmb, core, createAotCompiler, getParseErrors, isFormattedError, isSyntaxError} from '@angular/compiler'; import * as fs from 'fs'; import * as path from 'path'; import * as ts from 'typescript'; @@ -14,14 +14,13 @@ import * as ts from 'typescript'; import {TypeCheckHost, translateDiagnostics} from '../diagnostics/translate_diagnostics'; import {ModuleMetadata, createBundleIndexHost} from '../metadata/index'; -import {CompilerHost, CompilerOptions, CustomTransformers, DEFAULT_ERROR_CODE, Diagnostic, EmitFlags, LazyRoute, LibrarySummary, Program, SOURCE, TsEmitArguments, TsEmitCallback} from './api'; +import {CompilerHost, CompilerOptions, CustomTransformers, DEFAULT_ERROR_CODE, Diagnostic, DiagnosticMessageChain, EmitFlags, LazyRoute, LibrarySummary, Program, SOURCE, TsEmitArguments, TsEmitCallback} from './api'; import {CodeGenerator, TsCompilerAotCompilerTypeCheckHostAdapter, getOriginalReferences} from './compiler_host'; import {LowerMetadataCache, getExpressionLoweringTransformFactory} from './lower_expressions'; import {getAngularEmitterTransformFactory} from './node_emitter_transform'; import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, isInRootDir, ngToTsDiagnostic, tsStructureIsReused} from './util'; - /** * Maximum number of files that are emitable via calling ts.Program.emit * passing individual targetSourceFiles. @@ -378,10 +377,12 @@ class AngularCompilerProgram implements Program { } private get structuralDiagnostics(): Diagnostic[] { - if (!this._structuralDiagnostics) { + let diagnostics = this._structuralDiagnostics; + if (!diagnostics) { this.initSync(); + diagnostics = (this._structuralDiagnostics = this._structuralDiagnostics || []); } - return this._structuralDiagnostics !; + return diagnostics; } private get tsProgram(): ts.Program { @@ -430,16 +431,9 @@ class AngularCompilerProgram implements Program { this.rootNames, this.options, this.host, this.metadataCache, codegen, this.oldProgramLibrarySummaries); const aotOptions = getAotCompilerOptions(this.options); - this._structuralDiagnostics = []; - const errorCollector = - (this.options.collectAllErrors || this.options.fullTemplateTypeCheck) ? (err: any) => { - this._structuralDiagnostics !.push({ - messageText: err.toString(), - category: ts.DiagnosticCategory.Error, - source: SOURCE, - code: DEFAULT_ERROR_CODE - }); - } : undefined; + const errorCollector = (this.options.collectAllErrors || this.options.fullTemplateTypeCheck) ? + (err: any) => this._addStructuralDiagnostics(err) : + undefined; this._compiler = createAotCompiler(this._hostAdapter, aotOptions, errorCollector).compiler; } @@ -522,33 +516,26 @@ class AngularCompilerProgram implements Program { this._hostAdapter.isSourceFile = () => false; this._tsProgram = ts.createProgram(this.rootNames, this.options, this.hostAdapter); if (isSyntaxError(e)) { - const parserErrors = getParseErrors(e); - if (parserErrors && parserErrors.length) { - this._structuralDiagnostics = [ - ...(this._structuralDiagnostics || []), - ...parserErrors.map(e => ({ - messageText: e.contextualMessage(), - category: ts.DiagnosticCategory.Error, - span: e.span, - source: SOURCE, - code: DEFAULT_ERROR_CODE - })) - ]; - } else { - this._structuralDiagnostics = [ - ...(this._structuralDiagnostics || []), { - messageText: e.message, - category: ts.DiagnosticCategory.Error, - source: SOURCE, - code: DEFAULT_ERROR_CODE - } - ]; - } + this._addStructuralDiagnostics(e); return; } throw e; } + private _addStructuralDiagnostics(error: Error) { + const diagnostics = this._structuralDiagnostics || (this._structuralDiagnostics = []); + if (isSyntaxError(error)) { + diagnostics.push(...syntaxErrorToDiagnostics(error)); + } else { + diagnostics.push({ + messageText: error.toString(), + category: ts.DiagnosticCategory.Error, + source: SOURCE, + code: DEFAULT_ERROR_CODE + }); + } + } + // Note: this returns a ts.Diagnostic so that we // can return errors in a ts.EmitResult private generateFilesForEmit(emitFlags: EmitFlags): @@ -843,3 +830,56 @@ function mergeEmitResults(emitResults: ts.EmitResult[]): ts.EmitResult { } return {diagnostics, emitSkipped, emittedFiles}; } + +function diagnosticSourceOfSpan(span: ParseSourceSpan): ts.SourceFile { + // For diagnostics, TypeScript only uses the fileName and text properties. + // The redundant '()' are here is to avoid having clang-format breaking the line incorrectly. + return ({ fileName: span.start.file.url, text: span.start.file.content } as any); +} + +function diagnosticSourceOfFileName(fileName: string, program: ts.Program): ts.SourceFile { + const sourceFile = program.getSourceFile(fileName); + if (sourceFile) return sourceFile; + + // If we are reporting diagnostics for a source file that is not in the project then we need + // to fake a source file so the diagnostic formatting routines can emit the file name. + // The redundant '()' are here is to avoid having clang-format breaking the line incorrectly. + return ({ fileName, text: '' } as any); +} + + +function diagnosticChainFromFormattedDiagnosticChain(chain: FormattedMessageChain): + DiagnosticMessageChain { + return { + messageText: chain.message, + next: chain.next && diagnosticChainFromFormattedDiagnosticChain(chain.next), + position: chain.position + }; +} + +function syntaxErrorToDiagnostics(error: Error): Diagnostic[] { + const parserErrors = getParseErrors(error); + if (parserErrors && parserErrors.length) { + return parserErrors.map(e => ({ + messageText: e.contextualMessage(), + file: diagnosticSourceOfSpan(e.span), + start: e.span.start.offset, + length: e.span.end.offset - e.span.start.offset, + category: ts.DiagnosticCategory.Error, + source: SOURCE, + code: DEFAULT_ERROR_CODE + })); + } else { + if (isFormattedError(error)) { + return [{ + messageText: error.message, + chain: error.chain && diagnosticChainFromFormattedDiagnosticChain(error.chain), + category: ts.DiagnosticCategory.Error, + source: SOURCE, + code: DEFAULT_ERROR_CODE, + position: error.position + }]; + } + } + return []; +} \ No newline at end of file diff --git a/packages/compiler-cli/test/metadata/collector_spec.ts b/packages/compiler-cli/test/metadata/collector_spec.ts index 1b9b97e9d4..e999922247 100644 --- a/packages/compiler-cli/test/metadata/collector_spec.ts +++ b/packages/compiler-cli/test/metadata/collector_spec.ts @@ -112,7 +112,13 @@ describe('Collector', () => { __symbolic: 'class', decorators: [{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Component', + line: 4, + character: 7 + }, arguments: [{ selector: 'my-hero-detail', template: ` @@ -132,8 +138,13 @@ describe('Collector', () => { __symbolic: 'property', decorators: [{ __symbolic: 'call', - expression: - {__symbolic: 'reference', module: 'angular2/core', name: 'Input'} + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Input', + line: 18, + character: 9 + } }] }] } @@ -153,7 +164,13 @@ describe('Collector', () => { __symbolic: 'class', decorators: [{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Component', + line: 9, + character: 7 + }, arguments: [{ selector: 'my-app', template: ` @@ -172,20 +189,52 @@ describe('Collector', () => { __symbolic: 'reference', module: './hero-detail.component', name: 'HeroDetailComponent', + line: 22, + character: 21 }, - {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor'} + { + __symbolic: 'reference', + module: 'angular2/common', + name: 'NgFor', + line: 22, + character: 42 + } ], - providers: [{__symbolic: 'reference', module: './hero.service', default: true}], + providers: [{ + __symbolic: 'reference', + module: './hero.service', + default: true, + line: 23, + character: 20 + }], pipes: [ - {__symbolic: 'reference', module: 'angular2/common', name: 'LowerCasePipe'}, - {__symbolic: 'reference', module: 'angular2/common', name: 'UpperCasePipe'} + { + __symbolic: 'reference', + module: 'angular2/common', + name: 'LowerCasePipe', + line: 24, + character: 16 + }, + { + __symbolic: 'reference', + module: 'angular2/common', + name: 'UpperCasePipe', + line: 24, + character: 38 + } ] }] }], members: { __ctor__: [{ __symbolic: 'constructor', - parameters: [{__symbolic: 'reference', module: './hero.service', default: true}] + parameters: [{ + __symbolic: 'reference', + module: './hero.service', + default: true, + line: 31, + character: 42 + }] }], onSelect: [{__symbolic: 'method'}], ngOnInit: [{__symbolic: 'method'}], @@ -236,22 +285,23 @@ describe('Collector', () => { }); it('should record annotations on set and get declarations', () => { - const propertyData = { + const propertyData = (line: number) => ({ name: [{ __symbolic: 'property', decorators: [{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Input'}, + expression: + {__symbolic: 'reference', module: 'angular2/core', name: 'Input', line, character: 9}, arguments: ['firstName'] }] }] - }; + }); const caseGetProp = casesMetadata.metadata['GetProp']; - expect(caseGetProp.members).toEqual(propertyData); + expect(caseGetProp.members).toEqual(propertyData(11)); const caseSetProp = casesMetadata.metadata['SetProp']; - expect(caseSetProp.members).toEqual(propertyData); + expect(caseSetProp.members).toEqual(propertyData(19)); const caseFullProp = casesMetadata.metadata['FullProp']; - expect(caseFullProp.members).toEqual(propertyData); + expect(caseFullProp.members).toEqual(propertyData(27)); }); it('should record references to parameterized types', () => { @@ -260,7 +310,13 @@ describe('Collector', () => { __symbolic: 'class', decorators: [{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Injectable'} + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Injectable', + line: 40, + character: 7 + } }], members: { __ctor__: [{ @@ -313,7 +369,7 @@ describe('Collector', () => { const ctor = someClass.members !['__ctor__'][0]; const parameters = ctor.parameters; expect(parameters).toEqual([ - {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor'} + {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor', line: 6, character: 29} ]); }); @@ -398,7 +454,7 @@ describe('Collector', () => { const ctor = someClass.members !['__ctor__'][0]; const parameters = ctor.parameters; expect(parameters).toEqual([ - {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor'} + {__symbolic: 'reference', module: 'angular2/common', name: 'NgFor', line: 6, character: 29} ]); }); @@ -427,7 +483,13 @@ describe('Collector', () => { B: 1, C: 30, D: 40, - E: {__symbolic: 'reference', module: './exported-consts', name: 'constValue'} + E: { + __symbolic: 'reference', + module: './exported-consts', + name: 'constValue', + line: 5, + character: 75 + } }); }); @@ -457,13 +519,25 @@ describe('Collector', () => { expect(classData).toBeDefined(); expect(classData.decorators).toEqual([{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Component', + line: 4, + character: 5 + }, arguments: [{ providers: { __symbolic: 'call', expression: { __symbolic: 'select', - expression: {__symbolic: 'reference', module: './static-method', name: 'MyModule'}, + expression: { + __symbolic: 'reference', + module: './static-method', + name: 'MyModule', + line: 5, + character: 17 + }, member: 'with' }, arguments: ['a'] @@ -489,13 +563,25 @@ describe('Collector', () => { expect(classData).toBeDefined(); expect(classData.decorators).toEqual([{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Component', + line: 4, + character: 5 + }, arguments: [{ providers: [{ provide: 'a', useValue: { __symbolic: 'select', - expression: {__symbolic: 'reference', module: './static-field', name: 'MyModule'}, + expression: { + __symbolic: 'reference', + module: './static-field', + name: 'MyModule', + line: 5, + character: 45 + }, member: 'VALUE' } }] @@ -578,8 +664,20 @@ describe('Collector', () => { const metadata = collector.getMetadata(source) !; expect(metadata.metadata).toEqual({ MyClass: Object({__symbolic: 'class'}), - OtherModule: {__symbolic: 'reference', module: './static-field-reference', name: 'Foo'}, - MyOtherModule: {__symbolic: 'reference', module: './static-field', name: 'MyModule'} + OtherModule: { + __symbolic: 'reference', + module: './static-field-reference', + name: 'Foo', + line: 4, + character: 12 + }, + MyOtherModule: { + __symbolic: 'reference', + module: './static-field', + name: 'MyModule', + line: 4, + character: 25 + } }); }); @@ -598,7 +696,13 @@ describe('Collector', () => { __symbolic: 'class', decorators: [{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Component', + line: 11, + character: 5 + }, arguments: [{providers: [{__symbolic: 'reference', name: 'REQUIRED_VALIDATOR'}]}] }] } @@ -620,7 +724,13 @@ describe('Collector', () => { __symbolic: 'class', decorators: [{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Component'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Component', + line: 11, + character: 5 + }, arguments: [{providers: [{__symbolic: 'reference', name: 'REQUIRED_VALIDATOR'}]}] }] } @@ -653,7 +763,13 @@ describe('Collector', () => { __symbolic: 'constructor', parameterDecorators: [[{ __symbolic: 'call', - expression: {__symbolic: 'reference', module: 'angular2/core', name: 'Inject'}, + expression: { + __symbolic: 'reference', + module: 'angular2/core', + name: 'Inject', + line: 6, + character: 19 + }, arguments: ['a'] }]], parameters: [{__symbolic: 'reference', name: 'any'}] @@ -687,13 +803,20 @@ describe('Collector', () => { __symbolic: 'reference', module: './external', name: 'external', + line: 0, + character: 68, } }); }); it('should simplify a redundant template', () => { - e('`${external}`', 'import {external} from "./external";') - .toEqual({__symbolic: 'reference', module: './external', name: 'external'}); + e('`${external}`', 'import {external} from "./external";').toEqual({ + __symbolic: 'reference', + module: './external', + name: 'external', + line: 0, + character: 59 + }); }); it('should be able to collect complex template with imported references', () => { @@ -710,11 +833,18 @@ describe('Collector', () => { __symbolic: 'binop', operator: '+', left: 'foo:', - right: {__symbolic: 'reference', module: './external', name: 'foo'} + right: { + __symbolic: 'reference', + module: './external', + name: 'foo', + line: 0, + character: 63 + } }, right: ', bar:' }, - right: {__symbolic: 'reference', module: './external', name: 'bar'} + right: + {__symbolic: 'reference', module: './external', name: 'bar', line: 0, character: 75} }, right: ', end' }); @@ -741,11 +871,11 @@ describe('Collector', () => { __ctor__: [{ __symbolic: 'constructor', parameters: [ - {__symbolic: 'reference', module: './foo', name: 'Foo'}, - {__symbolic: 'reference', module: './foo', name: 'Foo'}, - {__symbolic: 'reference', module: './foo', name: 'Foo'}, - {__symbolic: 'reference', module: './foo', name: 'Foo'}, - {__symbolic: 'reference', module: './foo', name: 'Foo'} + {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24}, + {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24}, + {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24}, + {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24}, + {__symbolic: 'reference', module: './foo', name: 'Foo', line: 3, character: 24} ] }] }); @@ -825,7 +955,9 @@ describe('Collector', () => { extends: { __symbolic: 'reference', module: './class-inheritance-parent', - name: 'ParentClassFromOtherFile' + name: 'ParentClassFromOtherFile', + line: 9, + character: 45, } }); }); diff --git a/packages/compiler-cli/test/metadata/evaluator_spec.ts b/packages/compiler-cli/test/metadata/evaluator_spec.ts index d097e1955d..b08492dda4 100644 --- a/packages/compiler-cli/test/metadata/evaluator_spec.ts +++ b/packages/compiler-cli/test/metadata/evaluator_spec.ts @@ -149,12 +149,14 @@ describe('Evaluator', () => { const newExpression = program.getSourceFile('newExpression.ts'); expect(evaluator.evaluateNode(findVarInitializer(newExpression, 'someValue'))).toEqual({ __symbolic: 'new', - expression: {__symbolic: 'reference', name: 'Value', module: './classes'}, + expression: + {__symbolic: 'reference', name: 'Value', module: './classes', line: 4, character: 33}, arguments: ['name', 12] }); expect(evaluator.evaluateNode(findVarInitializer(newExpression, 'complex'))).toEqual({ __symbolic: 'new', - expression: {__symbolic: 'reference', name: 'Value', module: './classes'}, + expression: + {__symbolic: 'reference', name: 'Value', module: './classes', line: 5, character: 42}, arguments: ['name', 12] }); }); @@ -173,8 +175,7 @@ describe('Evaluator', () => { const errors = program.getSourceFile('errors.ts'); const fDecl = findVar(errors, 'f') !; expect(evaluator.evaluateNode(fDecl.initializer !)) - .toEqual( - {__symbolic: 'error', message: 'Function call not supported', line: 1, character: 12}); + .toEqual({__symbolic: 'error', message: 'Lambda not supported', line: 1, character: 12}); const eDecl = findVar(errors, 'e') !; expect(evaluator.evaluateNode(eDecl.type !)).toEqual({ __symbolic: 'error', diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index e92ee8ba8b..07afa4e23c 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -184,8 +184,7 @@ describe('ngc transformer command-line', () => { const exitCode = main(['-p', basePath], errorSpy); expect(errorSpy).toHaveBeenCalledTimes(1); - expect(errorSpy.calls.mostRecent().args[0]) - .toContain('Error at ' + path.join(basePath, 'mymodule.ts.MyComp.html')); + expect(errorSpy.calls.mostRecent().args[0]).toContain('mymodule.ts.MyComp.html'); expect(errorSpy.calls.mostRecent().args[0]) .toContain(`Property 'unknownProp' does not exist on type 'MyComp'`); @@ -215,8 +214,7 @@ describe('ngc transformer command-line', () => { const exitCode = main(['-p', basePath], errorSpy); expect(errorSpy).toHaveBeenCalledTimes(1); - expect(errorSpy.calls.mostRecent().args[0]) - .toContain('Error at ' + path.join(basePath, 'my.component.html(1,5):')); + expect(errorSpy.calls.mostRecent().args[0]).toContain('my.component.html(1,5):'); expect(errorSpy.calls.mostRecent().args[0]) .toContain(`Property 'unknownProp' does not exist on type 'MyComp'`); @@ -1566,4 +1564,49 @@ describe('ngc transformer command-line', () => { expect(main(['-p', path.join(basePath, 'src/tsconfig.json')])).toBe(0); }); }); + + describe('formatted messages', () => { + it('should emit a formatted error message for a structural error', () => { + write('src/tsconfig.json', `{ + "extends": "../tsconfig-base.json", + "files": ["test-module.ts"] + }`); + write('src/lib/indirect2.ts', ` + declare var f: any; + + export const t2 = f\`

hello

\`; + `); + write('src/lib/indirect1.ts', ` + import {t2} from './indirect2'; + export const t1 = t2 + ' '; + `); + write('src/lib/test.component.ts', ` + import {Component} from '@angular/core'; + import {t1} from './indirect1'; + + @Component({ + template: t1, + styleUrls: ['./test.component.css'] + }) + export class TestComponent {} + `); + write('src/test-module.ts', ` + import {NgModule} from '@angular/core'; + import {TestComponent} from './lib/test.component'; + + @NgModule({declarations: [TestComponent]}) + export class TestModule {} + `); + const messages: string[] = []; + const exitCode = + main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message)); + expect(exitCode).toBe(1, 'Compile was expected to fail'); + expect(messages[0]) + .toEqual(`lib/test.component.ts(6,21): Error during template compile of 'TestComponent' + Tagged template expressions are not supported in metadata in 't1' + 't1' references 't2' at lib/indirect1.ts(3,27) + 't2' contains the error at lib/indirect2.ts(4,27). +`); + }); + }); }); diff --git a/packages/compiler-cli/test/perform_watch_spec.ts b/packages/compiler-cli/test/perform_watch_spec.ts index d436ed4dda..5d30f371e6 100644 --- a/packages/compiler-cli/test/perform_watch_spec.ts +++ b/packages/compiler-cli/test/perform_watch_spec.ts @@ -143,7 +143,7 @@ describe('perform watch', () => { const errDiags = host.diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error); expect(errDiags.length).toBe(1); - expect(errDiags[0].messageText).toContain('Function calls are not supported.'); + expect(errDiags[0].messageText).toContain('Function expressions are not supported'); } }); }); diff --git a/packages/compiler-cli/test/transformers/program_spec.ts b/packages/compiler-cli/test/transformers/program_spec.ts index 2c64be1f56..61fdf7791e 100644 --- a/packages/compiler-cli/test/transformers/program_spec.ts +++ b/packages/compiler-cli/test/transformers/program_spec.ts @@ -930,7 +930,7 @@ describe('ng program', () => { const structuralErrors = program.getNgStructuralDiagnostics(); expect(structuralErrors.length).toBe(1); - expect(structuralErrors[0].messageText).toContain('Function calls are not supported.'); + expect(structuralErrors[0].messageText).toContain('Function expressions are not supported'); }); it('should not throw on structural errors but collect them (loadNgStructureAsync)', (done) => { @@ -943,7 +943,7 @@ describe('ng program', () => { program.loadNgStructureAsync().then(() => { const structuralErrors = program.getNgStructuralDiagnostics(); expect(structuralErrors.length).toBe(1); - expect(structuralErrors[0].messageText).toContain('Function calls are not supported.'); + expect(structuralErrors[0].messageText).toContain('Function expressions are not supported'); done(); }); }); @@ -982,7 +982,8 @@ describe('ng program', () => { const program = ng.createProgram({rootNames: allRootNames, options, host}); const structuralErrors = program.getNgStructuralDiagnostics(); expect(structuralErrors.length).toBe(1); - expect(structuralErrors[0].messageText).toContain('Function calls are not supported.'); + expect(structuralErrors[0].messageText) + .toContain('Function expressions are not supported'); }); }); }); diff --git a/packages/compiler/src/aot/formatted_error.ts b/packages/compiler/src/aot/formatted_error.ts new file mode 100644 index 0000000000..ffe2d1f416 --- /dev/null +++ b/packages/compiler/src/aot/formatted_error.ts @@ -0,0 +1,60 @@ +/** + * @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 {syntaxError} from '../util'; + +export interface Position { + fileName: string; + line: number; + column: number; +} + +export interface FormattedMessageChain { + message: string; + position?: Position; + next?: FormattedMessageChain; +} + +export type FormattedError = Error & { + chain: FormattedMessageChain; + position?: Position; +}; + +const FORMATTED_MESSAGE = 'ngFormattedMessage'; + +function indentStr(level: number): string { + if (level <= 0) return ''; + if (level < 6) return ['', ' ', ' ', ' ', ' ', ' '][level]; + const half = indentStr(Math.floor(level / 2)); + return half + half + (level % 2 === 1 ? ' ' : ''); +} + +function formatChain(chain: FormattedMessageChain | undefined, indent: number = 0): string { + if (!chain) return ''; + const position = chain.position ? + `${chain.position.fileName}(${chain.position.line+1},${chain.position.column+1})` : + ''; + const prefix = position && indent === 0 ? `${position}: ` : ''; + const postfix = position && indent !== 0 ? ` at ${position}` : ''; + const message = `${prefix}${chain.message}${postfix}`; + + return `${indentStr(indent)}${message}${(chain.next && ('\n' + formatChain(chain.next, indent + 2))) || ''}`; +} + +export function formattedError(chain: FormattedMessageChain): FormattedError { + const message = formatChain(chain) + '.'; + const error = syntaxError(message) as FormattedError; + (error as any)[FORMATTED_MESSAGE] = true; + error.chain = chain; + error.position = chain.position; + return error; +} + +export function isFormattedError(error: Error): error is FormattedError { + return !!(error as any)[FORMATTED_MESSAGE]; +} diff --git a/packages/compiler/src/aot/static_reflector.ts b/packages/compiler/src/aot/static_reflector.ts index 5300134cc1..7f20150036 100644 --- a/packages/compiler/src/aot/static_reflector.ts +++ b/packages/compiler/src/aot/static_reflector.ts @@ -13,6 +13,7 @@ import * as o from '../output/output_ast'; import {SummaryResolver} from '../summary_resolver'; import {syntaxError} from '../util'; +import {FormattedMessageChain, formattedError} from './formatted_error'; import {StaticSymbol} from './static_symbol'; import {StaticSymbolResolver} from './static_symbol_resolver'; @@ -98,11 +99,16 @@ export class StaticReflector implements CompileReflector { findSymbolDeclaration(symbol: StaticSymbol): StaticSymbol { const resolvedSymbol = this.symbolResolver.resolveSymbol(symbol); - if (resolvedSymbol && resolvedSymbol.metadata instanceof StaticSymbol) { - return this.findSymbolDeclaration(resolvedSymbol.metadata); - } else { - return symbol; + if (resolvedSymbol) { + let resolvedMetadata = resolvedSymbol.metadata; + if (resolvedMetadata && resolvedMetadata.__symbolic === 'resolved') { + resolvedMetadata = resolvedMetadata.symbol; + } + if (resolvedMetadata instanceof StaticSymbol) { + return this.findSymbolDeclaration(resolvedSymbol.metadata); + } } + return symbol; } public annotations(type: StaticSymbol): any[] { @@ -130,9 +136,12 @@ export class StaticReflector implements CompileReflector { (requiredType) => ownAnnotations.some(ann => requiredType.isTypeOf(ann))); if (!typeHasRequiredAnnotation) { this.reportError( - syntaxError( - `Class ${type.name} in ${type.filePath} extends from a ${CompileSummaryKind[summary.type.summaryKind!]} in another compilation unit without duplicating the decorator. ` + - `Please add a ${requiredAnnotationTypes.map((type) => type.ngMetadataName).join(' or ')} decorator to the class.`), + formatMetadataError( + metadataError( + `Class ${type.name} in ${type.filePath} extends from a ${CompileSummaryKind[summary.type.summaryKind!]} in another compilation unit without duplicating the decorator`, + /* summary */ undefined, + `Please add a ${requiredAnnotationTypes.map((type) => type.ngMetadataName).join(' or ')} decorator to the class`), + type), type); } } @@ -334,14 +343,6 @@ export class StaticReflector implements CompileReflector { return this.symbolResolver.getStaticSymbol(declarationFile, name, members); } - private reportError(error: Error, context: StaticSymbol, path?: string) { - if (this.errorRecorder) { - this.errorRecorder(error, (context && context.filePath) || path); - } else { - throw error; - } - } - /** * Simplify but discard any errors */ @@ -358,6 +359,7 @@ export class StaticReflector implements CompileReflector { const self = this; let scope = BindingScope.empty; const calling = new Map(); + const rootContext = context; function simplifyInContext( context: StaticSymbol, value: any, depth: number, references: number): any { @@ -366,17 +368,64 @@ export class StaticReflector implements CompileReflector { return resolvedSymbol ? resolvedSymbol.metadata : null; } - function simplifyCall(functionSymbol: StaticSymbol, targetFunction: any, args: any[]) { + function simplifyEagerly(value: any): any { + return simplifyInContext(context, value, depth, 0); + } + + function simplifyLazily(value: any): any { + return simplifyInContext(context, value, depth, references + 1); + } + + function simplifyNested(nestedContext: StaticSymbol, value: any): any { + if (nestedContext === context) { + // If the context hasn't changed let the exception propagate unmodified. + return simplifyInContext(nestedContext, value, depth + 1, references); + } + try { + return simplifyInContext(nestedContext, value, depth + 1, references); + } catch (e) { + if (isMetadataError(e)) { + // Propagate the message text up but add a message to the chain that explains how we got + // here. + // e.chain implies e.symbol + const summaryMsg = e.chain ? 'references \'' + e.symbol !.name + '\'' : errorSummary(e); + const summary = `'${nestedContext.name}' ${summaryMsg}`; + const chain = {message: summary, position: e.position, next: e.chain}; + // TODO(chuckj): retrieve the position information indirectly from the collectors node + // map if the metadata is from a .ts file. + self.error( + { + message: e.message, + advise: e.advise, + context: e.context, chain, + symbol: nestedContext + }, + context); + } else { + // It is probably an internal error. + throw e; + } + } + } + + function simplifyCall( + functionSymbol: StaticSymbol, targetFunction: any, args: any[], targetExpression: any) { if (targetFunction && targetFunction['__symbolic'] == 'function') { if (calling.get(functionSymbol)) { - throw new Error('Recursion not supported'); + self.error( + { + message: 'Recursion is not supported', + summary: `called '${functionSymbol.name}' recursively`, + value: targetFunction + }, + functionSymbol); } try { const value = targetFunction['value']; if (value && (depth != 0 || value.__symbolic != 'error')) { const parameters: string[] = targetFunction['parameters']; const defaults: any[] = targetFunction.defaults; - args = args.map(arg => simplifyInContext(context, arg, depth + 1, references)) + args = args.map(arg => simplifyNested(context, arg)) .map(arg => shouldIgnore(arg) ? undefined : arg); if (defaults && defaults.length > args.length) { args.push(...defaults.slice(args.length).map((value: any) => simplify(value))); @@ -390,7 +439,7 @@ export class StaticReflector implements CompileReflector { let result: any; try { scope = functionScope.done(); - result = simplifyInContext(functionSymbol, value, depth + 1, references); + result = simplifyNested(functionSymbol, value); } finally { scope = oldScope; } @@ -407,8 +456,22 @@ export class StaticReflector implements CompileReflector { // non-angular decorator, and we should just ignore it. return IGNORE; } - return simplify( - {__symbolic: 'error', message: 'Function call not supported', context: functionSymbol}); + let position: Position|undefined = undefined; + if (targetExpression && targetExpression.__symbolic == 'resolved') { + const line = targetExpression.line; + const character = targetExpression.character; + const fileName = targetExpression.fileName; + if (fileName != null && line != null && character != null) { + position = {fileName, line, column: character}; + } + } + self.error( + { + message: FUNCTION_CALL_NOT_SUPPORTED, + context: functionSymbol, + value: targetFunction, position + }, + context); } function simplify(expression: any): any { @@ -422,7 +485,7 @@ export class StaticReflector implements CompileReflector { if (item && item.__symbolic === 'spread') { // We call with references as 0 because we require the actual value and cannot // tolerate a reference here. - const spreadArray = simplifyInContext(context, item.expression, depth, 0); + const spreadArray = simplifyEagerly(item.expression); if (Array.isArray(spreadArray)) { for (const spreadItem of spreadArray) { result.push(spreadItem); @@ -448,7 +511,7 @@ export class StaticReflector implements CompileReflector { const staticSymbol = expression; const declarationValue = resolveReferenceValue(staticSymbol); if (declarationValue != null) { - return simplifyInContext(staticSymbol, declarationValue, depth + 1, references); + return simplifyNested(staticSymbol, declarationValue); } else { return staticSymbol; } @@ -525,8 +588,8 @@ export class StaticReflector implements CompileReflector { } return null; case 'index': - let indexTarget = simplifyInContext(context, expression['expression'], depth, 0); - let index = simplifyInContext(context, expression['index'], depth, 0); + let indexTarget = simplifyEagerly(expression['expression']); + let index = simplifyEagerly(expression['index']); if (indexTarget && isPrimitive(index)) return indexTarget[index]; return null; case 'select': @@ -539,26 +602,41 @@ export class StaticReflector implements CompileReflector { self.getStaticSymbol(selectTarget.filePath, selectTarget.name, members); const declarationValue = resolveReferenceValue(selectContext); if (declarationValue != null) { - return simplifyInContext( - selectContext, declarationValue, depth + 1, references); + return simplifyNested(selectContext, declarationValue); } else { return selectContext; } } if (selectTarget && isPrimitive(member)) - return simplifyInContext( - selectContext, selectTarget[member], depth + 1, references); + return simplifyNested(selectContext, selectTarget[member]); return null; case 'reference': - // Note: This only has to deal with variable references, - // as symbol references have been converted into StaticSymbols already - // in the StaticSymbolResolver! + // Note: This only has to deal with variable references, as symbol references have + // been converted into 'resolved' + // in the StaticSymbolResolver. const name: string = expression['name']; const localValue = scope.resolve(name); if (localValue != BindingScope.missing) { return localValue; } break; + case 'resolved': + try { + return simplify(expression.symbol); + } catch (e) { + // If an error is reported evaluating the symbol record the position of the + // reference in the error so it can + // be reported in the error message generated from the exception. + if (isMetadataError(e) && expression.fileName != null && + expression.line != null && expression.character != null) { + e.position = { + fileName: expression.fileName, + line: expression.line, + column: expression.character + }; + } + throw e; + } case 'class': return context; case 'function': @@ -580,29 +658,34 @@ export class StaticReflector implements CompileReflector { const argExpressions: any[] = expression['arguments'] || []; let converter = self.conversionMap.get(staticSymbol); if (converter) { - const args = - argExpressions - .map(arg => simplifyInContext(context, arg, depth + 1, references)) - .map(arg => shouldIgnore(arg) ? undefined : arg); + const args = argExpressions.map(arg => simplifyNested(context, arg)) + .map(arg => shouldIgnore(arg) ? undefined : arg); return converter(context, args); } else { // Determine if the function is one we can simplify. const targetFunction = resolveReferenceValue(staticSymbol); - return simplifyCall(staticSymbol, targetFunction, argExpressions); + return simplifyCall( + staticSymbol, targetFunction, argExpressions, expression['expression']); } } return IGNORE; case 'error': - let message = produceErrorMessage(expression); - if (expression['line']) { - message = - `${message} (position ${expression['line']+1}:${expression['character']+1} in the original .ts file)`; - self.reportError( - positionalError( - message, context.filePath, expression['line'], expression['character']), + let message = expression.message; + if (expression['line'] != null) { + self.error( + { + message, + context: expression.context, + value: expression, + position: { + fileName: expression['fileName'], + line: expression['line'], + column: expression['character'] + } + }, context); } else { - self.reportError(new Error(message), context); + self.error({message, context: expression.context}, context); } return IGNORE; case 'ignore': @@ -620,7 +703,7 @@ export class StaticReflector implements CompileReflector { return simplify(value); } } - return simplifyInContext(context, value, depth, references + 1); + return simplifyLazily(value); } return simplify(value); }); @@ -628,29 +711,19 @@ export class StaticReflector implements CompileReflector { return IGNORE; } - try { - return simplify(value); - } catch (e) { - const members = context.members.length ? `.${context.members.join('.')}` : ''; - const message = - `${e.message}, resolving symbol ${context.name}${members} in ${context.filePath}`; - if (e.fileName) { - throw positionalError(message, e.fileName, e.line, e.column); - } - throw syntaxError(message); - } + return simplify(value); } - const recordedSimplifyInContext = (context: StaticSymbol, value: any) => { - try { - return simplifyInContext(context, value, 0, 0); - } catch (e) { + let result: any; + try { + result = simplifyInContext(context, value, 0, 0); + } catch (e) { + if (this.errorRecorder) { this.reportError(e, context); + } else { + throw formatMetadataError(e, context); } - }; - - const result = this.errorRecorder ? recordedSimplifyInContext(context, value) : - simplifyInContext(context, value, 0, 0); + } if (shouldIgnore(result)) { return undefined; } @@ -662,40 +735,166 @@ export class StaticReflector implements CompileReflector { return resolvedSymbol && resolvedSymbol.metadata ? resolvedSymbol.metadata : {__symbolic: 'class'}; } -} -function expandedMessage(error: any): string { - switch (error.message) { - case 'Reference to non-exported class': - if (error.context && error.context.className) { - return `Reference to a non-exported class ${error.context.className}. Consider exporting the class`; - } - break; - case 'Variable not initialized': - return 'Only initialized variables and constants can be referenced because the value of this variable is needed by the template compiler'; - case 'Destructuring not supported': - return 'Referencing an exported destructured variable or constant is not supported by the template compiler. Consider simplifying this to avoid destructuring'; - case 'Could not resolve type': - if (error.context && error.context.typeName) { - return `Could not resolve type ${error.context.typeName}`; - } - break; - case 'Function call not supported': - let prefix = - error.context && error.context.name ? `Calling function '${error.context.name}', f` : 'F'; - return prefix + - 'unction calls are not supported. Consider replacing the function or lambda with a reference to an exported function'; - case 'Reference to a local symbol': - if (error.context && error.context.name) { - return `Reference to a local (non-exported) symbol '${error.context.name}'. Consider exporting the symbol`; - } - break; + private reportError(error: Error, context: StaticSymbol, path?: string) { + if (this.errorRecorder) { + this.errorRecorder( + formatMetadataError(error, context), (context && context.filePath) || path); + } else { + throw error; + } + } + + private error( + {message, summary, advise, position, context, value, symbol, chain}: { + message: string, + summary?: string, + advise?: string, + position?: Position, + context?: any, + value?: any, + symbol?: StaticSymbol, + chain?: MetadataMessageChain + }, + reportingContext: StaticSymbol) { + this.reportError( + metadataError(message, summary, advise, position, symbol, context, chain), + reportingContext); } - return error.message; } -function produceErrorMessage(error: any): string { - return `Error encountered resolving symbol values statically. ${expandedMessage(error)}`; +interface Position { + fileName: string; + line: number; + column: number; +} + +interface MetadataMessageChain { + message: string; + summary?: string; + position?: Position; + context?: any; + symbol?: StaticSymbol; + next?: MetadataMessageChain; +} + +type MetadataError = Error & { + position?: Position; + advise?: string; + summary?: string; + context?: any; + symbol?: StaticSymbol; + chain?: MetadataMessageChain; +}; + +const METADATA_ERROR = 'ngMetadataError'; + +function metadataError( + message: string, summary?: string, advise?: string, position?: Position, symbol?: StaticSymbol, + context?: any, chain?: MetadataMessageChain): MetadataError { + const error = syntaxError(message) as MetadataError; + (error as any)[METADATA_ERROR] = true; + if (advise) error.advise = advise; + if (position) error.position = position; + if (summary) error.summary = summary; + if (context) error.context = context; + if (chain) error.chain = chain; + if (symbol) error.symbol = symbol; + return error; +} + +function isMetadataError(error: Error): error is MetadataError { + return !!(error as any)[METADATA_ERROR]; +} + +const REFERENCE_TO_NONEXPORTED_CLASS = 'Reference to non-exported class'; +const VARIABLE_NOT_INITIALIZED = 'Variable not initialized'; +const DESTRUCTURE_NOT_SUPPORTED = 'Destructuring not supported'; +const COULD_NOT_RESOLVE_TYPE = 'Could not resolve type'; +const FUNCTION_CALL_NOT_SUPPORTED = 'Function call not supported'; +const REFERENCE_TO_LOCAL_SYMBOL = 'Reference to a local symbol'; +const LAMBDA_NOT_SUPPORTED = 'Lambda not supported'; + +function expandedMessage(message: string, context: any): string { + switch (message) { + case REFERENCE_TO_NONEXPORTED_CLASS: + if (context && context.className) { + return `References to a non-exported class are not supported in decorators but ${context.className} was referenced.`; + } + break; + case VARIABLE_NOT_INITIALIZED: + return 'Only initialized variables and constants can be referenced in decorators because the value of this variable is needed by the template compiler'; + case DESTRUCTURE_NOT_SUPPORTED: + return 'Referencing an exported destructured variable or constant is not supported in decorators and this value is needed by the template compiler'; + case COULD_NOT_RESOLVE_TYPE: + if (context && context.typeName) { + return `Could not resolve type ${context.typeName}`; + } + break; + case FUNCTION_CALL_NOT_SUPPORTED: + if (context && context.name) { + return `Function calls are not supported in decorators but '${context.name}' was called`; + } + return 'Function calls are not supported in decorators'; + case REFERENCE_TO_LOCAL_SYMBOL: + if (context && context.name) { + return `Reference to a local (non-exported) symbols are not supported in decorators but '${context.name}' was referenced`; + } + break; + case LAMBDA_NOT_SUPPORTED: + return `Function expressions are not supported in decorators`; + } + return message; +} + +function messageAdvise(message: string, context: any): string|undefined { + switch (message) { + case REFERENCE_TO_NONEXPORTED_CLASS: + if (context && context.className) { + return `Consider exporting '${context.className}'`; + } + break; + case DESTRUCTURE_NOT_SUPPORTED: + return 'Consider simplifying to avoid destructuring'; + case REFERENCE_TO_LOCAL_SYMBOL: + if (context && context.name) { + return `Consider exporting '${context.name}'`; + } + break; + case LAMBDA_NOT_SUPPORTED: + return `Consider changing the function expression into an exported function`; + } + return undefined; +} + +function errorSummary(error: MetadataError): string { + if (error.summary) { + return error.summary; + } + switch (error.message) { + case REFERENCE_TO_NONEXPORTED_CLASS: + if (error.context && error.context.className) { + return `references non-exported class ${error.context.className}`; + } + break; + case VARIABLE_NOT_INITIALIZED: + return 'is not initialized'; + case DESTRUCTURE_NOT_SUPPORTED: + return 'is a destructured variable'; + case COULD_NOT_RESOLVE_TYPE: + return 'could not be resolved'; + case FUNCTION_CALL_NOT_SUPPORTED: + if (error.context && error.context.name) { + return `calls '${error.context.name}'`; + } + return `calls a function`; + case REFERENCE_TO_LOCAL_SYMBOL: + if (error.context && error.context.name) { + return `references local variable ${error.context.name}`; + } + return `references a local variable`; + } + return 'contains the error'; } function mapStringMap(input: {[key: string]: any}, transform: (value: any, key: string) => any): @@ -751,10 +950,30 @@ class PopulatedScope extends BindingScope { } } -function positionalError(message: string, fileName: string, line: number, column: number): Error { - const result = syntaxError(message); - (result as any).fileName = fileName; - (result as any).line = line; - (result as any).column = column; - return result; -} \ No newline at end of file +function formatMetadataMessageChain( + chain: MetadataMessageChain, advise: string | undefined): FormattedMessageChain { + const expanded = expandedMessage(chain.message, chain.context); + const nesting = chain.symbol ? ` in '${chain.symbol.name}'` : ''; + const message = `${expanded}${nesting}`; + const position = chain.position; + const next: FormattedMessageChain|undefined = chain.next ? + formatMetadataMessageChain(chain.next, advise) : + advise ? {message: advise} : undefined; + return {message, position, next}; +} + +function formatMetadataError(e: Error, context: StaticSymbol): Error { + if (isMetadataError(e)) { + // Produce a formatted version of the and leaving enough information in the original error + // to recover the formatting information to eventually produce a diagnostic error message. + const position = e.position; + const chain: MetadataMessageChain = { + message: `Error during template compile of '${context.name}'`, + position: position, + next: {message: e.message, next: e.chain, context: e.context, symbol: e.symbol} + }; + const advise = e.advise || messageAdvise(e.message, e.context); + return formattedError(formatMetadataMessageChain(chain, advise)); + } + return e; +} diff --git a/packages/compiler/src/aot/static_symbol_resolver.ts b/packages/compiler/src/aot/static_symbol_resolver.ts index cb4241d5e8..2ae58e28c3 100644 --- a/packages/compiler/src/aot/static_symbol_resolver.ts +++ b/packages/compiler/src/aot/static_symbol_resolver.ts @@ -146,9 +146,9 @@ export class StaticSymbolResolver { if (isGeneratedFile(staticSymbol.filePath)) { return null; } - let resolvedSymbol = this.resolveSymbol(staticSymbol); + let resolvedSymbol = unwrapResolvedMetadata(this.resolveSymbol(staticSymbol)); while (resolvedSymbol && resolvedSymbol.metadata instanceof StaticSymbol) { - resolvedSymbol = this.resolveSymbol(resolvedSymbol.metadata); + resolvedSymbol = unwrapResolvedMetadata(this.resolveSymbol(resolvedSymbol.metadata)); } return (resolvedSymbol && resolvedSymbol.metadata && resolvedSymbol.metadata.arity) || null; } @@ -204,7 +204,7 @@ export class StaticSymbolResolver { if (!baseResolvedSymbol) { return null; } - const baseMetadata = baseResolvedSymbol.metadata; + let baseMetadata = unwrapResolvedMetadata(baseResolvedSymbol.metadata); if (baseMetadata instanceof StaticSymbol) { return new ResolvedStaticSymbol( staticSymbol, this.getStaticSymbol(baseMetadata.filePath, baseMetadata.name, members)); @@ -374,6 +374,19 @@ export class StaticSymbolResolver { return new ResolvedStaticSymbol(sourceSymbol, transformedMeta); } + let _originalFileMemo: string|undefined; + const getOriginalName: () => string = () => { + if (!_originalFileMemo) { + // Guess what hte original file name is from the reference. If it has a `.d.ts` extension + // replace it with `.ts`. If it already has `.ts` just leave it in place. If it doesn't have + // .ts or .d.ts, append `.ts'. Also, if it is in `node_modules`, trim the `node_module` + // location as it is not important to finding the file. + _originalFileMemo = + topLevelPath.replace(/((\.ts)|(\.d\.ts)|)$/, '.ts').replace(/^.*node_modules[/\\]/, ''); + } + return _originalFileMemo; + }; + const self = this; class ReferenceTransformer extends ValueTransformer { @@ -397,10 +410,19 @@ export class StaticSymbolResolver { if (!filePath) { return { __symbolic: 'error', - message: `Could not resolve ${module} relative to ${sourceSymbol.filePath}.` + message: `Could not resolve ${module} relative to ${sourceSymbol.filePath}.`, + line: map.line, + character: map.character, + fileName: getOriginalName() }; } - return self.getStaticSymbol(filePath, name); + return { + __symbolic: 'resolved', + symbol: self.getStaticSymbol(filePath, name), + line: map.line, + character: map.character, + fileName: getOriginalName() + }; } else if (functionParams.indexOf(name) >= 0) { // reference to a function parameter return {__symbolic: 'reference', name: name}; @@ -411,14 +433,17 @@ export class StaticSymbolResolver { // ambient value null; } + } else if (symbolic === 'error') { + return {...map, fileName: getOriginalName()}; } else { return super.visitStringMap(map, functionParams); } } } const transformedMeta = visitValue(metadata, new ReferenceTransformer(), []); - if (transformedMeta instanceof StaticSymbol) { - return this.createExport(sourceSymbol, transformedMeta); + let unwrappedTransformedMeta = unwrapResolvedMetadata(transformedMeta); + if (unwrappedTransformedMeta instanceof StaticSymbol) { + return this.createExport(sourceSymbol, unwrappedTransformedMeta); } return new ResolvedStaticSymbol(sourceSymbol, transformedMeta); } @@ -505,3 +530,10 @@ export class StaticSymbolResolver { export function unescapeIdentifier(identifier: string): string { return identifier.startsWith('___') ? identifier.substr(1) : identifier; } + +export function unwrapResolvedMetadata(metadata: any): any { + if (metadata && metadata.__symbolic === 'resolved') { + return metadata.symbol; + } + return metadata; +} \ No newline at end of file diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index 2cb46f6170..8f8bf4a973 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -35,6 +35,7 @@ export * from './aot/compiler'; export * from './aot/generated_file'; export * from './aot/compiler_options'; export * from './aot/compiler_host'; +export * from './aot/formatted_error'; export * from './aot/static_reflector'; export * from './aot/static_symbol'; export * from './aot/static_symbol_resolver'; diff --git a/packages/compiler/test/aot/compiler_spec.ts b/packages/compiler/test/aot/compiler_spec.ts index 6cb2dcda1a..82de7df5ce 100644 --- a/packages/compiler/test/aot/compiler_spec.ts +++ b/packages/compiler/test/aot/compiler_spec.ts @@ -768,9 +768,9 @@ describe('compiler (unbundled Angular)', () => { childClassDecorator: '', childModuleDecorator: '@NgModule({providers: [Extends]})', })) - .toThrowError( - 'Class Extends in /app/main.ts extends from a Injectable in another compilation unit without duplicating the decorator. ' + - 'Please add a Injectable or Pipe or Directive or Component or NgModule decorator to the class.'); + .toThrowError(`Error during template compile of 'Extends' + Class Extends in /app/main.ts extends from a Injectable in another compilation unit without duplicating the decorator + Please add a Injectable or Pipe or Directive or Component or NgModule decorator to the class.`); }); }); @@ -792,9 +792,9 @@ describe('compiler (unbundled Angular)', () => { childClassDecorator: '', childModuleDecorator: '@NgModule({declarations: [Extends]})', })) - .toThrowError( - 'Class Extends in /app/main.ts extends from a Directive in another compilation unit without duplicating the decorator. ' + - 'Please add a Directive or Component decorator to the class.'); + .toThrowError(`Error during template compile of 'Extends' + Class Extends in /app/main.ts extends from a Directive in another compilation unit without duplicating the decorator + Please add a Directive or Component decorator to the class.`); }); }); @@ -816,9 +816,9 @@ describe('compiler (unbundled Angular)', () => { childClassDecorator: '', childModuleDecorator: '@NgModule({declarations: [Extends]})', })) - .toThrowError( - 'Class Extends in /app/main.ts extends from a Directive in another compilation unit without duplicating the decorator. ' + - 'Please add a Directive or Component decorator to the class.'); + .toThrowError(`Error during template compile of 'Extends' + Class Extends in /app/main.ts extends from a Directive in another compilation unit without duplicating the decorator + Please add a Directive or Component decorator to the class.`); }); }); @@ -840,9 +840,9 @@ describe('compiler (unbundled Angular)', () => { childClassDecorator: '', childModuleDecorator: '@NgModule({declarations: [Extends]})', })) - .toThrowError( - 'Class Extends in /app/main.ts extends from a Pipe in another compilation unit without duplicating the decorator. ' + - 'Please add a Pipe decorator to the class.'); + .toThrowError(`Error during template compile of 'Extends' + Class Extends in /app/main.ts extends from a Pipe in another compilation unit without duplicating the decorator + Please add a Pipe decorator to the class.`); }); }); @@ -864,9 +864,9 @@ describe('compiler (unbundled Angular)', () => { childClassDecorator: '', childModuleDecorator: '', })) - .toThrowError( - 'Class Extends in /app/main.ts extends from a NgModule in another compilation unit without duplicating the decorator. ' + - 'Please add a NgModule decorator to the class.'); + .toThrowError(`Error during template compile of 'Extends' + Class Extends in /app/main.ts extends from a NgModule in another compilation unit without duplicating the decorator + Please add a NgModule decorator to the class.`); }); }); } diff --git a/packages/compiler/test/aot/static_reflector_spec.ts b/packages/compiler/test/aot/static_reflector_spec.ts index 5403fb8b89..156f72ee8d 100644 --- a/packages/compiler/test/aot/static_reflector_spec.ts +++ b/packages/compiler/test/aot/static_reflector_spec.ts @@ -107,8 +107,11 @@ describe('StaticReflector', () => { it('should provide context for errors reported by the collector', () => { const SomeClass = reflector.findDeclaration('src/error-reporting', 'SomeClass'); expect(() => reflector.annotations(SomeClass)) - .toThrow(new Error( - 'Error encountered resolving symbol values statically. A reasonable error message (position 13:34 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')); + .toThrow(new Error(`Error during template compile of 'SomeClass' + A reasonable error message in 'Link1' + 'Link1' references 'Link2' + 'Link2' references 'ErrorSym' + 'ErrorSym' contains the error at /tmp/src/error-references.ts(13,34).`)); }); it('should simplify primitive into itself', () => { @@ -330,10 +333,12 @@ describe('StaticReflector', () => { it('should error on direct recursive calls', () => { expect( () => simplify( - reflector.getStaticSymbol('/tmp/src/function-reference.ts', ''), + reflector.getStaticSymbol('/tmp/src/function-reference.ts', 'MyComp'), reflector.getStaticSymbol('/tmp/src/function-reference.ts', 'recursion'))) - .toThrow(new Error( - 'Recursion not supported, resolving symbol recursive in /tmp/src/function-recursive.d.ts, resolving symbol recursion in /tmp/src/function-reference.ts, resolving symbol in /tmp/src/function-reference.ts')); + .toThrow(new Error(`Error during template compile of 'MyComp' + Recursion is not supported in 'recursion' + 'recursion' references 'recursive' + 'recursive' called 'recursive' recursively.`)); }); it('should throw a SyntaxError without stack trace when the required resource cannot be resolved', @@ -345,8 +350,8 @@ describe('StaticReflector', () => { message: 'Could not resolve ./does-not-exist.component relative to /tmp/src/function-reference.ts' }))) - .toThrowError( - 'Error encountered resolving symbol values statically. Could not resolve ./does-not-exist.component relative to /tmp/src/function-reference.ts, resolving symbol AppModule in /tmp/src/function-reference.ts'); + .toThrowError(`Error during template compile of 'AppModule' + Could not resolve ./does-not-exist.component relative to /tmp/src/function-reference.ts.`); }); it('should record data about the error in the exception', () => { @@ -361,7 +366,7 @@ describe('StaticReflector', () => { simplify( reflector.getStaticSymbol('/tmp/src/invalid-metadata.ts', ''), classData.decorators[0]); } catch (e) { - expect(e.fileName).toBe('/tmp/src/invalid-metadata.ts'); + expect(e.position).toBeDefined(); threw = true; } expect(threw).toBe(true); @@ -370,10 +375,13 @@ describe('StaticReflector', () => { it('should error on indirect recursive calls', () => { expect( () => simplify( - reflector.getStaticSymbol('/tmp/src/function-reference.ts', ''), + reflector.getStaticSymbol('/tmp/src/function-reference.ts', 'MyComp'), reflector.getStaticSymbol('/tmp/src/function-reference.ts', 'indirectRecursion'))) - .toThrow(new Error( - 'Recursion not supported, resolving symbol indirectRecursion2 in /tmp/src/function-recursive.d.ts, resolving symbol indirectRecursion1 in /tmp/src/function-recursive.d.ts, resolving symbol indirectRecursion in /tmp/src/function-reference.ts, resolving symbol in /tmp/src/function-reference.ts')); + .toThrow(new Error(`Error during template compile of 'MyComp' + Recursion is not supported in 'indirectRecursion' + 'indirectRecursion' references 'indirectRecursion1' + 'indirectRecursion1' references 'indirectRecursion2' + 'indirectRecursion2' called 'indirectRecursion1' recursively.`)); }); it('should simplify a spread expression', () => { @@ -401,7 +409,8 @@ describe('StaticReflector', () => { () => reflector.annotations( reflector.getStaticSymbol('/tmp/src/invalid-calls.ts', 'MyComponent'))) .toThrow(new Error( - `Error encountered resolving symbol values statically. Calling function 'someFunction', function calls are not supported. Consider replacing the function or lambda with a reference to an exported function, resolving symbol MyComponent in /tmp/src/invalid-calls.ts, resolving symbol MyComponent in /tmp/src/invalid-calls.ts`)); + `/tmp/src/invalid-calls.ts(8,29): Error during template compile of 'MyComponent' + Function calls are not supported in decorators but 'someFunction' was called.`)); }); it('should be able to get metadata for a class containing a static method call', () => { @@ -962,7 +971,7 @@ describe('StaticReflector', () => { }); // Regression #18170 - it('should agressively evaluate enums selects', () => { + it('should eagerly evaluate enums selects', () => { const data = Object.create(DEFAULT_TEST_DATA); const file = '/tmp/src/my_component.ts'; data[file] = ` @@ -1078,6 +1087,228 @@ describe('StaticReflector', () => { expect(symbolResolver.getKnownModuleName(symbol.filePath)).toBe('a'); }); }); + + describe('formatted error reporting', () => { + describe('function calls', () => { + const fileName = '/tmp/src/invalid/components.ts'; + beforeEach(() => { + const localData = { + '/tmp/src/invalid/function-call.ts': ` + import {functionToCall} from 'some-module'; + export const CALL_FUNCTION = functionToCall(); + `, + '/tmp/src/invalid/indirect.ts': ` + import {CALL_FUNCTION} from './function-call'; + + export const INDIRECT_CALL_FUNCTION = CALL_FUNCTION + 1; + `, + '/tmp/src/invalid/two-levels-indirect.ts': ` + import {INDIRECT_CALL_FUNCTION} from './indirect'; + + export const TWO_LEVELS_INDIRECT_CALL_FUNCTION = INDIRECT_CALL_FUNCTION + 1; + `, + '/tmp/src/invalid/components.ts': ` + import {functionToCall} from 'some-module'; + import {Component} from '@angular/core'; + import {CALL_FUNCTION} from './function-call'; + import {INDIRECT_CALL_FUNCTION} from './indirect'; + import {TWO_LEVELS_INDIRECT_CALL_FUNCTION} from './two-levels-indirect'; + + @Component({ + value: functionToCall() + }) + export class CallImportedFunction {} + + @Component({ + value: CALL_FUNCTION + }) + export class ReferenceCalledFunction {} + + @Component({ + value: INDIRECT_CALL_FUNCTION + }) + export class IndirectReferenceCalledFunction {} + + @Component({ + value: TWO_LEVELS_INDIRECT_CALL_FUNCTION + }) + export class TwoLevelsIndirectReferenceCalledFunction {} + ` + }; + init({...DEFAULT_TEST_DATA, ...localData}); + }); + + it('should report a formatted error for a direct function call', () => { + expect(() => { + return reflector.annotations(reflector.getStaticSymbol(fileName, 'CallImportedFunction')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(9,18): Error during template compile of 'CallImportedFunction' + Function calls are not supported in decorators but 'functionToCall' was called.`); + }); + + it('should report a formatted error for a refernce to a function call', () => { + expect(() => { + return reflector.annotations( + reflector.getStaticSymbol(fileName, 'ReferenceCalledFunction')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(14,18): Error during template compile of 'ReferenceCalledFunction' + Function calls are not supported in decorators but 'functionToCall' was called in 'CALL_FUNCTION' + 'CALL_FUNCTION' calls 'functionToCall' at /tmp/src/invalid/function-call.ts(3,38).`); + }); + + it('should report a formatted error for an indirect reference to a function call', () => { + expect(() => { + return reflector.annotations( + reflector.getStaticSymbol(fileName, 'IndirectReferenceCalledFunction')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(19,18): Error during template compile of 'IndirectReferenceCalledFunction' + Function calls are not supported in decorators but 'functionToCall' was called in 'INDIRECT_CALL_FUNCTION' + 'INDIRECT_CALL_FUNCTION' references 'CALL_FUNCTION' at /tmp/src/invalid/indirect.ts(4,47) + 'CALL_FUNCTION' calls 'functionToCall' at /tmp/src/invalid/function-call.ts(3,38).`); + }); + + it('should report a formatted error for a double-indirect refernce to a function call', () => { + expect(() => { + return reflector.annotations( + reflector.getStaticSymbol(fileName, 'TwoLevelsIndirectReferenceCalledFunction')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(24,18): Error during template compile of 'TwoLevelsIndirectReferenceCalledFunction' + Function calls are not supported in decorators but 'functionToCall' was called in 'TWO_LEVELS_INDIRECT_CALL_FUNCTION' + 'TWO_LEVELS_INDIRECT_CALL_FUNCTION' references 'INDIRECT_CALL_FUNCTION' at /tmp/src/invalid/two-levels-indirect.ts(4,58) + 'INDIRECT_CALL_FUNCTION' references 'CALL_FUNCTION' at /tmp/src/invalid/indirect.ts(4,47) + 'CALL_FUNCTION' calls 'functionToCall' at /tmp/src/invalid/function-call.ts(3,38).`); + }); + }); + + describe('macro functions', () => { + const fileName = '/tmp/src/invalid/components.ts'; + beforeEach(() => { + const localData = { + '/tmp/src/invalid/function-call.ts': ` + import {functionToCall} from 'some-module'; + export const CALL_FUNCTION = functionToCall(); + `, + '/tmp/src/invalid/indirect.ts': ` + import {CALL_FUNCTION} from './function-call'; + + export const INDIRECT_CALL_FUNCTION = CALL_FUNCTION + 1; + `, + '/tmp/src/invalid/macros.ts': ` + export function someMacro(value: any) { + return [ { provide: 'key', value: value } ]; + } + `, + '/tmp/src/invalid/components.ts': ` + import {Component} from '@angular/core'; + import {functionToCall} from 'some-module'; + import {someMacro} from './macros'; + import {CALL_FUNCTION} from './function-call'; + import {INDIRECT_CALL_FUNCTION} from './indirect'; + + @Component({ + template: someMacro(functionToCall()) + }) + export class DirectCall {} + + @Component({ + template: someMacro(CALL_FUNCTION) + }) + export class IndirectCall {} + + @Component({ + template: someMacro(INDIRECT_CALL_FUNCTION) + }) + export class DoubleIndirectCall {} + ` + }; + init({...DEFAULT_TEST_DATA, ...localData}); + }); + + it('should report a formatted error for a direct function call', () => { + expect(() => { + return reflector.annotations(reflector.getStaticSymbol(fileName, 'DirectCall')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(9,31): Error during template compile of 'DirectCall' + Function calls are not supported in decorators but 'functionToCall' was called.`); + }); + + it('should report a formatted error for a reference to a function call', () => { + expect(() => { + return reflector.annotations(reflector.getStaticSymbol(fileName, 'IndirectCall')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(14,31): Error during template compile of 'IndirectCall' + Function calls are not supported in decorators but 'functionToCall' was called in 'CALL_FUNCTION' + 'CALL_FUNCTION' calls 'functionToCall' at /tmp/src/invalid/function-call.ts(3,38).`); + }); + + it('should report a formatted error for an indirect refernece to a function call', () => { + expect(() => { + return reflector.annotations(reflector.getStaticSymbol(fileName, 'DoubleIndirectCall')); + }) + .toThrowError( + `/tmp/src/invalid/components.ts(19,31): Error during template compile of 'DoubleIndirectCall' + Function calls are not supported in decorators but 'functionToCall' was called in 'INDIRECT_CALL_FUNCTION' + 'INDIRECT_CALL_FUNCTION' references 'CALL_FUNCTION' at /tmp/src/invalid/indirect.ts(4,47) + 'CALL_FUNCTION' calls 'functionToCall' at /tmp/src/invalid/function-call.ts(3,38).`); + }); + }); + + describe('and give advice', () => { + // If in a reference expression, advice the user to replace with a reference. + const fileName = '/tmp/src/invalid/components.ts'; + + function collectError(symbol: string): string { + try { + reflector.annotations(reflector.getStaticSymbol(fileName, symbol)); + } catch (e) { + return e.message; + } + fail('Expected an exception to be thrown'); + return ''; + } + + function initWith(content: string) { + init({ + ...DEFAULT_TEST_DATA, + [fileName]: `import {Component} from '@angular/core';\n${content}` + }); + } + + it('should advise exorting a local', () => { + initWith(`const f: string; @Component({value: f}) export class MyComp {}`); + expect(collectError('MyComp')).toContain(`Consider exporting 'f'`); + }); + + it('should advise export a class', () => { + initWith('class Foo {} @Component({value: Foo}) export class MyComp {}'); + expect(collectError('MyComp')).toContain(`Consider exporting 'Foo'`); + }); + + it('should advise avoiding destructuring', () => { + initWith( + 'export const {foo, bar} = {foo: 1, bar: 2}; @Component({value: foo}) export class MyComp {}'); + expect(collectError('MyComp')).toContain(`Consider simplifying to avoid destructuring`); + }); + + it('should advise converting an arrow function into an exported function', () => { + initWith('@Component({value: () => true}) export class MyComp {}'); + expect(collectError('MyComp')) + .toContain(`Consider changing the function expression into an exported function`); + }); + + it('should advise converting a function expression into an exported function', () => { + initWith('@Component({value: function () { return true; }}) export class MyComp {}'); + expect(collectError('MyComp')) + .toContain(`Consider changing the function expression into an exported function`); + }); + }); + }); }); const DEFAULT_TEST_DATA: {[key: string]: any} = { @@ -1467,5 +1698,5 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { export class Dep { @Input f: Forward; } - ` + `, }; diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts index 1a6aa31eb6..2359d0fff9 100644 --- a/packages/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/packages/compiler/test/aot/static_symbol_resolver_spec.ts @@ -234,15 +234,25 @@ describe('StaticSymbolResolver', () => { }); expect(symbolResolver.resolveSymbol(symbolCache.get('/test.ts', 'a')).metadata) .toEqual(symbolCache.get('/test2.ts', 'b')); - expect(symbolResolver.resolveSymbol(symbolCache.get('/test.ts', 'x')).metadata).toEqual([ - symbolCache.get('/test2.ts', 'y') - ]); + expect(symbolResolver.resolveSymbol(symbolCache.get('/test.ts', 'x')).metadata).toEqual([{ + __symbolic: 'resolved', + symbol: symbolCache.get('/test2.ts', 'y'), + line: 3, + character: 24, + fileName: '/test.ts' + }]); expect(symbolResolver.resolveSymbol(symbolCache.get('/test.ts', 'simpleFn')).metadata).toEqual({ __symbolic: 'function', parameters: ['fnArg'], value: [ - symbolCache.get('/test.ts', 'a'), symbolCache.get('/test2.ts', 'y'), - Object({__symbolic: 'reference', name: 'fnArg'}) + symbolCache.get('/test.ts', 'a'), { + __symbolic: 'resolved', + symbol: symbolCache.get('/test2.ts', 'y'), + line: 6, + character: 21, + fileName: '/test.ts' + }, + {__symbolic: 'reference', name: 'fnArg'} ] }); }); diff --git a/packages/language-service/src/diagnostics.ts b/packages/language-service/src/diagnostics.ts index 23ec4316bf..755a7e1df9 100644 --- a/packages/language-service/src/diagnostics.ts +++ b/packages/language-service/src/diagnostics.ts @@ -10,7 +10,7 @@ import {NgAnalyzedModules, StaticSymbol} from '@angular/compiler'; import {DiagnosticTemplateInfo, getTemplateExpressionDiagnostics} from '@angular/compiler-cli/src/language_services'; import {AstResult} from './common'; -import {Declarations, Diagnostic, DiagnosticKind, Diagnostics, Span, TemplateSource} from './types'; +import {Declarations, Diagnostic, DiagnosticKind, DiagnosticMessageChain, Diagnostics, Span, TemplateSource} from './types'; import {offsetSpan, spanOf} from './utils'; export interface AstProvider { @@ -56,7 +56,7 @@ export function getDeclarationDiagnostics( let directives: Set|undefined = undefined; for (const declaration of declarations) { - const report = (message: string, span?: Span) => { + const report = (message: string | DiagnosticMessageChain, span?: Span) => { results.push({ kind: DiagnosticKind.Error, span: span || declaration.declarationSpan, message diff --git a/packages/language-service/src/ts_plugin.ts b/packages/language-service/src/ts_plugin.ts index cb1637e278..77f73fbbe1 100644 --- a/packages/language-service/src/ts_plugin.ts +++ b/packages/language-service/src/ts_plugin.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {createLanguageService} from './language_service'; -import {Completion, Diagnostic, LanguageService, LanguageServiceHost} from './types'; +import {Completion, Diagnostic, DiagnosticMessageChain, LanguageService, LanguageServiceHost} from './types'; import {TypeScriptServiceHost} from './typescript_host'; const projectHostMap = new WeakMap(); @@ -188,12 +188,30 @@ export function create(info: any /* ts.server.PluginCreateInfo */): ts.LanguageS }; } + function diagnosticChainToDiagnosticChain(chain: DiagnosticMessageChain): + ts.DiagnosticMessageChain { + return { + messageText: chain.message, + category: ts.DiagnosticCategory.Error, + code: 0, + next: chain.next ? diagnosticChainToDiagnosticChain(chain.next) : undefined + }; + } + + function diagnosticMessageToDiagnosticMessageText(message: string | DiagnosticMessageChain): + string|ts.DiagnosticMessageChain { + if (typeof message === 'string') { + return message; + } + return diagnosticChainToDiagnosticChain(message); + } + function diagnosticToDiagnostic(d: Diagnostic, file: ts.SourceFile): ts.Diagnostic { const result = { file, start: d.span.start, length: d.span.end - d.span.start, - messageText: d.message, + messageText: diagnosticMessageToDiagnosticMessageText(d.message), category: ts.DiagnosticCategory.Error, code: 0, source: 'ng' diff --git a/packages/language-service/src/types.ts b/packages/language-service/src/types.ts index 8ea1a5c535..2666d3bfa3 100644 --- a/packages/language-service/src/types.ts +++ b/packages/language-service/src/types.ts @@ -81,7 +81,7 @@ export type TemplateSources = TemplateSource[] | undefined; /** * Error information found getting declaration information * - * A host type; see `LanagueServiceHost`. + * A host type; see `LanguageServiceHost`. * * @experimental */ @@ -92,9 +92,10 @@ export interface DeclarationError { readonly span: Span; /** - * The message to display describing the error. + * The message to display describing the error or a chain + * of messages. */ - readonly message: string; + readonly message: string|DiagnosticMessageChain; } /** @@ -255,6 +256,28 @@ export enum DiagnosticKind { Warning, } +/** + * A template diagnostics message chain. This is similar to the TypeScript + * DiagnosticMessageChain. The messages are intended to be formatted as separate + * sentence fragments and indented. + * + * For compatiblity previous implementation, the values are expected to override + * toString() to return a formatted message. + * + * @experimental + */ +export interface DiagnosticMessageChain { + /** + * The text of the diagnostic message to display. + */ + message: string; + + /** + * The next message in the chain. + */ + next?: DiagnosticMessageChain; +} + /** * An template diagnostic message to display. * @@ -272,9 +295,9 @@ export interface Diagnostic { span: Span; /** - * The text of the diagnostic message to display. + * The text of the diagnostic message to display or a chain of messages. */ - message: string; + message: string|DiagnosticMessageChain; } /** diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index fce0b45791..a35ae27091 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AotSummaryResolver, CompileMetadataResolver, CompilerConfig, DEFAULT_INTERPOLATION_CONFIG, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, HtmlParser, InterpolationConfig, JitSummaryResolver, NgAnalyzedModules, NgModuleResolver, ParseTreeResult, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, SummaryResolver, analyzeNgModules, createOfflineCompileUrlResolver} from '@angular/compiler'; +import {AotSummaryResolver, CompileMetadataResolver, CompilerConfig, DEFAULT_INTERPOLATION_CONFIG, DirectiveNormalizer, DirectiveResolver, DomElementSchemaRegistry, FormattedError, FormattedMessageChain, HtmlParser, InterpolationConfig, JitSummaryResolver, NgAnalyzedModules, NgModuleResolver, ParseTreeResult, PipeResolver, ResourceLoader, StaticReflector, StaticSymbol, StaticSymbolCache, StaticSymbolResolver, SummaryResolver, analyzeNgModules, createOfflineCompileUrlResolver, isFormattedError} from '@angular/compiler'; import {CompilerOptions, getClassMembersFromDeclaration, getPipesTable, getSymbolQuery} from '@angular/compiler-cli/src/language_services'; import {ViewEncapsulation, ɵConsole as Console} from '@angular/core'; import * as fs from 'fs'; @@ -15,10 +15,11 @@ import * as ts from 'typescript'; import {createLanguageService} from './language_service'; import {ReflectorHost} from './reflector_host'; -import {BuiltinType, Declaration, DeclarationError, DeclarationKind, Declarations, Definition, LanguageService, LanguageServiceHost, PipeInfo, Pipes, Signature, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable, TemplateSource, TemplateSources} from './types'; +import {BuiltinType, Declaration, DeclarationError, DeclarationKind, Declarations, Definition, DiagnosticMessageChain, LanguageService, LanguageServiceHost, PipeInfo, Pipes, Signature, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable, TemplateSource, TemplateSources} from './types'; import {isTypescriptVersion} from './utils'; + /** * Create a `LanguageServiceHost` */ @@ -494,7 +495,13 @@ export class TypeScriptServiceHost implements LanguageServiceHost { private getCollectedErrors(defaultSpan: Span, sourceFile: ts.SourceFile): DeclarationError[] { const errors = (this.collectedErrors && this.collectedErrors.get(sourceFile.fileName)); return (errors && errors.map((e: any) => { - return {message: e.message, span: spanAt(sourceFile, e.line, e.column) || defaultSpan}; + const line = e.line || (e.position && e.position.line); + const column = e.column || (e.position && e.position.column); + const span = spanAt(sourceFile, line, column) || defaultSpan; + if (isFormattedError(e)) { + return errorToDiagnosticWithChain(e, span); + } + return {message: e.message, span}; })) || []; } @@ -599,3 +606,20 @@ function spanAt(sourceFile: ts.SourceFile, line: number, column: number): Span|u } } } + +function chainedMessage(chain: DiagnosticMessageChain, indent = ''): string { + return indent + chain.message + (chain.next ? chainedMessage(chain.next, indent + ' ') : ''); +} + +class DiagnosticMessageChainImpl implements DiagnosticMessageChain { + constructor(public message: string, public next?: DiagnosticMessageChain) {} + toString(): string { return chainedMessage(this); } +} + +function convertChain(chain: FormattedMessageChain): DiagnosticMessageChain { + return {message: chain.message, next: chain.next ? convertChain(chain.next) : undefined}; +} + +function errorToDiagnosticWithChain(error: FormattedError, span: Span): DeclarationError { + return {message: error.chain ? convertChain(error.chain) : error.message, span}; +} \ No newline at end of file diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts index db22fc3b58..eab1c1e6ac 100644 --- a/packages/language-service/test/diagnostics_spec.ts +++ b/packages/language-service/test/diagnostics_spec.ts @@ -13,7 +13,7 @@ import {Diagnostics} from '../src/types'; import {TypeScriptServiceHost} from '../src/typescript_host'; import {toh} from './test_data'; -import {MockTypescriptHost, includeDiagnostic, noDiagnostics} from './test_utils'; +import {MockTypescriptHost, diagnosticMessageContains, findDiagnostic, includeDiagnostic, noDiagnostics} from './test_utils'; describe('diagnostics', () => { let documentRegistry = ts.createDocumentRegistry(); @@ -123,7 +123,8 @@ describe('diagnostics', () => { addCode(code, (fileName, content) => { const diagnostics = ngService.getDiagnostics(fileName); includeDiagnostic( - diagnostics !, 'Function calls are not supported.', '() => \'foo\'', content); + diagnostics !, 'Function expressions are not supported in decorators', '() => \'foo\'', + content); }); }); @@ -168,8 +169,7 @@ describe('diagnostics', () => { const code = ` @Component({template: '

Using an invalid pipe {{data | dat}}

'}) export class MyComponent { data = 'some data'; }`; addCode(code, fileName => { - const diagnostic = - ngService.getDiagnostics(fileName) !.filter(d => d.message.indexOf('pipe') > 0)[0]; + const diagnostic = findDiagnostic(ngService.getDiagnostics(fileName) !, 'pipe') !; expect(diagnostic).not.toBeUndefined(); expect(diagnostic.span.end - diagnostic.span.start).toBeLessThan(11); }); @@ -216,8 +216,8 @@ describe('diagnostics', () => { `, fileName => { const diagnostics = ngService.getDiagnostics(fileName) !; - const expected = diagnostics.find(d => d.message.startsWith('Invalid providers for')); - const notExpected = diagnostics.find(d => d.message.startsWith('Cannot read property')); + const expected = findDiagnostic(diagnostics, 'Invalid providers for'); + const notExpected = findDiagnostic(diagnostics, 'Cannot read property'); expect(expected).toBeDefined(); expect(notExpected).toBeUndefined(); }); @@ -355,12 +355,12 @@ describe('diagnostics', () => { expect(diagnostics.length).toBe(1); if (diagnostics.length > 1) { for (const diagnostic of diagnostics) { - if (diagnostic.message.indexOf('MyComponent') >= 0) continue; + if (diagnosticMessageContains(diagnostic.message, 'MyComponent')) continue; fail(`(${diagnostic.span.start}:${diagnostic.span.end}): ${diagnostic.message}`); } return; } - expect(diagnostics[0].message.indexOf('MyComponent') >= 0).toBeTruthy(); + expect(diagnosticMessageContains(diagnostics[0].message, 'MyComponent')).toBeTruthy(); } }); }); diff --git a/packages/language-service/test/test_utils.ts b/packages/language-service/test/test_utils.ts index 4d31809b9f..0f2e246b98 100644 --- a/packages/language-service/test/test_utils.ts +++ b/packages/language-service/test/test_utils.ts @@ -10,7 +10,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as ts from 'typescript'; -import {Diagnostic, Diagnostics, Span} from '../src/types'; +import {Diagnostic, DiagnosticMessageChain, Diagnostics, Span} from '../src/types'; export type MockData = string | MockDirectory; @@ -317,6 +317,25 @@ export function noDiagnostics(diagnostics: Diagnostics) { } } +export function diagnosticMessageContains( + message: string | DiagnosticMessageChain, messageFragment: string): boolean { + if (typeof message == 'string') { + return message.indexOf(messageFragment) >= 0; + } + if (message.message.indexOf(messageFragment) >= 0) { + return true; + } + if (message.next) { + return diagnosticMessageContains(message.next, messageFragment); + } + return false; +} + +export function findDiagnostic(diagnostics: Diagnostic[], messageFragment: string): Diagnostic| + undefined { + return diagnostics.find(d => diagnosticMessageContains(d.message, messageFragment)); +} + export function includeDiagnostic( diagnostics: Diagnostics, message: string, text?: string, len?: string): void; export function includeDiagnostic( @@ -324,14 +343,18 @@ export function includeDiagnostic( export function includeDiagnostic(diagnostics: Diagnostics, message: string, p1?: any, p2?: any) { expect(diagnostics).toBeDefined(); if (diagnostics) { - const diagnostic = diagnostics.find(d => d.message.indexOf(message) >= 0) as Diagnostic; - expect(diagnostic).toBeDefined(); + const diagnostic = findDiagnostic(diagnostics, message); + expect(diagnostic).toBeDefined(`no diagnostic contains '${message}`); if (diagnostic && p1 != null) { const at = typeof p1 === 'number' ? p1 : p2.indexOf(p1); const len = typeof p2 === 'number' ? p2 : p1.length; - expect(diagnostic.span.start).toEqual(at); + expect(diagnostic.span.start) + .toEqual( + at, + `expected message '${message}' was reported at ${diagnostic.span.start} but should be ${at}`); if (len != null) { - expect(diagnostic.span.end - diagnostic.span.start).toEqual(len); + expect(diagnostic.span.end - diagnostic.span.start) + .toEqual(len, `expected '${message}'s span length to be ${len}`); } } }