diff --git a/.vscode/launch.json b/.vscode/launch.json index 9b4a2d973..4669281eb 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -39,6 +39,7 @@ "skipFiles": [ "/**", "**/node_modules/mocha/**", + "**/node_modules/lodash/**", "**/node_modules/ts-node/**" ], "sourceMaps": true, @@ -53,7 +54,9 @@ "args": ["--logLevel", "Verbose"], "request": "launch", "skipFiles": [ - "/**" + "/**", + "**/node_modules/lodash/**", + "**/node_modules/**/handlebars/**" ], "sourceMaps": true, "type": "node" diff --git a/packages/typedoc-plugin-appium/lib/converter/builtin-external-driver.ts b/packages/typedoc-plugin-appium/lib/converter/builtin-external-driver.ts index ecdfb73e2..433627079 100644 --- a/packages/typedoc-plugin-appium/lib/converter/builtin-external-driver.ts +++ b/packages/typedoc-plugin-appium/lib/converter/builtin-external-driver.ts @@ -4,7 +4,7 @@ import {isAppiumTypesReflection, isExternalDriverDeclarationReflection} from '.. import {AppiumPluginLogger} from '../logger'; import {BaseConverter} from './base-converter'; import {AppiumTypesReflection, KnownMethods} from './types'; -import {findAsyncMethodsInReflection, findParentReflectionByName} from './utils'; +import {findParentReflectionByName, findCommandMethodsInReflection} from './utils'; /** * Name of the module containing `ExternalDriver` @@ -30,15 +30,15 @@ export class BuiltinExternalDriverConverter extends BaseConverter } #convertMethodDeclarations(refl: AppiumTypesReflection): KnownMethods { - const externalDriver = refl.getChildByName(NAME_EXTERNAL_DRIVER); + const externalDriverRefl = refl.getChildByName(NAME_EXTERNAL_DRIVER); let methods: KnownMethods = new Map(); - if (!isExternalDriverDeclarationReflection(externalDriver)) { + if (!isExternalDriverDeclarationReflection(externalDriverRefl)) { this.log.error('Could not find %s', NAME_EXTERNAL_DRIVER); return methods; } - methods = findAsyncMethodsInReflection(externalDriver); + methods = findCommandMethodsInReflection(externalDriverRefl); if (!methods.size) { this.log.error('No methods found in %s', NAME_EXTERNAL_DRIVER); @@ -56,11 +56,10 @@ export class BuiltinExternalDriverConverter extends BaseConverter public override convert(): KnownMethods { const {project} = this.ctx; - let methods: KnownMethods = new Map(); const typesModule = findParentReflectionByName(project, NAME_TYPES_MODULE); if (!isAppiumTypesReflection(typesModule)) { this.log.error('Could not find %s', NAME_TYPES_MODULE); - return methods; + return new Map(); } this.log.verbose('Found %s; converting', NAME_EXTERNAL_DRIVER); diff --git a/packages/typedoc-plugin-appium/lib/converter/builtin-method-map.ts b/packages/typedoc-plugin-appium/lib/converter/builtin-method-map.ts index e917f5543..e47c254fc 100644 --- a/packages/typedoc-plugin-appium/lib/converter/builtin-method-map.ts +++ b/packages/typedoc-plugin-appium/lib/converter/builtin-method-map.ts @@ -5,13 +5,13 @@ import { isMethodMapDeclarationReflection, } from '../guards'; import {AppiumPluginLogger} from '../logger'; -import {BaseConverter} from './base-converter'; import {BuiltinCommands} from '../model/builtin-commands'; +import {BaseConverter} from './base-converter'; import {convertMethodMap} from './method-map'; import {KnownMethods} from './types'; import { findChildByNameAndGuard, - findAsyncMethodsInReflection, + findCommandMethodsInReflection, findParentReflectionByName, } from './utils'; @@ -36,7 +36,7 @@ export class BuiltinMethodMapConverter extends BaseConverter { constructor( ctx: Context, log: AppiumPluginLogger, - protected readonly knownMethods: KnownMethods + protected readonly knownBuiltinMethods: KnownMethods ) { super(ctx, log.createChildLogger(NAME_BUILTIN_COMMAND_MODULE)); } @@ -76,11 +76,13 @@ export class BuiltinMethodMapConverter extends BaseConverter { return new BuiltinCommands(); } + const knownClassMethods = findCommandMethodsInReflection(baseDriverClassRefl); const baseDriverRoutes = convertMethodMap({ log: this.log, methodMapRefl: methodMap, parentRefl: baseDriverModuleRefl, - methods: findAsyncMethodsInReflection(baseDriverClassRefl), + knownClassMethods, + knownBuiltinMethods: this.knownBuiltinMethods, }); if (!baseDriverRoutes.size) { diff --git a/packages/typedoc-plugin-appium/lib/converter/comment.ts b/packages/typedoc-plugin-appium/lib/converter/comment.ts index 0ff99f636..ea6935143 100644 --- a/packages/typedoc-plugin-appium/lib/converter/comment.ts +++ b/packages/typedoc-plugin-appium/lib/converter/comment.ts @@ -5,8 +5,10 @@ import _ from 'lodash'; import {SetOptional, ValueOf} from 'type-fest'; -import {Comment, CommentTag} from 'typedoc'; -import {CommandMethodDeclarationReflection, KnownMethods} from './types'; +import {Comment, CommentTag, Reflection} from 'typedoc'; +import {isDeclarationReflection, isParameterReflection} from '../guards'; +import {findCallSignature} from '../utils'; +import {KnownMethods} from './types'; export const NAME_EXAMPLE_TAG = '@example'; @@ -49,9 +51,9 @@ interface CommentFinder { * @internal */ interface CommentFinderGetterOptions { - refl?: CommandMethodDeclarationReflection; + refl?: Reflection; comment?: Comment; - knownMethods?: KnownMethods; + knownBuiltinMethods?: KnownMethods; } /** @@ -112,18 +114,27 @@ export enum CommentSourceType { * the `@example` block tag from the `ExternalDriver` interface. */ Multiple = 'multiple', + + /** + * A comment found in a `ParameterReflection` + */ + Parameter = 'parameter', + /** + * A comment found in a `ParameterReflection` within a builtin method (e.g., from `ExternalDriver`) + */ + BuiltinParameter = 'builtin-parameter', } /** * Options for {@linkcode deriveComment} */ interface DeriveCommentOptions { - refl?: CommandMethodDeclarationReflection; + refl?: Reflection; comment?: Comment; knownMethods?: KnownMethods; } -const knownComments: Map = new Map(); +const knownDeclarationRefComments: Map = new Map(); /** * Array of strategies for finding comments. They can come from a variety of places depending on @@ -132,7 +143,7 @@ const knownComments: Map = new Map(); * * These have an order (of precedence), which is why this is an array. */ -const commentFinders: Readonly = [ +const methodCommentFinders: Readonly = [ { /** * @@ -152,7 +163,11 @@ const commentFinders: Readonly = [ /** * @returns The comment from the method's signature (may be inherited) */ - getter: ({refl}) => refl?.getAllSignatures().find((sig) => sig.comment?.summary)?.comment, + getter: ({refl}) => { + if (isDeclarationReflection(refl)) { + return refl.getAllSignatures().find((sig) => sig.comment?.summary)?.comment; + } + }, commentSource: CommentSourceType.MethodSignature, }, { @@ -160,13 +175,13 @@ const commentFinders: Readonly = [ * @returns The comment from some method that this one implements or overwrites or w/e; * typically coming from interfaces in `@appium/types` */ - getter: ({refl, knownMethods}) => { - if (refl && knownComments.has(refl.name)) { - return knownComments.get(refl.name); + getter: ({refl, knownBuiltinMethods}) => { + if (refl && knownDeclarationRefComments.has(refl.name)) { + return knownDeclarationRefComments.get(refl.name); } // if the `refl` is a known command, it should be in `knownMethods`; // if it isn't (or doesn't exist) we aren't going to display it anyway, so abort - const otherRefl = refl && knownMethods?.get(refl.name); + const otherRefl = refl && knownBuiltinMethods?.get(refl.name); if (!otherRefl) { return; } @@ -178,15 +193,15 @@ const commentFinders: Readonly = [ // // after looping thru the finders, if we have a comment in the list of `commentData` // objects, return the first one found. - const comment = commentFinders + const comment = methodCommentFinders .filter(({commentSource}) => commentSource !== CommentSourceType.OtherMethod) .map(({getter, commentSource}) => ({ - comment: getter({refl: otherRefl, knownMethods}), + comment: getter({refl: otherRefl, knownBuiltinMethods: knownBuiltinMethods}), commentSource, })) .find(({comment}) => Boolean(comment))?.comment; if (comment) { - knownComments.set(refl.name, comment); + knownDeclarationRefComments.set(refl.name, comment); } return comment; }, @@ -194,6 +209,59 @@ const commentFinders: Readonly = [ }, ]; +const paramCommentFinders: Readonly = [ + { + getter({refl}) { + if (!isParameterReflection(refl)) { + return; + } + return refl.comment?.hasVisibleComponent() ? refl.comment : undefined; + }, + commentSource: CommentSourceType.Parameter, + }, + + { + /** + * @returns The comment from some method that this one implements or overwrites or w/e; + * typically coming from interfaces in `@appium/types` + */ + getter: ({refl, knownBuiltinMethods}) => { + if (!isParameterReflection(refl)) { + return; + } + const signatureRefl = refl.parent; + if (!signatureRefl) { + return; + } + const methodRefl = signatureRefl.parent; + if (!methodRefl) { + return; + } + const paramIdx = signatureRefl.parameters?.indexOf(refl); + if (paramIdx === undefined || paramIdx < 0) { + return; + } + + const builtinMethodRefl = knownBuiltinMethods?.get(methodRefl.name); + + if (!builtinMethodRefl) { + return; + } + + const builtinParams = findCallSignature(builtinMethodRefl)?.parameters; + + if (!builtinParams || !builtinParams[paramIdx]) { + return; + } + const builtinParam = builtinParams[paramIdx]; + if (builtinParam.comment?.hasVisibleComponent()) { + return builtinParam.comment; + } + }, + commentSource: CommentSourceType.BuiltinParameter, + }, +]; + /** * Tries to figure out a comment for a command * @param opts Options @@ -202,6 +270,9 @@ const commentFinders: Readonly = [ */ export function deriveComment(opts: DeriveCommentOptions = {}): CommentData | undefined { const {refl, comment, knownMethods} = opts; + + const commentFinders = isParameterReflection(refl) ? paramCommentFinders : methodCommentFinders; + /** * The result of running thru all of the comment finders. Each value will have a * {@linkcode CommentSourceType} corresponding to the finder, and the a `comment` property _if and @@ -209,7 +280,7 @@ export function deriveComment(opts: DeriveCommentOptions = {}): CommentData | un */ const rawCommentData: SetOptional[] = commentFinders.map( ({getter, commentSource}) => ({ - comment: getter({refl, comment, knownMethods}), + comment: getter({refl, comment, knownBuiltinMethods: knownMethods}), commentSource, }) ); diff --git a/packages/typedoc-plugin-appium/lib/converter/exec-method-map.ts b/packages/typedoc-plugin-appium/lib/converter/exec-method-map.ts index e2cd813a8..92b19bbe1 100644 --- a/packages/typedoc-plugin-appium/lib/converter/exec-method-map.ts +++ b/packages/typedoc-plugin-appium/lib/converter/exec-method-map.ts @@ -1,6 +1,7 @@ import _ from 'lodash'; import {DeclarationReflection, ReflectionKind} from 'typedoc'; import { + isCommandMethodDeclarationReflection, isCommandPropDeclarationReflection, isExecMethodDefParamsPropDeclarationReflection, } from '../guards'; @@ -34,7 +35,7 @@ export interface ConvertExecuteMethodMapOpts { /** * Builtin methods from `@appium/types` */ - builtinMethods: KnownMethods; + knownMethods: KnownMethods; /** * If `true`, do not add a route if the method it references cannot be found */ @@ -54,7 +55,7 @@ export function convertExecuteMethodMap({ log, parentRefl, execMethodMapRefl, - builtinMethods, + knownMethods, strict = false, isPluginCommand = false, }: ConvertExecuteMethodMapOpts): ExecMethodDataSet { @@ -98,20 +99,21 @@ export function convertExecuteMethodMap({ const requiredParams = convertRequiredCommandParams(paramsProp); const optionalParams = convertOptionalCommandParams(paramsProp); - const methodRefl = builtinMethods.get(command); + const methodRefl = knownMethods.get(command); - if (strict && !methodRefl) { - log.error('No method found for command "%s" from script "%s"', command, script); + if (!methodRefl) { + if (strict) { + log.error('No method found for command "%s" from script "%s"', command, script); + } continue; } - const commentData = deriveComment({refl: methodRefl, comment, knownMethods: builtinMethods}); + const commentData = deriveComment({refl: methodRefl, comment, knownMethods: knownMethods}); commandRefs.add( - new ExecMethodData(log, command, script, { + new ExecMethodData(log, command, methodRefl, script, { requiredParams, optionalParams, - refl: methodRefl, comment: commentData?.comment, commentSource: commentData?.commentSource, isPluginCommand, diff --git a/packages/typedoc-plugin-appium/lib/converter/external.ts b/packages/typedoc-plugin-appium/lib/converter/external.ts index 09ef6adb8..a914f368a 100644 --- a/packages/typedoc-plugin-appium/lib/converter/external.ts +++ b/packages/typedoc-plugin-appium/lib/converter/external.ts @@ -17,15 +17,15 @@ import { RouteMap, } from '../model'; import {BaseConverter} from './base-converter'; -import {convertOverrides} from './overrides'; import {convertExecuteMethodMap} from './exec-method-map'; import {convertMethodMap} from './method-map'; +import {convertOverrides} from './overrides'; import {ClassDeclarationReflection, KnownMethods} from './types'; import { filterChildrenByGuard, findChildByGuard, findChildByNameAndGuard, - findAsyncMethodsInReflection, + findCommandMethodsInReflection, } from './utils'; /** @@ -152,7 +152,7 @@ export class ExternalConverter extends BaseConverter { findChildByGuard(classRefl, isConstructorDeclarationReflection) ); - const methods = findAsyncMethodsInReflection(classRefl); + const methods = new Map(findCommandMethodsInReflection(classRefl)); if (!methods.size) { // may or may not be expected @@ -220,7 +220,7 @@ export class ExternalConverter extends BaseConverter { log, parentRefl, execMethodMapRefl, - builtinMethods: methods, + knownMethods: methods, strict: true, isPluginCommand, }); @@ -253,8 +253,8 @@ export class ExternalConverter extends BaseConverter { log, methodMapRefl: newMethodMapRefl, parentRefl, - methods, - knownMethods: this.builtinMethods, + knownClassMethods: methods, + knownBuiltinMethods: this.builtinMethods, strict: true, isPluginCommand, }); diff --git a/packages/typedoc-plugin-appium/lib/converter/method-map.ts b/packages/typedoc-plugin-appium/lib/converter/method-map.ts index e229381a8..6754a479f 100644 --- a/packages/typedoc-plugin-appium/lib/converter/method-map.ts +++ b/packages/typedoc-plugin-appium/lib/converter/method-map.ts @@ -25,7 +25,7 @@ export interface ConvertMethodMapOpts { /** * All builtin methods from `@appium/types` */ - knownMethods?: KnownMethods; + knownBuiltinMethods: KnownMethods; /** * Logger */ @@ -37,7 +37,7 @@ export interface ConvertMethodMapOpts { /** * All async methods in `parentRefl` */ - methods: KnownMethods; + knownClassMethods: KnownMethods; /** * The parent of `methodMapRef`; could be a class or module */ @@ -62,8 +62,8 @@ export function convertMethodMap({ log, methodMapRefl, parentRefl, - methods, - knownMethods = new Map(), + knownClassMethods, + knownBuiltinMethods, strict = false, isPluginCommand = false, }: ConvertMethodMapOpts): RouteMap { @@ -108,18 +108,25 @@ export function convertMethodMap({ const command = String(commandProp.type.value); - const method = methods.get(command); + const method = knownClassMethods.get(command); - if (strict && !method) { - log.warn('(%s) No method found for command "%s"; this is a bug', parentRefl.name, command); + if (!method) { + if (strict) { + log.error( + '(%s) No method found for command "%s"; this may be a bug', + parentRefl.name, + command + ); + } continue; } const commentData = deriveComment({ refl: method, comment: mapComment, - knownMethods, + knownMethods: knownBuiltinMethods, }); + const {comment, commentSource} = commentData ?? {}; const payloadParamsProp = findChildByGuard( httpMethodProp, @@ -132,14 +139,14 @@ export function convertMethodMap({ const commandSet: CommandSet = routes.get(route) ?? new Set(); commandSet.add( - new CommandData(log, command, httpMethod, route, { + new CommandData(log, command, method, httpMethod, route, { requiredParams, optionalParams, - comment: commentData?.comment, - commentSource: commentData?.commentSource, - refl: method, + comment, + commentSource, parentRefl, isPluginCommand, + knownBuiltinMethods, }) ); diff --git a/packages/typedoc-plugin-appium/lib/converter/overrides.ts b/packages/typedoc-plugin-appium/lib/converter/overrides.ts index e1da3fc30..e1653893d 100644 --- a/packages/typedoc-plugin-appium/lib/converter/overrides.ts +++ b/packages/typedoc-plugin-appium/lib/converter/overrides.ts @@ -81,8 +81,8 @@ export function convertOverrides({ knownMethods: builtinMethods, }); const newCommandData = commandData.clone({ - refl: method, parentRefl, + knownBuiltinMethods: builtinMethods, ...commentData, }); log.verbose('Linked route %s %s for command "%s"', commandData.httpMethod, route, command); diff --git a/packages/typedoc-plugin-appium/lib/converter/utils.ts b/packages/typedoc-plugin-appium/lib/converter/utils.ts index 9f09d7f0b..de9922e05 100644 --- a/packages/typedoc-plugin-appium/lib/converter/utils.ts +++ b/packages/typedoc-plugin-appium/lib/converter/utils.ts @@ -3,16 +3,30 @@ * @module */ -import {DeclarationReflection, LiteralType, ProjectReflection, ReflectionKind} from 'typedoc'; +import _ from 'lodash'; import { - isAsyncMethodDeclarationReflection, + DeclarationReflection, + LiteralType, + ParameterReflection, + ProjectReflection, + ReflectionFlag, + ReflectionFlags, + ReflectionKind, + SignatureReflection, + SomeType, +} from 'typedoc'; +import { + isCommandMethodDeclarationReflection, isMethodDefParamNamesDeclarationReflection, isReflectionWithReflectedType, } from '../guards'; import {ParentReflection} from '../model'; +import {findCallSignature} from '../utils'; +import {deriveComment} from './comment'; import {NAME_OPTIONAL, NAME_REQUIRED} from './external'; import { ClassDeclarationReflection, + CommandMethodDeclarationReflection, Guard, InterfaceDeclarationReflection, KnownMethods, @@ -103,15 +117,15 @@ export function filterChildrenByKind( } /** - * Finds _all_ async methods in a class or interface + * Finds _all_ async command methods in a class or interface * @param refl Class reflection * @returns Map of method names to method reflections */ -export function findAsyncMethodsInReflection( +export function findCommandMethodsInReflection( refl: ClassDeclarationReflection | InterfaceDeclarationReflection ): KnownMethods { return new Map( - filterChildrenByGuard(refl, isAsyncMethodDeclarationReflection).map((method) => [ + filterChildrenByGuard(refl, isCommandMethodDeclarationReflection).map((method) => [ method.name, method, ]) @@ -171,3 +185,167 @@ export function convertRequiredCommandParams( ): string[] { return convertCommandParams(NAME_REQUIRED, methodDefRefl); } + +/** + * List of fields to shallow copy from a `ParameterReflection` to a clone + * @internal + */ +const PARAMETER_REFLECTION_CLONE_FIELDS = [ + 'anchor', + 'cssClasses', + 'defaultValue', + 'hasOwnDocument', + 'label', + 'originalName', + 'sources', + 'type', + 'url', +]; + +/** + * Clones a `ParameterReflection`. + * + * @privateRemarks I think. + * @param pRefl A `ParameterReflection` + * @param param Desired name of parameter + * @param knownMethods Builtin methods for aggregating comments + * @param sig Custom signature reflection + * @param optional If the parameter is considered "optional" + * @returns A new `ParameterReflection` based on the first + */ +export function cloneParameterReflection( + pRefl: ParameterReflection, + param: string, + knownMethods: KnownMethods, + sig?: SignatureReflection, + optional = false +) { + sig = sig ?? pRefl.parent; + if (!sig) { + throw new Error('ParameterReflection has no parent'); + } + const newPRefl = new ParameterReflection(param, ReflectionKind.CallSignature, sig); + _.assign(newPRefl, _.pick(pRefl, PARAMETER_REFLECTION_CLONE_FIELDS)); + // attempt to derive param comments + newPRefl.comment = + deriveComment({ + refl: newPRefl, + knownMethods: knownMethods, + })?.comment ?? pRefl.comment; + // there doesn't seem to be a straightforward way to clone flags. + newPRefl.flags = new ReflectionFlags(...pRefl.flags); + newPRefl.flags.setFlag(ReflectionFlag.Optional, optional); + return newPRefl; +} + +/** + * List of fields to shallow copy from a `SignatureReflection` to a clone + * @internal + */ +const SIGNATURE_REFLECTION_CLONE_FIELDS = [ + 'anchor', + 'comment', + 'flags', + 'hasOwnDocument', + 'implementationOf', + 'inheritedFrom', + 'kindString', + 'label', + 'originalName', + 'overwrites', + 'parameters', + 'sources', + 'typeParameters', + 'url', +]; + +/** + * This loops over a list of command parameter names as defined in the method/execute map and attempts + * to create a new `ParameterReflection` for each, based on the given data. + * + * Because the command param names are essentially properties of a JSON object and the + * `ParameterReflection` instances represent the arguments of a method, we must match them by + * index. In JS, optional arguments cannot become before required arguments in a function + * signature, so we can do those first. If there are _more_ method arguments than command param + * names, we toss them out, because they may not be part of the public API. + * @param methodRefl Command method declaration reflection + * @param opts Options + * @returns List of refls with names matching `commandParams`, throwing out any extra refls + */ +export function createNewParamRefls( + methodRefl: CommandMethodDeclarationReflection, + { + builtinMethods = new Map(), + commandParams = [], + sig, + isOptional, + isPluginCommand, + }: CreateNewParamReflsOpts = {} +): ParameterReflection[] { + const newParamRefls: ParameterReflection[] = []; + sig = sig ?? findCallSignature(methodRefl); + const allParams = sig?.parameters ?? []; + const pRefls = isPluginCommand ? allParams.slice(2) : allParams; + for (const [idx, param] of commandParams.entries()) { + const pRefl = pRefls[idx]; + if (pRefl) { + const newPRefl = cloneParameterReflection(pRefl, param, builtinMethods, sig, isOptional); + newParamRefls.push(newPRefl); + } + } + + return newParamRefls; +} + +/** + * Clones a `SignatureReflection` with a new parent and type. + * + * @privateRemarks I'm not sure this is sufficient. + * @param sig A `SignatureReflection` to clone + * @param parent The desired parent of the new `SignatureReflection` + * @param type The desired type of the new `SignatureReflection`; if not provided, the original type + * will be used + * @returns A clone of `sig` with the given parent and type + */ +export function cloneSignatureReflection( + sig: SignatureReflection, + parent: CommandMethodDeclarationReflection, + type?: SomeType +) { + const newSig = new SignatureReflection(sig.name, ReflectionKind.CallSignature, parent); + + _.assign(newSig, _.pick(sig, SIGNATURE_REFLECTION_CLONE_FIELDS)); + + if (type) { + newSig.type = type; + } + return newSig; +} + +export interface CreateNewParamReflsOpts { + /** + * Map of known methods + */ + builtinMethods?: KnownMethods; + /** + * If the parameter is marked as optional in the method def + */ + isOptional?: boolean; + /** + * If the class containing the method is a Plugin. + * + * This is important because the `PluginCommand` type has a different signature than the + * `DriverCommand` type; the former always has two specific arguments heading its parameter list, + * and we do not need to include in the generated docs. + */ + isPluginCommand?: boolean; + /** + * List of parameter names from method def + */ + commandParams?: string[]; + + /** + * Custom signature reflection to use + */ + sig?: SignatureReflection; +} diff --git a/packages/typedoc-plugin-appium/lib/guards.ts b/packages/typedoc-plugin-appium/lib/guards.ts index aed17e217..973135a3a 100644 --- a/packages/typedoc-plugin-appium/lib/guards.ts +++ b/packages/typedoc-plugin-appium/lib/guards.ts @@ -4,8 +4,10 @@ */ import { + ContainerReflection, DeclarationReflection, LiteralType, + ParameterReflection, ProjectReflection, ReferenceType, Reflection, @@ -280,7 +282,7 @@ export function isExternalDriverDeclarationReflection( * call signature returning a `Promise`. * @param value */ -export function isAsyncMethodDeclarationReflection( +export function isCommandMethodDeclarationReflection( value: any ): value is CommandMethodDeclarationReflection { if ( @@ -364,3 +366,11 @@ export function isBasePluginConstructorDeclarationReflection( : undefined; return ref?.name === `${NAME_BASE_PLUGIN}.constructor`; } + +/** + * Guard for {@linkcode ParameterReflection} + * @param value any + */ +export function isParameterReflection(value: any): value is ParameterReflection { + return value instanceof ParameterReflection; +} diff --git a/packages/typedoc-plugin-appium/lib/index.ts b/packages/typedoc-plugin-appium/lib/index.ts index ebcc59eae..43f3fb3bb 100644 --- a/packages/typedoc-plugin-appium/lib/index.ts +++ b/packages/typedoc-plugin-appium/lib/index.ts @@ -45,38 +45,48 @@ export const setup: (app: Application) => Application = _.flow(configureTheme, c /** * Finds commands and creates new reflections for them, adding them to the project. * - * Resolves after {@linkcode Converter.EVENT_RESOLVE_BEGIN} emits and when it's finished. + * Resolves after {@linkcode Converter.EVENT_RESOLVE_END} emits and when it's finished. * @param app Typedoc Application * @returns A {@linkcode ConvertResult} receipt from the conversion */ export async function convert(app: Application): Promise { return new Promise((resolve) => { - app.converter.once(Converter.EVENT_RESOLVE_BEGIN, (ctx: Context) => { - let extensionReflections: ExtensionReflection[] | undefined; - let projectCommands: ProjectCommands | undefined; + app.converter.once( + Converter.EVENT_RESOLVE_END, + /** + * This listener _must_ trigger on {@linkcode Converter.EVENT_RESOLVE_END}, because TypeDoc's + * internal plugins do some post-processing on the project's reflections--specifically, it + * finds `@param` tags in a `SignatureReflection`'s `comment` and "moves" them into the + * appropriate `ParameterReflections`. Without this in place, we won't be able aggregate + * parameter comments and they will not display in the generated docs. + */ + (ctx: Context) => { + let extensionReflections: ExtensionReflection[] | undefined; + let projectCommands: ProjectCommands | undefined; - // we don't want to do this work if we're not using the custom theme! - log = log ?? new AppiumPluginLogger(app.logger, NS); + // we don't want to do this work if we're not using the custom theme! + log = log ?? new AppiumPluginLogger(app.logger, NS); - // this should not be necessary given the `AppiumPluginOptionsReader` forces the issue, but - // it's a safeguard nonetheless. - if (app.renderer.themeName === THEME_NAME) { - // this queries the declarations created by TypeDoc and extracts command information - projectCommands = convertCommands(ctx, log); + // this should not be necessary given the `AppiumPluginOptionsReader` forces the issue, but + // it's a safeguard nonetheless. + if (app.renderer.themeName === THEME_NAME) { + // this queries the declarations created by TypeDoc and extracts command information + projectCommands = convertCommands(ctx, log); - if (!projectCommands) { - log.verbose('Skipping creation of reflections'); - resolve({ctx}); - return; + if (!projectCommands) { + log.verbose('Skipping creation of reflections'); + resolve({ctx}); + return; + } + // this creates new custom reflections from the data we gathered and registers them + // with TypeDoc + extensionReflections = createReflections(ctx, log, projectCommands); + } else { + log.warn(`Appium theme disabled! Use "theme: 'appium'" in your typedoc.json`); } - // this creates new custom reflections from the data we gathered and registers them - // with TypeDoc - extensionReflections = createReflections(ctx, log, projectCommands); - } else { - log.warn(`Appium theme disabled! Use "theme: 'appium'" in your typedoc.json`); + resolve({ctx, extensionReflections, projectCommands}); } - resolve({ctx, extensionReflections, projectCommands}); - }); + ); }); } @@ -85,7 +95,7 @@ export async function convert(app: Application): Promise { */ export interface ConvertResult { /** - * Context at time of {@linkcode Context.EVENT_RESOLVE_BEGIN} + * Context at time of {@linkcode Context.EVENT_RESOLVE_END} */ ctx: Context; /** diff --git a/packages/typedoc-plugin-appium/lib/model/command-data.ts b/packages/typedoc-plugin-appium/lib/model/command-data.ts index 6cb7326fd..6f0ae6999 100644 --- a/packages/typedoc-plugin-appium/lib/model/command-data.ts +++ b/packages/typedoc-plugin-appium/lib/model/command-data.ts @@ -1,56 +1,17 @@ import _ from 'lodash'; +import {Comment, DeclarationReflection, ParameterReflection, SignatureReflection} from 'typedoc'; import { - Comment, - DeclarationReflection, - ParameterReflection, - ReflectionFlag, - ReflectionFlags, - ReflectionKind, - SignatureReflection, -} from 'typedoc'; -import {CommandMethodDeclarationReflection, CommentSourceType} from '../converter'; -import {isCallSignatureReflection, isReferenceType} from '../guards'; + cloneSignatureReflection, + CommandMethodDeclarationReflection, + CommentSourceType, + createNewParamRefls, + KnownMethods, +} from '../converter'; +import {isReferenceType} from '../guards'; import {AppiumPluginLogger} from '../logger'; +import {findCallSignature} from '../utils'; import {AllowedHttpMethod, Command, Route} from './types'; -/** - * List of fields to shallow copy from a `SignatureReflection` to a clone - * @internal - */ -const SIGNATURE_REFLECTION_CLONE_FIELDS = [ - 'anchor', - 'comment', - 'flags', - 'hasOwnDocument', - 'implementationOf', - 'inheritedFrom', - 'kindString', - 'label', - 'originalName', - 'overwrites', - 'parameters', - 'sources', - 'typeParameters', - 'url', -]; - -/** - * List of fields to shallow copy from a `ParameterReflection` to a clone - * @internal - */ -const PARAMETER_REFLECTION_CLONE_FIELDS = [ - 'anchor', - 'comment', - 'cssClasses', - 'defaultValue', - 'hasOwnDocument', - 'label', - 'originalName', - 'sources', - 'type', - 'url', -]; - /** * Abstract representation of metadata for some sort of Appium command */ @@ -80,10 +41,8 @@ export abstract class BaseCommandData { public readonly isPluginCommand: boolean; /** * Actual method reflection. - * - * @todo Determine if this should be required */ - public readonly methodRefl?: CommandMethodDeclarationReflection; + public readonly methodRefl: CommandMethodDeclarationReflection; /** * List of optional parameter names derived from a method map */ @@ -94,12 +53,30 @@ export abstract class BaseCommandData { public readonly requiredParams?: string[]; /** - * Loops through signatures of the command's method declaration and returns the first that is a - * `CallSignatureReflection` (if any). This is what we think of when we think "function signature" + * Map of known builtin methods */ - public static findCallSignature = _.memoize((cmd: BaseCommandData) => - cmd.methodRefl?.getAllSignatures()?.find(isCallSignatureReflection) - ); + public readonly knownBuiltinMethods?: KnownMethods; + + /** + * Parameter reflections for this command's method declaration, to eventually be displayed in rendered docs + * + * These are _not_ the same objects as in the `parameters` property of a the call signature + * reflection in `methodRefl`; the comments therein have been aggregated and the parameters have + * been renamed and possibly truncated. + */ + public readonly parameters?: ParameterReflection[]; + /** + * Signature reflection for this command's method declaration, to eventually be displayed in + * rendered docs + * + * `methodRefl` is a {@linkcode CommandMethodDeclarationReflection}, so it returns a `Promise`, by + * definition. This signature reflection is modified so that it returns `T` instead, since + * `Promise`s don't make much sense in the rendered documentaion. + * + * The default TypeDoc output uses the original `SignatureReflection`, so you _will_ see + * `Promise` there. + */ + public readonly signature?: SignatureReflection; /** * Returns a list of `ParameterReflection` objects in the command's method declaration; @@ -107,71 +84,25 @@ export abstract class BaseCommandData { **/ public static rewriteParameters = _.memoize((cmd: BaseCommandData) => { if (!cmd.hasCommandParams) { - return []; - } - const sig = BaseCommandData.findCallSignature(cmd); - - if (!sig) { - return []; + return; } - const pRefls = (cmd.isPluginCommand ? sig.parameters?.slice(2) : sig.parameters?.slice()) ?? []; + const newParamRefls = [ + ...createNewParamRefls(cmd.methodRefl, { + builtinMethods: cmd.knownBuiltinMethods, + commandParams: cmd.requiredParams, + isPluginCommand: cmd.isPluginCommand, + sig: cmd.signature, + }), + ...createNewParamRefls(cmd.methodRefl, { + builtinMethods: cmd.knownBuiltinMethods, + commandParams: cmd.optionalParams, + isPluginCommand: cmd.isPluginCommand, + sig: cmd.signature, + isOptional: true, + }), + ]; - if (pRefls.length < cmd.requiredParams!.length + cmd.optionalParams!.length) { - cmd.log.warn( - '(%s) Method %s has fewer parameters (%d) than specified in the method map (%d)', - cmd.parentRefl!.name, - cmd.methodRefl!.name, - pRefls.length, - cmd.requiredParams!.length + cmd.optionalParams!.length - ); - } - - /** - * This loops over the command parameter names as defined in the method/execute map and attempts - * to associate a `ParameterReflection` object with each. - * - * Because the command param names are essentially properties of a JSON object and the - * `ParameterReflection` instances represent the arguments of a method, we must match them by - * index. In JS, Required arguments always come first, so we can do those first. If there are - * _more_ method arguments than command param names, we toss them out, because they may not be - * part of the public API. - * @param kind Either `required` or `optional` - * @returns List of refls with names matching `commandParams`, throwing out any extra refls - */ - const createNewRefls = (kind: 'required' | 'optional'): ParameterReflection[] => { - const commandParams = cmd[`${kind}Params`]; - if (!commandParams?.length) { - return []; - } - const paramCount = commandParams.length; - - const newParamRefls: ParameterReflection[] = []; - for (let i = 0; i < paramCount; i++) { - const pRefl = pRefls.shift(); - if (pRefl) { - // if there isn't one, the warning above will have been logged already - const newPRefl = new ParameterReflection( - commandParams[i], - ReflectionKind.CallSignature, - sig - ); - _.assign(newPRefl, _.pick(pRefl, PARAMETER_REFLECTION_CLONE_FIELDS)); - - // there doesn't seem to be a straightforward way to clone flags. - newPRefl.flags = new ReflectionFlags(...pRefl.flags); - newPRefl.flags.setFlag(ReflectionFlag.Optional, kind === 'optional'); - newParamRefls.push(newPRefl); - } - } - return newParamRefls; - }; - - const newParamRefls = [...createNewRefls('required'), ...createNewRefls('optional')]; - - if (!newParamRefls.length) { - return []; - } return newParamRefls; }); @@ -186,27 +117,21 @@ export abstract class BaseCommandData { * name `Promise`. */ public static unwrapSignatureType = _.memoize((cmd: BaseCommandData) => { - const callSig = BaseCommandData.findCallSignature(cmd); + const callSig = findCallSignature(cmd.methodRefl); if (!callSig) { return; } if (isReferenceType(callSig.type) && callSig.type.name === 'Promise') { - const newCallSig = new SignatureReflection( - callSig.name, - ReflectionKind.CallSignature, - cmd.methodRefl! - ); - _.assign(newCallSig, _.pick(callSig, SIGNATURE_REFLECTION_CLONE_FIELDS)); - - // this is the actual unwrapping. `Promise` only has a single type argument `T`, + // this does the actual unwrapping. `Promise` only has a single type argument `T`, // so we can safely use the first one. - newCallSig.type = callSig.type.typeArguments?.[0]; + const newType = callSig.type.typeArguments?.[0]; + const newCallSig = cloneSignatureReflection(callSig, cmd.methodRefl, newType); if (!newCallSig.type) { cmd.log.warn( '(%s) No type arg T found for return type Promise in %s; this is a bug', cmd.parentRefl!.name, - cmd.methodRefl!.name + cmd.methodRefl.name ); return; } @@ -219,16 +144,26 @@ export abstract class BaseCommandData { */ parentRefl?: DeclarationReflection; - constructor(log: AppiumPluginLogger, command: Command, opts: CommandDataOpts = {}) { + constructor( + log: AppiumPluginLogger, + command: Command, + methodRefl: CommandMethodDeclarationReflection, + opts: CommandDataOpts = {} + ) { this.command = command; + this.methodRefl = methodRefl; + this.log = log; + this.optionalParams = opts.optionalParams; this.requiredParams = opts.requiredParams; this.comment = opts.comment; this.commentSource = opts.commentSource; - this.methodRefl = opts.refl; this.parentRefl = opts.parentRefl; - this.log = log; + this.knownBuiltinMethods = opts.knownBuiltinMethods; this.isPluginCommand = Boolean(opts.isPluginCommand); + + this.signature = BaseCommandData.unwrapSignatureType(this); + this.parameters = BaseCommandData.rewriteParameters(this); } /** @@ -237,21 +172,6 @@ export abstract class BaseCommandData { public get hasCommandParams(): boolean { return Boolean(this.optionalParams?.length || this.requiredParams?.length); } - - /** - * Gets a list of function parameters (for use in rendering) - */ - public get parameters() { - return BaseCommandData.rewriteParameters(this); - } - - /** - * Gets the call signature (for use in rendering) - */ - public get signature() { - return BaseCommandData.unwrapSignatureType(this); - } - /** * Should create a shallow clone of the implementing instance * @param opts New options to pass to the new instance @@ -290,17 +210,15 @@ export interface CommandDataOpts { */ parentRefl?: DeclarationReflection; - /** - * Actual method reflection. - * - * @todo Determine if this should be required - */ - refl?: CommandMethodDeclarationReflection; - /** * List of required parameter names derived from a method map */ requiredParams?: string[]; + + /** + * Known methods in the project + */ + knownBuiltinMethods?: KnownMethods; } /** @@ -320,11 +238,12 @@ export class CommandData extends BaseCommandData { constructor( log: AppiumPluginLogger, command: Command, + methodRefl: CommandMethodDeclarationReflection, httpMethod: AllowedHttpMethod, route: Route, opts: CommandDataOpts = {} ) { - super(log, command, opts); + super(log, command, methodRefl, opts); this.httpMethod = httpMethod; this.route = route; } @@ -339,7 +258,10 @@ export class CommandData extends BaseCommandData { * @returns Cloned instance */ public override clone(opts: CommandDataOpts = {}): CommandData { - return new CommandData(this.log, this.command, this.httpMethod, this.route, {...this, ...opts}); + return new CommandData(this.log, this.command, this.methodRefl, this.httpMethod, this.route, { + ...this, + ...opts, + }); } } @@ -362,14 +284,12 @@ export class ExecMethodData extends BaseCommandData { constructor( log: AppiumPluginLogger, command: Command, + methodRefl: CommandMethodDeclarationReflection, script: string, opts: CommandDataOpts = {} ) { - super(log, command, opts); + super(log, command, methodRefl, opts); this.script = script; - if (!this.methodRefl) { - this.log.verbose(`No reflection for script ${script}`); - } } /** @@ -381,6 +301,9 @@ export class ExecMethodData extends BaseCommandData { * @returns Cloned instance */ public override clone(opts: CommandDataOpts): ExecMethodData { - return new ExecMethodData(this.log, this.command, this.script, {...this, ...opts}); + return new ExecMethodData(this.log, this.command, this.methodRefl, this.script, { + ...this, + ...opts, + }); } } diff --git a/packages/typedoc-plugin-appium/lib/utils.ts b/packages/typedoc-plugin-appium/lib/utils.ts new file mode 100644 index 000000000..e3d6bd131 --- /dev/null +++ b/packages/typedoc-plugin-appium/lib/utils.ts @@ -0,0 +1,16 @@ +/** + * Utils used across entire package + * @module + */ + +import _ from 'lodash'; +import {DeclarationReflection} from 'typedoc'; +import {isCallSignatureReflection} from './guards'; + +/** + * Loops through signatures of the command's method declaration and returns the first that is a + * `CallSignatureReflection` (if any). This is what we think of when we think "function signature" + */ +export const findCallSignature = _.memoize((refl?: DeclarationReflection) => + refl?.getAllSignatures()?.find(isCallSignatureReflection) +); diff --git a/packages/typedoc-plugin-appium/test/e2e/converter/builtin-method-map.e2e.spec.ts b/packages/typedoc-plugin-appium/test/e2e/converter/builtin-method-map.e2e.spec.ts index 8b3796676..4410edbd7 100644 --- a/packages/typedoc-plugin-appium/test/e2e/converter/builtin-method-map.e2e.spec.ts +++ b/packages/typedoc-plugin-appium/test/e2e/converter/builtin-method-map.e2e.spec.ts @@ -9,10 +9,10 @@ import { NAME_BUILTIN_COMMAND_MODULE, NAME_TYPES_MODULE, } from '../../../lib/converter'; -import {BuiltinCommands} from '../../../lib/model/builtin-commands'; import {AppiumPluginLogger} from '../../../lib/logger'; -import {initConverter, NAME_FAKE_DRIVER_MODULE} from '../helpers'; import {CommandData} from '../../../lib/model'; +import {BuiltinCommands} from '../../../lib/model/builtin-commands'; +import {initConverter, NAME_FAKE_DRIVER_MODULE} from '../helpers'; describe('@appium/typedoc-plugin-appium', function () { describe('BuiltinMethodMapConverter', function () { @@ -72,10 +72,21 @@ describe('@appium/typedoc-plugin-appium', function () { }); it('should contain the expected properties in the getSession command data', function () { - expect(_.omit(cmdData, 'methodRefl', 'parentRefl', 'comment', 'log')).to.eql({ + expect( + _.omit( + cmdData, + 'methodRefl', + 'parentRefl', + 'knownBuiltinMethods', + 'comment', + 'log', + 'parameters', + 'signature' + ) + ).to.eql({ command: 'createSession', httpMethod: 'POST', - commentSource: 'method-signature', + commentSource: 'multiple', requiredParams: [], route: '/session', optionalParams: ['desiredCapabilities', 'requiredCapabilities', 'capabilities'], diff --git a/packages/typedoc-plugin-appium/test/e2e/converter/converter.e2e.spec.ts b/packages/typedoc-plugin-appium/test/e2e/converter/converter.e2e.spec.ts index 77652533e..2fbb8f98c 100644 --- a/packages/typedoc-plugin-appium/test/e2e/converter/converter.e2e.spec.ts +++ b/packages/typedoc-plugin-appium/test/e2e/converter/converter.e2e.spec.ts @@ -28,7 +28,7 @@ describe('@appium/typedoc-plugin-appium', function () { entryPoints: [NAME_TYPES_MODULE, NAME_FAKE_DRIVER_MODULE, NAME_BUILTIN_COMMAND_MODULE], }); ctx = await new Promise((resolve) => { - app.converter.once(Converter.EVENT_RESOLVE_BEGIN, (ctx: Context) => { + app.converter.once(Converter.EVENT_RESOLVE_END, (ctx: Context) => { resolve(ctx); }); app.convert(); diff --git a/packages/typedoc-plugin-appium/test/e2e/converter/external.e2e.spec.ts b/packages/typedoc-plugin-appium/test/e2e/converter/external.e2e.spec.ts index 5b1e7ddb7..f4d295ef9 100644 --- a/packages/typedoc-plugin-appium/test/e2e/converter/external.e2e.spec.ts +++ b/packages/typedoc-plugin-appium/test/e2e/converter/external.e2e.spec.ts @@ -9,10 +9,10 @@ import { NAME_BUILTIN_COMMAND_MODULE, NAME_TYPES_MODULE, } from '../../../lib/converter'; -import {BuiltinCommands} from '../../../lib/model/builtin-commands'; import {isCallSignatureReflectionWithArity} from '../../../lib/guards'; import {AppiumPluginLogger} from '../../../lib/logger'; import {CommandSet, ModuleCommands, ProjectCommands} from '../../../lib/model'; +import {BuiltinCommands} from '../../../lib/model/builtin-commands'; import {initConverter, NAME_FAKE_DRIVER_MODULE} from '../helpers'; describe('@appium/typedoc-plugin-appium', function () { describe('ExternalConverter', function () { @@ -105,41 +105,25 @@ describe('@appium/typedoc-plugin-appium', function () { it('should prefer method map parameters over method parameters', function () { const postRoute = [...sessionCmdSet].find((cmdData) => cmdData.httpMethod === 'POST')!; - const pRefls = postRoute.methodRefl!.signatures!.find( - isCallSignatureReflectionWithArity - )!.parameters!; - // the method has 4 parameters, but the method map has 3 - expect(pRefls).to.have.lengthOf(4); expect(postRoute.parameters).to.have.lengthOf(3); - // the first parameter is required in the method, but optional in the method map - // and the names are different. - expect(postRoute.parameters[0]) + expect(postRoute.parameters![0]) .to.deep.include({ name: 'desiredCapabilities', }) .and.to.have.nested.property('flags.isOptional', true); - expect(pRefls[0]) - .to.deep.include({name: 'w3cCapabilities1'}) - .and.to.have.nested.property('flags.isOptional', false); - expect(postRoute.parameters[1]) + expect(postRoute.parameters![1]) .to.deep.include({ name: 'requiredCapabilities', }) .and.to.have.nested.property('flags.isOptional', true); - expect(pRefls[1]) - .to.deep.include({name: 'w3cCapabilities2'}) - .and.to.have.nested.property('flags.isOptional', true); - expect(postRoute.parameters[2]) + expect(postRoute.parameters![2]) .to.deep.include({ name: 'capabilities', }) .and.to.have.nested.property('flags.isOptional', true); - expect(pRefls[2]) - .to.deep.include({name: 'w3cCapabilities3'}) - .and.to.have.nested.property('flags.isOptional', true); }); }); diff --git a/packages/typedoc-plugin-appium/test/e2e/helpers.ts b/packages/typedoc-plugin-appium/test/e2e/helpers.ts index 60bf0353f..cb905cb78 100644 --- a/packages/typedoc-plugin-appium/test/e2e/helpers.ts +++ b/packages/typedoc-plugin-appium/test/e2e/helpers.ts @@ -130,13 +130,13 @@ async function convert, Args extends readonly any[ resolve(new cls(ctx, log)); } }; - app.converter.once(Converter.EVENT_RESOLVE_BEGIN, listener); + app.converter.once(Converter.EVENT_RESOLVE_END, listener); try { app.convert(); } catch (err) { reject(err); } finally { - app.converter.off(Converter.EVENT_RESOLVE_BEGIN, listener); + app.converter.off(Converter.EVENT_RESOLVE_END, listener); } }); } diff --git a/packages/typedoc-plugin-appium/test/unit/converter/comment.spec.ts b/packages/typedoc-plugin-appium/test/unit/converter/comment.spec.ts index a813e7068..237682ea0 100644 --- a/packages/typedoc-plugin-appium/test/unit/converter/comment.spec.ts +++ b/packages/typedoc-plugin-appium/test/unit/converter/comment.spec.ts @@ -1,5 +1,4 @@ import {expect} from 'chai'; -import _ from 'lodash'; import { Comment, CommentTag, @@ -8,11 +7,11 @@ import { ReferenceType, } from 'typedoc'; import { - KnownMethods, - deriveComment, + cloneComment, CommandMethodDeclarationReflection, CommentSourceType, - cloneComment, + deriveComment, + KnownMethods, } from '../../../lib/converter'; import {AppiumPluginReflectionKind, ExtensionReflection, ModuleCommands} from '../../../lib/model';