refactor(container-runtime): Enable various eslint rules from our "recommended" config and fix violations

common/build/eslint-config-fluid/printed-configs/default.json

@@ -1297,7 +1297,7 @@
             "error"
         ],
         "unicorn/no-array-push-push": [
-            "error"
+            "off"
         ],
         "unicorn/no-array-reduce": [
             "error"

common/build/eslint-config-fluid/printed-configs/react.json

@@ -1371,7 +1371,7 @@
             "error"
         ],
         "unicorn/no-array-push-push": [
-            "error"
+            "off"
         ],
         "unicorn/no-array-reduce": [
             "error"

common/build/eslint-config-fluid/printed-configs/recommended.json

@@ -1297,7 +1297,7 @@
             "error"
         ],
         "unicorn/no-array-push-push": [
-            "error"
+            "off"
         ],
         "unicorn/no-array-reduce": [
             "error"

common/build/eslint-config-fluid/printed-configs/strict-biome.json

@@ -1583,7 +1583,7 @@
             "error"
         ],
         "unicorn/no-array-push-push": [
-            "error"
+            "off"
         ],
         "unicorn/no-array-reduce": [
             "error"

common/build/eslint-config-fluid/printed-configs/strict.json

@@ -1343,7 +1343,7 @@
             "error"
         ],
         "unicorn/no-array-push-push": [
-            "error"
+            "off"
         ],
         "unicorn/no-array-reduce": [
             "error"

common/build/eslint-config-fluid/printed-configs/test.json

@@ -1302,7 +1302,7 @@
             "error"
         ],
         "unicorn/no-array-push-push": [
-            "error"
+            "off"
         ],
         "unicorn/no-array-reduce": [
             "error"

common/build/eslint-config-fluid/recommended.js

@@ -44,6 +44,11 @@ module.exports = {
 			},
 		],
 
+		// #region `unicorn` rule overrides
+
+		// False positives on non-array `push` methods.
+		"unicorn/no-array-push-push": "off",
+
 		"unicorn/empty-brace-spaces": "off",
 
 		// Rationale: Destructuring of `Array.entries()` in order to get the index variable results in a
