diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 12b78991cdf72..abaf8d08263e5 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -26,7 +26,7 @@ import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300'; import {SubsetOfKeys} from '../../util/src/typescript'; import {ResourceLoader} from './api'; -import {getDirectiveDiagnostics, getProviderDiagnostics} from './diagnostics'; +import {createValueHasWrongTypeError, getDirectiveDiagnostics, getProviderDiagnostics} from './diagnostics'; import {extractDirectiveMetadata, parseFieldArrayValue} from './directive'; import {compileNgFactoryDefField} from './factory'; import {generateSetClassMetadataCall} from './metadata'; @@ -243,8 +243,8 @@ export class ComponentDecoratorHandler implements const templateUrlExpr = component.get('templateUrl')!; const templateUrl = this.evaluator.evaluate(templateUrlExpr); if (typeof templateUrl !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); + throw createValueHasWrongTypeError( + templateUrlExpr, templateUrl, 'templateUrl must be a string'); } const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile); template = this._extractExternalTemplate(node, component, templateUrlExpr, resourceUrl); @@ -617,9 +617,8 @@ export class ComponentDecoratorHandler implements if (value instanceof EnumValue && isAngularCoreReference(value.enumRef, enumSymbolName)) { resolved = value.resolved as number; } else { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, - `${field} must be a member of ${enumSymbolName} enum from @angular/core`); + throw createValueHasWrongTypeError( + expr, value, `${field} must be a member of ${enumSymbolName} enum from @angular/core`); } } return resolved; @@ -634,8 +633,8 @@ export class ComponentDecoratorHandler implements const styleUrlsExpr = component.get('styleUrls')!; const styleUrls = this.evaluator.evaluate(styleUrlsExpr); if (!Array.isArray(styleUrls) || !styleUrls.every(url => typeof url === 'string')) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, styleUrlsExpr, 'styleUrls must be an array of strings'); + throw createValueHasWrongTypeError( + styleUrlsExpr, styleUrls, 'styleUrls must be an array of strings'); } styleUrls.push(...extraUrls); return styleUrls as string[]; @@ -649,8 +648,8 @@ export class ComponentDecoratorHandler implements const templateUrlExpr = component.get('templateUrl')!; const templateUrl = this.evaluator.evaluate(templateUrlExpr); if (typeof templateUrl !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); + throw createValueHasWrongTypeError( + templateUrlExpr, templateUrl, 'templateUrl must be a string'); } const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile); const templatePromise = this.resourceLoader.preload(resourceUrl); @@ -729,8 +728,8 @@ export class ComponentDecoratorHandler implements } else { const resolvedTemplate = this.evaluator.evaluate(templateExpr); if (typeof resolvedTemplate !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, templateExpr, 'template must be a string'); + throw createValueHasWrongTypeError( + templateExpr, resolvedTemplate, 'template must be a string'); } templateStr = resolvedTemplate; sourceMapping = { @@ -755,8 +754,7 @@ export class ComponentDecoratorHandler implements const expr = component.get('preserveWhitespaces')!; const value = this.evaluator.evaluate(expr); if (typeof value !== 'boolean') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, 'preserveWhitespaces must be a boolean'); + throw createValueHasWrongTypeError(expr, value, 'preserveWhitespaces must be a boolean'); } preserveWhitespaces = value; } @@ -767,9 +765,8 @@ export class ComponentDecoratorHandler implements const value = this.evaluator.evaluate(expr); if (!Array.isArray(value) || value.length !== 2 || !value.every(element => typeof element === 'string')) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, - 'interpolation must be an array with 2 elements of string type'); + throw createValueHasWrongTypeError( + expr, value, 'interpolation must be an array with 2 elements of string type'); } interpolation = InterpolationConfig.fromArray(value as [string, string]); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts index 6a7a221345594..195254d627b7c 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts @@ -8,15 +8,57 @@ import * as ts from 'typescript'; -import {ErrorCode, makeDiagnostic} from '../../diagnostics'; +import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; import {Reference} from '../../imports'; import {InjectableClassRegistry, MetadataReader} from '../../metadata'; -import {PartialEvaluator} from '../../partial_evaluator'; +import {describeResolvedType, DynamicValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../partial_evaluator'; import {ClassDeclaration, ReflectionHost} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope'; +import {identifierOfNode} from '../../util/src/typescript'; import {makeDuplicateDeclarationError, readBaseClass} from './util'; +/** + * Creates a `FatalDiagnosticError` for a node that did not evaluate to the expected type. The + * diagnostic that is created will include details on why the value is incorrect, i.e. it includes + * a representation of the actual type that was unsupported, or in the case of a dynamic value the + * trace to the node where the dynamic value originated. + * + * @param node The node for which the diagnostic should be produced. + * @param value The evaluated value that has the wrong type. + * @param messageText The message text of the error. + */ +export function createValueHasWrongTypeError( + node: ts.Node, value: ResolvedValue, messageText: string): FatalDiagnosticError { + let chainedMessage: string; + let relatedInformation: ts.DiagnosticRelatedInformation[]|undefined; + if (value instanceof DynamicValue) { + chainedMessage = 'Value could not be determined statically.'; + relatedInformation = traceDynamicValue(node, value); + } else if (value instanceof Reference) { + const target = value.debugName !== null ? `'${value.debugName}'` : 'an anonymous declaration'; + chainedMessage = `Value is a reference to ${target}.`; + + const referenceNode = identifierOfNode(value.node) ?? value.node; + relatedInformation = [makeRelatedInformation(referenceNode, 'Reference is declared here.')]; + } else { + chainedMessage = `Value is of type '${describeResolvedType(value)}'.`; + } + + const chain: ts.DiagnosticMessageChain = { + messageText, + category: ts.DiagnosticCategory.Error, + code: 0, + next: [{ + messageText: chainedMessage, + category: ts.DiagnosticCategory.Message, + code: 0, + }] + }; + + return new FatalDiagnosticError(ErrorCode.VALUE_HAS_WRONG_TYPE, node, chain, relatedInformation); +} + /** * Gets the diagnostics for a set of provider classes. * @param providerClasses Classes that should be checked. @@ -44,7 +86,7 @@ Either add the @Injectable() decorator to '${ provider.node.name .text}', or configure a different provider (such as a provider with 'useFactory'). `, - [{node: provider.node, messageText: `'${provider.node.name.text}' is declared here.`}])); + [makeRelatedInformation(provider.node, `'${provider.node.name.text}' is declared here.`)])); } return diagnostics; diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 644f1353bb5af..04c6f3f2c426a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -18,7 +18,7 @@ import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembe import {LocalModuleScopeRegistry} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform'; -import {getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic} from './diagnostics'; +import {createValueHasWrongTypeError, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic} from './diagnostics'; import {compileNgFactoryDefField} from './factory'; import {generateSetClassMetadataCall} from './metadata'; import {createSourceSpan, findAngularDecorator, getConstructorDependencies, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from './util'; @@ -278,8 +278,7 @@ export function extractDirectiveMetadata( const expr = directive.get('selector')!; const resolved = evaluator.evaluate(expr); if (typeof resolved !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, `selector must be a string`); + throw createValueHasWrongTypeError(expr, resolved, `selector must be a string`); } // use default selector in case selector is an empty string selector = resolved === '' ? defaultSelector : resolved; @@ -310,8 +309,7 @@ export function extractDirectiveMetadata( const expr = directive.get('exportAs')!; const resolved = evaluator.evaluate(expr); if (typeof resolved !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, `exportAs must be a string`); + throw createValueHasWrongTypeError(expr, resolved, `exportAs must be a string`); } exportAs = resolved.split(',').map(part => part.trim()); } @@ -381,8 +379,7 @@ export function extractQueryMetadata( } else if (isStringArrayOrDie(arg, `@${name} predicate`, node)) { predicate = arg; } else { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, node, `@${name} predicate cannot be interpreted`); + throw createValueHasWrongTypeError(node, arg, `@${name} predicate cannot be interpreted`); } // Extract the read and descendants options. @@ -405,9 +402,8 @@ export function extractQueryMetadata( const descendantsExpr = options.get('descendants')!; const descendantsValue = evaluator.evaluate(descendantsExpr); if (typeof descendantsValue !== 'boolean') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, descendantsExpr, - `@${name} options.descendants must be a boolean`); + throw createValueHasWrongTypeError( + descendantsExpr, descendantsValue, `@${name} options.descendants must be a boolean`); } descendants = descendantsValue; } @@ -415,8 +411,8 @@ export function extractQueryMetadata( if (options.has('static')) { const staticValue = evaluator.evaluate(options.get('static')!); if (typeof staticValue !== 'boolean') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, node, `@${name} options.static must be a boolean`); + throw createValueHasWrongTypeError( + node, staticValue, `@${name} options.static must be a boolean`); } isStatic = staticValue; } @@ -482,9 +478,8 @@ function isStringArrayOrDie(value: any, name: string, node: ts.Expression): valu for (let i = 0; i < value.length; i++) { if (typeof value[i] !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, node, - `Failed to resolve ${name} at position ${i} to a string`); + throw createValueHasWrongTypeError( + node, value[i], `Failed to resolve ${name} at position ${i} to a string`); } } return true; @@ -501,9 +496,8 @@ export function parseFieldArrayValue( const expression = directive.get(field)!; const value = evaluator.evaluate(expression); if (!isStringArrayOrDie(value, field, expression)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expression, - `Failed to resolve @Directive.${field} to a string array`); + throw createValueHasWrongTypeError( + expression, value, `Failed to resolve @Directive.${field} to a string array`); } return value; @@ -548,8 +542,8 @@ function parseDecoratedFields( } else if (decorator.args.length === 1) { const property = evaluator.evaluate(decorator.args[0]); if (typeof property !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, Decorator.nodeForError(decorator), + throw createValueHasWrongTypeError( + Decorator.nodeForError(decorator), property, `@${decorator.name} decorator argument must resolve to a string`); } results[fieldName] = mapValueResolver(property, fieldName); @@ -613,8 +607,8 @@ function evaluateHostExpressionBindings( hostExpr: ts.Expression, evaluator: PartialEvaluator): ParsedHostBindings { const hostMetaMap = evaluator.evaluate(hostExpr); if (!(hostMetaMap instanceof Map)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, hostExpr, `Decorator host metadata must be an object`); + throw createValueHasWrongTypeError( + hostExpr, hostMetaMap, `Decorator host metadata must be an object`); } const hostMetadata: StringMap = {}; hostMetaMap.forEach((value, key) => { @@ -624,8 +618,8 @@ function evaluateHostExpressionBindings( } if (typeof key !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, hostExpr, + throw createValueHasWrongTypeError( + hostExpr, key, `Decorator host metadata must be a string -> string object, but found unparseable key`); } @@ -634,8 +628,8 @@ function evaluateHostExpressionBindings( } else if (value instanceof DynamicValue) { hostMetadata[key] = new WrappedNodeExpr(value.node as ts.Expression); } else { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, hostExpr, + throw createValueHasWrongTypeError( + hostExpr, value, `Decorator host metadata must be a string -> string object, but found unparseable value`); } }); @@ -678,8 +672,8 @@ export function extractHostBindings( const resolved = evaluator.evaluate(decorator.args[0]); if (typeof resolved !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, Decorator.nodeForError(decorator), + throw createValueHasWrongTypeError( + Decorator.nodeForError(decorator), resolved, `@HostBinding's argument must be a string`); } @@ -704,8 +698,8 @@ export function extractHostBindings( const resolved = evaluator.evaluate(decorator.args[0]); if (typeof resolved !== 'string') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, decorator.args[0], + throw createValueHasWrongTypeError( + decorator.args[0], resolved, `@HostListener's event name argument must be a string`); } @@ -715,8 +709,8 @@ export function extractHostBindings( const expression = decorator.args[1]; const resolvedArgs = evaluator.evaluate(decorator.args[1]); if (!isStringArrayOrDie(resolvedArgs, '@HostListener.args', expression)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, decorator.args[1], + throw createValueHasWrongTypeError( + decorator.args[1], resolvedArgs, `@HostListener's second argument must be a string array`); } args = resolvedArgs; diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index c12cb44722baf..950ae78b98946 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -9,7 +9,7 @@ import {compileInjector, compileNgModule, CUSTOM_ELEMENTS_SCHEMA, Expression, ExternalExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, NO_ERRORS_SCHEMA, R3Identifiers, R3InjectorMetadata, R3NgModuleMetadata, R3Reference, SchemaMetadata, Statement, STRING_TYPE, WrappedNodeExpr} from '@angular/compiler'; import * as ts from 'typescript'; -import {ErrorCode, FatalDiagnosticError, makeDiagnostic} from '../../diagnostics'; +import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; import {DefaultImportRecorder, Reference, ReferenceEmitter} from '../../imports'; import {InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata'; import {PartialEvaluator, ResolvedValue, ResolvedValueArray} from '../../partial_evaluator'; @@ -20,7 +20,7 @@ import {FactoryTracker} from '../../shims/api'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; import {getSourceFile} from '../../util/src/typescript'; -import {getProviderDiagnostics} from './diagnostics'; +import {createValueHasWrongTypeError, getProviderDiagnostics} from './diagnostics'; import {generateSetClassMetadataCall} from './metadata'; import {ReferencesRegistry} from './references_registry'; import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, resolveProvidersRequiringFactory, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from './util'; @@ -133,10 +133,8 @@ export class NgModuleDecoratorHandler implements `Cannot declare '${ ref.node.name .text}' in an NgModule as it's not a part of the current compilation.`, - [{ - node: ref.node.name, - messageText: `'${ref.node.name.text}' is declared here.`, - }])); + [makeRelatedInformation( + ref.node.name, `'${ref.node.name.text}' is declared here.`)])); } } } @@ -172,21 +170,18 @@ export class NgModuleDecoratorHandler implements const rawExpr = ngModule.get('schemas')!; const result = this.evaluator.evaluate(rawExpr); if (!Array.isArray(result)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr, `NgModule.schemas must be an array`); + throw createValueHasWrongTypeError(rawExpr, result, `NgModule.schemas must be an array`); } for (const schemaRef of result) { if (!(schemaRef instanceof Reference)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr, - 'NgModule.schemas must be an array of schemas'); + throw createValueHasWrongTypeError( + rawExpr, result, 'NgModule.schemas must be an array of schemas'); } const id = schemaRef.getIdentityIn(schemaRef.node.getSourceFile()); if (id === null || schemaRef.ownedByModuleGuess !== '@angular/core') { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr, - 'NgModule.schemas must be an array of schemas'); + throw createValueHasWrongTypeError( + rawExpr, result, 'NgModule.schemas must be an array of schemas'); } // Since `id` is the `ts.Identifer` within the schema ref's declaration file, it's safe to // use `id.text` here to figure out which schema is in use. Even if the actual reference was @@ -199,9 +194,8 @@ export class NgModuleDecoratorHandler implements schemas.push(NO_ERRORS_SCHEMA); break; default: - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, rawExpr, - `'${schemaRef.debugName}' is not a valid NgModule schema`); + throw createValueHasWrongTypeError( + rawExpr, schemaRef, `'${schemaRef.debugName}' is not a valid NgModule schema`); } } } @@ -556,8 +550,8 @@ export class NgModuleDecoratorHandler implements arrayName: string): Reference[] { const refList: Reference[] = []; if (!Array.isArray(resolvedList)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, + throw createValueHasWrongTypeError( + expr, resolvedList, `Expected array when reading the NgModule.${arrayName} of ${className}`); } @@ -573,18 +567,18 @@ export class NgModuleDecoratorHandler implements refList.push(...this.resolveTypeList(expr, entry, className, arrayName)); } else if (isDeclarationReference(entry)) { if (!this.isClassDeclarationReference(entry)) { - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, entry.node, + throw createValueHasWrongTypeError( + entry.node, entry, `Value at position ${idx} in the NgModule.${arrayName} of ${ className} is not a class`); } refList.push(entry); } else { // TODO(alxhub): Produce a better diagnostic here - the array index may be an inner array. - throw new FatalDiagnosticError( - ErrorCode.VALUE_HAS_WRONG_TYPE, expr, + throw createValueHasWrongTypeError( + expr, entry, `Value at position ${idx} in the NgModule.${arrayName} of ${ - className} is not a reference: ${entry}`); + className} is not a reference`); } }); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index dcef9231eb7ee..f79d9ad07066d 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -16,6 +16,7 @@ import {PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; +import {createValueHasWrongTypeError} from './diagnostics'; import {compileNgFactoryDefField} from './factory'; import {generateSetClassMetadataCall} from './metadata'; @@ -82,8 +83,7 @@ export class PipeDecoratorHandler implements DecoratorHandler { + describe('ngtsc annotation diagnostics', () => { + describe('createValueError()', () => { + it('should include a trace for dynamic values', () => { + const error = createError('', 'nonexistent', 'Error message'); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText) + .toBe(`Value could not be determined statically.`); + + expect(error.relatedInformation).toBeDefined(); + expect(error.relatedInformation!.length).toBe(1); + + expect(error.relatedInformation![0].messageText).toBe('Unknown reference.'); + expect(error.relatedInformation![0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(error.relatedInformation![0])).toBe('nonexistent'); + }); + + it('should include a pointer for a reference to a named declaration', () => { + const error = createError( + `import {Foo} from './foo';`, 'Foo', 'Error message', + [{name: _('/foo.ts'), contents: 'export class Foo {}'}]); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText).toBe(`Value is a reference to 'Foo'.`); + + expect(error.relatedInformation).toBeDefined(); + expect(error.relatedInformation!.length).toBe(1); + expect(error.relatedInformation![0].messageText).toBe('Reference is declared here.'); + expect(error.relatedInformation![0].file!.fileName).toBe(_('/foo.ts')); + expect(getSourceCode(error.relatedInformation![0])).toBe('Foo'); + }); + + it('should include a pointer for a reference to an anonymous declaration', () => { + const error = createError( + `import Foo from './foo';`, 'Foo', 'Error message', + [{name: _('/foo.ts'), contents: 'export default class {}'}]); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText) + .toBe(`Value is a reference to an anonymous declaration.`); + + expect(error.relatedInformation).toBeDefined(); + expect(error.relatedInformation!.length).toBe(1); + expect(error.relatedInformation![0].messageText).toBe('Reference is declared here.'); + expect(error.relatedInformation![0].file!.fileName).toBe(_('/foo.ts')); + expect(getSourceCode(error.relatedInformation![0])).toBe('export default class {}'); + }); + + it('should include a representation of the value\'s type', () => { + const error = createError('', '{a: 2}', 'Error message'); + + if (typeof error.message === 'string') { + return fail('Created error must have a message chain'); + } + expect(error.message.messageText).toBe('Error message'); + expect(error.message.next!.length).toBe(1); + expect(error.message.next![0].messageText).toBe(`Value is of type '{ a: number }'.`); + + expect(error.relatedInformation).not.toBeDefined(); + }); + }); + }); +}); + +function getSourceCode(diag: ts.DiagnosticRelatedInformation): string { + const text = diag.file!.text; + return text.substr(diag.start!, diag.length!); +} + +function createError( + code: string, expr: string, messageText: string, + supportingFiles: TestFile[] = []): FatalDiagnosticError { + const {program} = makeProgram( + [{name: _('/entry.ts'), contents: `${code}; const target$ = ${expr}`}, ...supportingFiles], + /* options */ undefined, /* host */ undefined, /* checkForErrors */ false); + const checker = program.getTypeChecker(); + const decl = getDeclaration(program, _('/entry.ts'), 'target$', ts.isVariableDeclaration); + const valueExpr = decl.initializer!; + + const reflectionHost = new TypeScriptReflectionHost(checker); + const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null); + + const value = evaluator.evaluate(valueExpr); + return createValueHasWrongTypeError(valueExpr, value, messageText); +} diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/index.ts b/packages/compiler-cli/src/ngtsc/diagnostics/index.ts index e9c30007a220b..991719e867dad 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/index.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/index.ts @@ -6,6 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -export {FatalDiagnosticError, isFatalDiagnosticError, makeDiagnostic} from './src/error'; +export {FatalDiagnosticError, isFatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from './src/error'; export {ErrorCode, ngErrorCode} from './src/error_code'; export {replaceTsWithNgInErrors} from './src/util'; diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts index 734466e230910..0822840a9aa7a 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts @@ -8,10 +8,13 @@ import * as ts from 'typescript'; -import {ErrorCode} from './error_code'; +import {ErrorCode, ngErrorCode} from './error_code'; export class FatalDiagnosticError { - constructor(readonly code: ErrorCode, readonly node: ts.Node, readonly message: string) {} + constructor( + readonly code: ErrorCode, readonly node: ts.Node, + readonly message: string|ts.DiagnosticMessageChain, + readonly relatedInformation?: ts.DiagnosticRelatedInformation[]) {} /** * @internal @@ -19,37 +22,36 @@ export class FatalDiagnosticError { _isFatalDiagnosticError = true; toDiagnostic(): ts.DiagnosticWithLocation { - return makeDiagnostic(this.code, this.node, this.message); + return makeDiagnostic(this.code, this.node, this.message, this.relatedInformation); } } -export function makeDiagnostic(code: ErrorCode, node: ts.Node, messageText: string, relatedInfo?: { - node: ts.Node, - messageText: string, -}[]): ts.DiagnosticWithLocation { +export function makeDiagnostic( + code: ErrorCode, node: ts.Node, messageText: string|ts.DiagnosticMessageChain, + relatedInformation?: ts.DiagnosticRelatedInformation[]): ts.DiagnosticWithLocation { node = ts.getOriginalNode(node); - const diag: ts.DiagnosticWithLocation = { + return { category: ts.DiagnosticCategory.Error, - code: Number('-99' + code.valueOf()), + code: ngErrorCode(code), file: ts.getOriginalNode(node).getSourceFile(), start: node.getStart(undefined, false), length: node.getWidth(), messageText, + relatedInformation, + }; +} + +export function makeRelatedInformation( + node: ts.Node, messageText: string): ts.DiagnosticRelatedInformation { + node = ts.getOriginalNode(node); + return { + category: ts.DiagnosticCategory.Message, + code: 0, + file: node.getSourceFile(), + start: node.getStart(), + length: node.getWidth(), + messageText, }; - if (relatedInfo !== undefined) { - diag.relatedInformation = relatedInfo.map(info => { - const infoNode = ts.getOriginalNode(info.node); - return { - category: ts.DiagnosticCategory.Message, - code: 0, - file: infoNode.getSourceFile(), - start: infoNode.getStart(), - length: infoNode.getWidth(), - messageText: info.messageText, - }; - }); - } - return diag; } export function isFatalDiagnosticError(err: any): err is FatalDiagnosticError { diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/BUILD.bazel b/packages/compiler-cli/src/ngtsc/partial_evaluator/BUILD.bazel index 0c1476f4afd21..48e6b1e71f2ab 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/BUILD.bazel @@ -11,6 +11,7 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/incremental:api", "//packages/compiler-cli/src/ngtsc/reflection", diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/index.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/index.ts index 7b95a4335cf2d..7c3c566524ca7 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/index.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/index.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +export {describeResolvedType, traceDynamicValue} from './src/diagnostics'; export {DynamicValue} from './src/dynamic'; export {ForeignFunctionResolver, PartialEvaluator} from './src/interface'; export {EnumValue, KnownFn, ResolvedValue, ResolvedValueArray, ResolvedValueMap} from './src/result'; diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/diagnostics.ts new file mode 100644 index 0000000000000..e270cb213cba2 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/diagnostics.ts @@ -0,0 +1,187 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; + +import {makeRelatedInformation} from '../../diagnostics'; +import {Reference} from '../../imports'; +import {FunctionDefinition} from '../../reflection'; +import {DynamicValue, DynamicValueVisitor} from './dynamic'; +import {EnumValue, KnownFn, ResolvedModule, ResolvedValue} from './result'; + +/** + * Derives a type representation from a resolved value to be reported in a diagnostic. + * + * @param value The resolved value for which a type representation should be derived. + * @param maxDepth The maximum nesting depth of objects and arrays, defaults to 1 level. + */ +export function describeResolvedType(value: ResolvedValue, maxDepth: number = 1): string { + if (value === null) { + return 'null'; + } else if (value === undefined) { + return 'undefined'; + } else if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'string') { + return typeof value; + } else if (value instanceof Map) { + if (maxDepth === 0) { + return 'object'; + } + const entries = Array.from(value.entries()).map(([key, v]) => { + return `${quoteKey(key)}: ${describeResolvedType(v, maxDepth - 1)}`; + }); + return entries.length > 0 ? `{ ${entries.join('; ')} }` : '{}'; + } else if (value instanceof ResolvedModule) { + return '(module)'; + } else if (value instanceof EnumValue) { + return value.enumRef.debugName ?? '(anonymous)'; + } else if (value instanceof Reference) { + return value.debugName ?? '(anonymous)'; + } else if (Array.isArray(value)) { + if (maxDepth === 0) { + return 'Array'; + } + return `[${value.map(v => describeResolvedType(v, maxDepth - 1)).join(', ')}]`; + } else if (value instanceof DynamicValue) { + return '(not statically analyzable)'; + } else if (value instanceof KnownFn) { + return 'Function'; + } else { + return 'unknown'; + } +} + +function quoteKey(key: string): string { + if (/^[a-z0-9_]+$/i.test(key)) { + return key; + } else { + return `'${key.replace(/'/g, '\\\'')}'`; + } +} + +/** + * Creates an array of related information diagnostics for a `DynamicValue` that describe the trace + * of why an expression was evaluated as dynamic. + * + * @param node The node for which a `ts.Diagnostic` is to be created with the trace. + * @param value The dynamic value for which a trace should be created. + */ +export function traceDynamicValue( + node: ts.Node, value: DynamicValue): ts.DiagnosticRelatedInformation[] { + return value.accept(new TraceDynamicValueVisitor(node)); +} + +class TraceDynamicValueVisitor implements DynamicValueVisitor { + private currentContainerNode: ts.Node|null = null; + + constructor(private node: ts.Node) {} + + visitDynamicInput(value: DynamicValue): ts.DiagnosticRelatedInformation[] { + const trace = value.reason.accept(this); + if (this.shouldTrace(value.node)) { + const info = + makeRelatedInformation(value.node, 'Unable to evaluate this expression statically.'); + trace.unshift(info); + } + return trace; + } + + visitDynamicString(value: DynamicValue): ts.DiagnosticRelatedInformation[] { + return [makeRelatedInformation( + value.node, 'A string value could not be determined statically.')]; + } + + visitExternalReference(value: DynamicValue>): + ts.DiagnosticRelatedInformation[] { + const name = value.reason.debugName; + const description = name !== null ? `'${name}'` : 'an anonymous declaration'; + return [makeRelatedInformation( + value.node, + `A value for ${ + description} cannot be determined statically, as it is an external declaration.`)]; + } + + visitComplexFunctionCall(value: DynamicValue): + ts.DiagnosticRelatedInformation[] { + return [ + makeRelatedInformation( + value.node, + 'Unable to evaluate function call of complex function. A function must have exactly one return statement.'), + makeRelatedInformation(value.reason.node, 'Function is declared here.') + ]; + } + + visitInvalidExpressionType(value: DynamicValue): ts.DiagnosticRelatedInformation[] { + return [makeRelatedInformation(value.node, 'Unable to evaluate an invalid expression.')]; + } + + visitUnknown(value: DynamicValue): ts.DiagnosticRelatedInformation[] { + return [makeRelatedInformation(value.node, 'Unable to evaluate statically.')]; + } + + visitUnknownIdentifier(value: DynamicValue): ts.DiagnosticRelatedInformation[] { + return [makeRelatedInformation(value.node, 'Unknown reference.')]; + } + + visitUnsupportedSyntax(value: DynamicValue): ts.DiagnosticRelatedInformation[] { + return [makeRelatedInformation(value.node, 'This syntax is not supported.')]; + } + + /** + * Determines whether the dynamic value reported for the node should be traced, i.e. if it is not + * part of the container for which the most recent trace was created. + */ + private shouldTrace(node: ts.Node): boolean { + if (node === this.node) { + // Do not include a dynamic value for the origin node, as the main diagnostic is already + // reported on that node. + return false; + } + + const container = getContainerNode(node); + if (container === this.currentContainerNode) { + // The node is part of the same container as the previous trace entry, so this dynamic value + // should not become part of the trace. + return false; + } + + this.currentContainerNode = container; + return true; + } +} + +/** + * Determines the closest parent node that is to be considered as container, which is used to reduce + * the granularity of tracing the dynamic values to a single entry per container. Currently, full + * statements and destructuring patterns are considered as container. + */ +function getContainerNode(node: ts.Node): ts.Node { + let currentNode: ts.Node|undefined = node; + while (currentNode !== undefined) { + switch (currentNode.kind) { + case ts.SyntaxKind.ExpressionStatement: + case ts.SyntaxKind.VariableStatement: + case ts.SyntaxKind.ReturnStatement: + case ts.SyntaxKind.IfStatement: + case ts.SyntaxKind.SwitchStatement: + case ts.SyntaxKind.DoStatement: + case ts.SyntaxKind.WhileStatement: + case ts.SyntaxKind.ForStatement: + case ts.SyntaxKind.ForInStatement: + case ts.SyntaxKind.ForOfStatement: + case ts.SyntaxKind.ContinueStatement: + case ts.SyntaxKind.BreakStatement: + case ts.SyntaxKind.ThrowStatement: + case ts.SyntaxKind.ObjectBindingPattern: + case ts.SyntaxKind.ArrayBindingPattern: + return currentNode; + } + + currentNode = currentNode.parent; + } + return node.getSourceFile(); +} diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts index 9a0b295353d52..c19bfa3e3df01 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/dynamic.ts @@ -9,6 +9,7 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; +import {FunctionDefinition} from '../../reflection'; /** * The reason why a value cannot be determined statically. @@ -55,6 +56,11 @@ export const enum DynamicValueReason { */ INVALID_EXPRESSION_TYPE, + /** + * A function call could not be evaluated as the function's body is not a single return statement. + */ + COMPLEX_FUNCTION_CALL, + /** * A value could not be determined statically for any reason other the above. */ @@ -93,6 +99,11 @@ export class DynamicValue { return new DynamicValue(node, value, DynamicValueReason.INVALID_EXPRESSION_TYPE); } + static fromComplexFunctionCall(node: ts.Node, fn: FunctionDefinition): + DynamicValue { + return new DynamicValue(node, fn, DynamicValueReason.COMPLEX_FUNCTION_CALL); + } + static fromUnknown(node: ts.Node): DynamicValue { return new DynamicValue(node, undefined, DynamicValueReason.UNKNOWN); } @@ -121,7 +132,45 @@ export class DynamicValue { return this.code === DynamicValueReason.INVALID_EXPRESSION_TYPE; } + isFromComplexFunctionCall(this: DynamicValue): this is DynamicValue { + return this.code === DynamicValueReason.COMPLEX_FUNCTION_CALL; + } + isFromUnknown(this: DynamicValue): this is DynamicValue { return this.code === DynamicValueReason.UNKNOWN; } + + accept(visitor: DynamicValueVisitor): R { + switch (this.code) { + case DynamicValueReason.DYNAMIC_INPUT: + return visitor.visitDynamicInput(this as unknown as DynamicValue); + case DynamicValueReason.DYNAMIC_STRING: + return visitor.visitDynamicString(this); + case DynamicValueReason.EXTERNAL_REFERENCE: + return visitor.visitExternalReference( + this as unknown as DynamicValue>); + case DynamicValueReason.UNSUPPORTED_SYNTAX: + return visitor.visitUnsupportedSyntax(this); + case DynamicValueReason.UNKNOWN_IDENTIFIER: + return visitor.visitUnknownIdentifier(this); + case DynamicValueReason.INVALID_EXPRESSION_TYPE: + return visitor.visitInvalidExpressionType(this); + case DynamicValueReason.COMPLEX_FUNCTION_CALL: + return visitor.visitComplexFunctionCall( + this as unknown as DynamicValue); + case DynamicValueReason.UNKNOWN: + return visitor.visitUnknown(this); + } + } +} + +export interface DynamicValueVisitor { + visitDynamicInput(value: DynamicValue): R; + visitDynamicString(value: DynamicValue): R; + visitExternalReference(value: DynamicValue>): R; + visitUnsupportedSyntax(value: DynamicValue): R; + visitUnknownIdentifier(value: DynamicValue): R; + visitInvalidExpressionType(value: DynamicValue): R; + visitComplexFunctionCall(value: DynamicValue): R; + visitUnknown(value: DynamicValue): R; } diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index 76b27e7a87663..af2034229725e 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -490,8 +490,10 @@ export class StaticInterpreter { private visitFunctionBody(node: ts.CallExpression, fn: FunctionDefinition, context: Context): ResolvedValue { - if (fn.body === null || fn.body.length !== 1 || !ts.isReturnStatement(fn.body[0])) { + if (fn.body === null) { return DynamicValue.fromUnknown(node); + } else if (fn.body.length !== 1 || !ts.isReturnStatement(fn.body[0])) { + return DynamicValue.fromComplexFunctionCall(node, fn); } const ret = fn.body[0] as ts.ReturnStatement; diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/diagnostics_spec.ts new file mode 100644 index 0000000000000..e8bf6de5a0eb2 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/diagnostics_spec.ts @@ -0,0 +1,292 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; + +import {absoluteFrom as _} from '../../file_system'; +import {runInEachFileSystem} from '../../file_system/testing'; +import {Reference} from '../../imports'; +import {TypeScriptReflectionHost} from '../../reflection'; +import {getDeclaration, makeProgram} from '../../testing'; + +import {ObjectAssignBuiltinFn} from '../src/builtin'; +import {describeResolvedType, traceDynamicValue} from '../src/diagnostics'; +import {DynamicValue} from '../src/dynamic'; +import {PartialEvaluator} from '../src/interface'; +import {EnumValue, ResolvedModule} from '../src/result'; + +runInEachFileSystem(() => { + describe('partial evaluator', () => { + describe('describeResolvedType()', () => { + it('should describe primitives', () => { + expect(describeResolvedType(0)).toBe('number'); + expect(describeResolvedType(true)).toBe('boolean'); + expect(describeResolvedType(false)).toBe('boolean'); + expect(describeResolvedType(null)).toBe('null'); + expect(describeResolvedType(undefined)).toBe('undefined'); + expect(describeResolvedType('text')).toBe('string'); + }); + + it('should describe objects limited to a single level', () => { + expect(describeResolvedType(new Map())).toBe('{}'); + expect(describeResolvedType(new Map([['a', 0], ['b', true]]))) + .toBe('{ a: number; b: boolean }'); + expect(describeResolvedType(new Map([['a', new Map()]]))).toBe('{ a: object }'); + expect(describeResolvedType(new Map([['a', [1, 2, 3]]]))).toBe('{ a: Array }'); + }); + + it('should describe arrays limited to a single level', () => { + expect(describeResolvedType([])).toBe('[]'); + expect(describeResolvedType([1, 2, 3])).toBe('[number, number, number]'); + expect(describeResolvedType([[1, 2], [3, 4]])).toBe('[Array, Array]'); + expect(describeResolvedType([new Map([['a', 0]])])).toBe('[object]'); + }); + + it('should describe references', () => { + const namedFn = ts.createFunctionDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + /* asteriskToken */ undefined, + /* name */ 'test', + /* typeParameters */ undefined, + /* parameters */[], + /* type */ undefined, + /* body */ undefined, + ); + expect(describeResolvedType(new Reference(namedFn))).toBe('test'); + + const anonymousFn = ts.createFunctionDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + /* asteriskToken */ undefined, + /* name */ undefined, + /* typeParameters */ undefined, + /* parameters */[], + /* type */ undefined, + /* body */ undefined, + ); + expect(describeResolvedType(new Reference(anonymousFn))).toBe('(anonymous)'); + }); + + it('should describe enum values', () => { + const decl = ts.createEnumDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + /* name */ 'MyEnum', + /* members */[ts.createEnumMember('member', ts.createNumericLiteral('1'))], + ); + const ref = new Reference(decl); + expect(describeResolvedType(new EnumValue(ref, 'member', 1))).toBe('MyEnum'); + }); + + it('should describe dynamic values', () => { + const node = ts.createObjectLiteral(); + expect(describeResolvedType(DynamicValue.fromUnsupportedSyntax(node))) + .toBe('(not statically analyzable)'); + }); + + it('should describe known functions', () => { + expect(describeResolvedType(new ObjectAssignBuiltinFn())).toBe('Function'); + }); + + it('should describe external modules', () => { + expect(describeResolvedType(new ResolvedModule(new Map(), () => undefined))) + .toBe('(module)'); + }); + }); + + describe('traceDynamicValue()', () => { + it('should not include the origin node if points to a different dynamic node.', () => { + // In the below expression, the read of "value" is evaluated to be dynamic, but it's also + // the exact node for which the diagnostic is produced. Therefore, this node is not part + // of the trace. + const trace = traceExpression('const value = nonexistent;', 'value'); + + expect(trace.length).toBe(1); + expect(trace[0].messageText).toBe(`Unknown reference.`); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('nonexistent'); + }); + + it('should include the origin node if it is dynamic by itself', () => { + const trace = traceExpression('', 'nonexistent;'); + + expect(trace.length).toBe(1); + expect(trace[0].messageText).toBe(`Unknown reference.`); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('nonexistent'); + }); + + it('should include a trace for a dynamic subexpression in the origin expression', () => { + const trace = traceExpression('const value = nonexistent;', 'value.property'); + + expect(trace.length).toBe(2); + expect(trace[0].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('value'); + + expect(trace[1].messageText).toBe('Unknown reference.'); + expect(trace[1].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[1])).toBe('nonexistent'); + }); + + it('should reduce the granularity to a single entry per statement', () => { + // Dynamic values exist for each node that has been visited, but only the initial dynamic + // value within a statement is included in the trace. + const trace = traceExpression( + `const firstChild = document.body.childNodes[0]; + const child = firstChild.firstChild;`, + 'child !== undefined'); + + expect(trace.length).toBe(4); + expect(trace[0].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('child'); + + expect(trace[1].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[1].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[1])).toBe('firstChild'); + + expect(trace[2].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[2].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[2])).toBe('document.body'); + + expect(trace[3].messageText) + .toBe( + `A value for 'document' cannot be determined statically, as it is an external declaration.`); + expect(trace[3].file!.fileName).toBe(_('/lib.d.ts')); + expect(getSourceCode(trace[3])).toBe('document: any'); + }); + + it('should trace dynamic strings', () => { + const trace = traceExpression('', '`${document}`'); + + expect(trace.length).toBe(1); + expect(trace[0].messageText).toBe('A string value could not be determined statically.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('document'); + }); + + it('should trace invalid expression types', () => { + const trace = traceExpression('', 'true()'); + + expect(trace.length).toBe(1); + expect(trace[0].messageText).toBe('Unable to evaluate an invalid expression.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('true'); + }); + + it('should trace unknown syntax', () => { + const trace = traceExpression('', `new String('test')`); + + expect(trace.length).toBe(1); + expect(trace[0].messageText).toBe('This syntax is not supported.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('new String(\'test\')'); + }); + + it('should trace complex function invocations', () => { + const trace = traceExpression( + ` + function complex() { + console.log('test'); + return true; + }`, + 'complex()'); + + expect(trace.length).toBe(2); + expect(trace[0].messageText) + .toBe( + 'Unable to evaluate function call of complex function. A function must have exactly one return statement.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('complex()'); + + expect(trace[1].messageText).toBe('Function is declared here.'); + expect(trace[1].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[1])).toContain(`console.log('test');`); + }); + + it('should trace object destructuring of external reference', () => { + const trace = traceExpression('const {body: {firstChild}} = document;', 'firstChild'); + + expect(trace.length).toBe(2); + expect(trace[0].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('body: {firstChild}'); + + expect(trace[1].messageText) + .toBe( + `A value for 'document' cannot be determined statically, as it is an external declaration.`); + expect(trace[1].file!.fileName).toBe(_('/lib.d.ts')); + expect(getSourceCode(trace[1])).toBe('document: any'); + }); + + it('should trace deep object destructuring of external reference', () => { + const trace = + traceExpression('const {doc: {body: {firstChild}}} = {doc: document};', 'firstChild'); + + expect(trace.length).toBe(2); + expect(trace[0].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('body: {firstChild}'); + + expect(trace[1].messageText) + .toBe( + `A value for 'document' cannot be determined statically, as it is an external declaration.`); + expect(trace[1].file!.fileName).toBe(_('/lib.d.ts')); + expect(getSourceCode(trace[1])).toBe('document: any'); + }); + + it('should trace array destructuring of dynamic value', () => { + const trace = + traceExpression('const [firstChild] = document.body.childNodes;', 'firstChild'); + + expect(trace.length).toBe(3); + expect(trace[0].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[0].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[0])).toBe('firstChild'); + + expect(trace[1].messageText).toBe('Unable to evaluate this expression statically.'); + expect(trace[1].file!.fileName).toBe(_('/entry.ts')); + expect(getSourceCode(trace[1])).toBe('document.body'); + + expect(trace[2].messageText) + .toBe( + `A value for 'document' cannot be determined statically, as it is an external declaration.`); + expect(trace[2].file!.fileName).toBe(_('/lib.d.ts')); + expect(getSourceCode(trace[2])).toBe('document: any'); + }); + }); + }); +}); + +function getSourceCode(diag: ts.DiagnosticRelatedInformation): string { + const text = diag.file!.text; + return text.substr(diag.start!, diag.length!); +} + +function traceExpression(code: string, expr: string): ts.DiagnosticRelatedInformation[] { + const {program} = makeProgram( + [ + {name: _('/entry.ts'), contents: `${code}; const target$ = ${expr};`}, + {name: _('/lib.d.ts'), contents: `declare const document: any;`}, + ], + /* options */ undefined, /* host */ undefined, /* checkForErrors */ false); + const checker = program.getTypeChecker(); + const decl = getDeclaration(program, _('/entry.ts'), 'target$', ts.isVariableDeclaration); + const valueExpr = decl.initializer!; + + const reflectionHost = new TypeScriptReflectionHost(checker); + const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null); + + const value = evaluator.evaluate(valueExpr); + if (!(value instanceof DynamicValue)) { + throw new Error('Expected DynamicValue'); + } + return traceDynamicValue(valueExpr, value); +} diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts index 72695d9ad9b5f..7c8d53e9257ca 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts @@ -622,15 +622,18 @@ runInEachFileSystem(() => { expect(id.text).toEqual('Target'); }); - it('should resolve functions with more than one statement to an unknown value', () => { + it('should resolve functions with more than one statement to a complex function call', () => { const value = evaluate(`function foo(bar) { const b = bar; return b; }`, 'foo("test")'); if (!(value instanceof DynamicValue)) { return fail(`Should have resolved to a DynamicValue`); } - - expect(value.isFromUnknown()).toBe(true); + if (!value.isFromComplexFunctionCall()) { + return fail('Expected DynamicValue to be from complex function call'); + } expect((value.node as ts.CallExpression).expression.getText()).toBe('foo'); + expect((value.reason.node as ts.FunctionDeclaration).getText()) + .toContain('const b = bar; return b;'); }); describe('(with imported TypeScript helpers)', () => { diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 9287efb1eabaf..985058a157d98 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -9,7 +9,7 @@ import {ExternalExpr, SchemaMetadata} from '@angular/compiler'; import * as ts from 'typescript'; -import {ErrorCode, makeDiagnostic} from '../../diagnostics'; +import {ErrorCode, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; import {AliasingHost, Reexport, Reference, ReferenceEmitter} from '../../imports'; import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; @@ -358,7 +358,8 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop ngModule.ref.node.name .text}', but is not a directive, a component, or a pipe. ` + `Either remove it from the NgModule's declarations, or add an appropriate Angular decorator.`, - [{node: decl.node.name, messageText: `'${decl.node.name.text}' is declared here.`}])); + [makeRelatedInformation( + decl.node.name, `'${decl.node.name.text}' is declared here.`)])); continue; } @@ -643,8 +644,8 @@ function reexportCollision( To fix this problem please re-export one or both classes directly from this file. `.trim(), [ - {node: refA.node.name, messageText: childMessageText}, - {node: refB.node.name, messageText: childMessageText}, + makeRelatedInformation(refA.node.name, childMessageText), + makeRelatedInformation(refB.node.name, childMessageText), ]); } diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index c2b93c2e6480c..7e1ae0cde2f82 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -1622,7 +1622,8 @@ runInEachFileSystem(os => { expect(errors.length).toBe(1); const {code, messageText} = errors[0]; expect(code).toBe(ngErrorCode(errorCode)); - expect(trim(messageText as string)).toContain(errorMessage); + const text = ts.flattenDiagnosticMessageText(messageText, '\n'); + expect(trim(text)).toContain(errorMessage); } it('should throw if invalid arguments are provided in @NgModule', () => { @@ -3428,8 +3429,11 @@ runInEachFileSystem(os => { class CompA {} `); const errors = env.driveDiagnostics(); - expect(errors[0].messageText) + expect(errors.length).toBe(1); + const messageText = ts.flattenDiagnosticMessageText(errors[0].messageText, '\n'); + expect(messageText) .toContain('encapsulation must be a member of ViewEncapsulation enum from @angular/core'); + expect(messageText).toContain('Value is of type \'string\'.'); }); it('should handle `changeDetection` field', () => { @@ -3459,9 +3463,12 @@ runInEachFileSystem(os => { class CompA {} `); const errors = env.driveDiagnostics(); - expect(errors[0].messageText) + expect(errors.length).toBe(1); + const messageText = ts.flattenDiagnosticMessageText(errors[0].messageText, '\n'); + expect(messageText) .toContain( 'changeDetection must be a member of ChangeDetectionStrategy enum from @angular/core'); + expect(messageText).toContain('Value is of type \'string\'.'); }); it('should ignore empty bindings', () => { @@ -4700,7 +4707,10 @@ runInEachFileSystem(os => { `); const diags = await driveDiagnostics(); - expect(diags[0].messageText).toBe('styleUrls must be an array of strings'); + expect(diags.length).toBe(1); + const messageText = ts.flattenDiagnosticMessageText(diags[0].messageText, '\n'); + expect(messageText).toContain('styleUrls must be an array of strings'); + expect(messageText).toContain('Value is of type \'string\'.'); expect(diags[0].file!.fileName).toBe(absoluteFrom('/test.ts')); }); }); diff --git a/packages/compiler-cli/test/ngtsc/scope_spec.ts b/packages/compiler-cli/test/ngtsc/scope_spec.ts index c78584e824dac..69a8b88947a00 100644 --- a/packages/compiler-cli/test/ngtsc/scope_spec.ts +++ b/packages/compiler-cli/test/ngtsc/scope_spec.ts @@ -205,8 +205,10 @@ runInEachFileSystem(() => { `); const [error] = env.driveDiagnostics(); expect(error).not.toBeUndefined(); - expect(error.messageText).toContain('IsAModule'); - expect(error.messageText).toContain('NgModule.imports'); + const messageText = ts.flattenDiagnosticMessageText(error.messageText, '\n'); + expect(messageText) + .toContain('Value at position 0 in the NgModule.imports of IsAModule is not a class'); + expect(messageText).toContain('Value is a reference to \'NotAClass\'.'); expect(error.code).toEqual(ngErrorCode(ErrorCode.VALUE_HAS_WRONG_TYPE)); expect(diagnosticToNode(error, ts.isIdentifier).text).toEqual('NotAClass'); });