refactor(compiler): extract api docs for inherited members (angular#52389)

This commit expands docs extraction for classes and interfaces to include inherited members. This relies on the type checker to get the _resolved_ members of the type so that the extractor doesn't need to reason about inheritance rules, which can get tricky (especially with regards to method overloads).

PR Close angular#52389

Author: jelbourn

packages/compiler-cli/src/ngtsc/docs/src/class_extractor.ts

@@ -53,7 +53,7 @@ class ClassExtractor {
       isAbstract: this.isAbstract(),
       entryType: ts.isInterfaceDeclaration(this.declaration) ? EntryType.Interface :
                                                                EntryType.UndecoratedClass,
-      members: this.extractAllClassMembers(this.declaration),
+      members: this.extractAllClassMembers(),
       generics: extractGenerics(this.declaration),
       description: extractJsDocDescription(this.declaration),
       jsdocTags: extractJsDocTags(this.declaration),
@@ -62,10 +62,10 @@ class ClassExtractor {
   }
 
   /** Extracts doc info for a class's members. */
-  protected extractAllClassMembers(classDeclaration: ClassDeclarationLike): MemberEntry[] {
+  protected extractAllClassMembers(): MemberEntry[] {
     const members: MemberEntry[] = [];
 
-    for (const member of classDeclaration.members) {
+    for (const member of this.getMemberDeclarations()) {
       if (this.isMemberExcluded(member)) continue;
 
       const memberEntry = this.extractClassMember(member);
@@ -130,9 +130,40 @@ class ClassExtractor {
       tags.push(MemberTags.Optional);
     }
 
+    if (member.parent !== this.declaration) {
+      tags.push(MemberTags.Inherited);
+    }
+
     return tags;
   }
 
+  /** Gets all member declarations, including inherited members. */
+  private getMemberDeclarations(): MemberElement[] {
+    // We rely on TypeScript to resolve all the inherited members to their
+    // ultimate form via `getPropertiesOfType`. This is important because child
+    // classes may narrow types or add method overloads.
+    const type = this.typeChecker.getTypeAtLocation(this.declaration);
+    const members = type.getProperties();
+
+    // While the properties of the declaration type represent the properties that exist
+    // on a clas *instance*, static members are properties on the class symbol itself.
+    const typeOfConstructor = this.typeChecker.getTypeOfSymbol(type.symbol);
+    const staticMembers = typeOfConstructor.getProperties();
+
+    const result: MemberElement[] = [];
+    for (const member of [...members, ...staticMembers]) {
+      // A member may have multiple declarations in the case of function overloads.
+      const memberDeclarations = member.getDeclarations() ?? [];
+      for (const memberDeclaration of memberDeclarations) {
+        if (this.isDocumentableMember(memberDeclaration)) {
+          result.push(memberDeclaration);
+        }
+      }
+    }
+
+    return result;
+  }
+
   /** Get the tags for a member that come from the declaration modifiers. */
   private getMemberTagsFromModifiers(mods: Iterable<ts.ModifierLike>): MemberTags[] {
     const tags: MemberTags[] = [];
@@ -170,22 +201,22 @@ class ClassExtractor {
   private isMemberExcluded(member: MemberElement): boolean {
     return !member.name || !this.isDocumentableMember(member) ||
         !!member.modifiers?.some(mod => mod.kind === ts.SyntaxKind.PrivateKeyword) ||
-        isAngularPrivateName(member.name.getText());
+        member.name.getText() === 'prototype' || isAngularPrivateName(member.name.getText());
   }
 
   /** Gets whether a class member is a method, property, or accessor. */
-  private isDocumentableMember(member: MemberElement): member is MethodLike|PropertyLike {
+  private isDocumentableMember(member: ts.Node): member is MethodLike|PropertyLike {
     return this.isMethod(member) || this.isProperty(member) || ts.isAccessor(member);
   }
 
   /** Gets whether a member is a property. */
-  private isProperty(member: MemberElement): member is PropertyLike {
+  private isProperty(member: ts.Node): member is PropertyLike {
     // Classes have declarations, interface have signatures
     return ts.isPropertyDeclaration(member) || ts.isPropertySignature(member);
   }
 
   /** Gets whether a member is a method. */
-  private isMethod(member: MemberElement): member is MethodLike {
+  private isMethod(member: ts.Node): member is MethodLike {
     // Classes have declarations, interface have signatures
     return ts.isMethodDeclaration(member) || ts.isMethodSignature(member);
   }
@@ -197,20 +228,14 @@ class ClassExtractor {
   }
 
   /** Gets whether a method is the concrete implementation for an overloaded function. */
-  private isImplementationForOverload(method: MethodLike): boolean {
+  private isImplementationForOverload(method: MethodLike): boolean|undefined {
     // Method signatures (in an interface) are never implementations.
     if (method.kind === ts.SyntaxKind.MethodSignature) return false;
 
-    const methodsWithSameName =
-        this.declaration.members.filter(member => member.name?.getText() === method.name.getText())
-            .sort((a, b) => a.pos - b.pos);
-
-    // No overloads.
-    if (methodsWithSameName.length === 1) return false;
-
-    // The implementation is always the last declaration, so we know this is the
-    // implementation if it's the last position.
-    return method.pos === methodsWithSameName[methodsWithSameName.length - 1].pos;
+    const signature = this.typeChecker.getSignatureFromDeclaration(method);
+    return signature &&
+        this.typeChecker.isImplementationOfOverload(
+            signature.declaration as ts.SignatureDeclaration);
   }
 }
 

packages/compiler-cli/src/ngtsc/docs/src/entities.ts

@@ -47,6 +47,7 @@ export enum MemberTags {
   Optional = 'optional',
   Input = 'input',
   Output = 'output',
+  Inherited = 'override',
 }
 
 /** Documentation entity for single JsDoc tag. */

packages/compiler-cli/test/ngtsc/doc_extraction/class_doc_extraction_spec.ts

@@ -73,8 +73,8 @@ runInEachFileSystem(() => {
         export class UserProfile {
           ident(value: boolean): boolean
           ident(value: number): number
-          ident(value: number|boolean): number|boolean {
-            return value;
+          ident(value: number|boolean|string): number|boolean {
+            return 0;
           }
         }
       `);
@@ -207,13 +207,13 @@ runInEachFileSystem(() => {
         nameMember,
         ageMember,
         addressMember,
-        countryMember,
         birthdayMember,
         getEyeColorMember,
         getNameMember,
         getAgeMember,
-        getCountryMember,
         getBirthdayMember,
+        countryMember,
+        getCountryMember,
       ] = classEntry.members;
 
       // Properties
@@ -381,5 +381,114 @@ runInEachFileSystem(() => {
       expect(genericEntry.constraint).toBeUndefined();
       expect(genericEntry.default).toBeUndefined();
     });
+
+    it('should extract inherited members', () => {
+      env.write('index.ts', `
+        class Ancestor {
+          id: string;
+          value: string|number;
+
+          save(value: string|number): string|number { return 0; }
+        }
+
+        class Parent extends Ancestor {
+          name: string;
+        }
+
+        export class Child extends Parent {
+          age: number;
+          value: number;
+
+          save(value: number): number;
+          save(value: string|number): string|number { return 0; }
+        }`);
+
+      const docs: DocEntry[] = env.driveDocsExtraction('index.ts');
+      expect(docs.length).toBe(1);
+
+      const classEntry = docs[0] as ClassEntry;
+      expect(classEntry.members.length).toBe(5);
+
+      const [ageEntry, valueEntry, childSaveEntry, nameEntry, idEntry] = classEntry.members;
+
+      expect(ageEntry.name).toBe('age');
+      expect(ageEntry.memberType).toBe(MemberType.Property);
+      expect((ageEntry as PropertyEntry).type).toBe('number');
+      expect(ageEntry.memberTags).not.toContain(MemberTags.Inherited);
+
+      expect(valueEntry.name).toBe('value');
+      expect(valueEntry.memberType).toBe(MemberType.Property);
+      expect((valueEntry as PropertyEntry).type).toBe('number');
+      expect(valueEntry.memberTags).not.toContain(MemberTags.Inherited);
+
+      expect(childSaveEntry.name).toBe('save');
+      expect(childSaveEntry.memberType).toBe(MemberType.Method);
+      expect((childSaveEntry as MethodEntry).returnType).toBe('number');
+      expect(childSaveEntry.memberTags).not.toContain(MemberTags.Inherited);
+
+      expect(nameEntry.name).toBe('name');
+      expect(nameEntry.memberType).toBe(MemberType.Property);
+      expect((nameEntry as PropertyEntry).type).toBe('string');
+      expect(nameEntry.memberTags).toContain(MemberTags.Inherited);
+
+      expect(idEntry.name).toBe('id');
+      expect(idEntry.memberType).toBe(MemberType.Property);
+      expect((idEntry as PropertyEntry).type).toBe('string');
+      expect(idEntry.memberTags).toContain(MemberTags.Inherited);
+    });
+
+    it('should extract inherited getters/setters', () => {
+      env.write('index.ts', `
+        class Ancestor {
+          get name(): string { return ''; }
+          set name(v: string) { }
+
+          get id(): string { return ''; }
+          set id(v: string) { }
+
+          get age(): number { return 0; }
+          set age(v: number) { }
+        }
+
+        class Parent extends Ancestor {
+          name: string;
+        }
+
+        export class Child extends Parent {
+          get id(): string { return ''; }
+        }`);
+
+      const docs: DocEntry[] = env.driveDocsExtraction('index.ts');
+      expect(docs.length).toBe(1);
+
+      const classEntry = docs[0] as ClassEntry;
+      expect(classEntry.members.length).toBe(4);
+
+      const [idEntry, nameEntry, ageGetterEntry, ageSetterEntry] =
+          classEntry.members as PropertyEntry[];
+
+      // When the child class overrides an accessor pair with another accessor, it overrides
+      // *both* the getter and the setter, resulting (in this case) in just a getter.
+      expect(idEntry.name).toBe('id');
+      expect(idEntry.memberType).toBe(MemberType.Getter);
+      expect((idEntry as PropertyEntry).type).toBe('string');
+      expect(idEntry.memberTags).not.toContain(MemberTags.Inherited);
+
+      // When the child class overrides an accessor with a property, the property takes precedence.
+      expect(nameEntry.name).toBe('name');
+      expect(nameEntry.memberType).toBe(MemberType.Property);
+      expect(nameEntry.type).toBe('string');
+      expect(nameEntry.memberTags).toContain(MemberTags.Inherited);
+
+      expect(ageGetterEntry.name).toBe('age');
+      expect(ageGetterEntry.memberType).toBe(MemberType.Getter);
+      expect(ageGetterEntry.type).toBe('number');
+      expect(ageGetterEntry.memberTags).toContain(MemberTags.Inherited);
+
+      expect(ageSetterEntry.name).toBe('age');
+      expect(ageSetterEntry.memberType).toBe(MemberType.Setter);
+      expect(ageSetterEntry.type).toBe('number');
+      expect(ageSetterEntry.memberTags).toContain(MemberTags.Inherited);
+    });
   });
 });