refactor(container-runtime): Enable `@typescript-eslint/explicit-func…

…tion-return-type` eslint rule and fix violations (#23605)

One in a series of PRs working towards migrating the package to our
`recommended` config.

Plus a couple of misc. comment syntax fixes/improvements.


[AB#3027](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/3027)

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

@@ -17,6 +17,16 @@ module.exports = {
 
 		// #region TODO:AB#3027: remove overrides and upgrade config to `recommended`
 
+		"@typescript-eslint/explicit-function-return-type": [
+			"error",
+			{
+				allowExpressions: true,
+				allowTypedFunctionExpressions: true,
+				allowHigherOrderFunctions: true,
+				allowDirectConstAssertionInArrowFunctions: true,
+				allowConciseArrowFunctionExpressionsStartingWithVoid: false,
+			},
+		],
 		"@typescript-eslint/explicit-module-boundary-types": "error",
 		"@typescript-eslint/no-explicit-any": [
 			"error",
@@ -43,6 +53,9 @@ module.exports = {
 			// Rules only for test files
 			files: ["*.spec.ts", "src/test/**"],
 			rules: {
+				// TODO: remove these overrides and fix violations
+				"@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"] }],
 			},

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

@@ -329,7 +329,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 		);
 	}
 
-	private createAbortError(pending?: PendingBlob) {
+	private createAbortError(pending?: PendingBlob): LoggingError {
 		return new LoggingError("uploadBlob aborted", {
 			acked: pending?.acked,
 			uploadTime: pending?.uploadTime,
@@ -454,7 +454,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 		};
 		this.pendingBlobs.set(localId, pendingEntry);
 
-		const abortListener = () => {
+		const abortListener = (): void => {
 			if (!pendingEntry.acked) {
 				pendingEntry.handleP.reject(this.createAbortError(pendingEntry));
 			}
@@ -517,11 +517,11 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 	 * Set up a mapping in the redirect table from fromId to toId. Also, notify the runtime that a reference is added
 	 * which is required for GC.
 	 */
-	private setRedirection(fromId: string, toId: string | undefined) {
+	private setRedirection(fromId: string, toId: string | undefined): void {
 		this.redirectTable.set(fromId, toId);
 	}
 
-	private deletePendingBlobMaybe(id: string) {
+	private deletePendingBlobMaybe(id: string): void {
 		if (this.pendingBlobs.has(id)) {
 			const entry = this.pendingBlobs.get(id);
 			if (entry?.attached && entry?.acked) {
@@ -530,13 +530,16 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 		}
 	}
 
-	private deletePendingBlob(id: string) {
+	private deletePendingBlob(id: string): void {
 		if (this.pendingBlobs.delete(id) && !this.hasPendingBlobs) {
 			this.emit("noPendingBlobs");
 		}
 	}
 
-	private onUploadResolve(localId: string, response: ICreateBlobResponseWithTTL) {
+	private onUploadResolve(
+		localId: string,
+		response: ICreateBlobResponseWithTTL,
+	): ICreateBlobResponseWithTTL | undefined {
 		const entry = this.pendingBlobs.get(localId);
 		if (entry === undefined && this.pendingStashedBlobs.has(localId)) {
 			// The blob was already processed and deleted. This can happen if the blob was reuploaded by
@@ -705,16 +708,20 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 
 	/**
 	 * Delete blobs with the given routes from the redirect table.
+	 *
+	 * @remarks
 	 * The routes are GC nodes paths of format -`/<blobManagerBasePath>/<blobId>`. The blob ids are all local ids.
 	 * Deleting the blobs involves 2 steps:
+	 *
 	 * 1. The redirect table entry for the local ids are deleted.
+	 *
 	 * 2. If the storage ids corresponding to the deleted local ids are not in-use anymore, the redirect table entries
 	 * for the storage ids are deleted as well.
 	 *
 	 * Note that this does not delete the blobs from storage service immediately. Deleting the blobs from redirect table
 	 * will remove them the next summary. The service would them delete them some time in the future.
 	 */
-	private deleteBlobsFromRedirectTable(blobRoutes: readonly string[]) {
+	private deleteBlobsFromRedirectTable(blobRoutes: readonly string[]): void {
 		if (blobRoutes.length === 0) {
 			return;
 		}
@@ -764,7 +771,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 	 * Verifies that the blob with given id is not deleted, i.e., it has not been garbage collected. If the blob is GC'd,
 	 * log an error and throw if necessary.
 	 */
-	private verifyBlobNotDeleted(blobId: string) {
+	private verifyBlobNotDeleted(blobId: string): void {
 		if (!this.isBlobDeleted(getGCNodePathFromBlobId(blobId))) {
 			return;
 		}
@@ -852,7 +859,7 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
 										},
 										{ once: true },
 									);
-									const onBlobAttached = (attachedEntry) => {
+									const onBlobAttached = (attachedEntry): void => {
 										if (attachedEntry === entry) {
 											this.off("blobAttached", onBlobAttached);
 											resolve();
@@ -902,12 +909,13 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
  * This path must match the path of the blob handle returned by the createBlob API because blobs are marked
  * referenced by storing these handles in a referenced DDS.
  */
-const getGCNodePathFromBlobId = (blobId: string) => `/${blobManagerBasePath}/${blobId}`;
+const getGCNodePathFromBlobId = (blobId: string): string =>
+	`/${blobManagerBasePath}/${blobId}`;
 
 /**
  * For a given GC node path, return the blobId. The node path is of the format `/<basePath>/<blobId>`.
  */
-const getBlobIdFromGCNodePath = (nodePath: string) => {
+const getBlobIdFromGCNodePath = (nodePath: string): string => {
 	const pathParts = nodePath.split("/");
 	assert(areBlobPathParts(pathParts), 0x5bd /* Invalid blob node path */);
 	return pathParts[2];

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

@@ -396,7 +396,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 		this.parentContext.makeLocallyVisible();
 	}
 
-	private processAttachMessages(messageCollection: IRuntimeMessageCollection) {
+	private processAttachMessages(messageCollection: IRuntimeMessageCollection): void {
 		const { envelope, messagesContent, local } = messageCollection;
 		for (const { contents } of messagesContent) {
 			const attachMessage = contents as InboundAttachMessage;
@@ -779,7 +779,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 		return context.applyStashedOp(envelope.contents);
 	}
 
-	private async applyStashedAttachOp(message: IAttachMessage) {
+	private async applyStashedAttachOp(message: IAttachMessage): Promise<void> {
 		const { id, snapshot } = message;
 
 		// build the snapshot from the summary in the attach message
@@ -879,7 +879,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 		let currentMessagesContent: IRuntimeMessagesContent[] = [];
 
 		// Helper that sends the current bunch of messages to the data store. It validates that the data stores exists.
-		const sendBunchedMessages = () => {
+		const sendBunchedMessages = (): void => {
 			// Current message state will be undefined for the first message in the list.
 			if (currentMessageState === undefined) {
 				return;
@@ -1044,7 +1044,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
 		callSite: string,
 		requestHeaderData?: RuntimeHeaderData,
 		originalRequest?: IRequest,
-	) {
+	): boolean {
 		const dataStoreNodePath = `/${id}`;
 		if (!this.isDataStoreDeleted(dataStoreNodePath)) {
 			return false;
@@ -1588,7 +1588,7 @@ export function detectOutboundReferences(
 	const outboundPaths: string[] = [];
 	let ddsAddress: string | undefined;
 
-	function recursivelyFindHandles(obj: unknown) {
+	function recursivelyFindHandles(obj: unknown): void {
 		if (typeof obj === "object" && obj !== null) {
 			for (const [key, value] of Object.entries(obj)) {
 				// If 'value' is a serialized IFluidHandle, it represents a new outbound route.

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

@@ -301,7 +301,7 @@ class OpPerfTelemetry {
 		});
 	}
 
-	private reportGettingUpToDate() {
+	private reportGettingUpToDate(): void {
 		this.connectionOpSeqNumber = undefined;
 		this.logger.sendPerformanceEvent({
 			eventName: "ConnectionSpeed",
@@ -315,7 +315,7 @@ class OpPerfTelemetry {
 		});
 	}
 
-	private recordPingTime(latency: number) {
+	private recordPingTime(latency: number): void {
 		this.pingLatency = latency;
 
 		// Log if latency is longer than 1 min
@@ -335,7 +335,7 @@ class OpPerfTelemetry {
 		}
 	}
 
-	private beforeOpSubmit(message: IDocumentMessage) {
+	private beforeOpSubmit(message: IDocumentMessage): void {
 		// start with first client op and measure latency every 500 client ops
 		if (
 			this.opLatencyLogger.isSamplingDisabled ||
@@ -362,7 +362,7 @@ class OpPerfTelemetry {
 		}
 	}
 
-	private afterProcessingOp(message: ISequencedDocumentMessage) {
+	private afterProcessingOp(message: ISequencedDocumentMessage): void {
 		const sequenceNumber = message.sequenceNumber;
 
 		if (sequenceNumber === this.connectionOpSeqNumber) {