Skip to content

Commit 4a09c81

Browse files
chuckjazalxhub
chuckjaz
authored and
alxhub
committed
fix(language-service): do not throw for invalid metadata (angular#13261)
Fixes angular#13255
1 parent 16efb13 commit 4a09c81

File tree

4 files changed

+147
-57
lines changed

4 files changed

+147
-57
lines changed

‎modules/@angular/compiler/src/metadata_resolver.ts

Lines changed: 106 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {AnimationAnimateMetadata, AnimationEntryMetadata, AnimationGroupMetadata, AnimationKeyframesSequenceMetadata, AnimationMetadata, AnimationStateDeclarationMetadata, AnimationStateMetadata, AnimationStateTransitionMetadata, AnimationStyleMetadata, AnimationWithStepsMetadata, Attribute, ChangeDetectionStrategy, Component, Directive, Host, Inject, Injectable, ModuleWithProviders, Optional, Provider, Query, SchemaMetadata, Self, SkipSelf, Type, resolveForwardRef} from '@angular/core';
9+
import {AnimationAnimateMetadata, AnimationEntryMetadata, AnimationGroupMetadata, AnimationKeyframesSequenceMetadata, AnimationMetadata, AnimationStateDeclarationMetadata, AnimationStateMetadata, AnimationStateTransitionMetadata, AnimationStyleMetadata, AnimationWithStepsMetadata, Attribute, ChangeDetectionStrategy, Component, Directive, Host, Inject, Injectable, ModuleWithProviders, OpaqueToken, Optional, Provider, Query, SchemaMetadata, Self, SkipSelf, Type, resolveForwardRef} from '@angular/core';
1010

1111
import {StaticSymbol} from './aot/static_symbol';
1212
import {assertArrayOfStrings, assertInterpolationSymbols} from './assertions';
@@ -25,7 +25,8 @@ import {SummaryResolver} from './summary_resolver';
2525
import {getUrlScheme} from './url_resolver';
2626
import {MODULE_SUFFIX, SyncAsyncResult, ValueTransformer, visitValue} from './util';
2727

28-
28+
export type ErrorCollector = (error: any, type?: any) => void;
29+
export const ERROR_COLLECTOR_TOKEN = new OpaqueToken('ErrorCollector');
2930

3031
// Design notes:
3132
// - don't lazily create metadata:
@@ -47,7 +48,8 @@ export class CompileMetadataResolver {
4748
private _pipeResolver: PipeResolver, private _summaryResolver: SummaryResolver,
4849
private _schemaRegistry: ElementSchemaRegistry,
4950
private _directiveNormalizer: DirectiveNormalizer,
50-
private _reflector: ReflectorReader = reflector) {}
51+
private _reflector: ReflectorReader = reflector,
52+
@Optional() @Inject(ERROR_COLLECTOR_TOKEN) private _errorCollector?: ErrorCollector) {}
5153

