From 965a688c974d5eed37974e644a6c65f25c686695 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 14 May 2020 17:52:08 -0700 Subject: [PATCH] fix(compiler-cli): use ModuleWithProviders type if static eval fails (#37126) 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. PR Close #37126 --- .../partial_evaluator/src/interpreter.ts | 57 +++++++++++++++---- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 39 +++++++++++++ 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index f8e47f3edf..db65b4dfd3 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -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'; @@ -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 res; + return this.visitFfrExpression(expr, context); } - const body = fn.body; - if (body.length !== 1 || !ts.isReturnStatement(body[0])) { + 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; + } + + /** + * 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(); diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index bb45060a12..1f4ed4a5b6 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -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 { + 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'); + }); + it('should extract the generic type and include it in the module\'s declaration', () => { env.write(`test.ts`, ` import {NgModule} from '@angular/core';