fix(compiler): report an error for recursive module references

Modules that directly or indirectly export or imported themselves
reports an error instead of generating a stack fault.

Fixes: #19979
This commit is contained in:
Chuck Jazdzewski 2017-12-15 16:25:04 -08:00 committed by Alex Rickabaugh
parent 5efea2f6a0
commit 33c0ee3441
2 changed files with 91 additions and 5 deletions

View File

@ -1636,6 +1636,68 @@ describe('ngc transformer command-line', () => {
expect(messages[0]).toContain('Parser Error: Unexpected token'); expect(messages[0]).toContain('Parser Error: Unexpected token');
}); });
// Regression test for #19979
it('should not stack overflow on a recursive module export', () => {
write('src/tsconfig.json', `{
"extends": "../tsconfig-base.json",
"files": ["test-module.ts"]
}`);
write('src/test-module.ts', `
import {Component, NgModule} from '@angular/core';
@Component({
template: 'Hello'
})
export class MyFaultyComponent {}
@NgModule({
exports: [MyFaultyModule],
declarations: [MyFaultyComponent],
providers: [],
})
export class MyFaultyModule { }
`);
const messages: string[] = [];
expect(
main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message)))
.toBe(1, 'Compile was expected to fail');
expect(messages[0]).toContain(`module 'MyFaultyModule' is exported recursively`);
});
// Regression test for #19979
it('should not stack overflow on a recursive module import', () => {
write('src/tsconfig.json', `{
"extends": "../tsconfig-base.json",
"files": ["test-module.ts"]
}`);
write('src/test-module.ts', `
import {Component, NgModule, forwardRef} from '@angular/core';
@Component({
template: 'Hello'
})
export class MyFaultyComponent {}
@NgModule({
imports: [forwardRef(() => MyFaultyModule)]
})
export class MyFaultyImport {}
@NgModule({
imports: [MyFaultyImport],
declarations: [MyFaultyComponent]
})
export class MyFaultyModule { }
`);
const messages: string[] = [];
expect(
main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message)))
.toBe(1, 'Compile was expected to fail');
expect(messages[0]).toContain(`is imported recursively by the module 'MyFaultyImport`);
});
it('should allow using 2 classes with the same name in declarations with noEmitOnError=true', it('should allow using 2 classes with the same name in declarations with noEmitOnError=true',
() => { () => {
write('src/tsconfig.json', `{ write('src/tsconfig.json', `{

View File

@ -444,11 +444,12 @@ export class CompileMetadataResolver {
this._ngModuleResolver.isNgModule(type); this._ngModuleResolver.isNgModule(type);
} }
getNgModuleSummary(moduleType: any): cpl.CompileNgModuleSummary|null { getNgModuleSummary(moduleType: any, alreadyCollecting: Set<any>|null = null):
cpl.CompileNgModuleSummary|null {
let moduleSummary: cpl.CompileNgModuleSummary|null = let moduleSummary: cpl.CompileNgModuleSummary|null =
<cpl.CompileNgModuleSummary>this._loadSummary(moduleType, cpl.CompileSummaryKind.NgModule); <cpl.CompileNgModuleSummary>this._loadSummary(moduleType, cpl.CompileSummaryKind.NgModule);
if (!moduleSummary) { if (!moduleSummary) {
const moduleMeta = this.getNgModuleMetadata(moduleType, false); const moduleMeta = this.getNgModuleMetadata(moduleType, false, alreadyCollecting);
moduleSummary = moduleMeta ? moduleMeta.toSummary() : null; moduleSummary = moduleMeta ? moduleMeta.toSummary() : null;
if (moduleSummary) { if (moduleSummary) {
this._summaryCache.set(moduleType, moduleSummary); this._summaryCache.set(moduleType, moduleSummary);
@ -476,7 +477,9 @@ export class CompileMetadataResolver {
return Promise.all(loading); return Promise.all(loading);
} }
getNgModuleMetadata(moduleType: any, throwIfNotFound = true): cpl.CompileNgModuleMetadata|null { getNgModuleMetadata(
moduleType: any, throwIfNotFound = true,
alreadyCollecting: Set<any>|null = null): cpl.CompileNgModuleMetadata|null {
moduleType = resolveForwardRef(moduleType); moduleType = resolveForwardRef(moduleType);
let compileMeta = this._ngModuleCache.get(moduleType); let compileMeta = this._ngModuleCache.get(moduleType);
if (compileMeta) { if (compileMeta) {
@ -514,7 +517,18 @@ export class CompileMetadataResolver {
if (importedModuleType) { if (importedModuleType) {
if (this._checkSelfImport(moduleType, importedModuleType)) return; if (this._checkSelfImport(moduleType, importedModuleType)) return;
const importedModuleSummary = this.getNgModuleSummary(importedModuleType); if (!alreadyCollecting) alreadyCollecting = new Set();
if (alreadyCollecting.has(importedModuleType)) {
this._reportError(
syntaxError(
`${this._getTypeDescriptor(importedModuleType)} '${stringifyType(importedType)}' is imported recursively by the module '${stringifyType(moduleType)}'.`),
moduleType);
return;
}
alreadyCollecting.add(importedModuleType);
const importedModuleSummary =
this.getNgModuleSummary(importedModuleType, alreadyCollecting);
alreadyCollecting.delete(importedModuleType);
if (!importedModuleSummary) { if (!importedModuleSummary) {
this._reportError( this._reportError(
syntaxError( syntaxError(
@ -542,7 +556,17 @@ export class CompileMetadataResolver {
moduleType); moduleType);
return; return;
} }
const exportedModuleSummary = this.getNgModuleSummary(exportedType); if (!alreadyCollecting) alreadyCollecting = new Set();
if (alreadyCollecting.has(exportedType)) {
this._reportError(
syntaxError(
`${this._getTypeDescriptor(exportedType)} '${stringify(exportedType)}' is exported recursively by the module '${stringifyType(moduleType)}'`),
moduleType);
return;
}
alreadyCollecting.add(exportedType);
const exportedModuleSummary = this.getNgModuleSummary(exportedType, alreadyCollecting);
alreadyCollecting.delete(exportedType);
if (exportedModuleSummary) { if (exportedModuleSummary) {
exportedModules.push(exportedModuleSummary); exportedModules.push(exportedModuleSummary);
} else { } else {