chore(typedoc-plugin-appium): better sorting, refactors

trying to make some things less opaque.  maybe failed

packages/typedoc-plugin-appium/lib/converter/builder.ts

@@ -52,7 +52,11 @@ export function createCommandReflection(
 }
 
 /**
- * Create a new {@linkcode CommandsReflection} and all {@linkcode CommandReflection} children within it.
+ * Create a new {@linkcode CommandsReflection} and all {@linkcode CommandReflection} children within
+ * it.
+ *
+ * Note that the return value is mainly for informational purposes, since this method mutates
+ * TypeDoc's state.
  * @param log - Logger
  * @param ctx - Current context
  * @param parent - Parent module (or project)
@@ -75,16 +79,18 @@ export function createCommandsReflection(
   const parentCtx = ctx.withScope(commandsRefl);
   const {routeMap: routeMap, execMethodDataSet: execCommandsData} = moduleCmds;
 
-  // sort routes in alphabetical order
-  const sortedRouteMap = new Map([...routeMap.entries()].sort());
+  const sortedRouteMap = new Map([...routeMap].sort(([a], [b]) => a.localeCompare(b)));
+
   for (const [route, commandSet] of sortedRouteMap) {
     for (const data of commandSet) {
       createCommandReflection(log, parentCtx, data, commandsRefl, route);
     }
   }
 
-  // sort execute commands in alphabetical order
-  const sortedExecCommandsData = new Set([...execCommandsData].sort());
+  // sort execute methods in alphabetical order by script
+  const sortedExecCommandsData = [...execCommandsData].sort((a, b) =>
+    a.script.localeCompare(b.script)
+  );
   for (const data of sortedExecCommandsData) {
     createCommandReflection(log, parentCtx, data, commandsRefl);
   }

packages/typedoc-plugin-appium/lib/converter/builtin-method-map.ts

@@ -7,8 +7,9 @@ import {
 import {AppiumPluginLogger} from '../logger';
 import {ModuleCommands} from '../model';
 import {BaseConverter} from './base-converter';
+import {BuiltinCommands} from '../model/builtin-commands';
 import {convertMethodMap} from './method-map';
-import {BuiltinCommandSource, KnownMethods} from './types';
+import {KnownMethods} from './types';
 import {
   findChildByNameAndGuard,
   findMethodsInClassReflection,
@@ -27,7 +28,7 @@ export const NAME_BASE_DRIVER_CLASS = 'BaseDriver';
  */
 export const NAME_BUILTIN_COMMAND_MODULE = '@appium/base-driver';
 
