Skip to content

Commit

Permalink
Revert "Enable noUncheckedIndexedAccess for loader packages (microsof…
Browse files Browse the repository at this point in the history
…t#21485)" (microsoft#22380)

This reverts commit 15a9c2b.
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)
  • Loading branch information
RishhiB authored Sep 5, 2024
1 parent c590f48 commit e64c6dd
Show file tree
Hide file tree
Showing 13 changed files with 51 additions and 127 deletions.
12 changes: 3 additions & 9 deletions packages/loader/container-loader/src/connectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 }),
Expand Down
39 changes: 8 additions & 31 deletions packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
},
},
});
Expand Down Expand Up @@ -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) }),
});

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
]);
}

Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]];
Expand Down
24 changes: 6 additions & 18 deletions packages/loader/container-loader/src/deltaManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,22 +357,16 @@ export class DeltaManager<TConnectionManager extends IConnectionManager>

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 */,
);
}
Expand Down Expand Up @@ -898,12 +892,8 @@ export class DeltaManager<TConnectionManager extends IConnectionManager>
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
Expand Down Expand Up @@ -970,9 +960,7 @@ export class DeltaManager<TConnectionManager extends IConnectionManager>
}
}

// 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(
Expand Down
25 changes: 7 additions & 18 deletions packages/loader/container-loader/src/serializedStateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -343,23 +341,17 @@ 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()
) {
// can't refresh latest snapshot until we have processed the ops up to it.
// 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
Expand All @@ -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;
Expand All @@ -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,
Expand Down
21 changes: 6 additions & 15 deletions packages/loader/container-loader/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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");
}
}

Expand Down Expand Up @@ -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<IDocumentAttributes>(storage, attributesHash!);
const attributes = await readAndParse<IDocumentAttributes>(storage, attributesHash);

return attributes;
}
1 change: 1 addition & 0 deletions packages/loader/container-loader/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"compilerOptions": {
"rootDir": "./src",
"outDir": "./lib",
"noUncheckedIndexedAccess": false,
"exactOptionalPropertyTypes": false,
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}

/**
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 2 additions & 6 deletions packages/loader/driver-utils/src/insecureUrlResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 4 additions & 12 deletions packages/loader/driver-utils/src/parallelRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit e64c6dd

Please sign in to comment.