Merge #423 from neovim/versionparse

Author: justinmk

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## [5.4.0](https://github.com/neovim/node-client/compare/v5.3.0...v5.4.0)
 
-- TODO
+- feat(findNvim): version parsing robustness
 
 ## [5.3.0](https://github.com/neovim/node-client/compare/v5.2.1...v5.3.0)
 

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

@@ -21,6 +21,7 @@ describe('findNvim', () => {
   afterAll(() => {
     rmSync(testDir, { recursive: true, force: true });
   });
+
   it('parseVersion()', () => {
     expect(parseVersion('0.5.0-dev+1357-g192f89ea1')).toEqual([
       0,
@@ -74,6 +75,18 @@ describe('findNvim', () => {
         '0.3.0-dev-658+g06694203e-Homebrew'
       )
     ).toBe(1);
+
+    // Failure modes:
+    expect(compareVersions('0.3.0', 'nonsense')).toBe(1);
+    expect(() => compareVersions('nonsense', '0.3.0')).toThrow(
+      'Invalid version: "nonsense"'
+    );
+    expect(() => compareVersions('nonsense', 'nonsense')).toThrow(
+      'Invalid version: "nonsense"'
+    );
+    expect(() => compareVersions(undefined, undefined)).toThrow(
+      'Invalid version format: not a string'
+    );
   });
 
   /** Asserts that >=1 nvim binaries were found. */

packages/neovim/src/utils/findNvim.ts

@@ -67,7 +67,7 @@ export type FindNvimResult = {
 };
 
 const versionRegex = /^(\d+)\.(\d+)\.(\d+)(?:-(.+))?$/;
-const nvimVersionRegex = /^NVIM\s+v(.+)$/m;
+const nvimVersionRegex = /^[nN][vV][iI][mM]\s+v?(.+)$/m;
 const buildTypeRegex = /^Build\s+type:\s+(.+)$/m;
 const luaJitVersionRegex = /^LuaJIT\s+(.+)$/m;
 const windows = process.platform === 'win32';
@@ -83,6 +83,9 @@ function parseVersion(version: string): (number | string)[] | undefined {
   }
 
   const [, major, minor, patch, prerelease] = match;
+  if (major === undefined || minor === undefined || patch === undefined) {
+    throw new TypeError(`Invalid version string: "${version}"`);
+  }
   const majorNumber = Number(major);
   const minorNumber = Number(minor);
   const patchNumber = Number(patch);
@@ -114,11 +117,17 @@ function parseVersion(version: string): (number | string)[] | undefined {
 function compareVersions(a: string, b: string): number {
   const versionA = parseVersion(a);
   const versionB = parseVersion(b);
-  const length = Math.min(versionA?.length ?? 0, versionB?.length ?? 0);
+  if (versionA === undefined) {
+    throw new TypeError(`Invalid version: "${a}"`);
+  }
+  if (versionB === undefined) {
+    return 1;
+  }
 
+  const length = Math.min(versionA.length, versionB.length);
   for (let i = 0; i < length; i = i + 1) {
-    const partA = versionA?.[i] ?? 0;
-    const partB = versionB?.[i] ?? 0;
+    const partA = versionA[i] ?? 0;
+    const partB = versionB[i] ?? 0;
     if (partA < partB) {
       return -1;
     }
@@ -127,7 +136,7 @@ function compareVersions(a: string, b: string): number {
     }
   }
 
-  if ((versionB?.length ?? 0) > (versionA?.length ?? 0)) {
+  if (versionB.length > versionA.length) {
     return -1;
   }
 
@@ -209,6 +218,7 @@ export function findNvim(opt: FindNvimOptions = {}): Readonly<FindNvimResult> {
     if (existsSync(nvimPath) || normalizedPathsFromUser.includes(nvimPath)) {
       try {
         accessSync(nvimPath, constants.X_OK);
+        // TODO: fallback to `echo 'print(vim.version())' | nvim -l -` if parsing --version fails.
         const nvimVersionFull = execFileSync(nvimPath, [
           '--version',
         ]).toString();