Skip to content

Commit

Permalink
fix(compiler-cli): use ModuleWithProviders type if static eval fails
Browse files Browse the repository at this point in the history
When the compiler encounters a function call within an NgModule imports
section, it attempts to resolve it to an NgModule-annotated class by
looking at the function body and evaluating the statements there. This
evaluation can only understand simple functions which have a single
return statement as their body. If the function the user writes is more
complex than that, the compiler won't be able to understand it and
previously the PartialEvaluator would return a "DynamicValue" for
that import.

With this change, in the event the function body resolution fails the
PartialEvaluator will now attempt to use its foreign function resolvers to
determine the correct result from the function's type signtaure instead. If
the function is annotated with a correct ModuleWithProviders type, the
compiler will be able to understand the import without static analysis of
the function body.
  • Loading branch information
alxhub committed May 15, 2020
1 parent cda2530 commit 8d6fac8
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 11 deletions.
Expand Up @@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {Reference} from '../../imports';
import {OwningModule} from '../../imports/src/references';
import {DependencyTracker} from '../../incremental/api';
import {ConcreteDeclaration, Declaration, EnumMember, InlineDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
import {ConcreteDeclaration, Declaration, EnumMember, FunctionDefinition, InlineDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
import {isDeclaration} from '../../util/src/typescript';

import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
Expand Down Expand Up @@ -445,21 +445,54 @@ export class StaticInterpreter {
};
}

const res = this.visitExpression(expr, context);
if (res instanceof Reference) {
// This Reference was created synthetically, via a foreign function resolver. The real
// runtime value of the function expression may be different than the foreign function
// resolved value, so mark the Reference as synthetic to avoid it being misinterpreted.
res.synthetic = true;
return this.visitFfrExpression(expr, context);
}

let res: ResolvedValue = this.visitFunctionBody(node, fn, context);

// If the result of attempting to resolve the function body was a DynamicValue, attempt to use
// the foreignFunctionResolver if one is present. This could still potentially yield a usable
// value.
if (res instanceof DynamicValue && context.foreignFunctionResolver !== undefined) {
const ffrExpr = context.foreignFunctionResolver(lhs, node.arguments);
if (ffrExpr !== null) {
// The foreign function resolver was able to extract an expression from this function. See
// if that expression leads to a non-dynamic result.
const ffrRes = this.visitFfrExpression(ffrExpr, context);
if (!(ffrRes instanceof DynamicValue)) {
// FFR yielded an actual result that's not dynamic, so use that instead of the original
// resolution.
res = ffrRes;
}
}
return res;
}

const body = fn.body;
if (body.length !== 1 || !ts.isReturnStatement(body[0])) {
return res;
}

/**
* Visit an expression which was extracted from a foreign-function resolver.
*
* This will process the result and ensure it's correct for FFR-resolved values, including marking
* `Reference`s as synthetic.
*/
private visitFfrExpression(expr: ts.Expression, context: Context): ResolvedValue {
const res = this.visitExpression(expr, context);
if (res instanceof Reference) {
// This Reference was created synthetically, via a foreign function resolver. The real
// runtime value of the function expression may be different than the foreign function
// resolved value, so mark the Reference as synthetic to avoid it being misinterpreted.
res.synthetic = true;
}
return res;
}

private visitFunctionBody(node: ts.CallExpression, fn: FunctionDefinition, context: Context):
ResolvedValue {
if (fn.body === null || fn.body.length !== 1 || !ts.isReturnStatement(fn.body[0])) {
return DynamicValue.fromUnknown(node);
}
const ret = body[0] as ts.ReturnStatement;
const ret = fn.body[0] as ts.ReturnStatement;

const args = this.evaluateFunctionArguments(node, context);
const newScope: Scope = new Map<ts.ParameterDeclaration, ResolvedValue>();
Expand Down
39 changes: 39 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Expand Up @@ -2390,6 +2390,45 @@ runInEachFileSystem(os => {
});

describe('unwrapping ModuleWithProviders functions', () => {
it('should use a local ModuleWithProviders-annotated return type if a function is not statically analyzable',
() => {
env.write(`module.ts`, `
import {NgModule, ModuleWithProviders} from '@angular/core';
export function notStaticallyAnalyzable(): ModuleWithProviders<SomeModule> {
console.log('this interferes with static analysis');
return {
ngModule: SomeModule,
providers: [],
};
}
@NgModule()
export class SomeModule {}
`);

env.write('test.ts', `
import {NgModule} from '@angular/core';
import {notStaticallyAnalyzable} from './module';
@NgModule({
imports: [notStaticallyAnalyzable()]
})
export class TestModule {}
`);

env.driveMain();

const jsContents = env.getContents('test.js');
expect(jsContents).toContain('imports: [notStaticallyAnalyzable()]');

const dtsContents = env.getContents('test.d.ts');
expect(dtsContents).toContain(`import * as i1 from "./module";`);
expect(dtsContents)
.toContain(
'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.SomeModule], never>');
});

it('should extract the generic type and include it in the module\'s declaration', () => {
env.write(`test.ts`, `
import {NgModule} from '@angular/core';
Expand Down

0 comments on commit 8d6fac8

Please sign in to comment.