Use a more forgiving comparer when comparing version (#15912)

* Use a more forgiving comparer when comparing version

* Update tests

* Move sort interpreters function

Author: karthiknadig

src/client/interpreter/autoSelection/index.ts

@@ -4,13 +4,13 @@
 'use strict';
 
 import { inject, injectable, named } from 'inversify';
-import { compare } from 'semver';
 import { Event, EventEmitter, Uri } from 'vscode';
 import { IWorkspaceService } from '../../common/application/types';
 import '../../common/extensions';
 import { IFileSystem } from '../../common/platform/types';
 import { IPersistentState, IPersistentStateFactory, Resource } from '../../common/types';
 import { createDeferred, Deferred } from '../../common/utils/async';
+import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/pythonVersion';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
 import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
 import { EventName } from '../../telemetry/constants';
@@ -172,7 +172,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio
                 this.globallyPreferredInterpreter.value.version &&
                 interpreter &&
                 interpreter.version &&
-                compare(this.globallyPreferredInterpreter.value.version.raw, interpreter.version.raw) > 0
+                compareSemVerLikeVersions(this.globallyPreferredInterpreter.value.version, interpreter.version) > 0
             ) {
                 return;
             }

src/client/interpreter/autoSelection/rules/baseRule.ts

@@ -4,12 +4,12 @@
 'use strict';
 
 import { inject, injectable, unmanaged } from 'inversify';
-import { compare } from 'semver';
 import '../../../common/extensions';
 import { traceDecorators, traceVerbose } from '../../../common/logger';
 import { IFileSystem } from '../../../common/platform/types';
 import { IPersistentState, IPersistentStateFactory, Resource } from '../../../common/types';
 import { StopWatch } from '../../../common/utils/stopWatch';
+import { compareSemVerLikeVersions } from '../../../pythonEnvironments/base/info/pythonVersion';
 import { PythonEnvironment } from '../../../pythonEnvironments/info';
 import { sendTelemetryEvent } from '../../../telemetry';
 import { EventName } from '../../../telemetry/constants';