@@ -108,6 +113,8 @@ module.exports = {
 		 */
 		"unicorn/expiring-todo-comments": "off",
 
+		// #endregion
+
 		/**
 		 * Disallows the `any` type.
 		 * Using the `any` type defeats the purpose of using TypeScript.

packages/runtime/container-runtime/.eslintrc.cjs

@@ -15,6 +15,10 @@ module.exports = {
 		"@typescript-eslint/strict-boolean-expressions": "off",
 		"@fluid-internal/fluid/no-unchecked-record-access": "warn",
 
+		// False positives on non-array `push` methods.
+		// TODO:AB#28686: remove this override once this rule has been disabled in the root config.
+		"unicorn/no-array-push-push": "off",
+
 		// #region TODO:AB#3027: remove overrides and upgrade config to `recommended`
 
 		"@typescript-eslint/explicit-function-return-type": [
@@ -46,6 +50,30 @@ module.exports = {
 		"jsdoc/multiline-blocks": ["error", { noSingleLineBlocks: true }],
 		"jsdoc/require-description": ["error", { checkConstructors: false }],
 
+		"unicorn/catch-error-name": "error",
+		"unicorn/consistent-destructuring": "error",
+		"unicorn/new-for-builtins": "error",
+		"unicorn/no-array-callback-reference": "error",
+		"unicorn/no-array-for-each": "error",
+		"unicorn/no-null": "error",
+		"unicorn/no-zero-fractions": "error",
+		"unicorn/prefer-node-protocol": "error",
+		"unicorn/prefer-number-properties": "error",
+		"unicorn/prefer-optional-catch-binding": "error",
+		"unicorn/prefer-spread": "error",
+		"unicorn/prefer-string-slice": "error",
+		"unicorn/switch-case-braces": "error",
+		"unicorn/throw-new-error": "error",
+
+		// TODO:
+		// unicorn/consistent-function-scoping
+		// unicorn/no-negated-condition
+		// unicorn/no-lonely-if
+		// unicorn/no-new-array
+		// unicorn/prefer-includes
+		// unicorn/error-message
+		// unicorn/explicit-length-check
+
 		// #endregion
 	},
 	overrides: [
@@ -57,7 +85,7 @@ module.exports = {
 				"@typescript-eslint/explicit-function-return-type": "off",
 
 				// Test files are run in node only so additional node libraries can be used.
-				"import/no-nodejs-modules": ["error", { allow: ["assert", "crypto"] }],
+				"import/no-nodejs-modules": ["error", { allow: ["node:assert", "node:crypto"] }],
 			},
 		},
 	],

packages/runtime/container-runtime/src/blobManager/blobManager.ts

@@ -240,7 +240,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 		this.redirectTable = toRedirectTable(snapshot, this.mc.logger, this.runtime.attachState);
 
 		// Begin uploading stashed blobs from previous container instance
-		Object.entries(stashedBlobs ?? {}).forEach(([localId, entry]) => {
+		for (const [localId, entry] of Object.entries(stashedBlobs ?? {})) {
 			const { acked, storageId, minTTLInSeconds, uploadTime } = entry;
 			const blob = stringToBuffer(entry.blob, "base64");
 			const pendingEntry: PendingBlob = {
@@ -260,7 +260,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 				const timeLapseSinceLocalUpload = (Date.now() - uploadTime) / 1000;
 				// stashed entries with more than half-life in storage will not be reuploaded
 				if (minTTLInSeconds - timeLapseSinceLocalUpload > minTTLInSeconds / 2) {
-					return;
+					continue;
 				}
 			}
 			this.pendingStashedBlobs.set(localId, this.uploadBlob(localId, blob));
@@ -269,7 +269,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 				...stashedPendingBlobOverrides,
 				uploadP: this.pendingStashedBlobs.get(localId),
 			});
-		});
+		}
 
 		this.stashedBlobsUploadP = new LazyPromise(async () =>
 			PerformanceEvent.timedExecAsync(
@@ -337,7 +337,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 	}
 
 	public hasPendingStashedUploads(): boolean {
-		return Array.from(this.pendingBlobs.values()).some((e) => e.stashedUpload === true);
+		return [...this.pendingBlobs.values()].some((e) => e.stashedUpload === true);
 	}
 
 	public async getBlob(blobId: string): Promise<ArrayBufferLike> {
@@ -588,10 +588,10 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 			// If there is already an op for this storage ID, append the local ID to the list. Once any op for
 			// this storage ID is ack'd, all pending blobs for it can be resolved since the op will keep the
 			// blob alive in storage.
-			this.opsInFlight.set(
-				response.id,
-				(this.opsInFlight.get(response.id) ?? []).concat(localId),
-			);
+			this.opsInFlight.set(response.id, [
+				...(this.opsInFlight.get(response.id) ?? []),
+				localId,
+			]);
 		}
 		return response;
 	}
@@ -648,7 +648,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 				// For each op corresponding to this storage ID that we are waiting for, resolve the pending blob.
 				// This is safe because the server will keep the blob alive and the op containing the local ID to
 				// storage ID is already in flight and any op containing this local ID will be sequenced after that.
-				waitingBlobs.forEach((pendingLocalId) => {
+				for (const pendingLocalId of waitingBlobs) {
 					const entry = this.pendingBlobs.get(pendingLocalId);
 					assert(
 						entry !== undefined,
@@ -658,7 +658,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 					entry.acked = true;
 					entry.handleP.resolve(this.getBlobHandle(pendingLocalId));
 					this.deletePendingBlobMaybe(pendingLocalId);
-				});
+				}
 				this.opsInFlight.delete(blobId);
 			}
 			const localEntry = this.pendingBlobs.get(localId);
@@ -703,7 +703,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 	 */
 	public deleteSweepReadyNodes(sweepReadyBlobRoutes: readonly string[]): readonly string[] {
 		this.deleteBlobsFromRedirectTable(sweepReadyBlobRoutes);
-		return Array.from(sweepReadyBlobRoutes);
+		return [...sweepReadyBlobRoutes];
 	}
 
 	/**

packages/runtime/container-runtime/src/blobManager/blobManagerSnapSum.ts

@@ -73,7 +73,9 @@ export const toRedirectTable = (
 	if (snapshot.ids) {
 		// If we are detached, we don't have storage IDs yet, so set to undefined
 		// Otherwise, set identity (id -> id) entries.
-		snapshot.ids.forEach((entry) => redirectTable.set(entry, detached ? undefined : entry));
+		for (const entry of snapshot.ids) {
+			redirectTable.set(entry, detached ? undefined : entry);
+		}
 	}
 	return redirectTable;
 };
@@ -90,22 +92,19 @@ const summarizeV1 = (
 	const storageIds = getStorageIds(redirectTable, attachState);
 
 	// if storageIds is empty, it means we are detached and have only local IDs, or that there are no blobs attached
-	const blobIds =
-		storageIds.size > 0 ? Array.from(storageIds) : Array.from(redirectTable.keys());
+	const blobIds = storageIds.size > 0 ? [...storageIds] : [...redirectTable.keys()];
 	const builder = new SummaryTreeBuilder();
-	blobIds.forEach((blobId) => {
+	for (const blobId of blobIds) {
 		builder.addAttachment(blobId);
-	});
+	}
 
 	// Any non-identity entries in the table need to be saved in the summary
 	if (redirectTable.size > blobIds.length) {
 		builder.addBlob(
 			redirectTableBlobName,
 			// filter out identity entries
 			JSON.stringify(
-				Array.from(redirectTable.entries()).filter(
-					([localId, storageId]) => localId !== storageId,
-				),
+				[...redirectTable.entries()].filter(([localId, storageId]) => localId !== storageId),
 			),
 		);
 	}

packages/runtime/container-runtime/src/channelCollection.ts

@@ -707,13 +707,16 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 	public reSubmit(type: string, content: unknown, localOpMetadata: unknown): void {
 		switch (type) {
 			case ContainerMessageType.Attach:
-			case ContainerMessageType.Alias:
+			case ContainerMessageType.Alias: {
 				this.parentContext.submitMessage(type, content, localOpMetadata);
 				return;
-			case ContainerMessageType.FluidDataStoreOp:
+			}
+			case ContainerMessageType.FluidDataStoreOp: {
 				return this.reSubmitChannelOp(type, content, localOpMetadata);
-			default:
+			}
+			default: {
 				assert(false, 0x907 /* unknown op type */);
