Skip to content

Commit

Permalink
fix(ngcc): report errors from analyze and resolve processing (#33964
Browse files Browse the repository at this point in the history
)

Previously, these errors were being swallowed, which made it
hard to debug problems with packages.

See #33685 (comment)

PR Close #33964
  • Loading branch information
petebacondarwin authored and alxhub committed Nov 21, 2019
1 parent 1394781 commit ca5d772
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 8 deletions.
Expand Up @@ -212,9 +212,12 @@ export class DecorationAnalyzer {
analyzedFile.analyzedClasses.forEach(({declaration, matches}) => {
matches.forEach(({handler, analysis}) => {
if ((handler.resolve !== undefined) && analysis) {
const res = handler.resolve(declaration, analysis);
if (res.reexports !== undefined) {
this.addReexports(res.reexports, declaration);
const {reexports, diagnostics} = handler.resolve(declaration, analysis);
if (reexports !== undefined) {
this.addReexports(reexports, declaration);
}
if (diagnostics !== undefined) {
diagnostics.forEach(error => this.diagnosticHandler(error));
}
}
});
Expand Down
Expand Up @@ -40,7 +40,7 @@ runInEachFileSystem(() => {
let result: DecorationAnalyses;

// Helpers
const createTestHandler = () => {
const createTestHandler = (options: {analyzeError: boolean, resolveError: boolean}) => {
const handler = jasmine.createSpyObj<DecoratorHandlerWithResolve>('TestDecoratorHandler', [
'detect',
'analyze',
Expand Down Expand Up @@ -74,13 +74,20 @@ runInEachFileSystem(() => {
// The "test" analysis is an object with the name of the decorator being analyzed
handler.analyze.and.callFake((decl: ts.Declaration, dec: Decorator) => {
logs.push(`analyze: ${(decl as any).name.text}@${dec.name}`);
return {analysis: {decoratorName: dec.name}, diagnostics: undefined};
return {
analysis: {decoratorName: dec.name},
diagnostics: options.analyzeError ? [makeDiagnostic(9999, decl, 'analyze diagnostic')] :
undefined
};
});
// The "test" resolution is just setting `resolved: true` on the analysis
handler.resolve.and.callFake((decl: ts.Declaration, analysis: any) => {
logs.push(`resolve: ${(decl as any).name.text}@${analysis.decoratorName}`);
analysis.resolved = true;
return {};
return {
diagnostics: options.resolveError ? [makeDiagnostic(9998, decl, 'resolve diagnostic')] :
undefined
};
});
// The "test" compilation result is just the name of the decorator being compiled
// (suffixed with `(compiled)`)
Expand All @@ -92,7 +99,10 @@ runInEachFileSystem(() => {
return handler;
};

function setUpAnalyzer(testFiles: TestFile[]) {
function setUpAnalyzer(
testFiles: TestFile[],
options: {analyzeError: boolean,
resolveError: boolean} = {analyzeError: false, resolveError: false}) {
logs = [];
loadTestFiles(testFiles);
loadFakeCore(getFileSystem());
Expand All @@ -107,7 +117,7 @@ runInEachFileSystem(() => {
const analyzer = new DecorationAnalyzer(
getFileSystem(), bundle, reflectionHost, referencesRegistry,
(error) => diagnosticLogs.push(error));
testHandler = createTestHandler();
testHandler = createTestHandler(options);
analyzer.handlers = [testHandler];
migrationLogs = [];
const migration1 = new MockMigration('migration1', migrationLogs);
Expand Down Expand Up @@ -362,6 +372,26 @@ runInEachFileSystem(() => {
expect(diagnosticLogs[0]).toEqual(jasmine.objectContaining({code: -999999}));
expect(diagnosticLogs[1]).toEqual(jasmine.objectContaining({code: -996666}));
});

it('should report analyze and resolve diagnostics to the `diagnosticHandler` callback',
() => {
const analyzer = setUpAnalyzer(
[
{
name: _('/node_modules/test-package/index.js'),
contents: `
import {Component, Directive, Injectable} from '@angular/core';
export class MyComponent {}
MyComponent.decorators = [{type: Component}];
`,
},
],
{analyzeError: true, resolveError: true});
analyzer.analyzeProgram();
expect(diagnosticLogs.length).toEqual(2);
expect(diagnosticLogs[0]).toEqual(jasmine.objectContaining({code: -999999}));
expect(diagnosticLogs[1]).toEqual(jasmine.objectContaining({code: -999998}));
});
});
});
});
Expand Down

0 comments on commit ca5d772

Please sign in to comment.