From 9291ffc418c687e66089a97af5eb477af9daef46 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Wed, 25 Oct 2023 20:47:09 -0700 Subject: [PATCH] refactor(compiler): extract api docs for inherited members (#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 #52389 --- .../src/ngtsc/docs/src/class_extractor.ts | 61 ++++++--- .../src/ngtsc/docs/src/entities.ts | 1 + .../class_doc_extraction_spec.ts | 117 ++++++++++++++++- .../interface_doc_extraction_spec.ts | 124 +++++++++++++++++- 4 files changed, 277 insertions(+), 26 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/docs/src/class_extractor.ts b/packages/compiler-cli/src/ngtsc/docs/src/class_extractor.ts index 04e6878734e..7a51e08ecce 100644 --- a/packages/compiler-cli/src/ngtsc/docs/src/class_extractor.ts +++ b/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): 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); } } diff --git a/packages/compiler-cli/src/ngtsc/docs/src/entities.ts b/packages/compiler-cli/src/ngtsc/docs/src/entities.ts index af824389ab4..884cfc01d80 100644 --- a/packages/compiler-cli/src/ngtsc/docs/src/entities.ts +++ b/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. */ diff --git a/packages/compiler-cli/test/ngtsc/doc_extraction/class_doc_extraction_spec.ts b/packages/compiler-cli/test/ngtsc/doc_extraction/class_doc_extraction_spec.ts index 463bf0b666a..30bbaecdf36 100644 --- a/packages/compiler-cli/test/ngtsc/doc_extraction/class_doc_extraction_spec.ts +++ b/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); + }); }); }); diff --git a/packages/compiler-cli/test/ngtsc/doc_extraction/interface_doc_extraction_spec.ts b/packages/compiler-cli/test/ngtsc/doc_extraction/interface_doc_extraction_spec.ts index 4abb3e63b58..b8518ddeaed 100644 --- a/packages/compiler-cli/test/ngtsc/doc_extraction/interface_doc_extraction_spec.ts +++ b/packages/compiler-cli/test/ngtsc/doc_extraction/interface_doc_extraction_spec.ts @@ -7,7 +7,7 @@ */ import {DocEntry} from '@angular/compiler-cli/src/ngtsc/docs'; -import {EntryType, InterfaceEntry, MemberTags, MemberType, MethodEntry, PropertyEntry} from '@angular/compiler-cli/src/ngtsc/docs/src/entities'; +import {ClassEntry, EntryType, InterfaceEntry, MemberTags, MemberType, MethodEntry, PropertyEntry} from '@angular/compiler-cli/src/ngtsc/docs/src/entities'; import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; import {loadStandardTestFiles} from '@angular/compiler-cli/src/ngtsc/testing'; @@ -184,12 +184,12 @@ runInEachFileSystem(() => { it('should extract getters and setters', () => { // Test getter-only, a getter + setter, and setter-only. env.write('index.ts', ` - export interface UserProfile { + export interface UserProfile { get userId(): number; - + get userName(): string; set userName(value: string); - + set isAdmin(value: boolean); } `); @@ -211,5 +211,121 @@ runInEachFileSystem(() => { expect(isAdminSetter.name).toBe('isAdmin'); expect(isAdminSetter.memberType).toBe(MemberType.Setter); }); + + it('should extract inherited members', () => { + env.write('index.ts', ` + interface Ancestor { + id: string; + value: string|number; + + save(value: string|number): string|number; + } + + interface Parent extends Ancestor { + name: string; + } + + export interface Child extends Parent { + age: number; + value: number; + + save(value: number): number; + save(value: string|number): string|number; + }`); + + const docs: DocEntry[] = env.driveDocsExtraction('index.ts'); + expect(docs.length).toBe(1); + + const interfaceEntry = docs[0] as InterfaceEntry; + expect(interfaceEntry.members.length).toBe(6); + + const [ageEntry, valueEntry, numberSaveEntry, unionSaveEntry, nameEntry, idEntry] = + interfaceEntry.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(numberSaveEntry.name).toBe('save'); + expect(numberSaveEntry.memberType).toBe(MemberType.Method); + expect((numberSaveEntry as MethodEntry).returnType).toBe('number'); + expect(numberSaveEntry.memberTags).not.toContain(MemberTags.Inherited); + + expect(unionSaveEntry.name).toBe('save'); + expect(unionSaveEntry.memberType).toBe(MemberType.Method); + expect((unionSaveEntry as MethodEntry).returnType).toBe('string | number'); + expect(unionSaveEntry.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', ` + interface Ancestor { + get name(): string; + set name(v: string); + + get id(): string; + set id(v: string); + + get age(): number; + set age(v: number); + } + + interface Parent extends Ancestor { + name: string; + } + + export interface Child extends Parent { + get id(): string; + }`); + + const docs: DocEntry[] = env.driveDocsExtraction('index.ts'); + expect(docs.length).toBe(1); + + const interfaceEntry = docs[0] as InterfaceEntry; + expect(interfaceEntry.members.length).toBe(4); + + const [idEntry, nameEntry, ageGetterEntry, ageSetterEntry] = + interfaceEntry.members as PropertyEntry[]; + + // When the child interface 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 interface 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); + }); }); });