From 4656cae417a1ae50e62515fbe92940668e9598e1 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Tue, 22 Oct 2019 10:09:21 +0200
Subject: [PATCH] fix(core): missing-injectable migration should not update
 type definitions

Currently the `missing-injectable` migration seems to add
`@Injectable()` to third-party classes in type definitions.

This not an issue in general since we do not generate broken code
by inserting a decorator into a type definition file. Though, we can
avoid adding the decorator since it won't have any effect and in
general we should not write to non source files of the compilation unit.
---
 .../missing-injectable/transform.ts           |  8 ++++++++
 .../test/missing_injectable_migration_spec.ts | 20 ++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/packages/core/schematics/migrations/missing-injectable/transform.ts b/packages/core/schematics/migrations/missing-injectable/transform.ts
index 8a1fd27d33cc3..e124184906877 100644
--- a/packages/core/schematics/migrations/missing-injectable/transform.ts
+++ b/packages/core/schematics/migrations/missing-injectable/transform.ts
@@ -123,6 +123,14 @@ export class MissingInjectableTransform {
     this.visitedProviderClasses.add(node);
 
     const sourceFile = node.getSourceFile();
+
+    // We cannot migrate provider classes outside of source files. This is because the
+    // migration for third-party library files should happen in "ngcc", and in general
+    // would also involve metadata parsing.
+    if (sourceFile.isDeclarationFile) {
+      return;
+    }
+
     const ngDecorators =
         node.decorators ? getAngularDecorators(this.typeChecker, node.decorators) : null;
 
diff --git a/packages/core/schematics/test/missing_injectable_migration_spec.ts b/packages/core/schematics/test/missing_injectable_migration_spec.ts
index 75dbe9b86ce80..f9526300c2bcc 100644
--- a/packages/core/schematics/test/missing_injectable_migration_spec.ts
+++ b/packages/core/schematics/test/missing_injectable_migration_spec.ts
@@ -380,7 +380,6 @@ describe('Missing injectable migration', () => {
           .toContain(`{ ${type}, Injectable } from '@angular/core`);
     });
 
-
     it(`should migrate multiple nested providers in same ${type}`, async() => {
       writeFile('/index.ts', `
         import {${type}} from '@angular/core';
@@ -689,5 +688,24 @@ describe('Missing injectable migration', () => {
       expect(tree.readContent('/service.ts'))
           .toMatch(/import { Inject, Injectable } from '@angular\/core';/);
     });
+
+    it('should not migrate provider classes in library type definitions', async() => {
+      writeFile('/node_modules/my-lib/index.d.ts', `
+        export declare class MyService {}
+      `);
+
+      writeFile('/index.ts', `
+        import {MyService} from 'my-lib';
+        import {Pipe, ${type}} from '@angular/core';
+                  
+        @${type}({${propName}: [MyService]})
+        export class TestClass {}
+      `);
+
+      await runMigration();
+
+      expect(warnOutput.length).toBe(0);
+      expect(tree.readContent('/node_modules/my-lib/index.d.ts')).not.toContain('@Injectable');
+    });
   }
 });