+			}
 		}
 	}
 
@@ -757,14 +760,18 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 	public async applyStashedOp(content: unknown): Promise<unknown> {
 		const opContents = content as LocalContainerRuntimeMessage;
 		switch (opContents.type) {
-			case ContainerMessageType.Attach:
+			case ContainerMessageType.Attach: {
 				return this.applyStashedAttachOp(opContents.contents);
-			case ContainerMessageType.Alias:
+			}
+			case ContainerMessageType.Alias: {
 				return;
-			case ContainerMessageType.FluidDataStoreOp:
+			}
+			case ContainerMessageType.FluidDataStoreOp: {
 				return this.applyStashedChannelChannelOp(opContents.contents);
-			default:
+			}
+			default: {
 				assert(false, 0x908 /* unknon type of op */);
+			}
 		}
 	}
 
@@ -821,7 +828,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 			// if the client is not detached put in the pending attach list
 			// so that on ack of the stashed op, the context is found.
 			// detached client don't send ops, so should not expect and ack.
-			this.pendingAttach.set(message.id, message);
+			this.pendingAttach.set(id, message);
 		}
 	}
 
@@ -831,17 +838,21 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 	 */
 	public processMessages(messageCollection: IRuntimeMessageCollection): void {
 		switch (messageCollection.envelope.type) {
-			case ContainerMessageType.FluidDataStoreOp:
+			case ContainerMessageType.FluidDataStoreOp: {
 				this.processChannelMessages(messageCollection);
 				break;
-			case ContainerMessageType.Attach:
+			}
+			case ContainerMessageType.Attach: {
 				this.processAttachMessages(messageCollection);
 				break;
-			case ContainerMessageType.Alias:
+			}
+			case ContainerMessageType.Alias: {
 				this.processAliasMessages(messageCollection);
 				break;
-			default:
+			}
+			default: {
 				assert(false, 0x8e9 /* unreached */);
+			}
 		}
 	}
 
