Skip to content

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

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

Merged
alxhub merged 1 commit into from
Dec 20, 2017
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions packages/compiler-cli/test/ngc_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,68 @@ describe('ngc transformer command-line', () => {
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',
() => {
write('src/tsconfig.json', `{
Expand Down
34 changes: 29 additions & 5 deletions packages/compiler/src/metadata_resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,11 +444,12 @@ export class CompileMetadataResolver {
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 =
<cpl.CompileNgModuleSummary>this._loadSummary(moduleType, cpl.CompileSummaryKind.NgModule);
if (!moduleSummary) {
const moduleMeta = this.getNgModuleMetadata(moduleType, false);
const moduleMeta = this.getNgModuleMetadata(moduleType, false, alreadyCollecting);
moduleSummary = moduleMeta ? moduleMeta.toSummary() : null;
if (moduleSummary) {
this._summaryCache.set(moduleType, moduleSummary);
Expand Down Expand Up @@ -476,7 +477,9 @@ export class CompileMetadataResolver {
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);
let compileMeta = this._ngModuleCache.get(moduleType);
if (compileMeta) {
Expand Down Expand Up @@ -514,7 +517,18 @@ export class CompileMetadataResolver {

if (importedModuleType) {
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) {
this._reportError(
syntaxError(
Expand Down Expand Up @@ -542,7 +556,17 @@ export class CompileMetadataResolver {
moduleType);
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) {
exportedModules.push(exportedModuleSummary);
} else {
Expand Down