Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(compiler-cli): explain why an expression cannot be used in AOT compilations #37587

Closed
wants to merge 4 commits into from
Closed

feat(compiler-cli): explain why an expression cannot be used in AOT compilations #37587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
204a803
refactor(compiler-cli): create diagnostics using `ts.DiagnosticRelate…
JoostK Jun 12, 2020
9f1e7a4
feat(compiler-cli): explain why an expression cannot be used in AOT c…
JoostK Jun 15, 2020
11c668f
refactor(compiler-cli): more accurate reporting of complex function call
JoostK Jun 15, 2020
3f8660b
fixup! feat(compiler-cli): explain why an expression cannot be used i…
JoostK Jun 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 14 additions & 17 deletions packages/compiler-cli/src/ngtsc/annotations/src/component.ts
View file
Open in desktop
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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[];
Expand All @@ -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);
Expand Down Expand Up @@ -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 = {
Expand All @@ -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;
}
Expand All @@ -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]);
}
Expand Down
48 changes: 45 additions & 3 deletions packages/compiler-cli/src/ngtsc/annotations/src/diagnostics.ts
View file
Open in desktop
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
58 changes: 26 additions & 32 deletions packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
View file
Open in desktop
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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.
Expand All @@ -405,18 +402,17 @@ 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;
}

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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<string|Expression> = {};
hostMetaMap.forEach((value, key) => {
Expand All @@ -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`);
}

Expand All @@ -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`);
}
});
Expand Down Expand Up @@ -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`);
}

Expand All @@ -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`);
}

Expand All @@ -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;
Expand Down