fix(compiler-cli): don't crash when we can't resolve a resource (#40660)

Produces a diagnostic when we cannot resolve a component's external style sheet or external template.

The previous behavior was to throw an exception, which crashed the
Language Service.

fixes angular/vscode-ng-language-service#1079

PR Close #40660
This commit is contained in:
Zach Arend 2021-01-28 16:57:13 -08:00 committed by Alex Rickabaugh
parent bc5a694b82
commit 378da71f27
4 changed files with 265 additions and 58 deletions

View File

@ -14,6 +14,7 @@ export declare enum ErrorCode {
UNDECORATED_PROVIDER = 2005, UNDECORATED_PROVIDER = 2005,
DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006, DIRECTIVE_INHERITS_UNDECORATED_CTOR = 2006,
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007, UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
COMPONENT_RESOURCE_NOT_FOUND = 2008,
SYMBOL_NOT_EXPORTED = 3001, SYMBOL_NOT_EXPORTED = 3001,
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002, SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,
CONFIG_FLAT_MODULE_NO_INDEX = 4001, CONFIG_FLAT_MODULE_NO_INDEX = 4001,

View File

@ -16,7 +16,7 @@ import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from
import {DependencyTracker} from '../../incremental/api'; import {DependencyTracker} from '../../incremental/api';
import {IndexingContext} from '../../indexer'; import {IndexingContext} from '../../indexer';
import {ClassPropertyMapping, ComponentResources, DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, Resource, ResourceRegistry} from '../../metadata'; import {ClassPropertyMapping, ComponentResources, DirectiveMeta, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, Resource, ResourceRegistry} from '../../metadata';
import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {EnumValue, PartialEvaluator, ResolvedValue} from '../../partial_evaluator';
import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
import {ComponentScopeReader, LocalModuleScopeRegistry, TypeCheckScopeRegistry} from '../../scope'; import {ComponentScopeReader, LocalModuleScopeRegistry, TypeCheckScopeRegistry} from '../../scope';
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform';
@ -72,9 +72,9 @@ export interface ComponentAnalysisData {
resources: ComponentResources; resources: ComponentResources;
/** /**
* The literal `styleUrls` extracted from the decorator, if present. * `styleUrls` extracted from the decorator, if present.
*/ */
styleUrls: string[]|null; styleUrls: StyleUrlMeta[]|null;
/** /**
* Inline stylesheets extracted from the decorator, if present. * Inline stylesheets extracted from the decorator, if present.
@ -86,6 +86,31 @@ export interface ComponentAnalysisData {
export type ComponentResolutionData = Pick<R3ComponentMetadata, ComponentMetadataResolvedFields>; export type ComponentResolutionData = Pick<R3ComponentMetadata, ComponentMetadataResolvedFields>;
/**
* The literal style url extracted from the decorator, along with metadata for diagnostics.
*/
export interface StyleUrlMeta {
url: string;
nodeForError: ts.Node;
source: ResourceTypeForDiagnostics.StylesheetFromTemplate|
ResourceTypeForDiagnostics.StylesheetFromDecorator;
}
/**
* Information about the origin of a resource in the application code. This is used for creating
* diagnostics, so we can point to the root cause of an error in the application code.
*
* A template resource comes from the `templateUrl` property on the component decorator.
*
* Stylesheets resources can come from either the `styleUrls` property on the component decorator,
* or from inline `style` tags and style links on the external template.
*/
export const enum ResourceTypeForDiagnostics {
Template,
StylesheetFromTemplate,
StylesheetFromDecorator,
}
/** /**
* `DecoratorHandler` which handles the `@Component` annotation. * `DecoratorHandler` which handles the `@Component` annotation.
*/ */
@ -157,33 +182,48 @@ export class ComponentDecoratorHandler implements
const component = reflectObjectLiteral(meta); const component = reflectObjectLiteral(meta);
const containingFile = node.getSourceFile().fileName; const containingFile = node.getSourceFile().fileName;
// Convert a styleUrl string into a Promise to preload it. const resolveStyleUrl =
const resolveStyleUrl = (styleUrl: string): Promise<void> => { (styleUrl: string, nodeForError: ts.Node,
const resourceUrl = this.resourceLoader.resolve(styleUrl, containingFile); resourceType: ResourceTypeForDiagnostics): Promise<void>|undefined => {
const promise = this.resourceLoader.preload(resourceUrl); const resourceUrl =
return promise || Promise.resolve(); this._resolveResourceOrThrow(styleUrl, containingFile, nodeForError, resourceType);
return this.resourceLoader.preload(resourceUrl);
}; };
// A Promise that waits for the template and all <link>ed styles within it to be preloaded. // A Promise that waits for the template and all <link>ed styles within it to be preloaded.
const templateAndTemplateStyleResources = const templateAndTemplateStyleResources =
this._preloadAndParseTemplate(node, decorator, component, containingFile).then(template => { this._preloadAndParseTemplate(node, decorator, component, containingFile)
.then((template: ParsedTemplateWithSource|null): Promise<void>|undefined => {
if (template === null) { if (template === null) {
return undefined; return undefined;
} else {
return Promise.all(template.styleUrls.map(resolveStyleUrl)).then(() => undefined);
} }
const nodeForError = getTemplateDeclarationNodeForError(template.declaration);
return Promise
.all(template.styleUrls.map(
styleUrl => resolveStyleUrl(
styleUrl, nodeForError,
ResourceTypeForDiagnostics.StylesheetFromTemplate)))
.then(() => undefined);
}); });
// Extract all the styleUrls in the decorator. // Extract all the styleUrls in the decorator.
const styleUrls = this._extractStyleUrls(component, []); const componentStyleUrls = this._extractComponentStyleUrls(component);
if (styleUrls === null) { if (componentStyleUrls === null) {
// A fast path exists if there are no styleUrls, to just wait for // A fast path exists if there are no styleUrls, to just wait for
// templateAndTemplateStyleResources. // templateAndTemplateStyleResources.
return templateAndTemplateStyleResources; return templateAndTemplateStyleResources;
} else { } else {
// Wait for both the template and all styleUrl resources to resolve. // Wait for both the template and all styleUrl resources to resolve.
return Promise.all([templateAndTemplateStyleResources, ...styleUrls.map(resolveStyleUrl)]) return Promise
.all([
templateAndTemplateStyleResources,
...componentStyleUrls.map(
styleUrl => resolveStyleUrl(
styleUrl.url, styleUrl.nodeForError,
ResourceTypeForDiagnostics.StylesheetFromDecorator))
])
.then(() => undefined); .then(() => undefined);
} }
} }
@ -268,42 +308,38 @@ export class ComponentDecoratorHandler implements
// Figure out the set of styles. The ordering here is important: external resources (styleUrls) // Figure out the set of styles. The ordering here is important: external resources (styleUrls)
// precede inline styles, and styles defined in the template override styles defined in the // precede inline styles, and styles defined in the template override styles defined in the
// component. // component.
let styles: string[]|null = null; let styles: string[] = [];
const styleResources = this._extractStyleResources(component, containingFile); const styleResources = this._extractStyleResources(component, containingFile);
const styleUrls = this._extractStyleUrls(component, template.styleUrls); const styleUrls: StyleUrlMeta[] = [
if (styleUrls !== null) { ...this._extractComponentStyleUrls(component), ...this._extractTemplateStyleUrls(template)
if (styles === null) { ];
styles = [];
}
for (const styleUrl of styleUrls) { for (const styleUrl of styleUrls) {
const resourceUrl = this.resourceLoader.resolve(styleUrl, containingFile); const resourceType = styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator ?
ResourceTypeForDiagnostics.StylesheetFromDecorator :
ResourceTypeForDiagnostics.StylesheetFromTemplate;
const resourceUrl = this._resolveResourceOrThrow(
styleUrl.url, containingFile, styleUrl.nodeForError, resourceType);
const resourceStr = this.resourceLoader.load(resourceUrl); const resourceStr = this.resourceLoader.load(resourceUrl);
styles.push(resourceStr); styles.push(resourceStr);
if (this.depTracker !== null) { if (this.depTracker !== null) {
this.depTracker.addResourceDependency(node.getSourceFile(), absoluteFrom(resourceUrl)); this.depTracker.addResourceDependency(node.getSourceFile(), absoluteFrom(resourceUrl));
} }
} }
}
let inlineStyles: string[]|null = null; let inlineStyles: string[]|null = null;
if (component.has('styles')) { if (component.has('styles')) {
const litStyles = parseFieldArrayValue(component, 'styles', this.evaluator); const litStyles = parseFieldArrayValue(component, 'styles', this.evaluator);
if (litStyles !== null) { if (litStyles !== null) {
inlineStyles = [...litStyles]; inlineStyles = [...litStyles];
if (styles === null) {
styles = litStyles;
} else {
styles.push(...litStyles); styles.push(...litStyles);
} }
} }
}
if (template.styles.length > 0) { if (template.styles.length > 0) {
if (styles === null) {
styles = template.styles;
} else {
styles.push(...template.styles); styles.push(...template.styles);
} }
}
const encapsulation: number = const encapsulation: number =
this._resolveEnumValue(component, 'encapsulation', 'ViewEncapsulation') || 0; this._resolveEnumValue(component, 'encapsulation', 'ViewEncapsulation') || 0;
@ -329,7 +365,7 @@ export class ComponentDecoratorHandler implements
}, },
encapsulation, encapsulation,
interpolation: template.interpolationConfig ?? DEFAULT_INTERPOLATION_CONFIG, interpolation: template.interpolationConfig ?? DEFAULT_INTERPOLATION_CONFIG,
styles: styles || [], styles,
// These will be replaced during the compilation step, after all `NgModule`s have been // These will be replaced during the compilation step, after all `NgModule`s have been
// analyzed and the full compilation scope for the component can be realized. // analyzed and the full compilation scope for the component can be realized.
@ -609,7 +645,12 @@ export class ComponentDecoratorHandler implements
let styles: string[] = []; let styles: string[] = [];
if (analysis.styleUrls !== null) { if (analysis.styleUrls !== null) {
for (const styleUrl of analysis.styleUrls) { for (const styleUrl of analysis.styleUrls) {
const resolvedStyleUrl = this.resourceLoader.resolve(styleUrl, containingFile); const resourceType =
styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator ?
ResourceTypeForDiagnostics.StylesheetFromDecorator :
ResourceTypeForDiagnostics.StylesheetFromTemplate;
const resolvedStyleUrl = this._resolveResourceOrThrow(
styleUrl.url, containingFile, styleUrl.nodeForError, resourceType);
const styleText = this.resourceLoader.load(resolvedStyleUrl); const styleText = this.resourceLoader.load(resolvedStyleUrl);
styles.push(styleText); styles.push(styleText);
} }
@ -705,20 +746,54 @@ export class ComponentDecoratorHandler implements
return resolved; return resolved;
} }
private _extractStyleUrls(component: Map<string, ts.Expression>, extraUrls: string[]): private _extractComponentStyleUrls(
string[]|null { component: Map<string, ts.Expression>,
): StyleUrlMeta[] {
if (!component.has('styleUrls')) { if (!component.has('styleUrls')) {
return extraUrls.length > 0 ? extraUrls : null; return [];
} }
const styleUrlsExpr = component.get('styleUrls')!; return this._extractStyleUrlsFromExpression(component.get('styleUrls')!);
const styleUrls = this.evaluator.evaluate(styleUrlsExpr);
if (!Array.isArray(styleUrls) || !styleUrls.every(url => typeof url === 'string')) {
throw createValueHasWrongTypeError(
styleUrlsExpr, styleUrls, 'styleUrls must be an array of strings');
} }
styleUrls.push(...extraUrls);
return styleUrls as string[]; private _extractStyleUrlsFromExpression(styleUrlsExpr: ts.Expression): StyleUrlMeta[] {
const styleUrls: StyleUrlMeta[] = [];
if (ts.isArrayLiteralExpression(styleUrlsExpr)) {
for (const styleUrlExpr of styleUrlsExpr.elements) {
if (ts.isSpreadElement(styleUrlExpr)) {
styleUrls.push(...this._extractStyleUrlsFromExpression(styleUrlExpr.expression));
} else {
const styleUrl = this.evaluator.evaluate(styleUrlExpr);
if (typeof styleUrl !== 'string') {
throw createValueHasWrongTypeError(styleUrlExpr, styleUrl, 'styleUrl must be a string');
}
styleUrls.push({
url: styleUrl,
source: ResourceTypeForDiagnostics.StylesheetFromDecorator,
nodeForError: styleUrlExpr,
});
}
}
} else {
const evaluatedStyleUrls = this.evaluator.evaluate(styleUrlsExpr);
if (!isStringArray(evaluatedStyleUrls)) {
throw createValueHasWrongTypeError(
styleUrlsExpr, evaluatedStyleUrls, 'styleUrls must be an array of strings');
}
for (const styleUrl of evaluatedStyleUrls) {
styleUrls.push({
url: styleUrl,
source: ResourceTypeForDiagnostics.StylesheetFromDecorator,
nodeForError: styleUrlsExpr,
});
}
}
return styleUrls;
} }
private _extractStyleResources(component: Map<string, ts.Expression>, containingFile: string): private _extractStyleResources(component: Map<string, ts.Expression>, containingFile: string):
@ -734,7 +809,9 @@ export class ComponentDecoratorHandler implements
const styleUrlsExpr = component.get('styleUrls'); const styleUrlsExpr = component.get('styleUrls');
if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) { if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) {
for (const expression of stringLiteralElements(styleUrlsExpr)) { for (const expression of stringLiteralElements(styleUrlsExpr)) {
const resourceUrl = this.resourceLoader.resolve(expression.text, containingFile); const resourceUrl = this._resolveResourceOrThrow(
expression.text, containingFile, expression,
ResourceTypeForDiagnostics.StylesheetFromDecorator);
styles.add({path: absoluteFrom(resourceUrl), expression}); styles.add({path: absoluteFrom(resourceUrl), expression});
} }
} }
@ -751,7 +828,7 @@ export class ComponentDecoratorHandler implements
private _preloadAndParseTemplate( private _preloadAndParseTemplate(
node: ClassDeclaration, decorator: Decorator, component: Map<string, ts.Expression>, node: ClassDeclaration, decorator: Decorator, component: Map<string, ts.Expression>,
containingFile: string): Promise<ParsedTemplate|null> { containingFile: string): Promise<ParsedTemplateWithSource|null> {
if (component.has('templateUrl')) { if (component.has('templateUrl')) {
// Extract the templateUrl and preload it. // Extract the templateUrl and preload it.
const templateUrlExpr = component.get('templateUrl')!; const templateUrlExpr = component.get('templateUrl')!;
@ -760,7 +837,8 @@ export class ComponentDecoratorHandler implements
throw createValueHasWrongTypeError( throw createValueHasWrongTypeError(
templateUrlExpr, templateUrl, 'templateUrl must be a string'); templateUrlExpr, templateUrl, 'templateUrl must be a string');
} }
const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile); const resourceUrl = this._resolveResourceOrThrow(
templateUrl, containingFile, templateUrlExpr, ResourceTypeForDiagnostics.Template);
const templatePromise = this.resourceLoader.preload(resourceUrl); const templatePromise = this.resourceLoader.preload(resourceUrl);
// If the preload worked, then actually load and parse the template, and wait for any style // If the preload worked, then actually load and parse the template, and wait for any style
@ -936,7 +1014,8 @@ export class ComponentDecoratorHandler implements
throw createValueHasWrongTypeError( throw createValueHasWrongTypeError(
templateUrlExpr, templateUrl, 'templateUrl must be a string'); templateUrlExpr, templateUrl, 'templateUrl must be a string');
} }
const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile); const resourceUrl = this._resolveResourceOrThrow(
templateUrl, containingFile, templateUrlExpr, ResourceTypeForDiagnostics.Template);
return { return {
isInline: false, isInline: false,
@ -991,6 +1070,45 @@ export class ComponentDecoratorHandler implements
this.cycleAnalyzer.recordSyntheticImport(origin, imported); this.cycleAnalyzer.recordSyntheticImport(origin, imported);
} }
/**
* Resolve the url of a resource relative to the file that contains the reference to it.
*
* Throws a FatalDiagnosticError when unable to resolve the file.
*/
private _resolveResourceOrThrow(
file: string, basePath: string, nodeForError: ts.Node,
resourceType: ResourceTypeForDiagnostics): string {
try {
return this.resourceLoader.resolve(file, basePath);
} catch (e) {
let errorText: string;
switch (resourceType) {
case ResourceTypeForDiagnostics.Template:
errorText = `Could not find template file '${file}'.`;
break;
case ResourceTypeForDiagnostics.StylesheetFromTemplate:
errorText = `Could not find stylesheet file '${file}' linked from the template.`;
break;
case ResourceTypeForDiagnostics.StylesheetFromDecorator:
errorText = `Could not find stylesheet file '${file}'.`;
break;
}
throw new FatalDiagnosticError(
ErrorCode.COMPONENT_RESOURCE_NOT_FOUND, nodeForError, errorText);
}
}
private _extractTemplateStyleUrls(template: ParsedTemplateWithSource): StyleUrlMeta[] {
if (template.styleUrls === null) {
return [];
}
const nodeForError = getTemplateDeclarationNodeForError(template.declaration);
return template.styleUrls.map(
url => ({url, source: ResourceTypeForDiagnostics.StylesheetFromTemplate, nodeForError}));
}
} }
function getTemplateRange(templateExpr: ts.Expression) { function getTemplateRange(templateExpr: ts.Expression) {
@ -1016,6 +1134,23 @@ function sourceMapUrl(resourceUrl: string): string {
} }
} }
/** Determines if the result of an evaluation is a string array. */
function isStringArray(resolvedValue: ResolvedValue): resolvedValue is string[] {
return Array.isArray(resolvedValue) && resolvedValue.every(elem => typeof elem === 'string');
}
/** Determines the node to use for debugging purposes for the given TemplateDeclaration. */
function getTemplateDeclarationNodeForError(declaration: TemplateDeclaration): ts.Node {
// TODO(zarend): Change this to if/else when that is compatible with g3. This uses a switch
// because if/else fails to compile on g3. That is because g3 compiles this in non-strict mode
// where type inference does not work correctly.
switch (declaration.isInline) {
case true:
return declaration.expression;
case false:
return declaration.templateUrlExpression;
}
}
/** /**
* Information about the template which was extracted during parsing. * Information about the template which was extracted during parsing.

View File

@ -44,6 +44,12 @@ export enum ErrorCode {
*/ */
UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007, UNDECORATED_CLASS_USING_ANGULAR_FEATURES = 2007,
/**
* Raised when an component cannot resolve an external resource, such as a template or a style
* sheet.
*/
COMPONENT_RESOURCE_NOT_FOUND = 2008,
SYMBOL_NOT_EXPORTED = 3001, SYMBOL_NOT_EXPORTED = 3001,
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002, SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,

