Strengthen branch event types and remove usages of `oob()` by adding array helper utilities (#23157)

## Description

This adds a collection of utilities that help to strongly-type arrays
which are known to have one or more elements in them. This allows some
of the SharedTreeBranchEvent arrays of commits to be more strongly typed
and benefit from these helpers. This PR also removes many usages of
`oob()` in favor of the new utilities. While `oob()` is still
appropriate in many places, the new utilities can help retrieve either
the first or the last element of an array in a type-safe way that is
(unlike `oob()`) enforced by the compiler.

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

@@ -5,7 +5,7 @@
 
 import { assert, oob } from "@fluidframework/core-utils/internal";
 
-import type { Mutable } from "../../util/index.js";
+import { hasSome, type Mutable } from "../../util/index.js";
 
 import {
 	type ChangeRebaser,
@@ -248,13 +248,15 @@ export function rebaseBranch<TChange>(
 	// If the source and target rebase path begin with a range that has all the same revisions, remove it; it is
 	// equivalent on both branches and doesn't need to be rebased.
 	const targetRebasePath = [...targetCommits];
-	const minLength = Math.min(sourcePath.length, targetRebasePath.length);
-	for (let i = 0; i < minLength; i++) {
-		const firstSourcePath = sourcePath[0] ?? oob();
-		const firstTargetRebasePath = targetRebasePath[0] ?? oob();
-		if (firstSourcePath.revision === firstTargetRebasePath.revision) {
-			sourcePath.shift();
-			targetRebasePath.shift();
+	if (hasSome(sourcePath) && hasSome(targetRebasePath)) {
+		const minLength = Math.min(sourcePath.length, targetRebasePath.length);
+		for (let i = 0; i < minLength; i++) {
+			const firstSourcePath = sourcePath[0];
+			const firstTargetRebasePath = targetRebasePath[0];
+			if (firstSourcePath.revision === firstTargetRebasePath.revision) {
+				sourcePath.shift();
+				targetRebasePath.shift();
+			}
 		}
 	}
 
@@ -264,7 +266,7 @@ export function rebaseBranch<TChange>(
 	// are in the same order, and have no other commits interleaving them, then no rebasing needs to occur. Those commits can
 	// simply be removed from the source branch, and the remaining commits on the source branch are reparented off of the new
 	// base commit.
-	if (targetRebasePath.length === 0) {
+	if (!hasSome(targetRebasePath)) {
 		for (const c of sourcePath) {
 			sourceCommits.push(mintCommit(sourceCommits[sourceCommits.length - 1] ?? newBase, c));
 		}

packages/dds/tree/src/core/tree/detachedFieldIndexCodec.ts

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { assert, oob } from "@fluidframework/core-utils/internal";
+import { assert } from "@fluidframework/core-utils/internal";
 
 import {
 	type ICodecOptions,
@@ -24,6 +24,7 @@ import type {
 	Major,
 } from "./detachedFieldIndexTypes.js";
 import type { IIdCompressor } from "@fluidframework/id-compressor";
+import { hasSingle } from "../../util/index.js";
 
 class MajorCodec implements IJsonCodec<Major> {
 	public constructor(
@@ -83,8 +84,8 @@ export function makeDetachedNodeToFieldCodec(
 				for (const [minor, detachedField] of innerMap) {
 					rootRanges.push([minor, detachedField.root]);
 				}
-				if (rootRanges.length === 1) {
-					const firstRootRange = rootRanges[0] ?? oob();
+				if (hasSingle(rootRanges)) {
+					const firstRootRange = rootRanges[0];
 					const rootsForRevision: EncodedRootsForRevision = [
 						encodedRevision,
 						firstRootRange[0],

packages/dds/tree/src/feature-libraries/chunked-forest/chunkedForest.ts

@@ -29,7 +29,14 @@ import {
 	rootFieldKey,
 } from "../../core/index.js";
 import { createEmitter, type Listenable } from "../../events/index.js";
-import { assertValidRange, brand, fail, getOrAddEmptyToMap } from "../../util/index.js";
+import {
+	assertValidRange,
+	brand,
+	fail,
+	getLast,
+	getOrAddEmptyToMap,
+	hasSome,
+} from "../../util/index.js";
 
 import { BasicChunk, BasicChunkCursor, type SiblingsOrKey } from "./basicChunk.js";
 import type { ChunkedCursor, TreeChunk } from "./chunk.js";
@@ -100,8 +107,8 @@ export class ChunkedForest implements IEditableForest {
 			mutableChunkStack: [] as StackNode[],
 			mutableChunk: this.roots as BasicChunk | undefined,
 			getParent(): StackNode {
-				assert(this.mutableChunkStack.length > 0, 0x532 /* invalid access to root's parent */);
-				return this.mutableChunkStack[this.mutableChunkStack.length - 1] ?? oob();
+				assert(hasSome(this.mutableChunkStack), 0x532 /* invalid access to root's parent */);
+				return getLast(this.mutableChunkStack);
 			},
 			free(): void {
 				this.mutableChunk = undefined;

packages/dds/tree/src/feature-libraries/chunked-forest/uniformChunk.ts

@@ -15,7 +15,7 @@ import {
 	type UpPath,
 	type Value,
 } from "../../core/index.js";
-import { ReferenceCountedBase, fail } from "../../util/index.js";
+import { ReferenceCountedBase, fail, hasSome } from "../../util/index.js";
 import { SynchronousCursor, prefixFieldPath, prefixPath } from "../treeCursorUtils.js";
 
 import { type ChunkedCursor, type TreeChunk, cursorChunk, dummyRoot } from "./chunk.js";
@@ -517,12 +517,12 @@ class Cursor extends SynchronousCursor implements ChunkedCursor {
 
 	public firstField(): boolean {
 		const fieldsArray = this.nodeInfo(CursorLocationType.Nodes).shape.fieldsArray;
-		if (fieldsArray.length === 0) {
+		if (!hasSome(fieldsArray)) {
 			return false;
 		}
 		this.indexOfField = 0;
 		this.mode = CursorLocationType.Fields;
-		const fields = fieldsArray[0] ?? oob();
+		const fields = fieldsArray[0];
 		this.fieldKey = fields[0];
 		return true;
 	}

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

@@ -3,10 +3,10 @@
  * Licensed under the MIT License.
  */
 
-import { assert, unreachableCase, oob } from "@fluidframework/core-utils/internal";
+import { assert, unreachableCase } from "@fluidframework/core-utils/internal";
 
 import type { RevisionTag } from "../../core/index.js";
-import { type IdAllocator, type Mutable, fail } from "../../util/index.js";
+import { type IdAllocator, type Mutable, fail, hasSingle } from "../../util/index.js";
 import {
 	type CrossFieldManager,
 	CrossFieldTarget,
@@ -257,11 +257,11 @@ function invertMark(
 			}
 
 			assert(
-				detachInverses.length === 1,
+				hasSingle(detachInverses),
 				0x80d /* Only expected MoveIn marks to be split when inverting */,
 			);
 
-			let detachInverse = detachInverses[0] ?? oob();
+			let detachInverse = detachInverses[0];
 			assert(isAttach(detachInverse), 0x80e /* Inverse of a detach should be an attach */);
 
 			const inverses: Mark[] = [];

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

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { assert, unreachableCase, oob } from "@fluidframework/core-utils/internal";
+import { assert, unreachableCase } from "@fluidframework/core-utils/internal";
 
 import {
 	type DeltaDetachedNodeChanges,
@@ -12,7 +12,7 @@ import {
 	type DeltaMark,
 	areEqualChangeAtomIds,
 } from "../../core/index.js";
-import type { Mutable } from "../../util/index.js";
+import { getLast, hasSome, type Mutable } from "../../util/index.js";
 import { nodeIdFromChangeAtom } from "../deltaUtils.js";
 
 import { isMoveIn, isMoveOut } from "./moveEffectTable.js";
@@ -176,8 +176,8 @@ export function sequenceFieldToDelta(
 		}
 	}
 	// Remove trailing no-op marks
-	while (local.length > 0) {
-		const lastMark = local[local.length - 1] ?? oob();
+	while (hasSome(local)) {
+		const lastMark = getLast(local);
 		if (
 			lastMark.attach !== undefined ||
 			lastMark.detach !== undefined ||

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

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { assert, oob } from "@fluidframework/core-utils/internal";
+import { assert } from "@fluidframework/core-utils/internal";
 import { type TelemetryEventBatcher, measure } from "@fluidframework/telemetry-utils/internal";
 
 import {
@@ -24,7 +24,7 @@ import {
 import { createEmitter, type Listenable } from "../events/index.js";
 
 import { TransactionStack } from "./transactionStack.js";
-import { fail } from "../util/index.js";
+import { fail, getLast, hasSome } from "../util/index.js";
 
 /**
  * Describes a change to a `SharedTreeBranch`. Various operations can mutate the head of the branch;
@@ -46,12 +46,12 @@ export type SharedTreeBranchChange<TChange> =
 			type: "append";
 			kind: CommitKind;
 			change: TaggedChange<TChange>;
-			newCommits: readonly GraphCommit<TChange>[];
+			newCommits: readonly [GraphCommit<TChange>, ...GraphCommit<TChange>[]];
 	  }
 	| {
 			type: "remove";
 			change: TaggedChange<TChange> | undefined;
-			removedCommits: readonly GraphCommit<TChange>[];
+			removedCommits: readonly [GraphCommit<TChange>, ...GraphCommit<TChange>[]];
 	  }
 	| {
 			type: "replace";
@@ -314,7 +314,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> {
 		this.editor.exitTransaction();
 
 		this.#events.emit("transactionCommitted", this.transactions.size === 0);
-		if (commits.length === 0) {
+		if (!hasSome(commits)) {
 			return undefined;
 		}
 
@@ -355,7 +355,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> {
 		this.editor.exitTransaction();
 
 		this.#events.emit("transactionAborted", this.transactions.size === 0);
-		if (commits.length === 0) {
+		if (!hasSome(commits)) {
 			this.#events.emit("transactionRolledBack", this.transactions.size === 0);
 			return [undefined, []];
 		}
@@ -399,8 +399,13 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> {
 	private popTransaction(): [GraphCommit<TChange>, GraphCommit<TChange>[]] {
 		const { startRevision: startRevisionOriginal } = this.transactions.pop();
 		let startRevision = startRevisionOriginal;
-		while (this.initialTransactionRevToRebasedRev.has(startRevision)) {
-			startRevision = this.initialTransactionRevToRebasedRev.get(startRevision) ?? oob();
+
+		for (
+			let r: RevisionTag | undefined = startRevision;
+			r !== undefined;
+			r = this.initialTransactionRevToRebasedRev.get(startRevision)
+		) {
+			startRevision = r;
 		}
 
 		if (!this.isTransacting()) {
@@ -460,13 +465,13 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> {
 		// The net change to this branch is provided by the `rebaseBranch` API
 		const { newSourceHead, commits } = rebaseResult;
 		const { deletedSourceCommits, targetCommits, sourceCommits } = commits;
+		assert(hasSome(targetCommits), "Expected commit(s) for a non no-op rebase");
 
 		const newCommits = targetCommits.concat(sourceCommits);
+
 		if (this.isTransacting()) {
-			const firstCommit = targetCommits[0] ?? oob();
-			const lastCommit = targetCommits[targetCommits.length - 1] ?? oob();
-			const src = firstCommit.parent?.revision;
-			const dst = lastCommit.revision;
+			const src = targetCommits[0].parent?.revision;
+			const dst = getLast(targetCommits).revision;
 			if (src !== undefined && dst !== undefined) {
 				this.initialTransactionRevToRebasedRev.set(src, dst);
 			}
@@ -516,6 +521,7 @@ export class SharedTreeBranch<TEditor extends ChangeFamilyEditor, TChange> {
 
 		// Compute the net change to this branch
 		const sourceCommits = rebaseResult.commits.sourceCommits;
+		assert(hasSome(sourceCommits), "Expected source commits in non no-op merge");
 		const change = this.changeFamily.rebaser.compose(sourceCommits);
 		const taggedChange = makeAnonChange(change);
 		const changeEvent = {

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

@@ -5,7 +5,7 @@
 
 import { assert, oob } from "@fluidframework/core-utils/internal";
 import type { GraphCommit, TaggedChange } from "../core/index.js";
-import { disposeSymbol } from "../util/index.js";
+import { disposeSymbol, hasSome } from "../util/index.js";
 import type { ChangeEnricherReadonlyCheckout, ResubmitMachine } from "./index.js";
 
 /**
@@ -114,7 +114,8 @@ export class DefaultResubmitMachine<TChange> implements ResubmitMachine<TChange>
 			this.isInResubmitPhase,
 			0x982 /* No available commit to resubmit outside of resubmit phase */,
 		);
-		return this.resubmitQueue[0] ?? oob();
+		assert(hasSome(this.resubmitQueue), "Expected resubmit queue to be non-empty");
+		return this.resubmitQueue[0];
 	}
 
 	public get isInResubmitPhase(): boolean {

packages/dds/tree/src/shared-tree/sharedTreeChangeFamily.ts

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { assert, oob } from "@fluidframework/core-utils/internal";
+import { assert } from "@fluidframework/core-utils/internal";
 
 import type { ICodecFamily, ICodecOptions } from "../codec/index.js";
 import {
@@ -32,6 +32,7 @@ import {
 	type NestedSet,
 	addToNestedSet,
 	fail,
+	hasSingle,
 	nestedSetContains,
 } from "../util/index.js";
 
@@ -179,12 +180,12 @@ export class SharedTreeChangeFamily
 			return SharedTreeChangeFamily.emptyChange;
 		}
 		assert(
-			change.change.changes.length === 1 && over.change.changes.length === 1,
+			hasSingle(change.change.changes) && hasSingle(over.change.changes),
 			0x884 /* SharedTreeChange should have exactly one inner change if no schema change is present. */,
 		);
 
-		const dataChangeIntention = change.change.changes[0] ?? oob();
-		const dataChangeOver = over.change.changes[0] ?? oob();
+		const dataChangeIntention = change.change.changes[0];
+		const dataChangeOver = over.change.changes[0];
 		assert(
 			dataChangeIntention.type === "data" && dataChangeOver.type === "data",
 			0x885 /* Data change should be present. */,

packages/dds/tree/src/shared-tree/treeCheckout.ts

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { assert, oob } from "@fluidframework/core-utils/internal";
+import { assert } from "@fluidframework/core-utils/internal";
 import type { IIdCompressor } from "@fluidframework/id-compressor";
 import {
 	UsageError,
@@ -59,7 +59,15 @@ import {
 	getChangeReplaceType,
 	type SharedTreeBranchChange,
 } from "../shared-tree-core/index.js";
-import { Breakable, TransactionResult, disposeSymbol, fail } from "../util/index.js";
+import {
+	Breakable,
+	TransactionResult,
+	disposeSymbol,
+	fail,
+	getLast,
+	hasSingle,
+	hasSome,
+} from "../util/index.js";
 
 import { SharedTreeChangeFamily, hasSchemaChange } from "./sharedTreeChangeFamily.js";
 import type { SharedTreeChange } from "./sharedTreeChangeTypes.js";
@@ -468,12 +476,13 @@ export class TreeCheckout implements ITreeCheckoutFork {
 		// One important consequence of this is that we will not submit the op containing the invalid change, since op submissions happens in response to `afterChange`.
 		_branch.events.on("beforeChange", (event) => {
 			if (event.change !== undefined) {
-				const revision =
-					event.type === "replace"
-						? // Change events will always contain new commits
-							// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-							event.newCommits[event.newCommits.length - 1]!.revision
-						: event.change.revision;
+				let revision: RevisionTag | undefined;
+				if (event.type === "replace") {
+					assert(hasSome(event.newCommits), "Expected new commit for non no-op change event");
+					revision = getLast(event.newCommits).revision;
+				} else {
+					revision = event.change.revision;
+				}
 
 				// Conflicts due to schema will be empty and thus are not applied.
 				for (const change of event.change.change.changes) {
@@ -506,7 +515,11 @@ export class TreeCheckout implements ITreeCheckoutFork {
 				this.events.emit("afterBatch");
 			}
 			if (event.type === "replace" && getChangeReplaceType(event) === "transactionCommit") {
-				const firstCommit = event.newCommits[0] ?? oob();
+				assert(
+					hasSingle(event.newCommits),
+					"Expected exactly one new commit for transaction commit event",
+				);
+				const firstCommit = event.newCommits[0];
 				const transactionRevision = firstCommit.revision;
 				for (const transactionStep of event.removedCommits) {
 					this.removedRoots.updateMajor(transactionStep.revision, transactionRevision);

packages/dds/tree/src/simple-tree/api/simpleSchemaToJsonSchema.ts

@@ -3,10 +3,10 @@
  * Licensed under the MIT License.
  */
 
-import { oob, unreachableCase } from "@fluidframework/core-utils/internal";
+import { unreachableCase } from "@fluidframework/core-utils/internal";
 import { UsageError } from "@fluidframework/telemetry-utils/internal";
 import { ValueSchema } from "../../core/index.js";
-import { getOrCreate, type Mutable } from "../../util/index.js";
+import { getOrCreate, hasSingle, type Mutable } from "../../util/index.js";
 import type {
 	JsonArrayNodeSchema,
 	JsonFieldSchema,
@@ -42,9 +42,9 @@ export function toJsonSchema(schema: SimpleTreeSchema): JsonTreeSchema {
 		allowedTypes.push(createSchemaRef(allowedType));
 	}
 
-	return allowedTypes.length === 1
+	return hasSingle(allowedTypes)
 		? {
-				...(allowedTypes[0] ?? oob()),
+				...allowedTypes[0],
 				$defs: definitions,
 			}
 		: {
@@ -96,8 +96,9 @@ function convertArrayNodeSchema(schema: SimpleArrayNodeSchema): JsonArrayNodeSch
 		allowedTypes.push(createSchemaRef(type));
 	});
 
-	const items: JsonFieldSchema =
-		allowedTypes.length === 1 ? (allowedTypes[0] ?? oob()) : { anyOf: allowedTypes };
+	const items: JsonFieldSchema = hasSingle(allowedTypes)
+		? allowedTypes[0]
+		: { anyOf: allowedTypes };
 
 	return {
 		type: "array",
@@ -142,12 +143,11 @@ function convertObjectNodeSchema(schema: SimpleObjectNodeSchema): JsonObjectNode
 			allowedTypes.push(createSchemaRef(allowedType));
 		}
 
-		const output: Mutable<JsonFieldSchema> =
-			allowedTypes.length === 1
-				? (allowedTypes[0] ?? oob())
-				: {
-						anyOf: allowedTypes,
-					};
+		const output: Mutable<JsonFieldSchema> = hasSingle(allowedTypes)
+			? allowedTypes[0]
+			: {
+					anyOf: allowedTypes,
+				};
 
 		// Don't include "description" property at all if it's not present in the input.
 		if (value.metadata?.description !== undefined) {
@@ -178,12 +178,11 @@ function convertMapNodeSchema(schema: SimpleMapNodeSchema): JsonMapNodeSchema {
 		type: "object",
 		_treeNodeSchemaKind: NodeKind.Map,
 		patternProperties: {
-			"^.*$":
-				allowedTypes.length === 1
-					? (allowedTypes[0] ?? oob())
-					: {
-							anyOf: allowedTypes,
-						},
+			"^.*$": hasSingle(allowedTypes)
+				? allowedTypes[0]
+				: {
+						anyOf: allowedTypes,
+					},
 		},
 	};
 }

packages/dds/tree/src/simple-tree/toMapTree.ts

@@ -3,7 +3,7 @@
  * Licensed under the MIT License.
  */
 
-import { assert, oob } from "@fluidframework/core-utils/internal";
+import { assert } from "@fluidframework/core-utils/internal";
 import { UsageError } from "@fluidframework/telemetry-utils/internal";
 import { isFluidHandle } from "@fluidframework/runtime-utils/internal";
 
@@ -21,7 +21,7 @@ import {
 	valueSchemaAllows,
 	type NodeKeyManager,
 } from "../feature-libraries/index.js";
-import { brand, fail, isReadonlyArray, find } from "../util/index.js";
+import { brand, fail, isReadonlyArray, find, hasSome, hasSingle } from "../util/index.js";
 
 import { nullSchema } from "./leafNodeSchema.js";
 import {
@@ -465,10 +465,10 @@ function getType(
 		);
 	}
 	assert(
-		possibleTypes.length !== 0,
+		hasSome(possibleTypes),
 		0x84e /* data is incompatible with all types allowed by the schema */,
 	);
-	if (possibleTypes.length !== 1) {
+	if (!hasSingle(possibleTypes)) {
 		throw new UsageError(
 			`The provided data is compatible with more than one type allowed by the schema.
 The set of possible types is ${JSON.stringify([
@@ -478,7 +478,7 @@ Explicitly construct an unhydrated node of the desired type to disambiguate.
 For class-based schema, this can be done by replacing an expression like "{foo: 1}" with "new MySchema({foo: 1})".`,
 		);
 	}
-	return possibleTypes[0] ?? oob();
+	return possibleTypes[0];
 }
 
 /**

packages/dds/tree/src/util/index.ts

@@ -91,6 +91,9 @@ export {
 	compareStrings,
 	find,
 	count,
+	getLast,
+	hasSome,
+	hasSingle,
 } from "./utils.js";
 export { ReferenceCountedBase, type ReferenceCounted } from "./referenceCounting.js";
 

packages/dds/tree/src/util/utils.ts

@@ -79,6 +79,44 @@ export function makeArray<T>(size: number, filler: (index: number) => T): T[] {
 	return array;
 }
 
+/**
+ * Returns the last element of an array, or `undefined` if the array has no elements.
+ * @param array - The array to get the last element from.
+ * @remarks
+ * If the type of the array has been narrowed by e.g. {@link hasSome | hasSome(array)} or {@link hasSingle | hasOne(array)} then the return type will be `T` rather than `T | undefined`.
+ */
+export function getLast<T>(array: readonly [T, ...T[]]): T;
+export function getLast<T>(array: { [index: number]: T; length: number }): T | undefined;
+export function getLast<T>(array: { [index: number]: T; length: number }): T | undefined {
+	return array[array.length - 1];
+}
+
+/**
+ * Returns true if and only if the given array has at least one element.
+ * @param array - The array to check.
+ * @remarks
+ * If `array` contains at least one element, its type will be narrowed and can benefit from improved typing from e.g. `array[0]` and {@link getLast | getLast(array)}.
+ * This is especially useful when "noUncheckedIndexedAccess" is enabled in the TypeScript compiler options, since the return type of `array[0]` will be `T` rather than `T | undefined`.
+ */
+export function hasSome<T>(array: T[]): array is [T, ...T[]];
+export function hasSome<T>(array: readonly T[]): array is readonly [T, ...T[]];
+export function hasSome<T>(array: readonly T[]): array is [T, ...T[]] {
+	return array.length > 0;
+}
+
+/**
+ * Returns true if and only if the given array has exactly one element.
+ * @param array - The array to check.
+ * @remarks
+ * If `array` contains exactly one element, its type will be narrowed and can benefit from improved typing from e.g. `array[0]` and {@link getLast | getLast(array)}.
+ * This is especially useful when "noUncheckedIndexedAccess" is enabled in the TypeScript compiler options, since the return type of `array[0]` will be `T` rather than `T | undefined`.
+ */
+export function hasSingle<T>(array: T[]): array is [T];
+export function hasSingle<T>(array: readonly T[]): array is readonly [T];
+export function hasSingle<T>(array: readonly T[]): array is [T] {
+	return array.length === 1;
+}
+
 /**
  * Compares two sets using callbacks.
  * Early returns on first false comparison.