feat(core): missing-injectable migration should migrate empty object literal providers (#33709)

In View Engine, providers which neither used `useValue`, `useClass`,
`useFactory` or `useExisting`, were interpreted differently.

e.g.

```
{provide: X} -> {provide: X, useValue: undefined}, // this is how it works in View Engine
{provide: X} -> {provide: X, useClass: X}, // this is how it works in Ivy
```

The missing-injectable migration should migrate such providers to the
explicit `useValue` provider. This ensures that there is no unexpected
behavioral change when updating to v9.

PR Close #33709

integration/ng_update_migrations/src/app/migration-tests/providers-test-module.ts

@@ -34,9 +34,9 @@ class BaseProviderCase {
   constructor(zone: NgZone) {}
 }
 
-export class ProvideCase extends BaseProviderCase {}
+export class EmptyProviderLiteralCase {}
 
-export class ProviderClass {}
+export class ProviderClass extends BaseProviderCase {}
 
 export class DontNeedCase {}
 
@@ -46,7 +46,7 @@ export class DirectiveCase {}
 @NgModule({
   providers: [
     TypeCase,
-    {provide: ProvideCase},
+    {provide: EmptyProviderLiteralCase},
     {provide: DontNeedCase, useValue: 0},
     {provide: DontNeedCase, useFactory: () => null},
     {provide: DontNeedCase, useExisting: TypeCase},
@@ -56,4 +56,3 @@ export class DirectiveCase {}
   declarations: [ProvidersTestDirective, ProvidersTestComponent],
 })
 export class ProvidersTestModule {}
-

integration/ng_update_migrations/src/app/migration-tests/providers-test-module_expected.ts

@@ -41,11 +41,10 @@ class BaseProviderCase {
   constructor(zone: NgZone) {}
 }
 
-@Injectable()
+export class EmptyProviderLiteralCase {}
-export class ProvideCase extends BaseProviderCase {}
 
 @Injectable()
-export class ProviderClass {}
+export class ProviderClass extends BaseProviderCase {}
 
 export class DontNeedCase {}
 
@@ -55,7 +54,7 @@ export class DirectiveCase {}
 @NgModule({
   providers: [
     TypeCase,
-    {provide: ProvideCase},
+    { provide: EmptyProviderLiteralCase, useValue: undefined },
     {provide: DontNeedCase, useValue: 0},
     {provide: DontNeedCase, useFactory: () => null},
     {provide: DontNeedCase, useExisting: TypeCase},
@@ -65,4 +64,3 @@ export class DirectiveCase {}
   declarations: [ProvidersTestDirective, ProvidersTestComponent],
 })
 export class ProvidersTestModule {}
-

packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts

@@ -160,7 +160,7 @@ export class StaticInterpreter {
     return array;
   }
 
-  private visitObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: Context):
+  protected visitObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: Context):
       ResolvedValue {
     const map: ResolvedValueMap = new Map<string, ResolvedValue>();
     for (let i = 0; i < node.properties.length; i++) {

packages/core/schematics/migrations/missing-injectable/google3/tslint_update_recorder.ts

@@ -54,5 +54,13 @@ export class TslintUpdateRecorder implements UpdateRecorder {
         this.ruleName, fix));
   }
 
+
+  updateObjectLiteral(node: ts.ObjectLiteralExpression, newText: string): void {
+    this.failures.push(new RuleFailure(
+        this.sourceFile, node.getStart(), node.getEnd(),
+        `Object literal needs to be updated to: ${newText}`, this.ruleName,
+        Replacement.replaceFromTo(node.getStart(), node.getEnd(), newText)));
+  }
+
   commitUpdate() {}
 }

packages/core/schematics/migrations/missing-injectable/index.ts

@@ -107,6 +107,10 @@ function runMissingInjectableMigration(
         treeRecorder.remove(namedBindings.getStart(), namedBindings.getWidth());
         treeRecorder.insertRight(namedBindings.getStart(), newNamedBindings);
       },