@@ -74,7 +74,7 @@ export abstract class BaseRuleService implements IInterpreterAutoSelectionRule {
         const preferredInterpreter = manager.getAutoSelectedInterpreter(undefined);
         const comparison =
             preferredInterpreter && preferredInterpreter.version
-                ? compare(interpreter.version.raw, preferredInterpreter.version.raw)
+                ? compareSemVerLikeVersions(interpreter.version, preferredInterpreter.version)
                 : 1;
         if (comparison > 0) {
             await manager.setGlobalInterpreter(interpreter);

src/client/interpreter/helpers.ts

@@ -7,14 +7,14 @@ import { FileSystemPaths } from '../common/platform/fs-paths';
 import { IPythonExecutionFactory } from '../common/process/types';
 import { IExperimentService, IPersistentStateFactory, Resource } from '../common/types';
 import { IServiceContainer } from '../ioc/types';
+import { compareSemVerLikeVersions } from '../pythonEnvironments/base/info/pythonVersion';
 import { isMacDefaultPythonPath } from '../pythonEnvironments/discovery';
 import { getInterpreterHash } from '../pythonEnvironments/discovery/locators/services/hashProvider';
 import {
     EnvironmentType,
     getEnvironmentTypeName,
     InterpreterInformation,
     PythonEnvironment,
-    sortInterpreters,
 } from '../pythonEnvironments/info';
 import { IComponentAdapter, IInterpreterHelper, WorkspacePythonPath } from './contracts';
 
@@ -28,6 +28,21 @@ export function isInterpreterLocatedInWorkspace(interpreter: PythonEnvironment,
     return interpreterPath.startsWith(resourcePath);
 }
 
+/**
+ * Build a version-sorted list from the given one, with lowest first.
+ */
+function sortInterpreters(interpreters: PythonEnvironment[]): PythonEnvironment[] {
+    if (interpreters.length === 0) {
+        return [];
+    }
+    if (interpreters.length === 1) {
+        return [interpreters[0]];
+    }
+    const sorted = interpreters.slice();
+    sorted.sort((a, b) => (a.version && b.version ? compareSemVerLikeVersions(a.version, b.version) : 0));
+    return sorted;
+}
+
 @injectable()
 export class InterpreterHelper implements IInterpreterHelper {
     private readonly persistentFactory: IPersistentStateFactory;

src/client/pythonEnvironments/base/info/pythonVersion.ts

@@ -262,3 +262,29 @@ export function toSemverLikeVersion(
         prerelease: preRelease,
     };
 }
+
+/**
+ * Compares major, minor, patch for two versions of python
+ * @param v1 : semVer like version object
+ * @param v2 : semVer like version object
+ * @returns {1 | 0 | -1} :  0 if v1 === v2,
+ *                      1 if v1 > v2,
+ *                     -1 if v1 < v2
+ * Remarks: primarily used compare to old type of version info.
+ * @deprecated
+ */
+export function compareSemVerLikeVersions(
+    v1: { major: number; minor: number; patch: number },
+    v2: { major: number; minor: number; patch: number },
+): 1 | 0 | -1 {
+    if (v1.major === v2.major) {
+        if (v1.minor === v2.minor) {
+            if (v1.patch === v2.patch) {
+                return 0;
+            }
+            return v1.patch > v2.patch ? 1 : -1;
+        }
+        return v1.minor > v2.minor ? 1 : -1;
+    }
+    return v1.major > v2.major ? 1 : -1;
+}

src/client/pythonEnvironments/discovery/locators/services/condaLocatorService.ts

@@ -3,7 +3,6 @@
 
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
-import { compare } from 'semver';
 import { ConfigurationChangeEvent, Uri } from 'vscode';
 
 import { IWorkspaceService } from '../../../../common/application/types';
@@ -25,6 +24,7 @@ import {
     WINDOWS_REGISTRY_SERVICE,
 } from '../../../../interpreter/contracts';
 import { IServiceContainer } from '../../../../ioc/types';
+import { compareSemVerLikeVersions } from '../../../base/info/pythonVersion';
 import { EnvironmentType, PythonEnvironment } from '../../../info';
 import { CondaEnvironmentInfo, CondaInfo } from './conda';
 import { parseCondaEnvFileContents } from './condaHelper';
@@ -91,7 +91,9 @@ export class CondaLocatorService implements ICondaLocatorService {
     private static getLatestVersion(interpreters: PythonEnvironment[]): PythonEnvironment | undefined {
         const sortedInterpreters = interpreters.slice();
 
-        sortedInterpreters.sort((a, b) => (a.version && b.version ? compare(a.version.raw, b.version.raw) : 0));
+        sortedInterpreters.sort((a, b) =>
+            a.version && b.version ? compareSemVerLikeVersions(a.version, b.version) : 0,
+        );
         if (sortedInterpreters.length > 0) {
             return sortedInterpreters[sortedInterpreters.length - 1];
         }

src/client/pythonEnvironments/info/index.ts

@@ -3,7 +3,6 @@
 
 'use strict';
 
-import * as semver from 'semver';
 import { Architecture } from '../../common/utils/platform';
 import { PythonVersion } from './pythonVersion';
 
@@ -98,18 +97,3 @@ export function getEnvironmentTypeName(environmentType: EnvironmentType): string
         }
     }
 }
-
-/**
- * Build a version-sorted list from the given one, with lowest first.
- */
-export function sortInterpreters(interpreters: PythonEnvironment[]): PythonEnvironment[] {
-    if (interpreters.length === 0) {
-        return [];
-    }
-    if (interpreters.length === 1) {
-        return [interpreters[0]];
-    }
-    const sorted = interpreters.slice();
-    sorted.sort((a, b) => (a.version && b.version ? semver.compare(a.version.raw, b.version.raw) : 0));
-    return sorted;
-}

src/test/pythonEnvironments/base/info/pythonVersion.unit.test.ts

@@ -5,6 +5,7 @@ import * as assert from 'assert';
 
 import { PythonReleaseLevel, PythonVersion } from '../../../../client/pythonEnvironments/base/info';
 import {
+    compareSemVerLikeVersions,
     getEmptyVersion,
     getShortVersionString,
     parseVersion,
@@ -171,3 +172,60 @@ suite('pyenvs info - parseVersion', () => {
         });
     });
 });
+
+suite('pyenvs info - compareSemVerLikeVersions', () => {
+    const testData = [
+        {
+            v1: { major: 2, minor: 7, patch: 19 },
+            v2: { major: 3, minor: 7, patch: 4 },
+            expected: -1,
+        },
+        {
+            v1: { major: 2, minor: 7, patch: 19 },
+            v2: { major: 2, minor: 7, patch: 19 },
+            expected: 0,
+        },
+        {
+            v1: { major: 3, minor: 7, patch: 4 },
+            v2: { major: 2, minor: 7, patch: 19 },
+            expected: 1,
+        },
+        {
+            v1: { major: 3, minor: 8, patch: 1 },
+            v2: { major: 3, minor: 9, patch: 1 },
+            expected: -1,
+        },
+        {
+            v1: { major: 3, minor: 9, patch: 1 },
+            v2: { major: 3, minor: 9, patch: 1 },
+            expected: 0,
+        },
+        {
+            v1: { major: 3, minor: 9, patch: 1 },
+            v2: { major: 3, minor: 8, patch: 1 },
+            expected: 1,
+        },
+        {
+            v1: { major: 3, minor: 9, patch: 0 },
+            v2: { major: 3, minor: 9, patch: 1 },
+            expected: -1,
+        },
+        {
+            v1: { major: 3, minor: 9, patch: 1 },
+            v2: { major: 3, minor: 9, patch: 1 },
+            expected: 0,
+        },
+        {
+            v1: { major: 3, minor: 9, patch: 1 },
+            v2: { major: 3, minor: 9, patch: 0 },
+            expected: 1,
+        },
+    ];
+
+    testData.forEach((data) => {
+        test(`Compare versions ${JSON.stringify(data.v1)} and ${JSON.stringify(data.v2)}`, () => {
+            const actual = compareSemVerLikeVersions(data.v1, data.v2);
+            assert.deepStrictEqual(actual, data.expected);
+        });
+    });
+});