5254
clearCacheFor(type: Type<any>) {
5355
const dirMeta = this._directiveCache.get(type);
@@ -182,7 +184,8 @@ export class CompileMetadataResolver {
182184
return null;
183185
} else {
184186
if (isSync) {
185-
throw new ComponentStillLoadingError(directiveType);
187+
this._reportError(new ComponentStillLoadingError(directiveType), directiveType);
188+
return null;
186189
}
187190
return templateMeta.asyncResult.then(createDirectiveMetadata);
188191
}
@@ -234,7 +237,7 @@ export class CompileMetadataResolver {
234237
if (dirMeta.viewProviders) {
235238
viewProviders = this._getProvidersMetadata(
236239
dirMeta.viewProviders, entryComponentMetadata,
237-
`viewProviders for "${stringify(directiveType)}"`);
240+
`viewProviders for "${stringify(directiveType)}"`, [], directiveType);
238241
}
239242
if (dirMeta.entryComponents) {
240243
entryComponentMetadata = flattenAndDedupeArray(dirMeta.entryComponents)
@@ -247,14 +250,18 @@ export class CompileMetadataResolver {
247250
} else {
248251
// Directive
249252
if (!selector) {
250-
throw new Error(`Directive ${stringify(directiveType)} has no selector, please add it!`);
253+
this._reportError(
254+
new Error(`Directive ${stringify(directiveType)} has no selector, please add it!`),
255+
directiveType);
256+
selector = 'error';
251257
}
252258
}
253259

254260
let providers: cpl.CompileProviderMetadata[] = [];
255261
if (isPresent(dirMeta.providers)) {
256262
providers = this._getProvidersMetadata(
257-
dirMeta.providers, entryComponentMetadata, `providers for "${stringify(directiveType)}"`);
263+
dirMeta.providers, entryComponentMetadata, `providers for "${stringify(directiveType)}"`,
264+
[], directiveType);
258265
}
259266
let queries: cpl.CompileQueryMetadata[] = [];
260267
let viewQueries: cpl.CompileQueryMetadata[] = [];
@@ -289,8 +296,10 @@ export class CompileMetadataResolver {
289296
getDirectiveMetadata(directiveType: any): cpl.CompileDirectiveMetadata {
290297
const dirMeta = this._directiveCache.get(directiveType);
291298
if (!dirMeta) {
292-
throw new Error(
293-
`Illegal state: getDirectiveMetadata can only be called after loadNgModuleMetadata for a module that declares it. Directive ${stringify(directiveType)}.`);
299+
this._reportError(
300+
new Error(
301+
`Illegal state: getDirectiveMetadata can only be called after loadNgModuleMetadata for a module that declares it. Directive ${stringify(directiveType)}.`),
302+
directiveType);
294303
}
295304
return dirMeta;
296305
}
@@ -299,8 +308,10 @@ export class CompileMetadataResolver {
299308
const dirSummary =
300309
<cpl.CompileDirectiveSummary>this._loadSummary(dirType, cpl.CompileSummaryKind.Directive);
301310
if (!dirSummary) {
302-
throw new Error(
303-
`Illegal state: Could not load the summary for directive ${stringify(dirType)}.`);
311+
this._reportError(
312+
new Error(
313+
`Illegal state: Could not load the summary for directive ${stringify(dirType)}.`),
314+
dirType);
304315
}
305316
return dirSummary;
306317
}
@@ -372,29 +383,38 @@ export class CompileMetadataResolver {
372383
if (moduleWithProviders.providers) {
373384
providers.push(...this._getProvidersMetadata(
374385
moduleWithProviders.providers, entryComponents,
375-
`provider for the NgModule '${stringify(importedModuleType)}'`));
386+
`provider for the NgModule '${stringify(importedModuleType)}'`, [], importedType));
376387
}
377388
}
378389

379390
if (importedModuleType) {
380391
const importedModuleSummary = this.getNgModuleSummary(importedModuleType);
381392
if (!importedModuleSummary) {
382-
throw new Error(
383-
`Unexpected ${this._getTypeDescriptor(importedType)} '${stringify(importedType)}' imported by the module '${stringify(moduleType)}'`);
393+
this._reportError(
394+
new Error(
395+
`Unexpected ${this._getTypeDescriptor(importedType)} '${stringify(importedType)}' imported by the module '${stringify(moduleType)}'`),
396+
moduleType);
397+
return;
384398
}
385399
importedModules.push(importedModuleSummary);
386400
} else {
387-
throw new Error(
388-
`Unexpected value '${stringify(importedType)}' imported by the module '${stringify(moduleType)}'`);
401+
this._reportError(
402+
new Error(
403+
`Unexpected value '${stringify(importedType)}' imported by the module '${stringify(moduleType)}'`),
404+
moduleType);
405+
return;
389406
}
390407
});
391408
}
392409