+      updateObjectLiteral(node: ts.ObjectLiteralExpression, newText: string) {
+        treeRecorder.remove(node.getStart(), node.getWidth());
+        treeRecorder.insertRight(node.getStart(), newText);
+      },
       commitUpdate() { tree.commitUpdate(treeRecorder); }
     };
     updateRecorders.set(sourceFile, recorder);

packages/core/schematics/migrations/missing-injectable/providers_evaluator.ts

@@ -0,0 +1,58 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {forwardRefResolver} from '@angular/compiler-cli/src/ngtsc/annotations';
+import {ResolvedValue} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
+import {StaticInterpreter} from '@angular/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter';
+import * as ts from 'typescript';
+
+export interface ProviderLiteral {
+  node: ts.ObjectLiteralExpression;
+  resolvedValue: ResolvedValue;
+}
+
+/**
+ * Providers evaluator that extends the ngtsc static interpreter. This is necessary because
+ * the static interpreter by default only exposes the resolved value, but we are also interested
+ * in the TypeScript nodes that declare providers. It would be possible to manually traverse the
+ * AST to collect these nodes, but that would mean that we need to re-implement the static
+ * interpreter in order to handle all possible scenarios. (e.g. spread operator, function calls,
+ * callee scope). This can be avoided by simply extending the static interpreter and intercepting
+ * the "visitObjectLiteralExpression" method.
+ */
+export class ProvidersEvaluator extends StaticInterpreter {
+  private _providerLiterals: ProviderLiteral[] = [];
+
+  visitObjectLiteralExpression(node: ts.ObjectLiteralExpression, context: any) {
+    const resolvedValue =
+        super.visitObjectLiteralExpression(node, {...context, insideProviderDef: true});
+    // do not collect nested object literals. e.g. a provider could use a
+    // spread assignment (which resolves to another object literal). In that
+    // case the referenced object literal is not a provider object literal.
+    if (!context.insideProviderDef) {
+      this._providerLiterals.push({node, resolvedValue});
+    }
+    return resolvedValue;
+  }
+
+  /**
+   * Evaluates the given expression and returns its statically resolved value
+   * and a list of object literals which define Angular providers.
+   */
+  evaluate(expr: ts.Expression) {
+    this._providerLiterals = [];
+    const resolvedValue = this.visit(expr, {
+      originatingFile: expr.getSourceFile(),
+      absoluteModuleName: null,
+      resolutionContext: expr.getSourceFile().fileName,
+      scope: new Map(),
+      foreignFunctionResolver: forwardRefResolver
+    });
+    return {resolvedValue, literals: this._providerLiterals};
+  }
+}

packages/core/schematics/migrations/missing-injectable/transform.ts

@@ -6,9 +6,8 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {forwardRefResolver} from '@angular/compiler-cli/src/ngtsc/annotations/src/util';
 import {Reference} from '@angular/compiler-cli/src/ngtsc/imports';
