refactor: getNvimFromEnv

- don't export `parseVersion`, `compareVersions`
- rename options
- add test coverage
- remove TypeErrors for conditions which cannot happen (thus aren't testable)

.eslintrc.js

@@ -28,8 +28,8 @@ module.exports = {
         // `jest` against the compiled .js results (would require compiling
         // the test files as well)?
         'unicorn/prefer-at': 'off',
-      }
-    }
+      },
+    },
   ],
 
   rules: {
@@ -61,6 +61,7 @@ module.exports = {
 
     // Causes issues with enums
     'no-shadow': 'off',
+    'prefer-destructuring': 'off',  // Intentionally disabled trash.
 
     // prettier things
     'prettier/prettier': 'error',

packages/neovim/src/utils/getNvimFromEnv.test.ts

@@ -1,34 +1,46 @@
 /* eslint-env jest */
 // eslint-disable-next-line import/no-extraneous-dependencies
 import which from 'which';
-import {
-  getNvimFromEnv,
-  compareVersions,
-  parseVersion,
-} from './getNvimFromEnv';
+import { getNvimFromEnv, exportsForTesting } from './getNvimFromEnv';
+
+const parseVersion = exportsForTesting.parseVersion;
+const compareVersions = exportsForTesting.compareVersions;
 
 try {
   which.sync('nvim');
 } catch (e) {
   // eslint-disable-next-line no-console
-  console.error(
-    'A Neovim installation is required to run the tests',
-    '(see https://github.com/neovim/neovim/wiki/Installing)'
+  throw new Error(
+    'Nvim is required to run the tests (see https://github.com/neovim/neovim/wiki/Installing)'
   );
-  process.exit(1);
 }
 
-describe('get_nvim_from_env', () => {
-  it('Parse version', () => {
-    const a = parseVersion('0.5.0-dev+1357-g192f89ea1');
-    expect(a).toEqual([0, 5, 0, 'dev+1357-g192f89ea1']);
-    const b = parseVersion('0.5.0-dev+1357-g192f89ea1-Homebrew');
-    expect(b).toEqual([0, 5, 0, 'dev+1357-g192f89ea1-Homebrew']);
-    const c = parseVersion('0.9.1');
-    expect(c).toEqual([0, 9, 1, 'zzz']);
+describe('getNvimFromEnv', () => {
+  it('parseVersion()', () => {
+    expect(parseVersion('0.5.0-dev+1357-g192f89ea1')).toEqual([
+      0,
+      5,
+      0,
+      'dev+1357-g192f89ea1',
+    ]);
+    expect(parseVersion('0.5.0-dev+1357-g192f89ea1-Homebrew')).toEqual([
+      0,
+      5,
+      0,
+      'dev+1357-g192f89ea1-Homebrew',
+    ]);
+    expect(parseVersion('0.9.1')).toEqual([0, 9, 1, 'zzz']);
+
+    // Failure modes:
+    expect(() => parseVersion(42 as any)).toThrow(TypeError);
+    expect(parseVersion('x.y.z')).toEqual(undefined);
+    expect(parseVersion('1.y.z')).toEqual(undefined);
+    expect(parseVersion('1.2.z')).toEqual(undefined);
+    expect(parseVersion('x.2.3')).toEqual(undefined);
+    expect(parseVersion('1.y.3')).toEqual(undefined);
   });
 
-  it('Compare versions', () => {
+  it('compareVersions()', () => {
     expect(compareVersions('0.3.0', '0.3.0')).toBe(0);
     expect(compareVersions('0.3.0', '0.3.1')).toBe(-1);
     expect(compareVersions('0.3.1', '0.3.0')).toBe(1);
@@ -59,8 +71,8 @@ describe('get_nvim_from_env', () => {
     ).toBe(1);
   });
 
-  it('Get matching nvim from specified min version', () => {
-    const nvimRes = getNvimFromEnv({ minVersion: '0.3.0' });
+  it('gets Nvim matching specified min version', () => {
+    const nvimRes = getNvimFromEnv({ minVersion: '0.3.0', orderBy: 'desc' });
     expect(nvimRes).toBeTruthy();
     expect(nvimRes).toEqual({
       matches: expect.any(Array),
@@ -79,7 +91,7 @@ describe('get_nvim_from_env', () => {
     expect(nvimRes.errors.length).toEqual(0);
   });
 
-  it('Get matching nvim without specified min version', () => {
+  it('gets Nvim without specified min version', () => {
     const nvimRes = getNvimFromEnv();
     expect(nvimRes).toBeTruthy();
     expect(nvimRes).toEqual({

packages/neovim/src/utils/getNvimFromEnv.ts

@@ -19,16 +19,16 @@ export type GetNvimFromEnvOptions = {
   /**
    * (Optional) Sort order of list of `nvim` versions.
    *
-   * - `latest_nvim_first` - Latest Nvim version will be first.
+   * - "desc" - Sort by version in descending order (highest to lowest).
    *   - Example: `['0.5.0', '0.4.4', '0.4.3']`
-   * - `keep_path` - (Default) Order is that of the searched `$PATH` components.
+   * - "none" - (Default) Order is that of the searched `$PATH` components.
    *   - Example: `['0.4.4', '0.5.0', '0.4.3']`
    */
-  readonly orderBy?: 'latest_nvim_first' | 'keep_path';
+  readonly orderBy?: 'desc' | 'none';
 };
 
 export type GetNvimFromEnvError = {
-  /** Executeable path that failed. */
+  /** Executable path that failed. */
   readonly path: string;
   /** Error caught during operation. */
   readonly exception: Readonly<Error>;
@@ -58,31 +58,20 @@ const buildTypeRegex = /^Build\s+type:\s+(.+)$/m;
 const luaJitVersionRegex = /^LuaJIT\s+(.+)$/m;
 const windows = process.platform === 'win32';
 
-export function parseVersion(version: string): (number | string)[] | null {
+function parseVersion(version: string): (number | string)[] | undefined {
   if (typeof version !== 'string') {
     throw new TypeError('Invalid version format: not a string');
   }
 
   const match = version.match(versionRegex);
-  if (match === null) {
-    return null;
+  if (!match) {
+    return undefined;
   }
 
   const [, major, minor, patch, prerelease] = match;
   const majorNumber = Number(major);
-  if (Number.isNaN(majorNumber)) {
-    throw new TypeError('Invalid version format: major is not a number');
-  }
-
   const minorNumber = Number(minor);
-  if (Number.isNaN(minorNumber)) {
-    throw new TypeError('Invalid version format: minor is not a number');
-  }
-
   const patchNumber = Number(patch);
-  if (Number.isNaN(patchNumber)) {
-    throw new TypeError('Invalid version format: patch is not a number');
-  }
 
   const versionParts: Array<number | string> = [
     majorNumber,
@@ -98,17 +87,17 @@ export function parseVersion(version: string): (number | string)[] | null {
 }
 
 /**
- * Compare two versions.
+ * Compares two versions.
  * @param a - The first version to compare.
  * @param b - The second version to compare.
  * @returns -1 if a < b, 0 if a == b, 1 if a > b.
- * @throws {Error} If the versions are not valid.
+ * @throws {TypeError} If the versions are not valid.
  *
  * Format could be:
  * - 0.9.1
  * - 0.10.0-dev-658+g06694203e-Homebrew
  */
-export function compareVersions(a: string, b: string): number {
+function compareVersions(a: string, b: string): number {
   const versionA = parseVersion(a);
   const versionB = parseVersion(b);
   const length = Math.min(versionA.length, versionB.length);
@@ -181,7 +170,7 @@ export function getNvimFromEnv(
     }
   }
 
-  if (matches.length > 1 && opt.orderBy === 'latest_nvim_first') {
+  if (matches.length > 1 && opt.orderBy === 'desc') {
     matches.sort((a, b) => compareVersions(b.nvimVersion, a.nvimVersion));
   }
 
@@ -191,3 +180,14 @@ export function getNvimFromEnv(
     errors,
   } as const;
 }
+
+export let exportsForTesting: any;
+// jest sets NODE_ENV=test.
+if (process.env.NODE_ENV === 'test') {
+  // These functions are intentionally not exported. After `nvim` is found, clients can use Nvim's
+  // own `vim.version` module, so node-client shouldn't expose a half-baked "semver" implementation.
+  exportsForTesting = {
+    parseVersion,
+    compareVersions,
+  };
+}