Move replaceAll and splitLines out of string extensions. (#20626)

For #18871
microsoft · Feb 2, 2023 · 9271136 · 9271136
1 parent e743037
commit 9271136
Show file tree

Hide file tree

Showing 14 changed files with 102 additions and 101 deletions.
diff --git 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<T> {
     /**
      * Catches task error and ignores them.

diff --git 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
@@ -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
@@ -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
@@ -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<ILintMessage[]> {
-        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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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', () => {