From 73e526b5633250d49e4f25906b42f65ff835b2d3 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Wed, 1 Feb 2023 17:54:43 -0800 Subject: [PATCH] Move `replaceAll` and `splitLines` out of string extensions. --- src/client/common/extensions.ts | 59 ------------------- src/client/common/process/logger.ts | 3 +- src/client/common/stringUtils.ts | 46 +++++++++++++++ .../dynamicdebugConfigurationService.ts | 3 +- src/client/linters/baseLinter.ts | 3 +- .../common/environmentManagers/conda.ts | 3 +- .../common/environmentManagers/poetry.ts | 3 +- .../environmentManagers/simplevirtualenvs.ts | 3 +- .../provider/condaCreationProvider.ts | 3 +- .../pythonEnvironments/info/interpreter.ts | 3 +- .../testing/testController/unittest/runner.ts | 25 ++++---- .../testController/workspaceTestAdapter.ts | 12 ++-- src/test/common/extensions.unit.test.ts | 14 ----- src/test/common/stringUtils.unit.test.ts | 23 ++++++++ 14 files changed, 102 insertions(+), 101 deletions(-) create mode 100644 src/client/common/stringUtils.ts create mode 100644 src/test/common/stringUtils.unit.test.ts diff --git a/src/client/common/extensions.ts b/src/client/common/extensions.ts index 962349583776..e68e3838ee1d 100644 --- a/src/client/common/extensions.ts +++ b/src/client/common/extensions.ts @@ -1,21 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -/** - * @typedef {Object} SplitLinesOptions - * @property {boolean} [trim=true] - Whether to trim the lines. - * @property {boolean} [removeEmptyEntries=true] - Whether to remove empty entries. - */ - -// https://stackoverflow.com/questions/39877156/how-to-extend-string-prototype-and-use-it-next-in-typescript - declare interface String { - /** - * Split a string using the cr and lf characters and return them as an array. - * By default lines are trimmed and empty lines are removed. - * @param {SplitLinesOptions=} splitOptions - Options used for splitting the string. - */ - splitLines(splitOptions?: { trim: boolean; removeEmptyEntries?: boolean }): string[]; /** * Appropriately formats a string so it can be used as an argument for a command in a shell. * E.g. if an argument contains a space, then it will be enclosed within double quotes. @@ -37,33 +23,8 @@ declare interface String { * Removes leading and trailing quotes from a string */ trimQuotes(): string; - - /** - * String.replaceAll implementation - * Replaces all instances of a substring with a new string - */ - replaceAll(substr: string, newSubstr: string): string; } -/** - * Split a string using the cr and lf characters and return them as an array. - * By default lines are trimmed and empty lines are removed. - * @param {SplitLinesOptions=} splitOptions - Options used for splitting the string. - */ -String.prototype.splitLines = function ( - this: string, - splitOptions: { trim: boolean; removeEmptyEntries: boolean } = { removeEmptyEntries: true, trim: true }, -): string[] { - let lines = this.split(/\r?\n/g); - if (splitOptions && splitOptions.trim) { - lines = lines.map((line) => line.trim()); - } - if (splitOptions && splitOptions.removeEmptyEntries) { - lines = lines.filter((line) => line.length > 0); - } - return lines; -}; - /** * Appropriately formats a string so it can be used as an argument for a command in a shell. * E.g. if an argument contains a space, then it will be enclosed within double quotes. @@ -102,26 +63,6 @@ String.prototype.trimQuotes = function (this: string): string { return this.replace(/(^['"])|(['"]$)/g, ''); }; -/** - * String.replaceAll implementation - * Replaces all instances of a substring with a new substring. - */ -String.prototype.replaceAll = function (this: string, substr: string, newSubstr: string): string { - if (!this) { - return this; - } - - /** Escaping function from the MDN web docs site - * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping - * Escapes all the following special characters in a string . * + ? ^ $ { } ( ) | \ \\ */ - - function escapeRegExp(unescapedStr: string): string { - return unescapedStr.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string - } - - return this.replace(new RegExp(escapeRegExp(substr), 'g'), newSubstr); -}; - declare interface Promise { /** * Catches task error and ignores them. diff --git a/src/client/common/process/logger.ts b/src/client/common/process/logger.ts index 6a8e25d76e0b..ebb1ad019a48 100644 --- a/src/client/common/process/logger.ts +++ b/src/client/common/process/logger.ts @@ -11,6 +11,7 @@ import { Logging } from '../utils/localize'; import { getOSType, getUserHomeDir, OSType } from '../utils/platform'; import { IProcessLogger, SpawnOptions } from './types'; import { escapeRegExp } from 'lodash'; +import { replaceAll } from '../stringUtils'; @injectable() export class ProcessLogger implements IProcessLogger { @@ -57,7 +58,7 @@ function replaceMatchesWithCharacter(original: string, match: string, character: let pattern = escapeRegExp(match); if (getOSType() === OSType.Windows) { // Match both forward and backward slash versions of 'match' for Windows. - pattern = pattern.replaceAll('\\\\', '(\\\\|/)'); + pattern = replaceAll(pattern, '\\\\', '(\\\\|/)'); } let regex = new RegExp(pattern, 'ig'); return regex; diff --git a/src/client/common/stringUtils.ts b/src/client/common/stringUtils.ts new file mode 100644 index 000000000000..02ca51082ea8 --- /dev/null +++ b/src/client/common/stringUtils.ts @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export interface SplitLinesOptions { + trim?: boolean; + removeEmptyEntries?: boolean; +} + +/** + * Split a string using the cr and lf characters and return them as an array. + * By default lines are trimmed and empty lines are removed. + * @param {SplitLinesOptions=} splitOptions - Options used for splitting the string. + */ +export function splitLines( + source: string, + splitOptions: SplitLinesOptions = { removeEmptyEntries: true, trim: true }, +): string[] { + let lines = source.split(/\r?\n/g); + if (splitOptions?.trim) { + lines = lines.map((line) => line.trim()); + } + if (splitOptions?.removeEmptyEntries) { + lines = lines.filter((line) => line.length > 0); + } + return lines; +} + +/** + * Replaces all instances of a substring with a new substring. + */ +export function replaceAll(source: string, substr: string, newSubstr: string): string { + if (!source) { + return source; + } + + /** Escaping function from the MDN web docs site + * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping + * Escapes all the following special characters in a string . * + ? ^ $ { } ( ) | \ \\ + */ + + function escapeRegExp(unescapedStr: string): string { + return unescapedStr.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string + } + + return source.replace(new RegExp(escapeRegExp(substr), 'g'), newSubstr); +} diff --git a/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts b/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts index d13cf48c2a6d..2d80f0e3d6e8 100644 --- a/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts +++ b/src/client/debugger/extension/configuration/dynamicdebugConfigurationService.ts @@ -10,6 +10,7 @@ import { CancellationToken, DebugConfiguration, WorkspaceFolder } from 'vscode'; import { IDynamicDebugConfigurationService } from '../types'; import { DebuggerTypeName } from '../../constants'; import { asyncFilter } from '../../../common/utils/arrayUtils'; +import { replaceAll } from '../../../common/stringUtils'; const workspaceFolderToken = '${workspaceFolder}'; @@ -62,7 +63,7 @@ export class DynamicPythonDebugConfigurationService implements IDynamicDebugConf let fastApiPath = await DynamicPythonDebugConfigurationService.getFastApiPath(folder); if (fastApiPath) { - fastApiPath = path.relative(folder.uri.fsPath, fastApiPath).replaceAll(path.sep, '.').replace('.py', ''); + fastApiPath = replaceAll(path.relative(folder.uri.fsPath, fastApiPath), path.sep, '.').replace('.py', ''); providers.push({ name: 'Python: FastAPI', type: DebuggerTypeName, diff --git a/src/client/linters/baseLinter.ts b/src/client/linters/baseLinter.ts index 3598d4ef1dfc..bb24bee1637f 100644 --- a/src/client/linters/baseLinter.ts +++ b/src/client/linters/baseLinter.ts @@ -9,6 +9,7 @@ import { IWorkspaceService } from '../common/application/types'; import { isTestExecution } from '../common/constants'; import '../common/extensions'; import { IPythonToolExecutionService } from '../common/process/types'; +import { splitLines } from '../common/stringUtils'; import { ExecutionInfo, Flake8CategorySeverity, @@ -184,7 +185,7 @@ export abstract class BaseLinter implements ILinter { _token: vscode.CancellationToken, regEx: string, ): Promise { - const outputLines = output.splitLines({ removeEmptyEntries: false, trim: false }); + const outputLines = splitLines(output, { removeEmptyEntries: false, trim: false }); return this.parseLines(outputLines, regEx); } diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index 0e19c8e94801..969387f53a79 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -21,6 +21,7 @@ import { cache } from '../../../common/utils/decorators'; import { isTestExecution } from '../../../common/constants'; import { traceError, traceVerbose } from '../../../logging'; import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts'; +import { splitLines } from '../../../common/stringUtils'; export const AnacondaCompanyName = 'Anaconda, Inc.'; export const CONDAPATH_SETTING_KEY = 'condaPath'; @@ -185,7 +186,7 @@ export async function getPythonVersionFromConda(interpreterPath: string): Promis for (const configPath of configPaths) { if (await pathExists(configPath)) { try { - const lines = (await readFile(configPath)).splitLines(); + const lines = splitLines(await readFile(configPath)); // Sample data: // +defaults/linux-64::pip-20.2.4-py38_0 diff --git a/src/client/pythonEnvironments/common/environmentManagers/poetry.ts b/src/client/pythonEnvironments/common/environmentManagers/poetry.ts index 19e3bd80af59..48199b5bdc8f 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/poetry.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/poetry.ts @@ -19,6 +19,7 @@ import { StopWatch } from '../../../common/utils/stopWatch'; import { cache } from '../../../common/utils/decorators'; import { isTestExecution } from '../../../common/constants'; import { traceError, traceVerbose } from '../../../logging'; +import { splitLines } from '../../../common/stringUtils'; /** * Global virtual env dir for a project is named as: @@ -213,7 +214,7 @@ export class Poetry { */ const activated = '(Activated)'; const res = await Promise.all( - result.stdout.splitLines().map(async (line) => { + splitLines(result.stdout).map(async (line) => { if (line.endsWith(activated)) { line = line.slice(0, -activated.length); } diff --git a/src/client/pythonEnvironments/common/environmentManagers/simplevirtualenvs.ts b/src/client/pythonEnvironments/common/environmentManagers/simplevirtualenvs.ts index 80a60a0580ca..78a018138e2b 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/simplevirtualenvs.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/simplevirtualenvs.ts @@ -4,6 +4,7 @@ import * as fsapi from 'fs-extra'; import * as path from 'path'; import '../../../common/extensions'; +import { splitLines } from '../../../common/stringUtils'; import { getEnvironmentVariable, getOSType, getUserHomeDir, OSType } from '../../../common/utils/platform'; import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info'; import { comparePythonVersionSpecificity } from '../../base/info/env'; @@ -136,7 +137,7 @@ export async function getPythonVersionFromPyvenvCfg(interpreterPath: string): Pr for (const configPath of configPaths) { if (await pathExists(configPath)) { try { - const lines = (await readFile(configPath)).splitLines(); + const lines = splitLines(await readFile(configPath)); const pythonVersions = lines .map((line) => { diff --git a/src/client/pythonEnvironments/creation/provider/condaCreationProvider.ts b/src/client/pythonEnvironments/creation/provider/condaCreationProvider.ts index ac77e51f22f4..91954c620c01 100644 --- a/src/client/pythonEnvironments/creation/provider/condaCreationProvider.ts +++ b/src/client/pythonEnvironments/creation/provider/condaCreationProvider.ts @@ -27,6 +27,7 @@ import { CONDA_ENV_CREATED_MARKER, CONDA_ENV_EXISTING_MARKER, } from './condaProgressAndTelemetry'; +import { splitLines } from '../../../common/stringUtils'; function generateCommandArgs(version?: string, options?: CreateEnvironmentOptions): string[] { let addGitIgnore = true; @@ -112,7 +113,7 @@ async function createCondaEnv( let condaEnvPath: string | undefined; out.subscribe( (value) => { - const output = value.out.splitLines().join('\r\n'); + const output = splitLines(value.out).join('\r\n'); traceLog(output); if (output.includes(CONDA_ENV_CREATED_MARKER) || output.includes(CONDA_ENV_EXISTING_MARKER)) { condaEnvPath = getCondaEnvFromOutput(output); diff --git a/src/client/pythonEnvironments/info/interpreter.ts b/src/client/pythonEnvironments/info/interpreter.ts index 72ef670f2672..8925c7a6feb6 100644 --- a/src/client/pythonEnvironments/info/interpreter.ts +++ b/src/client/pythonEnvironments/info/interpreter.ts @@ -8,6 +8,7 @@ import { InterpreterInfoJson, } from '../../common/process/internal/scripts'; import { ShellExecFunc } from '../../common/process/types'; +import { replaceAll } from '../../common/stringUtils'; import { Architecture } from '../../common/utils/platform'; import { copyPythonExecInfo, PythonExecInfo } from '../exec'; @@ -68,7 +69,7 @@ export async function getInterpreterInfo( const argv = [info.command, ...info.args]; // Concat these together to make a set of quoted strings - const quoted = argv.reduce((p, c) => (p ? `${p} "${c}"` : `"${c.replaceAll('\\', '\\\\')}"`), ''); + const quoted = argv.reduce((p, c) => (p ? `${p} "${c}"` : `"${replaceAll(c, '\\', '\\\\')}"`), ''); // Try shell execing the command, followed by the arguments. This will make node kill the process if it // takes too long. diff --git a/src/client/testing/testController/unittest/runner.ts b/src/client/testing/testController/unittest/runner.ts index ccc14ae0b4c2..d6bbb59ee640 100644 --- a/src/client/testing/testController/unittest/runner.ts +++ b/src/client/testing/testController/unittest/runner.ts @@ -4,6 +4,7 @@ import { injectable, inject, named } from 'inversify'; import { Location, TestController, TestItem, TestMessage, TestRun, TestRunProfileKind } from 'vscode'; import * as internalScripts from '../../../common/process/internal/scripts'; +import { splitLines } from '../../../common/stringUtils'; import { IOutputChannel } from '../../../common/types'; import { noop } from '../../../common/utils/misc'; import { traceError, traceInfo } from '../../../logging'; @@ -23,6 +24,10 @@ interface ITestData { subtest?: string; } +function getTracebackForOutput(traceback: string): string { + return splitLines(traceback, { trim: false, removeEmptyEntries: true }).join('\r\n'); +} + @injectable() export class UnittestRunner implements ITestsRunner { constructor( @@ -111,9 +116,7 @@ export class UnittestRunner implements ITestsRunner { runInstance.appendOutput(fixLogLines(text)); counts.passed += 1; } else if (data.outcome === 'failed' || data.outcome === 'passed-unexpected') { - const traceback = data.traceback - ? data.traceback.splitLines({ trim: false, removeEmptyEntries: true }).join('\r\n') - : ''; + const traceback = data.traceback ? getTracebackForOutput(data.traceback) : ''; const text = `${rawTestCase.rawId} Failed: ${data.message ?? data.outcome}\r\n${traceback}\r\n`; const message = new TestMessage(text); @@ -128,9 +131,7 @@ export class UnittestRunner implements ITestsRunner { stopTesting = true; } } else if (data.outcome === 'error') { - const traceback = data.traceback - ? data.traceback.splitLines({ trim: false, removeEmptyEntries: true }).join('\r\n') - : ''; + const traceback = data.traceback ? getTracebackForOutput(data.traceback) : ''; const text = `${rawTestCase.rawId} Failed with Error: ${data.message}\r\n${traceback}\r\n`; const message = new TestMessage(text); @@ -145,9 +146,7 @@ export class UnittestRunner implements ITestsRunner { stopTesting = true; } } else if (data.outcome === 'skipped') { - const traceback = data.traceback - ? data.traceback.splitLines({ trim: false, removeEmptyEntries: true }).join('\r\n') - : ''; + const traceback = data.traceback ? getTracebackForOutput(data.traceback) : ''; const text = `${rawTestCase.rawId} Skipped: ${data.message}\r\n${traceback}\r\n`; runInstance.skipped(testCase); runInstance.appendOutput(fixLogLines(text)); @@ -196,9 +195,7 @@ export class UnittestRunner implements ITestsRunner { if (data.subtest) { runInstance.appendOutput(fixLogLines(`${data.subtest} Failed\r\n`)); - const traceback = data.traceback - ? data.traceback.splitLines({ trim: false, removeEmptyEntries: true }).join('\r\n') - : ''; + const traceback = data.traceback ? getTracebackForOutput(data.traceback) : ''; const text = `${data.subtest} Failed: ${data.message ?? data.outcome}\r\n${traceback}\r\n`; runInstance.appendOutput(fixLogLines(text)); @@ -226,9 +223,7 @@ export class UnittestRunner implements ITestsRunner { runInstance.errored(testCase, message); } } else if (data.outcome === 'error') { - const traceback = data.traceback - ? data.traceback.splitLines({ trim: false, removeEmptyEntries: true }).join('\r\n') - : ''; + const traceback = data.traceback ? getTracebackForOutput(data.traceback) : ''; const text = `${data.test} Failed with Error: ${data.message}\r\n${traceback}\r\n`; runInstance.appendOutput(fixLogLines(text)); } diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index f42152438cfb..32bc0d5c29ef 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -14,6 +14,7 @@ import { Uri, Location, } from 'vscode'; +import { splitLines } from '../../common/stringUtils'; import { createDeferred, Deferred } from '../../common/utils/async'; import { Testing } from '../../common/utils/localize'; import { traceError } from '../../logging'; @@ -138,11 +139,12 @@ export class WorkspaceTestAdapter { rawTestExecData.result[keyTemp].outcome === 'subtest-failure' || rawTestExecData.result[keyTemp].outcome === 'passed-unexpected' ) { - const traceback = rawTestExecData.result[keyTemp].traceback - ? rawTestExecData.result[keyTemp] - .traceback!.splitLines({ trim: false, removeEmptyEntries: true }) - .join('\r\n') - : ''; + const rawTraceback = rawTestExecData.result[keyTemp].traceback ?? ''; + const traceback = splitLines(rawTraceback, { + trim: false, + removeEmptyEntries: true, + }).join('\r\n'); + const text = `${rawTestExecData.result[keyTemp].test} failed: ${ rawTestExecData.result[keyTemp].message ?? rawTestExecData.result[keyTemp].outcome }\r\n${traceback}\r\n`; diff --git a/src/test/common/extensions.unit.test.ts b/src/test/common/extensions.unit.test.ts index ffa75515fd9c..75d48024b2e8 100644 --- a/src/test/common/extensions.unit.test.ts +++ b/src/test/common/extensions.unit.test.ts @@ -102,20 +102,6 @@ suite('String Extensions', () => { expect(quotedString3.trimQuotes()).to.be.equal(expectedString); expect(quotedString4.trimQuotes()).to.be.equal(expectedString); }); - test('String should replace all substrings with new substring', () => { - const oldString = `foo \\ foo \\ foo`; - const expectedString = `foo \\\\ foo \\\\ foo`; - const oldString2 = `\\ foo \\ foo`; - const expectedString2 = `\\\\ foo \\\\ foo`; - const oldString3 = `\\ foo \\`; - const expectedString3 = `\\\\ foo \\\\`; - const oldString4 = `foo foo`; - const expectedString4 = `foo foo`; - expect(oldString.replaceAll('\\', '\\\\')).to.be.equal(expectedString); - expect(oldString2.replaceAll('\\', '\\\\')).to.be.equal(expectedString2); - expect(oldString3.replaceAll('\\', '\\\\')).to.be.equal(expectedString3); - expect(oldString4.replaceAll('\\', '\\\\')).to.be.equal(expectedString4); - }); }); suite('Array extensions', () => { diff --git a/src/test/common/stringUtils.unit.test.ts b/src/test/common/stringUtils.unit.test.ts new file mode 100644 index 000000000000..f8b5f2947631 --- /dev/null +++ b/src/test/common/stringUtils.unit.test.ts @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import '../../client/common/extensions'; +import { replaceAll } from '../../client/common/stringUtils'; + +suite('String Extensions', () => { + test('String should replace all substrings with new substring', () => { + const oldString = `foo \\ foo \\ foo`; + const expectedString = `foo \\\\ foo \\\\ foo`; + const oldString2 = `\\ foo \\ foo`; + const expectedString2 = `\\\\ foo \\\\ foo`; + const oldString3 = `\\ foo \\`; + const expectedString3 = `\\\\ foo \\\\`; + const oldString4 = `foo foo`; + const expectedString4 = `foo foo`; + expect(replaceAll(oldString, '\\', '\\\\')).to.be.equal(expectedString); + expect(replaceAll(oldString2, '\\', '\\\\')).to.be.equal(expectedString2); + expect(replaceAll(oldString3, '\\', '\\\\')).to.be.equal(expectedString3); + expect(replaceAll(oldString4, '\\', '\\\\')).to.be.equal(expectedString4); + }); +});