View File

@ -7718,6 +7718,71 @@ export const Foo = Foo__PRE_R3__;
}); });
}); });
}); });
it('reports a COMPONENT_RESOURCE_NOT_FOUND for a component with a templateUrl' +
' that points to a non-existent file',
() => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-component',
templateUrl: './non-existent-file.html'
})
class TestComponent {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toEqual(1);
expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.COMPONENT_RESOURCE_NOT_FOUND));
expect(diags[0].messageText)
.toEqual(`Could not find template file './non-existent-file.html'.`);
});
it(`reports a COMPONENT_RESOURCE_NOT_FOUND when style sheet link in a component's template` +
` does not exist`,
() => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-component',
templateUrl: './test.html'
})
class TestComponent {}
`);
env.write('test.html', `
<link rel="stylesheet" href="./non-existent-file.css">
`);
const diags = env.driveDiagnostics();
expect(diags.length).toEqual(1);
expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.COMPONENT_RESOURCE_NOT_FOUND));
expect(diags[0].messageText)
.toEqual(
`Could not find stylesheet file './non-existent-file.css' linked from the template.`);
});
it('reports a COMPONENT_RESOURCE_NOT_FOUND for a component with a style url ' +
'defined in a spread that points to a non-existent file',
() => {
env.write('test.ts', `
import {Component} from '@angular/core';
@Component({
selector: 'test-component',
template: '',
styleUrls: [...['./non-existent-file.css']]
})
class TestComponent {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toEqual(1);
expect(diags[0].code).toEqual(ngErrorCode(ErrorCode.COMPONENT_RESOURCE_NOT_FOUND));
expect(diags[0].messageText)
.toEqual(`Could not find stylesheet file './non-existent-file.css'.`);
});
}); });
function expectTokenAtPosition<T extends ts.Node>( function expectTokenAtPosition<T extends ts.Node>(