fix(core): undecorated-classes-with-decorated-fields migration should avoid error if base class has no value declaration (#36543)

The undecorated-classes-with-decorated-fields migration relies on
the type checker to resolve base classes of individual classes.

It could happen that resolved base classes have no value declaration.
e.g. if they are declared through an interface in the default types.
Currently the migration will throw in such situations because it assumes
that `ts.Symbol#valueDeclaration` is always present. This is not the
case, but we don't get good type-checking here due to a bug in the
TypeScript types. See:
https://github.com/microsoft/TypeScript/issues/24706.

Fixes #36522.

PR Close #36543

packages/core/schematics/test/undecorated_classes_with_decorated_fields_migration_spec.ts

@@ -42,7 +42,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     shx.rm('-r', tmpDirPath);
   });
 
-  it(`should add an import for Directive if there isn't one already`, async() => {
+  it(`should add an import for Directive if there isn't one already`, async () => {
     writeFile('/index.ts', `
       import { Input } from '@angular/core';
 
@@ -56,7 +56,7 @@ describe('Undecorated classes with decorated fields migration', () => {
         .toContain(`import { Input, Directive } from '@angular/core';`);
   });
 
-  it('should not change the imports if there is an import for Directive already', async() => {
+  it('should not change the imports if there is an import for Directive already', async () => {
     writeFile('/index.ts', `
       import { Directive, Input } from '@angular/core';
 
@@ -74,8 +74,9 @@ describe('Undecorated classes with decorated fields migration', () => {
         .toContain(`import { Directive, Input } from '@angular/core';`);
   });
 
-  it('should not generate conflicting imports there is a different `Directive` symbol', async() => {
-    writeFile('/index.ts', `
+  it('should not generate conflicting imports there is a different `Directive` symbol',
+     async () => {
+       writeFile('/index.ts', `
       import { HostBinding } from '@angular/core';
       
       export class Directive {
@@ -88,14 +89,14 @@ describe('Undecorated classes with decorated fields migration', () => {
       }
     `);
 
-    await runMigration();
-    const fileContent = tree.readContent('/index.ts');
-    expect(fileContent)
-        .toContain(`import { HostBinding, Directive as Directive_1 } from '@angular/core';`);
-    expect(fileContent).toMatch(/@Directive_1\(\)\s+export class MyLibrarySharedBaseClass/);
-  });
+       await runMigration();
+       const fileContent = tree.readContent('/index.ts');
+       expect(fileContent)
+           .toContain(`import { HostBinding, Directive as Directive_1 } from '@angular/core';`);
+       expect(fileContent).toMatch(/@Directive_1\(\)\s+export class MyLibrarySharedBaseClass/);
+     });
 
-  it('should add @Directive to undecorated classes that have @Input', async() => {
+  it('should add @Directive to undecorated classes that have @Input', async () => {
     writeFile('/index.ts', `
       import { Input } from '@angular/core';
 
@@ -108,7 +109,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should not change decorated classes', async() => {
+  it('should not change decorated classes', async () => {
     writeFile('/index.ts', `
       import { Input, Component, Output, EventEmitter } from '@angular/core';
 
@@ -130,7 +131,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(content).toContain(`@Directive()\nexport class Child extends Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have @Output', async() => {
+  it('should add @Directive to undecorated classes that have @Output', async () => {
     writeFile('/index.ts', `
       import { Output, EventEmitter } from '@angular/core';
 
@@ -143,7 +144,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have a host binding', async() => {
+  it('should add @Directive to undecorated classes that have a host binding', async () => {
     writeFile('/index.ts', `
       import { HostBinding } from '@angular/core';
 
@@ -159,7 +160,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have a host listener', async() => {
+  it('should add @Directive to undecorated classes that have a host listener', async () => {
     writeFile('/index.ts', `
       import { HostListener } from '@angular/core';
 
@@ -175,7 +176,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have a ViewChild query', async() => {
+  it('should add @Directive to undecorated classes that have a ViewChild query', async () => {
     writeFile('/index.ts', `
       import { ViewChild, ElementRef } from '@angular/core';
 
@@ -188,7 +189,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have a ViewChildren query', async() => {
+  it('should add @Directive to undecorated classes that have a ViewChildren query', async () => {
     writeFile('/index.ts', `
       import { ViewChildren, ElementRef } from '@angular/core';
 
@@ -201,7 +202,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have a ContentChild query', async() => {
+  it('should add @Directive to undecorated classes that have a ContentChild query', async () => {
     writeFile('/index.ts', `
       import { ContentChild, ElementRef } from '@angular/core';
 
@@ -214,7 +215,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated classes that have a ContentChildren query', async() => {
+  it('should add @Directive to undecorated classes that have a ContentChildren query', async () => {
     writeFile('/index.ts', `
       import { ContentChildren, ElementRef } from '@angular/core';
 
@@ -227,7 +228,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`);
   });
 
-  it('should add @Directive to undecorated derived classes of a migrated class', async() => {
+  it('should add @Directive to undecorated derived classes of a migrated class', async () => {
     writeFile('/index.ts', `
       import { Input, Directive, NgModule } from '@angular/core';
 
@@ -259,7 +260,7 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(fileContent).toMatch(/}\s+export class MyCompWrapped/);
   });
 
-  it('should add @Directive to derived undecorated classes of abstract directives', async() => {
+  it('should add @Directive to derived undecorated classes of abstract directives', async () => {
     writeFile('/index.ts', `
       import { Input, Directive, NgModule } from '@angular/core';
 
@@ -292,6 +293,27 @@ describe('Undecorated classes with decorated fields migration', () => {
     expect(fileContent).toMatch(/}\s+export class MyCompWrapped/);
   });
 
+  it('should not throw if undecorated class extends from unresolved declaration', async () => {
+    writeFile('/lib.d.ts', `
+      // Fakes the ES5 error default lib types. Since we are in a virtual tree,
+      // the default lib types from TypeScript are not available.
+      interface ErrorConstructor {}
+      declare var Error: ErrorConstructor;
+    `);
+    writeFile('/index.ts', `
+      export class MyCustomErrorClass extends Error {}
+    `);
+
+    let error: any = null;
+    try {
+      await runMigration();
+    } catch (e) {
+      error = e;
+    }
+
+    expect(error).toBe(null);
+  });
+
   function writeFile(filePath: string, contents: string) {
     host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));
   }

packages/core/schematics/utils/typescript/find_base_classes.ts

@@ -20,7 +20,9 @@ export function findBaseClassDeclarations(node: ts.ClassDeclaration, typeChecker
       break;
     }
     const symbol = typeChecker.getTypeAtLocation(baseTypes[0]).getSymbol();
-    if (!symbol || !ts.isClassDeclaration(symbol.valueDeclaration)) {
+    // Note: `ts.Symbol#valueDeclaration` can be undefined. TypeScript has an incorrect type
+    // for this: https://github.com/microsoft/TypeScript/issues/24706.
+    if (!symbol || !symbol.valueDeclaration || !ts.isClassDeclaration(symbol.valueDeclaration)) {
       break;
     }
     result.push({identifier: baseTypes[0], node: symbol.valueDeclaration});