393410
if (meta.exports) {
394411
flattenAndDedupeArray(meta.exports).forEach((exportedType) => {
395412
if (!isValidType(exportedType)) {
396-
throw new Error(
397-
`Unexpected value '${stringify(exportedType)}' exported by the module '${stringify(moduleType)}'`);
413+
this._reportError(
414+
new Error(
415+
`Unexpected value '${stringify(exportedType)}' exported by the module '${stringify(moduleType)}'`),
416+
moduleType);
417+
return;
398418
}
399419
const exportedModuleSummary = this.getNgModuleSummary(exportedType);
400420
if (exportedModuleSummary) {
@@ -411,8 +431,11 @@ export class CompileMetadataResolver {
411431
if (meta.declarations) {
412432
flattenAndDedupeArray(meta.declarations).forEach((declaredType) => {
413433
if (!isValidType(declaredType)) {
414-
throw new Error(
415-
`Unexpected value '${stringify(declaredType)}' declared by the module '${stringify(moduleType)}'`);
434+
this._reportError(
435+
new Error(
436+
`Unexpected value '${stringify(declaredType)}' declared by the module '${stringify(moduleType)}'`),
437+
moduleType);
438+
return;
416439
}
417440
const declaredIdentifier = this._getIdentifierMetadata(declaredType);
418441
if (this._directiveResolver.isDirective(declaredType)) {
@@ -425,8 +448,11 @@ export class CompileMetadataResolver {
425448
declaredPipes.push(declaredIdentifier);
426449
this._addTypeToModule(declaredType, moduleType);
427450
} else {
428-
throw new Error(
429-
`Unexpected ${this._getTypeDescriptor(declaredType)} '${stringify(declaredType)}' declared by the module '${stringify(moduleType)}'`);
451+
this._reportError(
452+
new Error(
453+
`Unexpected ${this._getTypeDescriptor(declaredType)} '${stringify(declaredType)}' declared by the module '${stringify(moduleType)}'`),
454+
moduleType);
455+
return;
430456
}
431457
});
432458
}
@@ -441,16 +467,19 @@ export class CompileMetadataResolver {
441467
exportedPipes.push(exportedId);
442468
transitiveModule.addExportedPipe(exportedId);
443469
} else {
444-
throw new Error(
445-
`Can't export ${this._getTypeDescriptor(exportedId.reference)} ${stringify(exportedId.reference)} from ${stringify(moduleType)} as it was neither declared nor imported!`);
470+
this._reportError(
471+
new Error(
472+
`Can't export ${this._getTypeDescriptor(exportedId.reference)} ${stringify(exportedId.reference)} from ${stringify(moduleType)} as it was neither declared nor imported!`),
473+
moduleType);
446474
}
447475
});
448476

449477
// The providers of the module have to go last
450478
// so that they overwrite any other provider we already added.
451479
if (meta.providers) {
452480
providers.push(...this._getProvidersMetadata(
453-
meta.providers, entryComponents, `provider for the NgModule '${stringify(moduleType)}'`));
481+
meta.providers, entryComponents, `provider for the NgModule '${stringify(moduleType)}'`,
482+
[], moduleType));
454483
}
455484

456485
if (meta.entryComponents) {
@@ -459,14 +488,16 @@ export class CompileMetadataResolver {
459488
}
460489

461490
if (meta.bootstrap) {
462-
const typeMetadata = flattenAndDedupeArray(meta.bootstrap).map(type => {
491+
flattenAndDedupeArray(meta.bootstrap).forEach(type => {
463492
if (!isValidType(type)) {
464-
throw new Error(
465-
`Unexpected value '${stringify(type)}' used in the bootstrap property of module '${stringify(moduleType)}'`);
493+
this._reportError(
494+
new Error(
495+
`Unexpected value '${stringify(type)}' used in the bootstrap property of module '${stringify(moduleType)}'`),
496+
moduleType);
497+
return;
466498
}
467-
return this._getTypeMetadata(type);
499+
bootstrapComponents.push(this._getTypeMetadata(type));
468500
});
469-
bootstrapComponents.push(...typeMetadata);
470501
}
471502

472503
entryComponents.push(...bootstrapComponents);
@@ -522,10 +553,12 @@ export class CompileMetadataResolver {
522553
private _addTypeToModule(type: Type<any>, moduleType: Type<any>) {
523554
const oldModule = this._ngModuleOfTypes.get(type);
524555
if (oldModule && oldModule !== moduleType) {
525-
throw new Error(
526-
`Type ${stringify(type)} is part of the declarations of 2 modules: ${stringify(oldModule)} and ${stringify(moduleType)}! ` +
527-
`Please consider moving ${stringify(type)} to a higher module that imports ${stringify(oldModule)} and ${stringify(moduleType)}. ` +
528-
`You can also create a new NgModule that exports and includes ${stringify(type)} then import that NgModule in ${stringify(oldModule)} and ${stringify(moduleType)}.`);
556+
this._reportError(
557+
new Error(
558+
`Type ${stringify(type)} is part of the declarations of 2 modules: ${stringify(oldModule)} and ${stringify(moduleType)}! ` +
559+
`Please consider moving ${stringify(type)} to a higher module that imports ${stringify(oldModule)} and ${stringify(moduleType)}. ` +
560+
`You can also create a new NgModule that exports and includes ${stringify(type)} then import that NgModule in ${stringify(oldModule)} and ${stringify(moduleType)}.`),
561+
moduleType);
529562
}
530563
this._ngModuleOfTypes.set(type, moduleType);
531564
}
@@ -596,8 +629,10 @@ export class CompileMetadataResolver {
596629
getPipeMetadata(pipeType: any): cpl.CompilePipeMetadata {
597630
const pipeMeta = this._pipeCache.get(pipeType);
598631
if (!pipeMeta) {
599-
throw new Error(
600-
`Illegal state: getPipeMetadata can only be called after loadNgModuleMetadata for a module that declares it. Pipe ${stringify(pipeType)}.`);
632+
this._reportError(
633+
new Error(
634+
`Illegal state: getPipeMetadata can only be called after loadNgModuleMetadata for a module that declares it. Pipe ${stringify(pipeType)}.`),
635+
pipeType);
601636
}
602637
return pipeMeta;
603638
}
@@ -606,7 +641,9 @@ export class CompileMetadataResolver {
606641
const pipeSummary =
607642
<cpl.CompilePipeSummary>this._loadSummary(pipeType, cpl.CompileSummaryKind.Pipe);
608643
if (!pipeSummary) {
609-
throw new Error(`Illegal state: Could not load the summary for pipe ${stringify(pipeType)}.`);
644+
this._reportError(
645+
new Error(`Illegal state: Could not load the summary for pipe ${stringify(pipeType)}.`),
646+
pipeType);
610647
}
611648
return pipeSummary;
612649
}
@@ -686,8 +723,9 @@ export class CompileMetadataResolver {
686723
if (hasUnknownDeps) {
687724
const depsTokens =
688725
dependenciesMetadata.map((dep) => dep ? stringify(dep.token) : '?').join(', ');
689-
throw new Error(
690-
`Can't resolve all parameters for ${stringify(typeOrFunc)}: (${depsTokens}).`);
726+
this._reportError(
727+
new Error(`Can't resolve all parameters for ${stringify(typeOrFunc)}: (${depsTokens}).`),
728+
typeOrFunc);
691729
}
692730

693731
return dependenciesMetadata;
@@ -706,8 +744,8 @@ export class CompileMetadataResolver {
706744

707745
private _getProvidersMetadata(
708746
providers: Provider[], targetEntryComponents: cpl.CompileIdentifierMetadata[],
709-
debugInfo?: string,
710-
compileProviders: cpl.CompileProviderMetadata[] = []): cpl.CompileProviderMetadata[] {
747+
debugInfo?: string, compileProviders: cpl.CompileProviderMetadata[] = [],
748+
type?: any): cpl.CompileProviderMetadata[] {
711749
providers.forEach((provider: any, providerIdx: number) => {
712750
if (Array.isArray(provider)) {
713751
this._getProvidersMetadata(provider, targetEntryComponents, debugInfo, compileProviders);
@@ -733,11 +771,13 @@ export class CompileMetadataResolver {
733771
},
734772
[]))
735773
.join(', ');
736-
throw new Error(
737-
`Invalid ${debugInfo ? debugInfo : 'provider'} - only instances of Provider and Type are allowed, got: [${providersInfo}]`);
774+
this._reportError(
775+
new Error(
776+
`Invalid ${debugInfo ? debugInfo : 'provider'} - only instances of Provider and Type are allowed, got: [${providersInfo}]`),
777+
type);
738778
}
739779
if (providerMeta.token === resolveIdentifier(Identifiers.ANALYZE_FOR_ENTRY_COMPONENTS)) {
740-
targetEntryComponents.push(...this._getEntryComponentsFromProvider(providerMeta));
780+
targetEntryComponents.push(...this._getEntryComponentsFromProvider(providerMeta, type));
741781
} else {
742782
compileProviders.push(this.getProviderMetadata(providerMeta));
743783
}
@@ -746,17 +786,21 @@ export class CompileMetadataResolver {
746786
return compileProviders;
747787
}
748788

749-
private _getEntryComponentsFromProvider(provider: cpl.ProviderMeta):
789+
private _getEntryComponentsFromProvider(provider: cpl.ProviderMeta, type?: any):
750790
cpl.CompileIdentifierMetadata[] {
751791
const components: cpl.CompileIdentifierMetadata[] = [];
752792
const collectedIdentifiers: cpl.CompileIdentifierMetadata[] = [];
753793

754794
if (provider.useFactory || provider.useExisting || provider.useClass) {
755-
throw new Error(`The ANALYZE_FOR_ENTRY_COMPONENTS token only supports useValue!`);
795+
this._reportError(
796+
new Error(`The ANALYZE_FOR_ENTRY_COMPONENTS token only supports useValue!`), type);
797+
return [];
756798
}
757799

758800
if (!provider.multi) {
759-
throw new Error(`The ANALYZE_FOR_ENTRY_COMPONENTS token only supports 'multi = true'!`);
801+
this._reportError(
802+
new Error(`The ANALYZE_FOR_ENTRY_COMPONENTS token only supports 'multi = true'!`), type);
803+
return [];
760804
}
761805

762806
extractIdentifiers(provider.useValue, collectedIdentifiers);
@@ -822,8 +866,10 @@ export class CompileMetadataResolver {
822866
this._queryVarBindings(q.selector).map(varName => this._getTokenMetadata(varName));
823867
} else {
824868
if (!q.selector) {
825-
throw new Error(
826-
`Can't construct a query for the property "${propertyName}" of "${stringify(typeOrFunc)}" since the query selector wasn't defined.`);
869+
this._reportError(
870+
new Error(
871+
`Can't construct a query for the property "${propertyName}" of "${stringify(typeOrFunc)}" since the query selector wasn't defined.`),
872+
typeOrFunc);
827873
}
828874
selectors = [this._getTokenMetadata(q.selector)];
829875
}
@@ -835,6 +881,17 @@ export class CompileMetadataResolver {
835881
read: q.read ? this._getTokenMetadata(q.read) : null
836882
};
837883
}
884+
885+
private _reportError(error: any, type?: any, otherType?: any) {
886+
if (this._errorCollector) {
887+
this._errorCollector(error, type);
888+
if (otherType) {
889+
this._errorCollector(error, otherType);
890+
}
891+
} else {
892+
throw error;
893+
}
894+
}
838895
}
839896

840897
function flattenArray(tree: any[], out: Array<any> = []): Array<any> {

‎modules/@angular/language-service/src/typescript_host.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
120120

121121
result = this._resolver = new CompileMetadataResolver(
122122
moduleResolver, directiveResolver, pipeResolver, new SummaryResolver(),
123-
elementSchemaRegistry, directiveNormalizer, this.reflector);
123+
elementSchemaRegistry, directiveNormalizer, this.reflector,
124+
(error, type) => this.collectError(error, type && type.filePath));
124125
}
125126
return result;
126127
}

0 commit comments

Comments
 (0)