fix(ivy): wrap functions from "providers" in parentheses in Closure mode

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -89,17 +89,18 @@ export class DecorationAnalyzer {
         this.scopeRegistry, this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs,
         /* defaultPreserveWhitespaces */ false,
         /* i18nUseExternalIds */ true, /* i18nLegacyMessageIdFormat */ '', this.moduleResolver,
-        this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER),
+        this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER,
+        /* annotateForClosureCompiler */ false),
     new DirectiveDecoratorHandler(
         this.reflectionHost, this.evaluator, this.fullRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
-        this.isCore),
+        this.isCore, /* annotateForClosureCompiler */ false),
     new InjectableDecoratorHandler(
         this.reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore,
         /* strictCtorDeps */ false),
     new NgModuleDecoratorHandler(
         this.reflectionHost, this.evaluator, this.fullMetaReader, this.fullRegistry,
         this.scopeRegistry, this.referencesRegistry, this.isCore, /* routeAnalyzer */ null,
-        this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER),
+        this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER, /* annotateForClosureCompiler */ false),
     new PipeDecoratorHandler(
         this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
         this.isCore),

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -28,7 +28,7 @@ import {ResourceLoader} from './api';
 import {extractDirectiveMetadata, parseFieldArrayValue} from './directive';
 import {compileNgFactoryDefField} from './factory';
 import {generateSetClassMetadataCall} from './metadata';
-import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, readBaseClass, unwrapExpression} from './util';
+import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, readBaseClass, unwrapExpression, wrapFunctionExpressionsInParens} from './util';
 
 const EMPTY_MAP = new Map<string, Expression>();
 const EMPTY_ARRAY: any[] = [];
@@ -55,6 +55,7 @@ export class ComponentDecoratorHandler implements
       private i18nLegacyMessageIdFormat: string, private moduleResolver: ModuleResolver,
       private cycleAnalyzer: CycleAnalyzer, private refEmitter: ReferenceEmitter,
       private defaultImportRecorder: DefaultImportRecorder,
+      private annotateForClosureCompiler: boolean,
       private resourceDependencies:
           ResourceDependencyRecorder = new NoopResourceDependencyRecorder()) {}
 
@@ -146,7 +147,8 @@ export class ComponentDecoratorHandler implements
     // on it.
     const directiveResult = extractDirectiveMetadata(
         node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore,
-        flags, this.elementSchemaRegistry.getDefaultComponentElementName());
+        flags, this.annotateForClosureCompiler,
+        this.elementSchemaRegistry.getDefaultComponentElementName());
     if (directiveResult === undefined) {
       // `extractDirectiveMetadata` returns undefined when the @Directive has `jit: true`. In this
       // case, compilation of the decorator is skipped. Returning an empty object signifies
@@ -169,7 +171,10 @@ export class ComponentDecoratorHandler implements
     }, undefined) !;
 
     const viewProviders: Expression|null = component.has('viewProviders') ?
-        new WrappedNodeExpr(component.get('viewProviders') !) :
+        new WrappedNodeExpr(
+            this.annotateForClosureCompiler ?
+                wrapFunctionExpressionsInParens(component.get('viewProviders') !) :
+                component.get('viewProviders') !) :
         null;
 
     // Parse the template.
@@ -297,7 +302,8 @@ export class ComponentDecoratorHandler implements
           i18nUseExternalIds: this.i18nUseExternalIds, relativeContextFilePath
         },
         metadataStmt: generateSetClassMetadataCall(
-            node, this.reflector, this.defaultImportRecorder, this.isCore),
+            node, this.reflector, this.defaultImportRecorder, this.isCore,
+            this.annotateForClosureCompiler),
         parsedTemplate: template, parseTemplate, templateSourceMapping,
       },
       typeCheck: true,

packages/compiler-cli/src/ngtsc/annotations/src/directive.ts

@@ -19,7 +19,7 @@ import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFl
 
 import {compileNgFactoryDefField} from './factory';
 import {generateSetClassMetadataCall} from './metadata';
