Refactor how version information is stored against interpreter inform…

…ation (#3812)

For #3369

src/client/common/installer/moduleInstaller.ts

@@ -56,7 +56,7 @@ export abstract class ModuleInstaller {
         // If installing pylint on python 2.x, then use pylint~=1.9.0
         const interpreterService = this.serviceContainer.get<IInterpreterService>(IInterpreterService);
         const currentInterpreter = await interpreterService.getActiveInterpreter(resource);
-        if (currentInterpreter && currentInterpreter.version_info && currentInterpreter.version_info[0] === 2) {
+        if (currentInterpreter && currentInterpreter.version && currentInterpreter.version.major === 2) {
             const newArgs = [...args];
             // This command could be sent to the terminal, hence '<' needs to be escaped for UNIX.
             newArgs[indexOfPylint] = '"pylint<2.0.0"';

src/client/common/process/pythonProcess.ts

@@ -10,6 +10,7 @@ import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError';
 import { traceError } from '../logger';
 import { IFileSystem } from '../platform/types';
 import { Architecture } from '../utils/platform';
+import { convertPythonVersionToSemver } from '../utils/version';
 import { ExecutionResult, InterpreterInfomation, IProcessService, IPythonExecutionService, ObservableExecutionResult, PythonVersionInfo, SpawnOptions } from './types';
 
 @injectable()
@@ -27,12 +28,8 @@ export class PythonExecutionService implements IPythonExecutionService {
     public async getInterpreterInformation(): Promise<InterpreterInfomation | undefined> {
         const file = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'interpreterInfo.py');
         try {
-            const [version, jsonValue] = await Promise.all([
-                this.procService.exec(this.pythonPath, ['--version'], { mergeStdOutErr: true })
-                    .then(output => output.stdout.trim()),
-                this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true })
-                    .then(output => output.stdout.trim())
-            ]);
+            const jsonValue = await this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true })
+                .then(output => output.stdout.trim());
 
             let json: { versionInfo: PythonVersionInfo; sysPrefix: string; sysVersion: string; is64Bit: boolean };
             try {
@@ -41,22 +38,12 @@ export class PythonExecutionService implements IPythonExecutionService {
                 traceError(`Failed to parse interpreter information for '${this.pythonPath}' with JSON ${jsonValue}`, ex);
                 return;
             }
-            const version_info = json.versionInfo;
-            // Exclude PII from `version_info` to ensure we don't send this up via telemetry.
-            for (let index = 0; index < 3; index += 1) {
-                if (typeof version_info[index] !== 'number') {
-                    version_info[index] = 0;
-                }
-            }
-            if (['alpha', 'beta', 'candidate', 'final'].indexOf(version_info[3]) === -1) {
-                version_info[3] = 'unknown';
-            }
+            const versionValue = json.versionInfo.length === 4 ? `${json.versionInfo.slice(0, 3).join('.')}-${json.versionInfo[3]}` : json.versionInfo.join('.');
             return {
                 architecture: json.is64Bit ? Architecture.x64 : Architecture.x86,
                 path: this.pythonPath,
-                version,
+                version: convertPythonVersionToSemver(versionValue),
                 sysVersion: json.sysVersion,
-                version_info: json.versionInfo,
                 sysPrefix: json.sysPrefix
             };
         } catch (ex) {

src/client/common/process/types.ts

@@ -4,6 +4,7 @@ import { ChildProcess, ExecOptions, SpawnOptions as ChildProcessSpawnOptions } f
 import { Observable } from 'rxjs/Observable';
 import { CancellationToken, Uri } from 'vscode';
 
+import { SemVer } from 'semver';
 import { ExecutionInfo } from '../types';
 import { Architecture } from '../utils/platform';
 import { EnvironmentVariables } from '../variables/types';
@@ -20,7 +21,7 @@ export type Output<T extends string | Buffer> = {
 export type ObservableExecutionResult<T extends string | Buffer> = {
     proc: ChildProcess | undefined;
     out: Observable<Output<T>>;
-    dispose() : void;
+    dispose(): void;
 };
 
 // tslint:disable-next-line:interface-name
@@ -32,7 +33,7 @@ export type SpawnOptions = ChildProcessSpawnOptions & {
 };
 
 // tslint:disable-next-line:interface-name
-export type ShellOptions = ExecOptions & { throwOnStdErr? : boolean };
+export type ShellOptions = ExecOptions & { throwOnStdErr?: boolean };
 
 export type ExecutionResult<T extends string | Buffer> = {
     stdout: T;
@@ -60,14 +61,12 @@ export interface IPythonExecutionFactory {
     create(options: ExecutionFactoryCreationOptions): Promise<IPythonExecutionService>;
 }
 export type ReleaseLevel = 'alpha' | 'beta' | 'candidate' | 'final' | 'unknown';
-// tslint:disable-next-line:interface-name
 export type PythonVersionInfo = [number, number, number, ReleaseLevel];
 export type InterpreterInfomation = {
     path: string;
-    version: string;
+    version?: SemVer;
     sysVersion: string;
     architecture: Architecture;
-    version_info: PythonVersionInfo;
     sysPrefix: string;
 };
 export const IPythonExecutionService = Symbol('IPythonExecutionService');

src/client/common/terminal/service.ts

@@ -81,7 +81,7 @@ export class TerminalService implements ITerminalService, Disposable {
     private async sendTelemetry() {
         const pythonPath = this.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(this.resource).pythonPath;
         const interpreterInfo = await this.serviceContainer.get<IInterpreterService>(IInterpreterService).getInterpreterDetails(pythonPath);
-        const pythonVersion = (interpreterInfo && interpreterInfo.version_info) ? interpreterInfo.version_info.join('.') : undefined;
+        const pythonVersion = (interpreterInfo && interpreterInfo.version) ? interpreterInfo.version.raw : undefined;
         const interpreterType = interpreterInfo ? interpreterInfo.type : undefined;
         captureTelemetry(TERMINAL_CREATE, { terminal: this.terminalShellType, pythonVersion, interpreterType });
     }

src/client/common/utils/version.ts

@@ -24,6 +24,33 @@ export function convertToSemver(version: string) {
     return versionParts.join('.');
 }
 
+export function convertPythonVersionToSemver(version: string): semver.SemVer | undefined {
+    if (!version || version.trim().length === 0) {
+        return;
+    }
+    const versionParts = (version || '')
+    .split('.')
+    .map(item => item.trim())
+    .filter(item => item.length > 0)
+    .filter((_, index) => index < 4);
+
+    if (versionParts.length > 0 && versionParts[versionParts.length - 1].indexOf('-') > 0) {
+        const lastPart = versionParts[versionParts.length - 1];
+        versionParts[versionParts.length - 1] = lastPart.split('-')[0].trim();
+        versionParts.push(lastPart.split('-')[1].trim());
+    }
+    while (versionParts.length < 4) {
+        versionParts.push('');
+    }
+    // Exclude PII from `version_info` to ensure we don't send this up via telemetry.
+    for (let index = 0; index < 3; index += 1) {
+        versionParts[index] = /^\d+$/.test(versionParts[index]) ? versionParts[index] : '0';
+    }
+    versionParts[3] = ['alpha', 'beta', 'candidate', 'final'].indexOf(versionParts[3]) === -1 ? 'unknown' : versionParts[3];
+
+    return new semver.SemVer(`${versionParts[0]}.${versionParts[1]}.${versionParts[2]}-${versionParts[3]}`);
+}
+
 export function compareVersion(versionA: string, versionB: string) {
     try {
         versionA = convertToSemver(versionA);

src/client/datascience/jupyterExecution.ts

@@ -92,8 +92,8 @@ class JupyterCommand {
 
     public mainVersion = async (): Promise<number> => {
         const interpreter = await this.interpreterPromise;
-        if (interpreter) {
-            return interpreter.version_info[0];
+        if (interpreter && interpreter.version) {
+            return interpreter.version.major;
         } else {
             return this.execVersion();
         }
@@ -417,14 +417,18 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
                         score += 1;
 
                         // Then start matching based on version
-                        if (list[i].version_info[0] === active.version_info[0]) {
-                            score += 32;
-                            if (list[i].version_info[1] === active.version_info[1]) {
-                                score += 16;
-                                if (list[i].version_info[2] === active.version_info[2]) {
-                                    score += 8;
-                                    if (list[i].version_info[3] === active.version_info[3]) {
-                                        score += 4;
+                        const listVersion = list[i].version;
+                        const activeVersion = active.version;
+                        if (listVersion && activeVersion) {
+                            if (listVersion.major === activeVersion.major) {
+                                score += 32;
+                                if (listVersion.minor === activeVersion.minor) {
+                                    score += 16;
+                                    if (listVersion.patch === activeVersion.patch) {
+                                        score += 8;
+                                        if (listVersion.raw === activeVersion.raw) {
+                                            score += 4;
+                                        }
                                     }
                                 }
                             }
@@ -632,33 +636,33 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
                 score += 1;
 
                 // See if the version is the same
-                if (info && info.version_info && specDetails[i]) {
+                if (info && info.version && specDetails[i]) {
                     const details = specDetails[i];
-                    if (details && details.version_info) {
-                        if (details.version_info[0] === info.version_info[0]) {
+                    if (details && details.version) {
+                        if (details.version.major === info.version.major) {
                             // Major version match
                             score += 4;
 
-                            if (details.version_info[1] === info.version_info[1]) {
+                            if (details.version.minor === info.version.minor) {
                                 // Minor version match
                                 score += 2;
 
-                                if (details.version_info[2] === info.version_info[2]) {
+                                if (details.version.patch === info.version.patch) {
                                     // Minor version match
                                     score += 1;
                                 }
                             }
                         }
                     }
-                } else if (info && info.version_info && spec && spec.path && spec.path.toLocaleLowerCase() === 'python' && spec.name) {
+                } else if (info && info.version && spec && spec.path && spec.path.toLocaleLowerCase() === 'python' && spec.name) {
                     // This should be our current python.
 
                     // Search for a digit on the end of the name. It should match our major version
                     const match = /\D+(\d+)/.exec(spec.name);
                     if (match && match !== null && match.length > 0) {
                         // See if the version number matches
                         const nameVersion = parseInt(match[0], 10);
-                        if (nameVersion && nameVersion === info.version_info[0]) {
+                        if (nameVersion && nameVersion === info.version.major) {
                             score += 4;
                         }
                     }

src/client/datascience/jupyterExporter.ts

@@ -19,7 +19,7 @@ import { CellState, ICell, IJupyterExecution, INotebookExporter, ISysInfo } from
 export class JupyterExporter implements INotebookExporter {
 
     constructor(
-        @inject(IJupyterExecution) private jupyterExecution : IJupyterExecution,
+        @inject(IJupyterExecution) private jupyterExecution: IJupyterExecution,
         @inject(ILogger) private logger: ILogger,
         @inject(IWorkspaceService) private workspaceService: IWorkspaceService,
         @inject(IFileSystem) private fileSystem: IFileSystem) {
@@ -29,7 +29,7 @@ export class JupyterExporter implements INotebookExporter {
         noop();
     }
 
-    public async translateToNotebook(cells: ICell[], changeDirectory?: string) : Promise<nbformat.INotebookContent | undefined> {
+    public async translateToNotebook(cells: ICell[], changeDirectory?: string): Promise<nbformat.INotebookContent | undefined> {
         // If requested, add in a change directory cell to fix relative paths
         if (changeDirectory) {
             cells = await this.addDirectoryChangeCell(cells, changeDirectory);
@@ -95,18 +95,17 @@ export class JupyterExporter implements INotebookExporter {
     // When we export we want to our change directory back to the first real file that we saw run from any workspace folder
     private firstWorkspaceFolder = async (cells: ICell[]): Promise<string | undefined> => {
         for (const cell of cells) {
-           const filename = cell.file;
+            const filename = cell.file;
 
-           // First check that this is an absolute file that exists (we add in temp files to run system cell)
-           if (path.isAbsolute(filename) && await this.fileSystem.fileExists(filename)) {
+            // First check that this is an absolute file that exists (we add in temp files to run system cell)
+            if (path.isAbsolute(filename) && await this.fileSystem.fileExists(filename)) {
                 // We've already check that workspace folders above
                 for (const folder of this.workspaceService.workspaceFolders!) {
-                    if (filename.toLowerCase().startsWith(folder.uri.fsPath.toLowerCase()))
-                    {
+                    if (filename.toLowerCase().startsWith(folder.uri.fsPath.toLowerCase())) {
                         return folder.uri.fsPath;
                     }
                 }
-           }
+            }
         }
 
         return undefined;
@@ -134,22 +133,22 @@ export class JupyterExporter implements INotebookExporter {
         }
     }
 
-    private pruneCells = (cells : ICell[]) : nbformat.IBaseCell[] => {
+    private pruneCells = (cells: ICell[]): nbformat.IBaseCell[] => {
         // First filter out sys info cells. Jupyter doesn't understand these
         return cells.filter(c => c.data.cell_type !== 'sys_info')
             // Then prune each cell down to just the cell data.
             .map(this.pruneCell);
     }
 
-    private pruneCell = (cell : ICell) : nbformat.IBaseCell => {
+    private pruneCell = (cell: ICell): nbformat.IBaseCell => {
         // Remove the #%% of the top of the source if there is any. We don't need
         // this to end up in the exported ipynb file.
-        const copy = {...cell.data};
+        const copy = { ...cell.data };
         copy.source = this.pruneSource(cell.data.source);
         return copy;
     }
 
-    private pruneSource = (source : nbformat.MultilineString) : nbformat.MultilineString => {
+    private pruneSource = (source: nbformat.MultilineString): nbformat.MultilineString => {
 
         if (Array.isArray(source) && source.length > 0) {
             if (RegExpValues.PythonCellMarker.test(source[0])) {
@@ -168,7 +167,7 @@ export class JupyterExporter implements INotebookExporter {
     private extractPythonMainVersion = async (cells: ICell[]): Promise<number> => {
         let pythonVersion;
         const sysInfoCells = cells.filter((targetCell: ICell) => {
-           return targetCell.data.cell_type === 'sys_info';
+            return targetCell.data.cell_type === 'sys_info';
         });
 
         if (sysInfoCells.length > 0) {
@@ -184,6 +183,6 @@ export class JupyterExporter implements INotebookExporter {
 
         // In this case, let's check the version on the active interpreter
         const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython();
-        return usableInterpreter ? usableInterpreter.version_info[0] : 3;
+        return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3;
     }
 }

src/client/extension.ts

@@ -297,10 +297,10 @@ async function sendStartupTelemetry(activatedPromise: Promise<any>, serviceConta
             interpreterService.getInterpreters(mainWorkspaceUri).catch<PythonInterpreter[]>(() => [])
         ]);
         const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0;
-        const pythonVersion = interpreter ? interpreter.version_info.join('.') : undefined;
+        const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined;
         const interpreterType = interpreter ? interpreter.type : undefined;
         const hasPython3 = interpreters
-            .filter(item => item && Array.isArray(item.version_info) ? item.version_info[0] === 3 : false)
+            .filter(item => item && item.version ? item.version.major === 3 : false)
             .length > 0;
 
         const props = { condaVersion, terminal: terminalShellType, pythonVersion, interpreterType, workspaceFolderCount, hasPython3 };

src/client/interpreter/configuration/interpreterComparer.ts

@@ -29,8 +29,8 @@ export class InterpreterComparer implements IInterpreterComparer {
         // * Architecture
         // * Interpreter Type
         // * Environment name
-        if (info.version_info && info.version_info.length > 0) {
-            sortNameParts.push(info.version_info.slice(0, 3).join('.'));
+        if (info.version) {
+            sortNameParts.push(info.version.raw);
         }
         if (info.architecture) {
             sortNameParts.push(getArchitectureDisplayName(info.architecture));

src/client/interpreter/configuration/pythonPathUpdaterService.ts

@@ -49,8 +49,8 @@ export class PythonPathUpdaterService implements IPythonPathUpdaterServiceManage
                 .then(value => value.length === 0 ? undefined : value)
                 .catch<string>(() => '');
             const [info, pipVersion] = await Promise.all([infoPromise, pipVersionPromise]);
-            if (info) {
-                telemtryProperties.pythonVersion = info.version_info.join('.');
+            if (info && info.version) {
+                telemtryProperties.pythonVersion = info.version.raw;
             }
             if (pipVersion) {
                 telemtryProperties.pipVersion = pipVersion;

src/client/interpreter/interpreterService.ts

@@ -86,7 +86,7 @@ export class InterpreterService implements Disposable, IInterpreterService {
             return;
         }
         // Always pick the highest version by default.
-        interpretersInWorkspace.sort((a, b) => a.version! > b.version! ? 1 : -1);
+        interpretersInWorkspace.sort((a, b) => (a.version && b.version) ? a.version.compare(b.version) : 0);
         const pythonPath = interpretersInWorkspace[0].path;
         // Ensure this new environment is at the same level as the current workspace.
         // In windows the interpreter is under scripts/python.exe on linux it is under bin/python.
@@ -196,8 +196,8 @@ export class InterpreterService implements Disposable, IInterpreterService {
         const displayNameParts: string[] = ['Python'];
         const envSuffixParts: string[] = [];
 
-        if (info.version_info && info.version_info.length > 0) {
-            displayNameParts.push(info.version_info.slice(0, 3).join('.'));
+        if (info.version) {
+            displayNameParts.push(`${info.version.major}.${info.version.minor}.${info.version.patch}`);
         }
         if (info.architecture) {
             displayNameParts.push(getArchitectureDisplayName(info.architecture));