From e64c6dde6570c9455c473c3a758d09ba4435adbd Mon Sep 17 00:00:00 2001 From: Rishhi Balakrishnan <107130183+RishhiB@users.noreply.github.com> Date: Thu, 5 Sep 2024 16:23:55 -0400 Subject: [PATCH] Revert "Enable noUncheckedIndexedAccess for loader packages (#21485)" (#22380) This reverts commit 15a9c2b8487bab986eb96c3d0440313a3d2d8f8e. Since we now have a lint rule that replaces our need for noUncheckedIndexedAccess, removing the code we added when enabling noUncheckedIndexedAccess [AB#11815](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/11815) --- .../container-loader/src/connectionManager.ts | 12 ++---- .../loader/container-loader/src/container.ts | 39 ++++--------------- .../src/containerStorageAdapter.ts | 2 +- .../container-loader/src/deltaManager.ts | 24 +++--------- .../src/serializedStateManager.ts | 25 ++++-------- packages/loader/container-loader/src/utils.ts | 21 +++------- .../loader/container-loader/tsconfig.json | 1 + ...ageServiceSummaryBlobCompressionAdapter.ts | 17 ++++---- .../driver-utils/src/insecureUrlResolver.ts | 8 +--- .../driver-utils/src/parallelRequests.ts | 16 ++------ .../src/prefetchDocumentStorageService.ts | 8 ++-- .../driver-utils/src/protocol/gitHelper.ts | 4 +- packages/loader/driver-utils/tsconfig.json | 1 + 13 files changed, 51 insertions(+), 127 deletions(-) diff --git a/packages/loader/container-loader/src/connectionManager.ts b/packages/loader/container-loader/src/connectionManager.ts index 3790f0f3b567..711e00e8d25b 100644 --- a/packages/loader/container-loader/src/connectionManager.ts +++ b/packages/loader/container-loader/src/connectionManager.ts @@ -933,12 +933,8 @@ export class ConnectionManager implements IConnectionManager { let last = -1; if (initialMessages.length > 0) { this._connectionVerboseProps.connectionInitialOpsFrom = - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - initialMessages[0]!.sequenceNumber; - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - last = initialMessages[initialMessages.length - 1]!.sequenceNumber; + initialMessages[0].sequenceNumber; + last = initialMessages[initialMessages.length - 1].sequenceNumber; this._connectionVerboseProps.connectionInitialOpsTo = last + 1; // Update knowledge of how far we are behind, before raising "connect" event // This is duplication of what incomingOpHandler() does, but we have to raise event before we get there, @@ -1225,9 +1221,7 @@ export class ConnectionManager implements IConnectionManager { // Always connect in write mode after getting nacked. private readonly nackHandler = (documentId: string, messages: INack[]): void => { - // TODO Why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const message = messages[0]!; + const message = messages[0]; if (this._readonlyPermissions === true) { this.props.closeHandler( createWriteError("writeOnReadOnlyDocument", { driverVersion: undefined }), diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 2ac2acfca375..ac47a7941395 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -883,9 +883,7 @@ export class Container : this.deltaManager?.lastMessage?.clientId, dmLastMsgClientSeq: () => this.deltaManager?.lastMessage?.clientSequenceNumber, connectionStateDuration: () => - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - performance.now() - this.connectionTransitionTimes[this.connectionState]!, + performance.now() - this.connectionTransitionTimes[this.connectionState], }, }, }); @@ -929,10 +927,7 @@ export class Container mode, category: this._lifecycleState === "loading" ? "generic" : category, duration: - performance.now() - - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.connectionTransitionTimes[ConnectionState.CatchingUp]!, + performance.now() - this.connectionTransitionTimes[ConnectionState.CatchingUp], ...(details === undefined ? {} : { details: JSON.stringify(details) }), }); @@ -1844,9 +1839,7 @@ export class Container const baseTree = getProtocolSnapshotTree(snapshotTreeWithBlobContents); const qValues = await readAndParse<[string, ICommittedProposal][]>( this.storageAdapter, - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - baseTree.blobs.quorumValues!, + baseTree.blobs.quorumValues, ); this.initializeProtocolState( attributes, @@ -1882,24 +1875,12 @@ export class Container const baseTree = getProtocolSnapshotTree(snapshot); [quorumSnapshot.members, quorumSnapshot.proposals, quorumSnapshot.values] = await Promise.all([ - readAndParse<[string, ISequencedClient][]>( - storage, - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - baseTree.blobs.quorumMembers!, - ), + readAndParse<[string, ISequencedClient][]>(storage, baseTree.blobs.quorumMembers), readAndParse<[number, ISequencedProposal, string[]][]>( storage, - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - baseTree.blobs.quorumProposals!, - ), - readAndParse<[string, ICommittedProposal][]>( - storage, - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - baseTree.blobs.quorumValues!, + baseTree.blobs.quorumProposals, ), + readAndParse<[string, ICommittedProposal][]>(storage, baseTree.blobs.quorumValues), ]); } @@ -2156,9 +2137,7 @@ export class Container // Log actual event const time = performance.now(); this.connectionTransitionTimes[value] = time; - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const duration = time - this.connectionTransitionTimes[oldState]!; + const duration = time - this.connectionTransitionTimes[oldState]; let durationFromDisconnected: number | undefined; let connectionInitiationReason: string | undefined; @@ -2170,9 +2149,7 @@ export class Container } else { if (value === ConnectionState.Connected) { durationFromDisconnected = - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - time - this.connectionTransitionTimes[ConnectionState.Disconnected]!; + time - this.connectionTransitionTimes[ConnectionState.Disconnected]; durationFromDisconnected = formatTick(durationFromDisconnected); } else if (value === ConnectionState.CatchingUp) { // This info is of most interesting while Catching Up. diff --git a/packages/loader/container-loader/src/containerStorageAdapter.ts b/packages/loader/container-loader/src/containerStorageAdapter.ts index 5073f0d55889..5ba5788368c8 100644 --- a/packages/loader/container-loader/src/containerStorageAdapter.ts +++ b/packages/loader/container-loader/src/containerStorageAdapter.ts @@ -174,7 +174,7 @@ export class ContainerStorageAdapter let snapshot: ISnapshot; if ( this.loadingGroupIdSnapshotsFromPendingState !== undefined && - snapshotFetchOptions?.loadingGroupIds?.[0] !== undefined + snapshotFetchOptions?.loadingGroupIds !== undefined ) { const localSnapshot = this.loadingGroupIdSnapshotsFromPendingState[snapshotFetchOptions.loadingGroupIds[0]]; diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index 3a816f0e1019..fa165fdef7f5 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -357,22 +357,16 @@ export class DeltaManager if (batch.length === 1) { assert( - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - (batch[0]!.metadata as IBatchMetadata)?.batch === undefined, + (batch[0].metadata as IBatchMetadata)?.batch === undefined, 0x3c9 /* no batch markup on single message */, ); } else { assert( - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - (batch[0]!.metadata as IBatchMetadata)?.batch === true, + (batch[0].metadata as IBatchMetadata)?.batch === true, 0x3ca /* no start batch markup */, ); assert( - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - (batch[batch.length - 1]!.metadata as IBatchMetadata)?.batch === false, + (batch[batch.length - 1].metadata as IBatchMetadata)?.batch === false, 0x3cb /* no end batch markup */, ); } @@ -898,12 +892,8 @@ export class DeltaManager return; } - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const from = messages[0]!.sequenceNumber; - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const last = messages[messages.length - 1]!.sequenceNumber; + const from = messages[0].sequenceNumber; + const last = messages[messages.length - 1].sequenceNumber; // Report stats about missing and duplicate ops // This helps better understand why we fetch ops from storage, and thus may delay @@ -970,9 +960,7 @@ export class DeltaManager } } - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.updateLatestKnownOpSeqNumber(messages[messages.length - 1]!.sequenceNumber); + this.updateLatestKnownOpSeqNumber(messages[messages.length - 1].sequenceNumber); const n = this.previouslyProcessedMessage?.sequenceNumber; assert( diff --git a/packages/loader/container-loader/src/serializedStateManager.ts b/packages/loader/container-loader/src/serializedStateManager.ts index ce839dca31e0..0f75491e04a7 100644 --- a/packages/loader/container-loader/src/serializedStateManager.ts +++ b/packages/loader/container-loader/src/serializedStateManager.ts @@ -188,9 +188,7 @@ export class SerializedStateManager { if (pendingLocalState && pendingLocalState.savedOps.length > 0) { const savedOpsSize = pendingLocalState.savedOps.length; this.lastSavedOpSequenceNumber = - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - pendingLocalState.savedOps[savedOpsSize - 1]!.sequenceNumber; + pendingLocalState.savedOps[savedOpsSize - 1].sequenceNumber; } containerEvent.on("saved", () => this.updateSnapshotAndProcessedOpsMaybe()); } @@ -343,9 +341,7 @@ export class SerializedStateManager { if ( snapshotSequenceNumber === undefined || this.processedOps.length === 0 || - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.processedOps[this.processedOps.length - 1]!.sequenceNumber < + this.processedOps[this.processedOps.length - 1].sequenceNumber < this.lastSavedOpSequenceNumber || this.containerDirty() ) { @@ -353,13 +349,9 @@ export class SerializedStateManager { // Pending state would be behind the latest snapshot. return -1; } - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const firstProcessedOpSequenceNumber = this.processedOps[0]!.sequenceNumber; + const firstProcessedOpSequenceNumber = this.processedOps[0].sequenceNumber; const lastProcessedOpSequenceNumber = - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.processedOps[this.processedOps.length - 1]!.sequenceNumber; + this.processedOps[this.processedOps.length - 1].sequenceNumber; if (snapshotSequenceNumber < firstProcessedOpSequenceNumber) { // Snapshot seq number is older than our first processed op, which could mean we're fetching @@ -385,9 +377,7 @@ export class SerializedStateManager { snapshotSequenceNumber, firstProcessedOpSequenceNumber, newFirstProcessedOpSequenceNumber: - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.processedOps.length === 0 ? undefined : this.processedOps[0]!.sequenceNumber, + this.processedOps.length === 0 ? undefined : this.processedOps[0].sequenceNumber, }); } return snapshotSequenceNumber; @@ -411,9 +401,8 @@ export class SerializedStateManager { ".protocol" in baseSnapshot.trees ? baseSnapshot.trees[".protocol"].blobs.attributes : baseSnapshot.blobs[".attributes"]; - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-non-null-assertion - const attributes = JSON.parse(snapshotBlobs[attributesHash!]!); + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const attributes = JSON.parse(snapshotBlobs[attributesHash]); assert( // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access attributes.sequenceNumber === 0, diff --git a/packages/loader/container-loader/src/utils.ts b/packages/loader/container-loader/src/utils.ts index 177aa7c11536..761673b56e0d 100644 --- a/packages/loader/container-loader/src/utils.ts +++ b/packages/loader/container-loader/src/utils.ts @@ -94,12 +94,8 @@ export function tryParseCompatibleResolvedUrl(url: string): IParsedUrl | undefin const match = regex.exec(parsed.pathname); return match?.length === 3 ? { - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - id: match[1]!, - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - path: match[2]!, + id: match[1], + path: match[2], query, // URLSearchParams returns null if the param is not provided. version: parsed.searchParams.get("version") ?? undefined, @@ -151,9 +147,7 @@ function convertSummaryToSnapshotAndBlobs(summary: ISummaryTree): SnapshotWithBl }; const keys = Object.keys(summary.tree); for (const key of keys) { - // TODO change this to use object.entries - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const summaryObject = summary.tree[key]!; + const summaryObject = summary.tree[key]; switch (summaryObject.type) { case SummaryType.Tree: { @@ -286,9 +280,8 @@ export const combineSnapshotTreeAndSnapshotBlobs = ( // Process blobs in the current level for (const [, id] of Object.entries(baseSnapshot.blobs)) { - const snapshot = snapshotBlobs[id]; - if (snapshot !== undefined) { - blobsContents[id] = stringToBuffer(snapshot, "utf8"); + if (snapshotBlobs[id]) { + blobsContents[id] = stringToBuffer(snapshotBlobs[id], "utf8"); } } @@ -427,9 +420,7 @@ export async function getDocumentAttributes( ? tree.trees[".protocol"].blobs.attributes : tree.blobs[".attributes"]; - // Non null asserting here because of the length check above - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const attributes = await readAndParse(storage, attributesHash!); + const attributes = await readAndParse(storage, attributesHash); return attributes; } diff --git a/packages/loader/container-loader/tsconfig.json b/packages/loader/container-loader/tsconfig.json index 9a9e0de4d11b..3ca432805ddb 100644 --- a/packages/loader/container-loader/tsconfig.json +++ b/packages/loader/container-loader/tsconfig.json @@ -5,6 +5,7 @@ "compilerOptions": { "rootDir": "./src", "outDir": "./lib", + "noUncheckedIndexedAccess": false, "exactOptionalPropertyTypes": false, }, } diff --git a/packages/loader/driver-utils/src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts b/packages/loader/driver-utils/src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts index d34d34d150f1..cc96947bb03f 100644 --- a/packages/loader/driver-utils/src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts +++ b/packages/loader/driver-utils/src/adapters/compression/summaryblob/documentStorageServiceSummaryBlobCompressionAdapter.ts @@ -61,9 +61,7 @@ export class DocumentStorageServiceCompressionAdapter extends DocumentStorageSer * @returns `true` if there is a compression markup byte in the blob, otherwise `false`. */ private static hasPrefix(blob: ArrayBufferLike): boolean { - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const firstByte = IsoBuffer.from(blob)[0]!; + const firstByte = IsoBuffer.from(blob)[0]; // eslint-disable-next-line no-bitwise return (firstByte & 0xf0) === 0xb0; } @@ -75,9 +73,8 @@ export class DocumentStorageServiceCompressionAdapter extends DocumentStorageSer private static readAlgorithmFromBlob(blob: ArrayBufferLike): number { return !this.hasPrefix(blob) ? SummaryCompressionAlgorithm.None - : // TODO why are we non null asserting here? - // eslint-disable-next-line no-bitwise, @typescript-eslint/no-non-null-assertion - IsoBuffer.from(blob)[0]! & 0x0f; + : // eslint-disable-next-line no-bitwise + IsoBuffer.from(blob)[0] & 0x0f; } /** @@ -91,9 +88,7 @@ export class DocumentStorageServiceCompressionAdapter extends DocumentStorageSer algorithm: number, ): ArrayBufferLike { if (algorithm === SummaryCompressionAlgorithm.None) { - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const firstByte = IsoBuffer.from(blob)[0]!; + const firstByte = IsoBuffer.from(blob)[0]; // eslint-disable-next-line no-bitwise if ((firstByte & 0xf0) !== 0xb0) { return blob; @@ -296,7 +291,9 @@ export class DocumentStorageServiceCompressionAdapter extends DocumentStorageSer */ private static findMetadataHolderSummary(summary: ISummaryTree): ISummaryTree | undefined { assert(typeof summary === "object", 0x6f7 /* summary must be a non-null object */); - for (const [key, value] of Object.entries(summary.tree)) { + for (const key of Object.keys(summary.tree)) { + const value = summary.tree[key]; + if (Boolean(value) && value.type === SummaryType.Tree) { const found = this.findMetadataHolderSummary(value); if (found) { diff --git a/packages/loader/driver-utils/src/insecureUrlResolver.ts b/packages/loader/driver-utils/src/insecureUrlResolver.ts index b99db3c673ae..5d01685210d8 100644 --- a/packages/loader/driver-utils/src/insecureUrlResolver.ts +++ b/packages/loader/driver-utils/src/insecureUrlResolver.ts @@ -54,14 +54,10 @@ export class InsecureUrlResolver implements IUrlResolver { if (this.isForNodeTest) { const [, documentId, tmpRelativePath] = parsedUrl.pathname.substr(1).split("/"); const relativePath = tmpRelativePath ?? ""; - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return this.resolveHelper(documentId!, relativePath, parsedUrl.search); + return this.resolveHelper(documentId, relativePath, parsedUrl.search); } else if (parsedUrl.host === window.location.host) { const fullPath = parsedUrl.pathname.substr(1); - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const documentId = fullPath.split("/")[0]!; + const documentId = fullPath.split("/")[0]; const documentRelativePath = fullPath.slice(documentId.length); return this.resolveHelper(documentId, documentRelativePath); } else { diff --git a/packages/loader/driver-utils/src/parallelRequests.ts b/packages/loader/driver-utils/src/parallelRequests.ts index 8eecf3b8b853..bc9f9b3ff013 100644 --- a/packages/loader/driver-utils/src/parallelRequests.ts +++ b/packages/loader/driver-utils/src/parallelRequests.ts @@ -595,21 +595,13 @@ export function requestOps( (deltas: ISequencedDocumentMessage[]) => { // Assert continuing and right start. if (lastFetch === undefined) { - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - assert(deltas[0]!.sequenceNumber === fromTotal, 0x26d /* "wrong start" */); + assert(deltas[0].sequenceNumber === fromTotal, 0x26d /* "wrong start" */); } else { - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - assert(deltas[0]!.sequenceNumber === lastFetch + 1, 0x26e /* "wrong start" */); + assert(deltas[0].sequenceNumber === lastFetch + 1, 0x26e /* "wrong start" */); } - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - lastFetch = deltas[deltas.length - 1]!.sequenceNumber; + lastFetch = deltas[deltas.length - 1].sequenceNumber; assert( - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - lastFetch - deltas[0]!.sequenceNumber + 1 === deltas.length, + lastFetch - deltas[0].sequenceNumber + 1 === deltas.length, 0x26f /* "continuous and no duplicates" */, ); length += deltas.length; diff --git a/packages/loader/driver-utils/src/prefetchDocumentStorageService.ts b/packages/loader/driver-utils/src/prefetchDocumentStorageService.ts index 5a5ba9fb7237..e99e5c60fe0c 100644 --- a/packages/loader/driver-utils/src/prefetchDocumentStorageService.ts +++ b/packages/loader/driver-utils/src/prefetchDocumentStorageService.ts @@ -84,19 +84,19 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const blobKey of Object.keys(tree.blobs)) { const blob = tree.blobs[blobKey]; if (blobKey.startsWith(".") || blobKey === "header" || blobKey.startsWith("quorum")) { - if (blob !== null && blob !== undefined) { + if (blob !== null) { // We don't care if the prefetch succeeds void this.cachedRead(blob); } } else if (!blobKey.startsWith("deltas")) { - if (blob !== null && blob !== undefined) { + if (blob !== null) { secondary.push(blob); } } } - for (const [_, snapshot] of Object.entries(tree.trees)) { - this.prefetchTreeCore(snapshot, secondary); + for (const subTree of Object.keys(tree.trees)) { + this.prefetchTreeCore(tree.trees[subTree], secondary); } } } diff --git a/packages/loader/driver-utils/src/protocol/gitHelper.ts b/packages/loader/driver-utils/src/protocol/gitHelper.ts index 7b218a03a1ad..e7f032a34a81 100644 --- a/packages/loader/driver-utils/src/protocol/gitHelper.ts +++ b/packages/loader/driver-utils/src/protocol/gitHelper.ts @@ -79,9 +79,7 @@ export function buildGitTreeHierarchy( const entryPathBase = entryPath.slice(lastIndex + 1); // The flat output is breadth-first so we can assume we see tree nodes prior to their contents - // TODO why are we non null asserting here? - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const node = lookup[entryPathDir]!; + const node = lookup[entryPathDir]; // Add in either the blob or tree if (entry.type === "tree") { diff --git a/packages/loader/driver-utils/tsconfig.json b/packages/loader/driver-utils/tsconfig.json index 9a9e0de4d11b..679025b46439 100644 --- a/packages/loader/driver-utils/tsconfig.json +++ b/packages/loader/driver-utils/tsconfig.json @@ -6,5 +6,6 @@ "rootDir": "./src", "outDir": "./lib", "exactOptionalPropertyTypes": false, + "noUncheckedIndexedAccess": false, }, }