-import {DynamicValue, PartialEvaluator, ResolvedValue} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
+import {DynamicValue, ResolvedValue} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
 import {TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
 import * as ts from 'typescript';
 
@@ -16,6 +15,7 @@ import {getAngularDecorators} from '../../utils/ng_decorators';
 
 import {ResolvedDirective, ResolvedNgModule} from './definition_collector';
 import {ImportManager} from './import_manager';
+import {ProviderLiteral, ProvidersEvaluator} from './providers_evaluator';
 import {UpdateRecorder} from './update_recorder';
 
 
@@ -30,16 +30,19 @@ export interface AnalysisFailure {
 export class MissingInjectableTransform {
   private printer = ts.createPrinter();
   private importManager = new ImportManager(this.getUpdateRecorder, this.printer);
-  private partialEvaluator: PartialEvaluator;
+  private providersEvaluator: ProvidersEvaluator;
 
   /** Set of provider class declarations which were already checked or migrated. */
   private visitedProviderClasses = new Set<ts.ClassDeclaration>();
 
+  /** Set of provider object literals which were already checked or migrated. */
+  private visitedProviderLiterals = new Set<ts.ObjectLiteralExpression>();
+
   constructor(
       private typeChecker: ts.TypeChecker,
       private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {
-    this.partialEvaluator =
+    this.providersEvaluator =
-        new PartialEvaluator(new TypeScriptReflectionHost(typeChecker), typeChecker);
+        new ProvidersEvaluator(new TypeScriptReflectionHost(typeChecker), typeChecker);
   }
 
   recordChanges() { this.importManager.recordChanges(); }
@@ -68,16 +71,17 @@ export class MissingInjectableTransform {
       return [];
     }
 
-    const evaluatedExpr = this._evaluateExpression(module.providersExpr);
+    const {resolvedValue, literals} = this.providersEvaluator.evaluate(module.providersExpr);
+    this._migrateLiteralProviders(literals);
 
-    if (!Array.isArray(evaluatedExpr)) {
+    if (!Array.isArray(resolvedValue)) {
       return [{
         node: module.providersExpr,
         message: 'Providers of module are not statically analyzable.'
       }];
     }
 
-    return this._visitProviderResolvedValue(evaluatedExpr, module);
+    return this._visitProviderResolvedValue(resolvedValue, module);
   }
 
 
@@ -90,24 +94,27 @@ export class MissingInjectableTransform {
 
     // Migrate "providers" on directives and components if defined.
     if (directive.providersExpr) {
-      const evaluatedExpr = this._evaluateExpression(directive.providersExpr);
+      const {resolvedValue, literals} = this.providersEvaluator.evaluate(directive.providersExpr);
-      if (!Array.isArray(evaluatedExpr)) {
+      this._migrateLiteralProviders(literals);
+      if (!Array.isArray(resolvedValue)) {
         return [
           {node: directive.providersExpr, message: `Providers are not statically analyzable.`}
         ];
       }
-      failures.push(...this._visitProviderResolvedValue(evaluatedExpr, directive));
+      failures.push(...this._visitProviderResolvedValue(resolvedValue, directive));
     }
 
     // Migrate "viewProviders" on components if defined.
     if (directive.viewProvidersExpr) {
-      const evaluatedExpr = this._evaluateExpression(directive.viewProvidersExpr);
+      const {resolvedValue, literals} =
-      if (!Array.isArray(evaluatedExpr)) {
+          this.providersEvaluator.evaluate(directive.viewProvidersExpr);
+      this._migrateLiteralProviders(literals);
+      if (!Array.isArray(resolvedValue)) {
         return [
           {node: directive.viewProvidersExpr, message: `Providers are not statically analyzable.`}
         ];
       }
-      failures.push(...this._visitProviderResolvedValue(evaluatedExpr, directive));
+      failures.push(...this._visitProviderResolvedValue(resolvedValue, directive));
     }
     return failures;
   }
@@ -160,11 +167,39 @@ export class MissingInjectableTransform {
   }
 
   /**
-   * Evaluates the given TypeScript expression using the partial evaluator with
+   * Migrates object literal providers which do not use "useValue", "useClass",
-   * the foreign function resolver for handling "forwardRef" calls.
+   * "useExisting" or "useFactory". These providers behave differently in Ivy. e.g.
+   *
+   * ```ts
+   *   {provide: X} -> {provide: X, useValue: undefined} // this is how it behaves in VE
+   *   {provide: X} -> {provide: X, useClass: X} // this is how it behaves in Ivy
+   * ```
+   *
+   * To ensure forward compatibility, we migrate these empty object literal providers
+   * to explicitly use `useValue: undefined`.
    */
-  private _evaluateExpression(expr: ts.Expression): ResolvedValue {
+  private _migrateLiteralProviders(literals: ProviderLiteral[]) {
-    return this.partialEvaluator.evaluate(expr, forwardRefResolver);
+    for (let {node, resolvedValue} of literals) {
+      if (this.visitedProviderLiterals.has(node)) {
+        continue;
+      }
+      this.visitedProviderLiterals.add(node);
+
+      if (!resolvedValue || !(resolvedValue instanceof Map) || !resolvedValue.has('provide') ||
+          resolvedValue.has('useClass') || resolvedValue.has('useValue') ||
+          resolvedValue.has('useExisting') || resolvedValue.has('useFactory')) {
+        continue;
+      }
+
+      const sourceFile = node.getSourceFile();
+      const newObjectLiteral = ts.updateObjectLiteral(
+          node, node.properties.concat(
+                    ts.createPropertyAssignment('useValue', ts.createIdentifier('undefined'))));
+
+      this.getUpdateRecorder(sourceFile)
+          .updateObjectLiteral(
+              node, this.printer.printNode(ts.EmitHint.Unspecified, newObjectLiteral, sourceFile));
+    }
   }
 
   /**
@@ -177,14 +212,11 @@ export class MissingInjectableTransform {
     if (value instanceof Reference && ts.isClassDeclaration(value.node)) {
       this.migrateProviderClass(value.node, module);
     } else if (value instanceof Map) {
-      if (!value.has('provide') || value.has('useValue') || value.has('useFactory') ||
+      // If a "ClassProvider" has the "deps" property set, then we do not need to
-          value.has('useExisting')) {
+      // decorate the class. This is because the class is instantiated through the
-        return [];
+      // specified "deps" and the class does not need a factory definition.
-      }
+      if (value.has('provide') && value.has('useClass') && value.get('deps') == null) {
-      if (value.has('useClass')) {
         return this._visitProviderResolvedValue(value.get('useClass') !, module);
-      } else {
-        return this._visitProviderResolvedValue(value.get('provide') !, module);
       }
     } else if (Array.isArray(value)) {
       return value.reduce((res, v) => res.concat(this._visitProviderResolvedValue(v, module)), [

packages/core/schematics/migrations/missing-injectable/update_recorder.ts

@@ -18,5 +18,6 @@ export interface UpdateRecorder {
   updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string): void;
   addClassDecorator(node: ts.ClassDeclaration, text: string, className: string): void;
   replaceDecorator(node: ts.Decorator, newText: string, className: string): void;
+  updateObjectLiteral(node: ts.ObjectLiteralExpression, newText: string): void;
   commitUpdate(): void;
 }

packages/core/schematics/test/google3/missing_injectable_rule_spec.ts

@@ -121,6 +121,7 @@ describe('Google3 missing injectable tslint rule', () => {
         export class MyServiceE {}
         export class MyServiceF {}
         export class MyServiceG {}
+        export class MyServiceH {}
             
         @${type}({${propName}: [
           WithPipe,
@@ -135,6 +136,7 @@ describe('Google3 missing injectable tslint rule', () => {
           {provide: null, useExisting: MyServiceE},
           {provide: MyServiceF, useFactory: () => null},
           {provide: MyServiceG, useValue: null},
+          {provide: MyServiceH, deps: []},
         ]})
         export class TestClass {}
       `);
@@ -149,11 +151,13 @@ describe('Google3 missing injectable tslint rule', () => {
           .toMatch(/WithDirective {}\s+@Component\(\)\s+export class WithComponent/);
       expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceA/);
       expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceB/);
-      expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceC/);
+      expect(getFile('/index.ts')).toMatch(/MyServiceB {}\s+export class MyServiceC/);
       expect(getFile('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyServiceD/);
       expect(getFile('/index.ts')).toMatch(/MyServiceD {}\s+export class MyServiceE/);
       expect(getFile('/index.ts')).toMatch(/MyServiceE {}\s+export class MyServiceF/);
       expect(getFile('/index.ts')).toMatch(/MyServiceF {}\s+export class MyServiceG/);
+      expect(getFile('/index.ts')).toMatch(/MyServiceG {}\s+export class MyServiceH/);
+      expect(getFile('/index.ts')).toContain(`{ provide: MyServiceC, useValue: undefined },`);
     });
 
     it(`should migrate provider once if referenced in multiple ${type} definitions`, () => {

packages/core/schematics/test/missing_injectable_migration_spec.ts

@@ -121,7 +121,7 @@ describe('Missing injectable migration', () => {
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
     });
 
-    it(`should migrate object literal provider in ${type}`, async() => {
+    it(`should migrate object literal provider in ${type} to explicit value provider`, async() => {
       writeFile('/index.ts', `
         import {${type}} from '@angular/core';
       
@@ -134,16 +134,17 @@ describe('Missing injectable migration', () => {
       await runMigration();
 
       expect(warnOutput.length).toBe(0);
-      expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyService/);
+      expect(tree.readContent('/index.ts')).toMatch(/@angular\/core';\s+export class MyService/);
       expect(tree.readContent('/index.ts'))
-          .toContain(`{ ${type}, Injectable } from '@angular/core`);
+          .toContain(`${propName}: [{ provide: MyService, useValue: undefined }]`);
+      expect(tree.readContent('/index.ts')).toContain(`{${type}} from '@angular/core`);
     });
 
     it(`should migrate object literal provider with forwardRef in ${type}`, async() => {
       writeFile('/index.ts', `
         import {${type}, forwardRef} from '@angular/core';
                   
-        @${type}({${propName}: [{provide: forwardRef(() => MyService)}]})
+        @${type}({${propName}: [forwardRef(() => MyService)]})
         export class TestClass {}
 
         export class MyService {}
@@ -173,6 +174,22 @@ describe('Missing injectable migration', () => {
       expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
     });
 
+    it(`should not migrate provider with "useClass" and "deps" in ${type}`, async() => {
+      writeFile('/index.ts', `
+        import {${type}} from '@angular/core';
+      
+        export class MyService {}
+            
+        @${type}({${propName}: [{provide: MyService, deps: []}]})
+        export class TestClass {}
+      `);
+
+      await runMigration();
+
+      expect(warnOutput.length).toBe(0);
+      expect(tree.readContent('/index.ts')).not.toContain('@Injectable');
+    });
+
     it(`should not migrate object literal provider with "useFactory" in ${type}`, async() => {
       writeFile('/index.ts', `
         import {${type}} from '@angular/core';
@@ -251,7 +268,7 @@ describe('Missing injectable migration', () => {
         import {MyService} from './index';
         
         export @${type}({
-          ${propName}: [{provide: MyService}],
+          ${propName}: [{provide: MyService, useClass: MyService}],
         })
         export class OtherClass {}
       `);
@@ -374,10 +391,12 @@ describe('Missing injectable migration', () => {
 
       expect(warnOutput.length).toBe(0);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
-      expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
+      expect(tree.readContent('/index.ts')).toMatch(/ServiceA {}\s+export class ServiceB/);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceC/);
       expect(tree.readContent('/index.ts'))
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
+      expect(tree.readContent('/index.ts'))
+          .toContain(`{ provide: ServiceB, useValue: undefined },`);
     });
 
     it(`should migrate multiple nested providers in same ${type}`, async() => {
@@ -387,13 +406,15 @@ describe('Missing injectable migration', () => {
         export class ServiceA {}
         export class ServiceB {}
         export class ServiceC {}
+        export class ServiceD {}
             
         @${type}({
           ${propName}: [
             ServiceA,
             [
-              {provide: ServiceB},
+              {provide: ServiceB, useClass: ServiceB},
               ServiceC,
+              {provide: ServiceD},
             ],
           ]})
         export class TestClass {}
@@ -405,8 +426,11 @@ describe('Missing injectable migration', () => {
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceC/);
+      expect(tree.readContent('/index.ts')).toMatch(/ServiceC {}\s+export class ServiceD/);
       expect(tree.readContent('/index.ts'))
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
+      expect(tree.readContent('/index.ts'))
+          .toContain(`{ provide: ServiceD, useValue: undefined },`);
     });
 
     it('should migrate providers referenced through identifier', async() => {
@@ -415,8 +439,15 @@ describe('Missing injectable migration', () => {
       
         export class ServiceA {}
         export class ServiceB {}
+        export class ServiceC {}
         
-        const PROVIDERS = [ServiceA, ServiceB];
+        const PROVIDERS = [
+          ServiceA,
+          ServiceB,
+          // trailing comma is by intention to check if do not create
+          // an invalid object literal.
+          {provide: ServiceC, },
+        ];
             
         @${type}({
           ${propName}: PROVIDERS,
@@ -429,8 +460,11 @@ describe('Missing injectable migration', () => {
       expect(warnOutput.length).toBe(0);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
+      expect(tree.readContent('/index.ts')).toMatch(/ServiceB {}\s+export class ServiceC/);
       expect(tree.readContent('/index.ts'))
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
+      expect(tree.readContent('/index.ts'))
+          .toContain(`{ provide: ServiceC, useValue: undefined },`);
     });
 
     it('should migrate providers created through static analyzable function call', async() => {
@@ -439,13 +473,14 @@ describe('Missing injectable migration', () => {
       
         export class ServiceA {}
         export class ServiceB {}
+        export class ServiceC {}
         
-        export function createProviders(x: any) {
+        export function createProviders(x: any, b: any) {
-          return [ServiceA, x]
+          return [ServiceA, x, b]
         }
             
         @${type}({
-          ${propName}: createProviders(ServiceB),
+          ${propName}: createProviders(ServiceB, {provide: ServiceC}),
         })
         export class TestClass {}
       `);
@@ -455,8 +490,11 @@ describe('Missing injectable migration', () => {
       expect(warnOutput.length).toBe(0);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
+      expect(tree.readContent('/index.ts')).toMatch(/ServiceB {}\s+export class ServiceC/);
       expect(tree.readContent('/index.ts'))
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
+      expect(tree.readContent('/index.ts'))
+          .toContain(`ServiceB, { provide: ServiceC, useValue: undefined }),`);
     });
 
     it('should migrate providers which are computed through spread operator', async() => {
@@ -465,8 +503,9 @@ describe('Missing injectable migration', () => {
       
         export class ServiceA {}
         export class ServiceB {}
+        export class ServiceC {}
         
-        const otherServices = [ServiceB];
+        const otherServices = [ServiceB, {provide: ServiceC}];
             
         @${type}({
           ${propName}: [ServiceA, ...otherServices],
@@ -479,8 +518,11 @@ describe('Missing injectable migration', () => {
       expect(warnOutput.length).toBe(0);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceA/);
       expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class ServiceB/);
+      expect(tree.readContent('/index.ts')).toMatch(/ServiceB {}\s+export class ServiceC/);
       expect(tree.readContent('/index.ts'))
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
+      expect(tree.readContent('/index.ts'))
+          .toContain(`ServiceB, { provide: ServiceC, useValue: undefined }];`);
     });
 
     it(`should migrate provider once if referenced in multiple ${type} definitions`, async() => {
@@ -515,6 +557,41 @@ describe('Missing injectable migration', () => {
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
     });
 
+    it(`should only migrate empty object provider literal once if referenced multiple times ` +
+           `in ${type} definitions`,
+       async() => {
+         writeFile('/provider.ts', `
+        export class MyService {}
+      
+        export const PROVIDER = {provide: MyService};
+      `);
+
+         writeFile('/index.ts', `
+        import {${type}} from '@angular/core';
+        import {PROVIDER} from './provider';
+                        
+        @${type}({${propName}: [PROVIDER]})
+        export class TestClassA {}
+      `);
+
+         writeFile('/second.ts', `
+        import {${type}} from '@angular/core';
+        import {PROVIDER} from './provider';
+        
+        export class ServiceB {}        
+        
+        @${type}({${propName}: [PROVIDER, ServiceB]})
+        export class TestClassB {}
+      `);
+
+         await runMigration();
+
+         expect(warnOutput.length).toBe(0);
+         expect(tree.readContent('/provider.ts')).toMatch(/^\s+export class MyService {}/);
+         expect(tree.readContent('/provider.ts'))
+             .toContain(`const PROVIDER = { provide: MyService, useValue: undefined };`);
+       });
+
     it('should create new import for @Injectable when migrating provider', async() => {
       writeFile('/index.ts', `
         import {${type}} from '@angular/core';