-import {findAngularDecorator, getConstructorDependencies, isAngularDecorator, readBaseClass, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies} from './util';
+import {findAngularDecorator, getConstructorDependencies, isAngularDecorator, readBaseClass, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens} from './util';
 
 const EMPTY_OBJECT: {[key: string]: string} = {};
 const FIELD_DECORATORS = [
@@ -40,7 +40,7 @@ export class DirectiveDecoratorHandler implements
   constructor(
       private reflector: ReflectionHost, private evaluator: PartialEvaluator,
       private metaRegistry: MetadataRegistry, private defaultImportRecorder: DefaultImportRecorder,
-      private isCore: boolean) {}
+      private isCore: boolean, private annotateForClosureCompiler: boolean) {}
 
   readonly precedence = HandlerPrecedence.PRIMARY;
 
@@ -76,7 +76,7 @@ export class DirectiveDecoratorHandler implements
       AnalysisOutput<DirectiveHandlerData> {
     const directiveResult = extractDirectiveMetadata(
         node, decorator, this.reflector, this.evaluator, this.defaultImportRecorder, this.isCore,
-        flags);
+        flags, this.annotateForClosureCompiler);
     const analysis = directiveResult && directiveResult.metadata;
 
     if (analysis === undefined) {
@@ -102,7 +102,8 @@ export class DirectiveDecoratorHandler implements
       analysis: {
         meta: analysis,
         metadataStmt: generateSetClassMetadataCall(
-            node, this.reflector, this.defaultImportRecorder, this.isCore),
+            node, this.reflector, this.defaultImportRecorder, this.isCore,
+            this.annotateForClosureCompiler),
       }
     };
   }
