From 3d0558efd86fb44898db2b77d708e316b3c74d03 Mon Sep 17 00:00:00 2001
From: Matt Bierner <matb@microsoft.com>
Date: Tue, 16 Oct 2018 16:52:54 -0700
Subject: [PATCH] Split coalesce into inplace and copy version

Using an overload makes the code more complex and prevents  patterns such as `.then(coalesce)` or `.map(coalesce)`. It also makes it clear which code paths are actually being most used in our code
---
 src/vs/base/common/arrays.ts                  | 35 ++++++++++---------
 src/vs/base/test/common/arrays.test.ts        |  8 ++---
 .../contrib/documentSymbols/outlineModel.ts   |  4 +--
 .../contrib/goToDefinition/goToDefinition.ts  |  2 +-
 src/vs/editor/contrib/hover/getHover.ts       |  2 +-
 5 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/src/vs/base/common/arrays.ts b/src/vs/base/common/arrays.ts
index ba4aaa552e4..7aaab83a0a8 100644
--- a/src/vs/base/common/arrays.ts
+++ b/src/vs/base/common/arrays.ts
@@ -297,29 +297,30 @@ function topStep<T>(array: T[], compare: (a: T, b: T) => number, result: T[], i:
 }
 
 /**
- * @returns a new array with all undefined or null values removed. The original array is not modified at all.
+ * @returns a new array with all falsy values removed. The original array IS NOT modified.
  */
-export function coalesce<T>(array: (T | undefined | null)[]): T[];
-export function coalesce<T>(array: (T | undefined | null)[], inplace: true): void;
-export function coalesce<T>(array: (T | undefined | null)[], inplace?: true): void | T[] {
+export function coalesce<T>(array: (T | undefined | null)[]): T[] {
 	if (!array) {
-		if (!inplace) {
-			return array;
-		}
+		return array;
 	}
-	if (!inplace) {
-		return <T[]>array.filter(e => !!e);
+	return <T[]>array.filter(e => !!e);
+}
 
-	} else {
-		let to = 0;
-		for (let i = 0; i < array.length; i++) {
-			if (!!array[i]) {
-				array[to] = array[i];
-				to += 1;
-			}
+/**
+ * Remove all falsey values from `array`. The original array IS modified.
+ */
+export function coalesceInPlace<T>(array: (T | undefined | null)[]): void {
+	if (!array) {
+		return;
+	}
+	let to = 0;
+	for (let i = 0; i < array.length; i++) {
+		if (!!array[i]) {
+			array[to] = array[i];
+			to += 1;
 		}
-		array.length = to;
 	}
+	array.length = to;
 }
 
 /**
diff --git a/src/vs/base/test/common/arrays.test.ts b/src/vs/base/test/common/arrays.test.ts
index de025835539..5297a4de659 100644
--- a/src/vs/base/test/common/arrays.test.ts
+++ b/src/vs/base/test/common/arrays.test.ts
@@ -299,14 +299,14 @@ suite('Arrays', () => {
 
 	test('coalesce - inplace', function () {
 		let a = [null, 1, null, 2, 3];
-		arrays.coalesce(a, true);
+		arrays.coalesceInPlace(a);
 		assert.equal(a.length, 3);
 		assert.equal(a[0], 1);
 		assert.equal(a[1], 2);
 		assert.equal(a[2], 3);
 
 		a = [null, 1, null, void 0, undefined, 2, 3];
-		arrays.coalesce(a, true);
+		arrays.coalesceInPlace(a);
 		assert.equal(a.length, 3);
 		assert.equal(a[0], 1);
 		assert.equal(a[1], 2);
@@ -316,7 +316,7 @@ suite('Arrays', () => {
 		b[10] = 1;
 		b[20] = 2;
 		b[30] = 3;
-		arrays.coalesce(b, true);
+		arrays.coalesceInPlace(b);
 		assert.equal(b.length, 3);
 		assert.equal(b[0], 1);
 		assert.equal(b[1], 2);
@@ -331,7 +331,7 @@ suite('Arrays', () => {
 
 		assert.equal(sparse.length, 1002);
 
-		arrays.coalesce(sparse, true);
+		arrays.coalesceInPlace(sparse);
 		assert.equal(sparse.length, 5);
 	});
 });
diff --git a/src/vs/editor/contrib/documentSymbols/outlineModel.ts b/src/vs/editor/contrib/documentSymbols/outlineModel.ts
index 23bb9a4cad5..0d852cd9ed9 100644
--- a/src/vs/editor/contrib/documentSymbols/outlineModel.ts
+++ b/src/vs/editor/contrib/documentSymbols/outlineModel.ts
@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { binarySearch, coalesce, isFalsyOrEmpty } from 'vs/base/common/arrays';
+import { binarySearch, isFalsyOrEmpty, coalesceInPlace } from 'vs/base/common/arrays';
 import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation';
 import { first, forEach, size } from 'vs/base/common/collections';
 import { onUnexpectedExternalError } from 'vs/base/common/errors';
@@ -214,7 +214,7 @@ export class OutlineGroup extends TreeElement {
 			};
 		}
 
-		coalesce(markers, true);
+		coalesceInPlace(markers);
 	}
 }
 
diff --git a/src/vs/editor/contrib/goToDefinition/goToDefinition.ts b/src/vs/editor/contrib/goToDefinition/goToDefinition.ts
index 6df077d607c..88507a558a8 100644
--- a/src/vs/editor/contrib/goToDefinition/goToDefinition.ts
+++ b/src/vs/editor/contrib/goToDefinition/goToDefinition.ts
@@ -30,7 +30,7 @@ function getDefinitions<T>(
 	});
 	return Promise.all(promises)
 		.then(flatten)
-		.then(references => coalesce(references));
+		.then(coalesce);
 }
 
 
diff --git a/src/vs/editor/contrib/hover/getHover.ts b/src/vs/editor/contrib/hover/getHover.ts
index 577c87d814a..ad0a7e082c5 100644
--- a/src/vs/editor/contrib/hover/getHover.ts
+++ b/src/vs/editor/contrib/hover/getHover.ts
@@ -24,7 +24,7 @@ export function getHover(model: ITextModel, position: Position, token: Cancellat
 		});
 	});
 
-	return Promise.all(promises).then(values => coalesce(values));
+	return Promise.all(promises).then(coalesce);
 }
 
 registerDefaultLanguageCommand('_executeHoverProvider', (model, position) => getHover(model, position, CancellationToken.None));
-- 
GitLab