-export class BuiltinMethodMapConverter extends BaseConverter<BuiltinCommandSource | undefined> {
+export class BuiltinMethodMapConverter extends BaseConverter<BuiltinCommands | undefined> {
   /**
    * Creates a child logger for this instance
    * @param ctx Typedoc Context
@@ -41,55 +42,57 @@ export class BuiltinMethodMapConverter extends BaseConverter<BuiltinCommandSourc
     super(ctx, log.createChildLogger('builtins'));
   }
 
-  public override convert(): BuiltinCommandSource {
+  /**
+   * Converts `@appium/base-driver` into a `RouteMap`, if it can.
+   *
+   * @returns Object containing a declaration reflection of `@appium/base-driver` and its associated
+   * route map (if found).
+   */
+  public override convert(): BuiltinCommands {
     const {project} = this.ctx;
-    let methods: KnownMethods = new Map();
-    let builtinCmdSrc = {} as BuiltinCommandSource;
-    const baseDriverRef = findParentReflectionByName(project, NAME_BUILTIN_COMMAND_MODULE);
+    const baseDriverModuleRefl = findParentReflectionByName(project, NAME_BUILTIN_COMMAND_MODULE);
 
-    if (!isBaseDriverDeclarationReflection(baseDriverRef)) {
-      this.log.verbose('Did not find %s', NAME_BUILTIN_COMMAND_MODULE);
-      return builtinCmdSrc;
+    if (!isBaseDriverDeclarationReflection(baseDriverModuleRefl)) {
+      this.log.error('Could not find %s', NAME_BUILTIN_COMMAND_MODULE);
+      return new BuiltinCommands();
     }
 
     this.log.verbose('Found %s', NAME_BUILTIN_COMMAND_MODULE);
 
     // we need base driver class to find methods implemented in it
-    const baseDriverClassRef = findChildByNameAndGuard(
-      baseDriverRef,
+    const baseDriverClassRefl = findChildByNameAndGuard(
+      baseDriverModuleRefl,
       NAME_BASE_DRIVER_CLASS,
       isClassDeclarationReflection
     );
-    if (!baseDriverClassRef) {
+    if (!baseDriverClassRefl) {
       this.log.error(
         'Could not find %s in %s',
         NAME_BASE_DRIVER_CLASS,
         NAME_BUILTIN_COMMAND_MODULE
       );
-    } else {
-      methods = findMethodsInClassReflection(baseDriverClassRef, this.knownMethods);
+      return new BuiltinCommands();
     }
 
-    const methodMap = baseDriverRef.getChildByName(NAME_METHOD_MAP);
+    const methodMap = baseDriverModuleRefl.getChildByName(NAME_METHOD_MAP);
 
     if (!isMethodMapDeclarationReflection(methodMap)) {
       this.log.error('Could not find %s in %s', NAME_METHOD_MAP, NAME_BUILTIN_COMMAND_MODULE);
-      return builtinCmdSrc;
+      return new BuiltinCommands();
     }
 
     const baseDriverRoutes = convertMethodMap({
       log: this.log,
       methodMapRef: methodMap,
-      parentRefl: baseDriverRef,
-      methods,
+      parentRefl: baseDriverModuleRefl,
+      methods: findMethodsInClassReflection(baseDriverClassRefl, this.knownMethods),
     });
 
     if (!baseDriverRoutes.size) {
-      this.log.error('Could not find any commands in %s!?', NAME_BUILTIN_COMMAND_MODULE);
-      return builtinCmdSrc;
+      this.log.error('Could not find any commands in %s', NAME_BUILTIN_COMMAND_MODULE);
+      return new BuiltinCommands();
     }
 
-    builtinCmdSrc = {refl: baseDriverRef, moduleCmds: new ModuleCommands(baseDriverRoutes)};
-    return builtinCmdSrc;
+    return new BuiltinCommands(baseDriverRoutes, baseDriverModuleRefl);
   }
 }

packages/typedoc-plugin-appium/lib/converter/index.ts

@@ -16,7 +16,7 @@
 
 import {Context} from 'typedoc';
 import {AppiumPluginLogger} from '../logger';
-import {ModuleCommands, ProjectCommands} from '../model';
+import {ProjectCommands} from '../model';
 import {BuiltinExternalDriverConverter} from './builtin-external-driver';
 import {BuiltinMethodMapConverter} from './builtin-method-map';
 import {ExternalConverter} from './external';
@@ -44,22 +44,20 @@ export function convertCommands(ctx: Context, parentLog: AppiumPluginLogger): Pr
   );
   const externalCommands = externalConverter.convert();
 
-  const allCommands = [
-    ...([[builtinCommands.refl.name, builtinCommands.moduleCmds]] as [string, ModuleCommands][]),
-    ...externalCommands,
-  ];
+  const allCommands = [...builtinCommands.toProjectCommands(), ...externalCommands];
 
   return new ProjectCommands(allCommands);
 }
 
+export * from './base-converter';
 export * from './builder';
+export * from '../model/builtin-commands';
 export * from './builtin-external-driver';
 export * from './builtin-method-map';
 export * from './comment';
-export * from './base-converter';
+export * from './exec-method-map';
 export * from './external';
+export * from './method-map';
 export * from './overrides';
 export * from './types';
-export * from './method-map';
 export * from './utils';
-export * from './exec-method-map';

packages/typedoc-plugin-appium/lib/converter/method-map.ts

@@ -22,6 +22,10 @@ import {
  * Options for {@linkcode convertMethodMap}
  */
 export interface ConvertMethodMapOpts {
+  /**
+   * All builtin methods from `@appium/types`
+   */
+  knownMethods?: KnownMethods;
   /**
    * Logger
    */
@@ -30,18 +34,14 @@ export interface ConvertMethodMapOpts {
    * A `MethodMap` object whose parent is `parentRefl`
    */
   methodMapRef: MethodMapDeclarationReflection;
-  /**
-   * The parent of `methodMapRef`; could be a class or module
-   */
-  parentRefl: DeclarationReflection;
   /**
    * All async methods in `parentRefl`
    */
   methods: KnownMethods;
   /**
-   * All builtin methods from `@appium/types`
+   * The parent of `methodMapRef`; could be a class or module
    */
-  knownMethods?: KnownMethods;
+  parentRefl: DeclarationReflection;
   /**
    * If `true`, do not add a route if the method it references cannot be found
    */

packages/typedoc-plugin-appium/lib/converter/types.ts

@@ -11,7 +11,7 @@ import {
   TupleType,
   TypeOperatorType,
 } from 'typedoc';
-import {AllowedHttpMethod, ModuleCommands, ParentReflection} from '../model';
+import {AllowedHttpMethod, ParentReflection} from '../model';
 import {NAME_EXTERNAL_DRIVER, NAME_TYPES_MODULE} from './builtin-external-driver';
 import {NAME_BUILTIN_COMMAND_MODULE, NAME_METHOD_MAP} from './builtin-method-map';
 import {NAME_NEW_METHOD_MAP, NAME_EXECUTE_METHOD_MAP, NAME_PARAMS} from './external';
@@ -159,11 +159,6 @@ export interface MethodDefParam {
 
 export type ClassDeclarationReflection = WithKind<ReflectionKind.Class, DeclarationReflection>;
 
-export type BuiltinCommandSource = {
-  refl: BaseDriverDeclarationReflection;
-  moduleCmds: ModuleCommands;
-};
-
 export type ParamsPropDeclarationReflection =
   | ExecMethodDefParamsPropDeclarationReflection
   | MethodDefParamsPropDeclarationReflection;

packages/typedoc-plugin-appium/lib/model/builtin-commands.ts

@@ -0,0 +1,42 @@
+import _ from 'lodash';
+import {ModuleCommands, ProjectCommands, RouteMap} from '.';
+import {BaseDriverDeclarationReflection} from '../converter/types';
+
+/**
+ * This is essentially a {@linkcode ProjectCommands} object with zero or one entries.
+ *
+ * The entry is intended to be reflection for `@appium/base-driver and the associated
+ * {@linkcode ModuleCommands} object.
+ *
+ * If no reflection is provided--even if the `RouteMap` is present--the result of
+ * {@linkcode BuiltinCommands.toProjectCommands} will be empty (because there will be no key).
+ */
+export class BuiltinCommands {
+  moduleCmds: ModuleCommands;
+
+  /**
+   * Note that since `@appium/base-driver` contains no execute methods, the `ModuleCommands` object
+   * won't either.  If, in the future, `@appium/base-driver` contains execute methods, this constructor
+   * will want to accept another argument _or_ just accept a `ModuleCommands` object in lieu of a
+   * `RouteMap`/`ExecMethodDataSet`.
+   * @param routeMap Builtin route map
+   * @param refl `@appium/base-driver` `BaseDriverDeclarationReflection`
+   */
+  constructor(
+    routeMap: RouteMap = new Map(),
+    public readonly refl?: BaseDriverDeclarationReflection
+  ) {
+    this.moduleCmds = new ModuleCommands(routeMap);
+  }
+
+  /**
+   * Converts this object to a {@linkcode ProjectCommands} object.
+   *
+   * Since the fields in this class are read-only, the contents of the `ProjectCommands` instance
+   * will be invariant; thus we only need to create it once.
+   * @returns A {@linkcode ProjectCommands} object with zero or one entry
+   */
+  public toProjectCommands = _.once(() => {
+    return new ProjectCommands(this.refl ? [[this.refl.name, this.moduleCmds]] : []);
+  });
+}

packages/typedoc-plugin-appium/test/unit/converter/builder.spec.ts

@@ -10,7 +10,12 @@ import {
   NAME_TYPES_MODULE,
 } from '../../../lib/converter';
 import {AppiumPluginLogger} from '../../../lib/logger';
-import {CommandReflection, CommandsReflection, ProjectCommands} from '../../../lib/model';
+import {
+  AppiumPluginReflectionKind,
+  CommandReflection,
+  CommandsReflection,
+  ProjectCommands,
+} from '../../../lib/model';
 import {initAppForPkgs, NAME_FAKE_DRIVER_MODULE, ROOT_TSCONFIG} from '../helpers';
 
 const {expect} = chai;
@@ -67,15 +72,32 @@ describe('command data builder', function () {
       expect(() => (cmdsRefls = createReflections(ctx, log, moduleCommands))).not.to.throw();
     });
 
-    describe('when the parameters from the method map do not match the method parameters', function () {
-      it('should prefer the method map parameters', function () {
-        const fakeDriverCmdsRefl = cmdsRefls.find(({name}) => name === NAME_FAKE_DRIVER_MODULE)!;
-        expect(fakeDriverCmdsRefl).to.exist;
-        const sessionRouteRefl = fakeDriverCmdsRefl.children!.filter(
-          (child: CommandReflection) => child.name === '/session' && child.httpMethod === 'POST'
-        );
-        sessionRouteRefl; //?
-      });
+    it('should sort commands in ascending order by route', function () {
+      for (const cmdsRefl of cmdsRefls) {
+        let lastRoute = '';
+        for (const cmdRefl of cmdsRefl.getChildrenByKind(
+          AppiumPluginReflectionKind.COMMAND as any
+        ) as CommandReflection[]) {
+          if (lastRoute) {
+            expect(cmdRefl.route.localeCompare(lastRoute)).to.greaterThanOrEqual(0);
+          }
+          lastRoute = cmdRefl.route;
+        }
+      }
+    });
+
+    it('should sort exec methods in ascending order by script', function () {
+      for (const cmdsRefl of cmdsRefls) {
+        let lastScript = '';
+        for (const cmdRefl of cmdsRefl.getChildrenByKind(
+          AppiumPluginReflectionKind.EXECUTE_METHOD as any
+        ) as CommandReflection[]) {
+          if (lastScript) {
+            expect(cmdRefl.script!.localeCompare(lastScript)).to.greaterThanOrEqual(0);
+          }
+          lastScript = cmdRefl.script!;
+        }
+      }
     });
   });
 });

packages/typedoc-plugin-appium/test/unit/converter/builtin-method-map.spec.ts

@@ -3,11 +3,11 @@ import _ from 'lodash';
 import {createSandbox, SinonSandbox} from 'sinon';
 import {Comment, Context} from 'typedoc';
 import {
-  BuiltinCommandSource,
   BuiltinMethodMapConverter,
   KnownMethods,
   NAME_BUILTIN_COMMAND_MODULE,
 } from '../../../lib/converter';
+import {BuiltinCommands} from '../../../lib/model/builtin-commands';
 import {AppiumPluginLogger} from '../../../lib/logger';
 import {initConverter, NAME_FAKE_DRIVER_MODULE} from '../helpers';
 
@@ -36,7 +36,7 @@ describe('BaseDriverConverter', function () {
   describe('instance method', function () {
     describe('convert()', function () {
       describe('when provided the correct module', function () {
-        let builtinSource: BuiltinCommandSource;
+        let builtinSource: BuiltinCommands;
         before(async function () {
           const converter = await initConverter(
             BuiltinMethodMapConverter,
@@ -51,13 +51,13 @@ describe('BaseDriverConverter', function () {
         });
 
         it('should map a method name to a route', function () {
-          expect(builtinSource.moduleCmds.routesByCommandName.get('createSession')).to.eql(
+          expect(builtinSource.moduleCmds!.routesByCommandName.get('createSession')).to.eql(
             new Set(['/session'])
           );
         });
 
         it('should contain command data per route', function () {
-          const firstCommand = [...builtinSource.moduleCmds.routeMap.get('/session')!.values()][0];
+          const firstCommand = [...builtinSource.moduleCmds!.routeMap.get('/session')!.values()][0];
           expect(firstCommand).to.exist;
           expect(_.omit(firstCommand, 'methodRefl', 'parentRefl', 'comment', 'log')).to.eql({
             command: 'createSession',
@@ -78,7 +78,7 @@ describe('BaseDriverConverter', function () {
         const converter = await initConverter(BuiltinMethodMapConverter, NAME_FAKE_DRIVER_MODULE, {
           extraArgs: [new Map()],
         });
-        expect(converter.convert()).to.be.empty;
+        expect(converter.convert().toProjectCommands()).to.be.empty;
       });
     });
   });