@@ -136,7 +137,8 @@ export class DirectiveDecoratorHandler implements
 export function extractDirectiveMetadata(
     clazz: ClassDeclaration, decorator: Decorator | null, reflector: ReflectionHost,
     evaluator: PartialEvaluator, defaultImportRecorder: DefaultImportRecorder, isCore: boolean,
-    flags: HandlerFlags, defaultSelector: string | null = null): {
+    flags: HandlerFlags, annotateForClosureCompiler: boolean,
+    defaultSelector: string | null = null): {
   decorator: Map<string, ts.Expression>,
   metadata: R3DirectiveMetadata,
 }|undefined {
@@ -230,8 +232,12 @@ export function extractDirectiveMetadata(
 
   const host = extractHostBindings(decoratedElements, evaluator, coreModule, directive);
 
-  const providers: Expression|null =
-      directive.has('providers') ? new WrappedNodeExpr(directive.get('providers') !) : null;
+  const providers: Expression|null = directive.has('providers') ?
+      new WrappedNodeExpr(
+          annotateForClosureCompiler ?
+              wrapFunctionExpressionsInParens(directive.get('providers') !) :
+              directive.get('providers') !) :
+      null;
 
   // Determine if `ngOnChanges` is a lifecycle hook defined on the component.
   const usesOnChanges = members.some(

packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts

@@ -12,7 +12,7 @@ import * as ts from 'typescript';
 import {DefaultImportRecorder} from '../../imports';
 import {CtorParameter, Decorator, ReflectionHost} from '../../reflection';
 
-import {valueReferenceToExpression} from './util';
+import {valueReferenceToExpression, wrapFunctionExpressionsInParens} from './util';
 
 
 /**
@@ -24,7 +24,7 @@ import {valueReferenceToExpression} from './util';
  */
 export function generateSetClassMetadataCall(
     clazz: ts.Declaration, reflection: ReflectionHost, defaultImportRecorder: DefaultImportRecorder,
-    isCore: boolean): Statement|null {
+    isCore: boolean, annotateForClosureCompiler?: boolean): Statement|null {
   if (!reflection.isClass(clazz)) {
     return null;
   }
@@ -37,7 +37,9 @@ export function generateSetClassMetadataCall(
     return null;
   }
   const ngClassDecorators =
-      classDecorators.filter(dec => isAngularDecorator(dec, isCore)).map(decoratorToMetadata);
+      classDecorators.filter(dec => isAngularDecorator(dec, isCore))
+          .map(
+              (decorator: Decorator) => decoratorToMetadata(decorator, annotateForClosureCompiler));
   if (ngClassDecorators.length === 0) {
     return null;
   }
@@ -109,8 +111,8 @@ function ctorParameterToMetadata(
 
   // If the parameter has decorators, include the ones from Angular.
   if (param.decorators !== null) {
-    const ngDecorators =
-        param.decorators.filter(dec => isAngularDecorator(dec, isCore)).map(decoratorToMetadata);
+    const ngDecorators = param.decorators.filter(dec => isAngularDecorator(dec, isCore))
+                             .map((decorator: Decorator) => decoratorToMetadata(decorator));
     const value = new WrappedNodeExpr(ts.createArrayLiteral(ngDecorators));
     mapEntries.push({key: 'decorators', value, quoted: false});
   }
@@ -122,16 +124,17 @@ function ctorParameterToMetadata(
  */
 function classMemberToMetadata(
     name: string, decorators: Decorator[], isCore: boolean): ts.PropertyAssignment {
-  const ngDecorators =
-      decorators.filter(dec => isAngularDecorator(dec, isCore)).map(decoratorToMetadata);
+  const ngDecorators = decorators.filter(dec => isAngularDecorator(dec, isCore))
+                           .map((decorator: Decorator) => decoratorToMetadata(decorator));
   const decoratorMeta = ts.createArrayLiteral(ngDecorators);
   return ts.createPropertyAssignment(name, decoratorMeta);
 }
 
 /**
  * Convert a reflected decorator to metadata.
  */
-function decoratorToMetadata(decorator: Decorator): ts.ObjectLiteralExpression {
+function decoratorToMetadata(
+    decorator: Decorator, wrapFunctionsInParens?: boolean): ts.ObjectLiteralExpression {
   if (decorator.identifier === null) {
     throw new Error('Illegal state: synthesized decorator cannot be emitted in class metadata.');
   }
@@ -141,7 +144,10 @@ function decoratorToMetadata(decorator: Decorator): ts.ObjectLiteralExpression {
   ];
   // Sometimes they have arguments.
   if (decorator.args !== null && decorator.args.length > 0) {
-    const args = decorator.args.map(arg => ts.getMutableClone(arg));
+    const args = decorator.args.map(arg => {
+      const expr = ts.getMutableClone(arg);
+      return wrapFunctionsInParens ? wrapFunctionExpressionsInParens(expr) : expr;
+    });
     properties.push(ts.createPropertyAssignment('args', ts.createArrayLiteral(args)));
   }
   return ts.createObjectLiteral(properties, true);

packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts

@@ -21,7 +21,7 @@ import {getSourceFile} from '../../util/src/typescript';
 
 import {generateSetClassMetadataCall} from './metadata';
 import {ReferencesRegistry} from './references_registry';
-import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, toR3Reference, unwrapExpression} from './util';
+import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens} from './util';
 
 export interface NgModuleAnalysis {
   mod: R3NgModuleMetadata;
@@ -44,7 +44,8 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
       private scopeRegistry: LocalModuleScopeRegistry,
       private referencesRegistry: ReferencesRegistry, private isCore: boolean,
       private routeAnalyzer: NgModuleRouteAnalyzer|null, private refEmitter: ReferenceEmitter,
-      private defaultImportRecorder: DefaultImportRecorder, private localeId?: string) {}
+      private defaultImportRecorder: DefaultImportRecorder,
+      private annotateForClosureCompiler: boolean, private localeId?: string) {}
 
   readonly precedence = HandlerPrecedence.PRIMARY;
 
@@ -212,7 +213,11 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
     };
 
     const rawProviders = ngModule.has('providers') ? ngModule.get('providers') ! : null;
-    const providers = rawProviders !== null ? new WrappedNodeExpr(rawProviders) : null;
+    const providers = rawProviders !== null ?
+        new WrappedNodeExpr(
+            this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(rawProviders) :
+                                              rawProviders) :
+        null;
 
     // At this point, only add the module's imports as the injectors' imports. Any exported modules
     // are added during `resolve`, as we need scope information to be able to filter out directives
@@ -244,7 +249,8 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
         declarations: declarationRefs,
         exports: exportRefs,
         metadataStmt: generateSetClassMetadataCall(
-            node, this.reflector, this.defaultImportRecorder, this.isCore),
+            node, this.reflector, this.defaultImportRecorder, this.isCore,
+            this.annotateForClosureCompiler),
       },
       factorySymbolName: node.name.text,
     };

packages/compiler-cli/src/ngtsc/annotations/src/util.ts

@@ -351,3 +351,28 @@ export function readBaseClass(
 
   return null;
 }
+
+const parensWrapperTransformerFactory: ts.TransformerFactory<ts.Expression> =
+    (context: ts.TransformationContext) => {
+      const visitor: ts.Visitor = (node: ts.Node): ts.Node => {
+        const visited = ts.visitEachChild(node, visitor, context);
+        if (ts.isArrowFunction(visited) || ts.isFunctionExpression(visited)) {
+          return ts.createParen(visited);
+        }
+        return visited;
+      };
+      return (node: ts.Expression) => ts.visitEachChild(node, visitor, context);
+    };
+
+/**
+ * Wraps all functions in a given expression in parentheses. This is needed to avoid problems
+ * where Tsickle annotations added between analyse and transform phases in Angular may trigger
+ * automatic semicolon insertion, e.g. if a function is the expression in a `return` statement. More
+ * info can be found in Tsickle source code here:
+ * https://github.com/angular/tsickle/blob/d7974262571c8a17d684e5ba07680e1b1993afdd/src/jsdoc_transformer.ts#L1021
+ *
+ * @param expression Expression where functions should be wrapped in parentheses
+ */
+export function wrapFunctionExpressionsInParens(expression: ts.Expression): ts.Expression {
+  return ts.transform(expression, [parensWrapperTransformerFactory]).transformed[0];
+}

packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts

@@ -60,9 +60,11 @@ runInEachFileSystem(() => {
       const refEmitter = new ReferenceEmitter([]);
 
       const handler = new ComponentDecoratorHandler(
-          reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry, false,
-          new NoopResourceLoader(), [''], false, true, '', moduleResolver, cycleAnalyzer,
-          refEmitter, NOOP_DEFAULT_IMPORT_RECORDER);
+          reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry,
+          /* isCore */ false, new NoopResourceLoader(), /* rootDirs */[''],
+          /* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true,
+          /* i18nLegacyMessageIdFormat */ '', moduleResolver, cycleAnalyzer, refEmitter,
+          NOOP_DEFAULT_IMPORT_RECORDER, /* annotateForClosureCompiler */ false);
       const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
       const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
       if (detected === undefined) {

packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts

@@ -49,7 +49,8 @@ runInEachFileSystem(() => {
           metaReader, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]),
           null);
       const handler = new DirectiveDecoratorHandler(
-          reflectionHost, evaluator, scopeRegistry, NOOP_DEFAULT_IMPORT_RECORDER, false);
+          reflectionHost, evaluator, scopeRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
+          /* isCore */ false, /* annotateForClosureCompiler */ false);
 
       const analyzeDirective = (dirName: string) => {
         const DirNode = getDeclaration(program, _('/entry.ts'), dirName, isNamedClassDeclaration);

packages/compiler-cli/src/ngtsc/annotations/test/ng_module_spec.ts

@@ -69,7 +69,8 @@ runInEachFileSystem(() => {
 
       const handler = new NgModuleDecoratorHandler(
           reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry,
-          false, null, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER);
+          false, null, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER,
+          /* annotateForClosureCompiler */ false);
       const TestModule =
           getDeclaration(program, _('/entry.ts'), 'TestModule', isNamedClassDeclaration);
       const detected =

packages/compiler-cli/src/ngtsc/program.ts

@@ -591,16 +591,18 @@ export class NgtscProgram implements api.Program {
           this.isCore, this.resourceManager, this.rootDirs,
           this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false,
           this.getI18nLegacyMessageFormat(), this.moduleResolver, this.cycleAnalyzer,
-          this.refEmitter, this.defaultImportTracker, this.incrementalState),
+          this.refEmitter, this.defaultImportTracker, this.closureCompilerEnabled,
+          this.incrementalState),
       new DirectiveDecoratorHandler(
-          this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
+          this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore,
+          this.closureCompilerEnabled),
       new InjectableDecoratorHandler(
           this.reflector, this.defaultImportTracker, this.isCore,
           this.options.strictInjectionParameters || false),
       new NgModuleDecoratorHandler(
           this.reflector, evaluator, this.metaReader, metaRegistry, scopeRegistry,
           referencesRegistry, this.isCore, this.routeAnalyzer, this.refEmitter,
-          this.defaultImportTracker, this.options.i18nInLocale),
+          this.defaultImportTracker, this.closureCompilerEnabled, this.options.i18nInLocale),
       new PipeDecoratorHandler(
           this.reflector, evaluator, metaRegistry, this.defaultImportTracker, this.isCore),
     ];