@@ -874,7 +885,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 	 * @param messageCollection - The collection of messages to process.
 	 */
 	private processChannelMessages(messageCollection: IRuntimeMessageCollection): void {
-		const { messagesContent, local } = messageCollection;
+		const { envelope, messagesContent, local } = messageCollection;
 		let currentMessageState: { address: string; type: string } | undefined;
 		let currentMessagesContent: IRuntimeMessagesContent[] = [];
 
@@ -888,7 +899,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 			assert(!!currentContext, 0xa66 /* Context not found */);
 
 			currentContext.processMessages({
-				envelope: { ...messageCollection.envelope, type: currentMessageState.type },
+				envelope: { ...envelope, type: currentMessageState.type },
 				messagesContent: currentMessagesContent,
 				local,
 			});
@@ -916,11 +927,11 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 				throw DataProcessingError.create(
 					"No context for op",
 					"processFluidDataStoreOp",
-					messageCollection.envelope as ISequencedDocumentMessage,
+					envelope as ISequencedDocumentMessage,
 					{
 						local,
 						messageDetails: JSON.stringify({
-							type: messageCollection.envelope.type,
+							type: envelope.type,
 							contentType: typeof contents,
 						}),
 						...tagCodeArtifacts({ address }),
@@ -948,16 +959,12 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 			this.gcNodeUpdated({
 				node: { type: "DataStore", path: `/${address}` },
 				reason: "Changed",
-				timestampMs: messageCollection.envelope.timestamp,
+				timestampMs: envelope.timestamp,
 				packagePath: context.isLoaded ? context.packagePath : undefined,
 			});
 
 			detectOutboundReferences(address, contextContents, (fromPath: string, toPath: string) =>
-				this.parentContext.addedGCOutboundRoute(
-					fromPath,
-					toPath,
-					messageCollection.envelope.timestamp,
-				),
+				this.parentContext.addedGCOutboundRoute(fromPath, toPath, envelope.timestamp),
 			);
 		}
 
@@ -1200,7 +1207,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 			},
 		);
 		// Get the outbound routes (aliased data stores) and add a GC node for this channel.
-		builder.addNode("/", Array.from(this.aliasedDataStores));
+		builder.addNode("/", [...this.aliasedDataStores]);
 		return builder.getGCData();
 	}
 
@@ -1412,7 +1419,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 
 			this.deleteChild(dataStoreId);
 		}
-		return Array.from(sweepReadyDataStoreRoutes);
+		return [...sweepReadyDataStoreRoutes];
 	}
 
 	/**
@@ -1613,7 +1620,9 @@ export function detectOutboundReferences(
 	// GC node paths are all absolute paths, hence the "" prefix.
 	// e.g. this will yield "/dataStoreId/ddsId"
 	const fromPath = ["", address, ddsAddress].join("/");
-	outboundPaths.forEach((toPath) => addedOutboundReference(fromPath, toPath));
+	for (const toPath of outboundPaths) {
+		addedOutboundReference(fromPath, toPath);
+	}
 }
 
 /**