Enabled unicorn/no-array-reduce in packages/tree (microsoft#26143)

Removes the unicorn/no-array-reduce: off override from the
@fluidframework/tree package and fixes the resulting errors. Converts
all Array.reduce() calls to explicit for loops. This is part of an
ongoing effort to incrementally remove global ESLint rule overrides from
eslint.config.mts and .eslintrc.cjs.

  Changes:
-   Production code (2 files)
-   Test code (6 files)

No inline overrides were needed - all reduce usages could be converted
to for loops.

Author: brrichards

packages/dds/tree/.eslintrc.cjs

@@ -52,7 +52,6 @@ module.exports = {
 
 		"unicorn/consistent-function-scoping": "off",
 		"unicorn/no-array-method-this-argument": "off",
-		"unicorn/no-array-reduce": "off",
 		"unicorn/no-await-expression-member": "off",
 		"unicorn/no-new-array": "off",
 		"unicorn/no-null": "off",

packages/dds/tree/eslint.config.mts

@@ -32,7 +32,6 @@ const config: Linter.Config[] = [
 			"jsdoc/require-description": "warn",
 			"unicorn/consistent-function-scoping": "off",
 			"unicorn/no-array-method-this-argument": "off",
-			"unicorn/no-array-reduce": "off",
 			"unicorn/no-await-expression-member": "off",
 			"unicorn/no-new-array": "off",
 			"unicorn/no-null": "off",

packages/dds/tree/src/core/rebase/utils.ts

@@ -403,10 +403,14 @@ export function rebaseChangeOverChanges<TChange>(
 		getRevInfoFromTaggedChanges([...changesToRebaseOver, changeToRebase]),
 	);
 
-	return changesToRebaseOver.reduce(
-		(a, b) => mapTaggedChange(changeToRebase, changeRebaser.rebase(a, b, revisionMetadata)),
-		changeToRebase,
-	).change;
+	let result = changeToRebase;
+	for (const b of changesToRebaseOver) {
+		result = mapTaggedChange(
+			changeToRebase,
+			changeRebaser.rebase(result, b, revisionMetadata),
+		);
+	}
+	return result.change;
 }
 
 // TODO: Deduplicate

packages/dds/tree/src/shared-tree-core/editManager.ts

@@ -1090,27 +1090,27 @@ class SharedBranch<TEditor extends ChangeFamilyEditor, TChangeset> {
 		const parentTrunkBase =
 			trunkRevisionCache.get(data.base ?? rootRevision) ??
 			fail(0xc61 /* Expected base revision to be in trunk cache */);
-		this.trunk.setHead(
-			data.trunk.reduce((base, c) => {
-				const sequenceId: SequenceId =
-					c.indexInBatch === undefined
-						? {
-								sequenceNumber: c.sequenceNumber,
-							}
-						: {
-								sequenceNumber: c.sequenceNumber,
-								indexInBatch: c.indexInBatch,
-							};
-				const commit = mintCommit(base, c);
-				this.sequenceIdToCommit.set(sequenceId, commit);
-				this.commitMetadata.set(c.revision, {
-					sequenceId,
-					sessionId: c.sessionId,
-				});
-				trunkRevisionCache.set(c.revision, commit);
-				return commit;
-			}, parentTrunkBase),
-		);
+		let trunkHead = parentTrunkBase;
+		for (const c of data.trunk) {
+			const sequenceId: SequenceId =
+				c.indexInBatch === undefined
+					? {
+							sequenceNumber: c.sequenceNumber,
+						}
+					: {
+							sequenceNumber: c.sequenceNumber,
+							indexInBatch: c.indexInBatch,
+						};
+			const commit = mintCommit(trunkHead, c);
+			this.sequenceIdToCommit.set(sequenceId, commit);
+			this.commitMetadata.set(c.revision, {
+				sequenceId,
+				sessionId: c.sessionId,
+			});
+			trunkRevisionCache.set(c.revision, commit);
+			trunkHead = commit;
+		}
+		this.trunk.setHead(trunkHead);
 
 		this.localBranch.setHead(this.trunk.getHead());
 
@@ -1119,13 +1119,13 @@ class SharedBranch<TEditor extends ChangeFamilyEditor, TChangeset> {
 				trunkRevisionCache.get(branch.base) ??
 				fail(0xad7 /* Expected summary branch to be based off of a revision in the trunk */);
 
+			let branchHead = commit;
+			for (const c of branch.commits) {
+				branchHead = mintCommit(branchHead, c);
+			}
 			this.peerLocalBranches.set(
 				sessionId,
-				new SharedTreeBranch(
-					branch.commits.reduce(mintCommit, commit),
-					this.changeFamily,
-					this.mintRevisionTag,
-				),
+				new SharedTreeBranch(branchHead, this.changeFamily, this.mintRevisionTag),
 			);
 		}
 	}

packages/dds/tree/src/test/core/tree/anchorSet.spec.ts

@@ -616,14 +616,15 @@ type PathStep = [FieldKey, number];
 
 function makePath(...steps: [PathStep, ...PathStep[]]): UpPath {
 	assert(steps.length > 0, "Path cannot be empty");
-	return steps.reduce(
-		(path: UpPath | undefined, step: PathStep) => ({
+	let path: UpPath | undefined;
+	for (const step of steps) {
+		path = {
 			parent: path,
 			parentField: step[0],
 			parentIndex: step[1],
-		}),
-		undefined,
-	) as UpPath;
+		};
+	}
+	return path as UpPath;
 }
 
 function makeFieldPath(field: FieldKey, ...stepsToFieldParent: PathStep[]): FieldUpPath {

packages/dds/tree/src/test/feature-libraries/modular-schema/modularChangesetUtil.ts

@@ -160,22 +160,20 @@ function fieldChangeMapFromDescription(
 			field: field.fieldKey,
 		};
 
-		const fieldChangeset = field.children.reduce(
-			(change: unknown, nodeDescription: NodeChangesetDescription) =>
-				addNodeToField(
-					family,
-					change,
-					nodeDescription,
-					fieldId,
-					changeHandler,
-					nodes,
-					nodeToParent,
-					crossFieldKeys,
-					idAllocator,
-				),
-
-			field.changeset,
-		);
+		let fieldChangeset: unknown = field.changeset;
+		for (const nodeDescription of field.children) {
+			fieldChangeset = addNodeToField(
+				family,
+				fieldChangeset,
+				nodeDescription,
+				fieldId,
+				changeHandler,
+				nodes,
+				nodeToParent,
+				crossFieldKeys,
+				idAllocator,
+			);
+		}
 
 		for (const { key, count } of changeHandler.getCrossFieldKeys(fieldChangeset)) {
 			crossFieldKeys.set(key, count, fieldId);

packages/dds/tree/src/test/feature-libraries/optional-field/optionalChangeRebaser.test.ts

@@ -227,12 +227,15 @@ function rebaseComposedWrapped(
 	change: TaggedChange<WrappedChangeset>,
 	...baseChanges: TaggedChange<WrappedChangeset>[]
 ): WrappedChangeset {
-	const composed =
-		baseChanges.length === 0
-			? makeAnonChange(ChangesetWrapper.create(Change.empty()))
-			: baseChanges.reduce((change1, change2) =>
-					makeAnonChange(composeWrapped(change1, change2)),
-				);
+	let composed: TaggedChange<WrappedChangeset>;
+	if (baseChanges.length === 0) {
+		composed = makeAnonChange(ChangesetWrapper.create(Change.empty()));
+	} else {
+		composed = baseChanges[0];
+		for (let i = 1; i < baseChanges.length; i++) {
+			composed = makeAnonChange(composeWrapped(composed, baseChanges[i]));
+		}
+	}
 
 	return rebaseWrapped(change, composed, metadata);
 }

packages/dds/tree/src/test/feature-libraries/sequence-field/utils.ts

@@ -242,15 +242,18 @@ export function composeDeep(
 ): WrappedChange {
 	const metadata = revisionMetadata ?? defaultRevisionMetadataFromChanges(changes);
 
-	return changes.length === 0
-		? ChangesetWrapper.create([])
-		: changes.reduce((change1, change2) =>
-				makeAnonChange(
-					ChangesetWrapper.compose(change1, change2, (c1, c2, composeChild) =>
-						composePair(c1.change, c2.change, composeChild, metadata, idAllocatorFromMaxId()),
-					),
-				),
-			).change;
+	if (changes.length === 0) {
+		return ChangesetWrapper.create([]);
+	}
+	let result = changes[0];
+	for (let i = 1; i < changes.length; i++) {
+		result = makeAnonChange(
+			ChangesetWrapper.compose(result, changes[i], (c1, c2, composeChild) =>
+				composePair(c1.change, c2.change, composeChild, metadata, idAllocatorFromMaxId()),
+			),
+		);
+	}
+	return result.change;
 }
 
 export function composeNoVerify(

packages/dds/tree/src/test/shared-tree-core/edit-manager/editManagerScenario.ts

@@ -245,17 +245,17 @@ export function runUnitTestScenario(
 
 				// For each peer, find its next step and extract the ref number.
 				// The min of all these ref numbers for all peers is the highest possible min sequence number across those peers.
-				const minPeerRef = activePeers
-					.map(
-						(peer) =>
-							steps
-								.filter(
-									(s): s is UnitTestPullStepWithIntention =>
-										s.type === "Pull" && s.from === peer,
-								)
-								.find((s) => s.seq > sequenceNumber)?.ref ?? Number.POSITIVE_INFINITY,
-					)
-					.reduce((p, c) => Math.min(p, c), Number.POSITIVE_INFINITY);
+				const peerRefs = activePeers.map(
+					(peer) =>
+						steps
+							.filter(
+								(s): s is UnitTestPullStepWithIntention =>
+									s.type === "Pull" && s.from === peer,
+							)
+							.find((s) => s.seq > sequenceNumber)?.ref ?? Number.POSITIVE_INFINITY,
+				);
+				const minPeerRef =
+					peerRefs.length > 0 ? Math.min(...peerRefs) : Number.POSITIVE_INFINITY;
 
 				// Compute the true min sequence number by including our local session's last seen sequence number as well.
 				return Math.min(sequenceNumber, minPeerRef);

packages/dds/tree/src/test/simple-tree/list.spec.ts

@@ -14,19 +14,21 @@ const schemaFactory = new SchemaFactory("test");
 describe("List", () => {
 	/** Formats 'args' array, inserting commas and eliding trailing `undefine`s.  */
 	function prettyArgs(...args: any[]) {
-		return args.reduce((prev: string, arg, index) => {
+		let result = "";
+		for (let index = 0; index < args.length; index++) {
 			// If all remaining arguments are 'undefined' elide them.
 			if (!args.slice(index).some((value) => value !== undefined)) {
-				return prev;
+				break;
 			}
 
 			// If not the first argument add a comma separator.
-			let next = index > 0 ? `${prev}, ` : prev;
-
-			next += pretty(arg);
+			if (index > 0) {
+				result += ", ";
+			}
 
-			return next;
-		}, "");
+			result += pretty(args[index]);
+		}
+		return result;
 	}
 
 	// eslint-disable-next-line @fluid-internal/fluid/no-markdown-links-in-jsdoc -- false positive AB#51719