fix: make findCommand deterministic

src/config/config.ts

@@ -93,7 +93,8 @@ export class Config implements IConfig {
   private _topics?: Topic[]
 
   // eslint-disable-next-line no-useless-constructor
-  constructor(public options: Options) {}
+  constructor(public options: Options) {
+  }
 
   static async load(opts: LoadOptions = (module.parent && module.parent.parent && module.parent.parent.filename) || __dirname) {
     // Handle the case when a file URL string is passed in such as 'import.meta.url'; covert to file path.
@@ -224,7 +225,7 @@ export class Config implements IConfig {
         log(message?: any, ...args: any[]) {
           process.stdout.write(format(message, ...args) + '\n')
         },
-        error(message, options: {code?: string; exit?: number} = {}) {
+        error(message, options: { code?: string; exit?: number } = {}) {
           error(message, options)
         },
         warn(message: string) {
@@ -287,21 +288,44 @@ export class Config implements IConfig {
     .toUpperCase()
   }
 
-  findCommand(id: string, opts: {must: true}): Command.Plugin
-
-  findCommand(id: string, opts?: {must: boolean}): Command.Plugin | undefined
-
-  findCommand(id: string, opts: {must?: boolean} = {}): Command.Plugin | undefined {
-    const command = this.commands.find(c => c.id === id || c.aliases.includes(id))
-    if (command) return command
-    if (opts.must) error(`command ${id} not found`)
+  findCommand(id: string, opts: { must: true }): Command.Plugin
+
+  findCommand(id: string, opts?: { must: boolean }): Command.Plugin | undefined
+
+  findCommand(id: string, opts: { must?: boolean } = {}): Command.Plugin | undefined {
+    const commands = this.commands.filter(c => c.id === id || c.aliases.includes(id))
+    if (opts.must && commands.length === 0) error(`command ${id} not found`)
+    if (commands.length === 1) return commands[0]
+
+    // more than one command found across available plugins
+    const oclifPlugins = this.pjson.oclif?.plugins ?? []
+    const commandPlugins = commands.sort((a, b) => {
+      const pluginAliasA = a.pluginAlias ?? 'A-Cannot-Find-This'
+      const pluginAliasB = b.pluginAlias ?? 'B-Cannot-Find-This'
+      const aIndex = oclifPlugins.indexOf(pluginAliasA)
+      const bIndex = oclifPlugins.indexOf(pluginAliasB)
+      if (a.pluginType === b.pluginType) {
+        if (aIndex === bIndex) {
+          return 0
+        }
+        if (aIndex < bIndex) {
+          return -1
+        }
+        return 1
+      }
+      if (a.pluginType === 'core' && b.pluginType !== 'core') {
+        return -1
+      }
+      return 1
+    })
+    return commandPlugins[0]
   }
 
-  findTopic(id: string, opts: {must: true}): Topic
+  findTopic(id: string, opts: { must: true }): Topic
 
-  findTopic(id: string, opts?: {must: boolean}): Topic | undefined
+  findTopic(id: string, opts?: { must: boolean }): Topic | undefined
 
-  findTopic(name: string, opts: {must?: boolean} = {}) {
+  findTopic(name: string, opts: { must?: boolean } = {}) {
     const topic = this.topics.find(t => t.name === name)
     if (topic) return topic
     if (opts.must) throw new Error(`topic ${name} not found`)
@@ -403,11 +427,12 @@ export class Config implements IConfig {
     try {
       const {enabled} = require('debug')(this.bin)
       if (enabled) return 1
-    } catch {}
+    } catch {
+    }
     return 0
   }
 
-  protected async loadPlugins(root: string, type: string, plugins: (string | {root?: string; name?: string; tag?: string})[], parent?: Plugin.Plugin) {
+  protected async loadPlugins(root: string, type: string, plugins: (string | { root?: string; name?: string; tag?: string })[], parent?: Plugin.Plugin) {
     if (!plugins || plugins.length === 0) return
     debug('loading plugins', plugins)
     await Promise.all((plugins || []).map(async plugin => {
@@ -437,7 +462,7 @@ export class Config implements IConfig {
     }))
   }
 
-  protected warn(err: string | Error | {name: string; detail: string}, scope?: string) {
+  protected warn(err: string | Error | { name: string; detail: string }, scope?: string) {
     if (this.warned) return
 
     if (typeof err === 'string') {
@@ -539,6 +564,7 @@ export async function toCached(c: Command.Class, plugin?: IPlugin): Promise<Comm
     strict: c.strict,
     usage: c.usage,
     pluginName: plugin && plugin.name,
+    pluginAlias: plugin && plugin.pluginAlias,
     pluginType: plugin && plugin.type,
     hidden: c.hidden,
     aliases: c.aliases || [],

src/config/plugin.ts

@@ -79,6 +79,8 @@ export class Plugin implements IPlugin {
 
   root!: string
 
+  pluginAlias!: string
+
   tag?: string
 
   manifest!: Manifest
@@ -112,6 +114,7 @@ export class Plugin implements IPlugin {
     this._debug('reading %s plugin %s', this.type, root)
     this.pjson = await loadJSON(path.join(root, 'package.json')) as any
     this.name = this.pjson.name
+    this.pluginAlias = this.options.name ?? this.pjson.name
     const pjsonPath = path.join(root, 'package.json')
     if (!this.name) throw new Error(`no name in ${pjsonPath}`)
     if (!isProd() && !this.pjson.files) this.warn(`files attribute must be specified in ${pjsonPath}`)
@@ -128,7 +131,7 @@ export class Plugin implements IPlugin {
 
     this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate))
     this.commands = Object.entries(this.manifest.commands)
-    .map(([id, c]) => ({...c, load: async () => this.findCommand(id, {must: true})}))
+    .map(([id, c]) => ({...c, pluginAlias: this.pluginAlias, pluginType: this.type, load: async () => this.findCommand(id, {must: true})}))
     this.commands.sort((a, b) => {
       if (a.id < b.id) return -1
       if (a.id > b.id) return 1

src/interfaces/command.ts

@@ -55,6 +55,7 @@ export interface Command extends CommandProps {
   type?: string;
   pluginName?: string;
   pluginType?: string;
+  pluginAlias?: string;
   flags: {[name: string]: Command.Flag};
   args: Command.Arg[];
   strict: boolean;

src/interfaces/plugin.ts

@@ -30,6 +30,10 @@ export interface Plugin {
    * name from package.json
    */
   name: string;
+  /**
+   * aliases from package.json dependencies
+   */
+  pluginAlias: string;
   /**
    * version from package.json
    *

test/config/config.test.ts

@@ -1,10 +1,13 @@
 import * as os from 'os'
 import * as path from 'path'
 
-import {Config, Interfaces} from '../../src'
+import {Config} from '../../src/config/config'
+import {Plugin as IPlugin} from '../../src/interfaces'
 import * as util from '../../src/config/util'
+import {Command as ICommand} from '../../src/interfaces'
 
 import {expect, fancy} from './test'
+import {Interfaces} from '../../src'
 
 interface Options {
   pjson?: any;
@@ -13,6 +16,33 @@ interface Options {
   env?: {[k: string]: string};
 }
 
+const pjson = {
+  name: 'foo',
+  version: '1.0.0',
+  files: [],
+  commands: {},
+  oclif: {
+    topics: {
+      t1: {
+        description: 'desc for t1',
+        subtopics: {
+          't1-1': {
+            description: 'desc for t1-1',
+            subtopics: {
+              't1-1-1': {
+                description: 'desc for t1-1-1',
+              },
+              't1-1-2': {
+                description: 'desc for t1-1-2',
+              },
+            },
+          },
+        },
+      },
+    },
+  },
+}
+
 describe('Config', () => {
   const testConfig = ({pjson, homedir = '/my/home', platform = 'darwin', env = {}}: Options = {}) => {
     let test = fancy
@@ -125,34 +155,116 @@ describe('Config', () => {
   })
 
   testConfig({
-    pjson: {
-      name: 'foo',
-      version: '1.0.0',
-      files: [],
-      commands: {},
-      oclif: {
-        topics: {
-          t1: {
-            description: 'desc for t1',
-            subtopics: {
-              't1-1': {
-                description: 'desc for t1-1',
-                subtopics: {
-                  't1-1-1': {
-                    description: 'desc for t1-1-1',
-                  },
-                  't1-1-2': {
-                    description: 'desc for t1-1-2',
-                  },
-                },
-              },
-            },
-          },
-        },
-      },
-    },
+    pjson,
   })
   .it('has subtopics', config => {
     expect(config.topics.map(t => t.name)).to.have.members(['t1', 't1:t1-1', 't1:t1-1:t1-1-1', 't1:t1-1:t1-1-2'])
   })
+
+  const findCommandTestConfig = ({pjson, homedir = '/my/home', platform = 'darwin', env = {}}: Options = {}) => {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+    // @ts-ignore
+    class MyComandClass implements ICommand.Class {
+      _base = ''
+
+      aliases: string[] = []
+
+      hidden = false
+
+      id = 'foo:bar'
+
+      new(): ICommand.Instance {
+        return {_run(): Promise<any> {
+          return Promise.resolve()
+        }}
+      }
+
+      run(): PromiseLike<any> {
+        return Promise.resolve(undefined)
+      }
+    }
+    const load = async (): Promise<void>  => {}
+    const findCommand = async (): Promise<ICommand.Class> => {
+      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+      // @ts-ignore
+      return new MyComandClass()
+    }
+    const commandPluginA: ICommand.Plugin = {
+      strict: false,
+      aliases: [], args: [], flags: {}, hidden: false, id: 'foo:bar', async load(): Promise<ICommand.Class> {
+        return new MyComandClass() as unknown as ICommand.Class
+      },
+    }
+    const commandPluginB: ICommand.Plugin = {
+      strict: false,
+      aliases: [], args: [], flags: {}, hidden: false, id: 'foo:bar', async load(): Promise<ICommand.Class> {
+        return new MyComandClass() as unknown as ICommand.Class
+      },
+    }
+    const hooks = {}
+    const pluginA: IPlugin = {load,
+      findCommand,
+      name: '@My/plugina',
+      pluginAlias: '@My/plugina',
+      commands: [commandPluginA],
+      _base: '',
+      pjson,
+      commandIDs: ['foo:bar'] as string[],
+      root: '',
+      version: '0.0.0',
+      type: 'core',
+      hooks,
+      topics: [],
+      valid: true,
+      tag: 'tag',
+    }
+    commandPluginA
+    const pluginB: IPlugin = {
+      load,
+      findCommand,
+      name: '@My/pluginb',
+      pluginAlias: '@My/pluginb',
+      commands: [commandPluginB],
+      _base: '',
+      pjson,
+      commandIDs: ['foo:bar'] as string[],
+      root: '',
+      version: '0.0.0',
+      type: 'core',
+      hooks,
+      topics: [],
+      valid: true,
+      tag: 'tag',
+    }
+    const plugins: IPlugin[] = [pluginA, pluginB]
+    let test = fancy
+    .resetConfig()
+    .env(env, {clear: true})
+    .stub(os, 'homedir', () => path.join(homedir))
+    .stub(os, 'platform', () => platform)
+
+    if (pjson) test = test.stub(util, 'loadJSON', () => Promise.resolve(pjson))
+    test = test.add('config', async () => {
+      const config = await Config.load()
+      config.plugins = plugins
+      config.pjson.oclif.plugins = ['@My/pluginb', '@My/plugina']
+      config.pjson.dependencies = {'@My/pluginb': '0.0.0', '@My/plugina': '0.0.0'}
+      return config
+    })
+    // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+    // @ts-ignore
+    return {
+      it(expectation: string, fn: (config: Interfaces.Config) => any) {
+        test
+        .do(({config}) => fn(config))
+        .it(expectation)
+        return this
+      },
+    }
+  }
+
+  findCommandTestConfig()
+  .it('find command with no duplicates', config => {
+    expect(config.findCommand('foo:bar', {must: true}))
+  })
 })