Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Revert "Enable noUncheckedIndexedAccess for DDSes excluding tree and merge-tree (#21420)" and disable noUncheckedIndexedAccess" #23065

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions packages/dds/ink/src/inkCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,24 @@ class Vector {
}

function drawPolygon(context: CanvasRenderingContext2D, points: IPoint[]) {
if (points.length === 0) {
const firstPoint = points[0];
if (firstPoint === undefined) {
return;
}

context.beginPath();
// Move to the first point
context.moveTo(points[0].x, points[0].y);
context.moveTo(firstPoint.x, firstPoint.y);

// Draw the rest of the segments
for (let i = 1; i < points.length; i++) {
context.lineTo(points[i].x, points[i].y);
// Non null asserting, this must exist because the we are iterating through the length of points
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
context.lineTo(points[i]!.x, points[i]!.y);
}

// And then close the shape
context.lineTo(points[0].x, points[0].y);
context.lineTo(firstPoint.x, firstPoint.y);
context.closePath();
context.fill();
}
Expand Down Expand Up @@ -152,7 +155,10 @@ export class InkCanvas {
const strokes = this.model.getStrokes();

// Time of the first operation in stroke 0 is our starting time
const startTime = strokes[0].points[0].time;
const startTime = strokes[0]?.points[0]?.time;
if (startTime === undefined) {
throw new Error("Couldn't get start time");
}
for (const stroke of strokes) {
this.animateStroke(stroke, 0, startTime);
}
Expand Down Expand Up @@ -224,8 +230,12 @@ export class InkCanvas {
}

// Draw the requested stroke
const current = stroke.points[operationIndex];
const previous = stroke.points[Math.max(0, operationIndex - 1)];
// Non null asserting, this must exist because the operationIndex must be less than the length of stroke.points
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const current = stroke.points[operationIndex]!;
// Non null asserting, this must exist because its either the first index or operationIndex minus one which is less than the length of stroke.points
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const previous = stroke.points[Math.max(0, operationIndex - 1)]!;
const time =
operationIndex === 0 ? current.time - startTime : current.time - previous.time;

Expand All @@ -247,7 +257,9 @@ export class InkCanvas {

const strokes = this.model.getStrokes();
for (const stroke of strokes) {
let previous = stroke.points[0];
// Non null asserting since previous would not be used if stroke.points is empty
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
let previous = stroke.points[0]!;
for (const current of stroke.points) {
// For the down, current === previous === stroke.operations[0]
this.drawStrokeSegment(stroke.pen, current, previous);
Expand All @@ -269,7 +281,9 @@ export class InkCanvas {
const stroke = this.model.getStroke(dirtyStrokeId);
// If this is the only point in the stroke, we'll use it for both the start and end of the segment
const prevPoint =
stroke.points[stroke.points.length - (stroke.points.length >= 2 ? 2 : 1)];
// Non null asserting, this must exist its within the length of stroke.points
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
stroke.points[stroke.points.length - (stroke.points.length >= 2 ? 2 : 1)]!;
this.drawStrokeSegment(stroke.pen, prevPoint, operation.point);
}
}
4 changes: 3 additions & 1 deletion packages/dds/ink/src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ export class InkData {
* {@inheritDoc IInk.getStroke}
*/
public getStroke(key: string): IInkStroke {
return this.strokes[this.strokeIndex[key]];
// Non null asserting, it should be okay that a bad key results in an error being thrown
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this.strokes[this.strokeIndex[key]!]!;
}

/**
Expand Down
1 change: 0 additions & 1 deletion packages/dds/ink/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"compilerOptions": {
"rootDir": "./src",
"outDir": "./lib",
"noUncheckedIndexedAccess": false,
"exactOptionalPropertyTypes": false,
},
}
21 changes: 15 additions & 6 deletions packages/dds/map/src/directory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,9 @@ export class SharedDirectory
const nodeList = absolutePath.split(posix.sep);
let start = 1;
while (start < nodeList.length) {
const subDirName = nodeList[start];
// Non null asserting, this loop only runs while start in in the bounds of the array
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const subDirName = nodeList[start]!;
if (currentParent.isSubDirectoryDeletePending(subDirName)) {
return true;
}
Expand Down Expand Up @@ -1492,7 +1494,10 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
dirs: this._subdirectories,
next(): IteratorResult<[string, IDirectory]> {
if (this.index < subdirNames.length) {
const subdirName = subdirNames[this.index++];
// Non null asserting, we've checked that the index is inside the bounds of the array.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const subdirName = subdirNames[this.index]!;
this.index++;
const subdir = this.dirs.get(subdirName);
assert(subdir !== undefined, 0x8ac /* Could not find expected sub-directory. */);
return { value: [subdirName, subdir], done: false };
Expand Down Expand Up @@ -2183,18 +2188,22 @@ class SubDirectory extends TypedEventEmitter<IDirectoryEvents> implements IDirec
): boolean {
if (this.pendingClearMessageIds.length > 0) {
if (local) {
// Remove all pendingMessageIds lower than first pendingClearMessageId.
// Non null asserting, because of pendingClearMessageIds length check above
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const firstPendingClearMessageId = this.pendingClearMessageIds[0]!;
assert(
localOpMetadata !== undefined &&
isKeyEditLocalOpMetadata(localOpMetadata) &&
localOpMetadata.pendingMessageId < this.pendingClearMessageIds[0],
localOpMetadata.pendingMessageId < firstPendingClearMessageId,
0x010 /* "Received out of order storage op when there is an unackd clear message" */,
);
// Remove all pendingMessageIds lower than first pendingClearMessageId.
const lowestPendingClearMessageId = this.pendingClearMessageIds[0];
const pendingKeyMessageIdArray = this.pendingKeys.get(op.key);
if (pendingKeyMessageIdArray !== undefined) {
let index = 0;
while (pendingKeyMessageIdArray[index] < lowestPendingClearMessageId) {
// Non-null asserting because we maintain that the pendingKeyMessageIdArray will only exist if it is non-empty.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
while (pendingKeyMessageIdArray[index]! < firstPendingClearMessageId) {
index += 1;
}
const newPendingKeyMessageId = pendingKeyMessageIdArray.splice(index);
Expand Down
19 changes: 9 additions & 10 deletions packages/dds/map/src/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,23 +202,22 @@ export class SharedMap extends SharedObject<ISharedMapEvents> implements IShared
// and result in non-incremental snapshot.
// This can be improved in the future, without being format breaking change, as loading sequence
// loads all blobs at once and partitioning schema has no impact on that process.
for (const key of Object.keys(data)) {
const value = data[key];
if (value.value && value.value.length >= MinValueSizeSeparateSnapshotBlob) {
for (const [key, { value, type }] of Object.entries(data)) {
if (value && value.length >= MinValueSizeSeparateSnapshotBlob) {
const blobName = `blob${counter}`;
counter++;
blobs.push(blobName);
const content: IMapDataObjectSerializable = {
[key]: {
type: value.type,
value: JSON.parse(value.value) as unknown,
type,
value: JSON.parse(value) as unknown,
},
};
builder.addBlob(blobName, JSON.stringify(content));
} else {
currentSize += value.type.length + 21; // Approximation cost of property header
if (value.value) {
currentSize += value.value.length;
currentSize += type.length + 21; // Approximation cost of property header
if (value) {
currentSize += value.length;
}

if (currentSize > MaxSnapshotBlobSize) {
Expand All @@ -230,8 +229,8 @@ export class SharedMap extends SharedObject<ISharedMapEvents> implements IShared
currentSize = 0;
}
headerBlob[key] = {
type: value.type,
value: value.value === undefined ? undefined : (JSON.parse(value.value) as unknown),
type,
value: value === undefined ? undefined : (JSON.parse(value) as unknown),
};
}
}
Expand Down
4 changes: 3 additions & 1 deletion packages/dds/map/src/mapKernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,9 @@ export class MapKernel {
assert(
localOpMetadata !== undefined &&
isMapKeyLocalOpMetadata(localOpMetadata) &&
localOpMetadata.pendingMessageId < this.pendingClearMessageIds[0],
// Non null asserting, above we checked that the length is greater than 0.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
localOpMetadata.pendingMessageId < this.pendingClearMessageIds[0]!,
0x013 /* "Received out of order op when there is an unackd clear message" */,
);
}
Expand Down
1 change: 0 additions & 1 deletion packages/dds/map/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"compilerOptions": {
"rootDir": "./src",
"outDir": "./lib",
"noUncheckedIndexedAccess": false,
"exactOptionalPropertyTypes": false,
},
}
18 changes: 14 additions & 4 deletions packages/dds/matrix/src/handlecache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ export class HandleCache implements IVectorConsumer<Handle> {
// checking that 'position' is in bounds until 'cacheMiss(..)'. This yields an
// ~40% speedup when the position is in the cache (node v12 x64).

return index < this.handles.length ? this.handles[index] : this.cacheMiss(position);
const handle = this.handles[index];
if (handle !== undefined) {
return handle;
}
return this.cacheMiss(position);
}

/**
Expand All @@ -62,7 +66,9 @@ export class HandleCache implements IVectorConsumer<Handle> {
const index = this.getIndex(position);
if (index < this.handles.length) {
assert(
!isHandleValid(this.handles[index]),
// Non null asserting, above we checked that the index is less than the length.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
!isHandleValid(this.handles[index]!),
0x018 /* "Trying to insert handle into position with already valid handle!" */,
);
this.handles[index] = handle;
Expand Down Expand Up @@ -104,15 +110,19 @@ export class HandleCache implements IVectorConsumer<Handle> {
if (_position < this.start) {
this.handles = [...this.getHandles(_position, this.start), ...this.handles];
this.start = _position;
return this.handles[0];
// TODO why are we non null asserting here?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this.handles[0]!;
} else {
ensureRange(_position, this.vector.getLength());

this.handles = [
...this.handles,
...this.getHandles(this.start + this.handles.length, _position + 1),
];
return this.handles[this.handles.length - 1];
// TODO why are we non null asserting here?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this.handles[this.handles.length - 1]!;
}
}

Expand Down
8 changes: 6 additions & 2 deletions packages/dds/matrix/src/matrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,9 @@ export class SharedMatrix<T = any>
const rowCount = inserted.cachedLength;
for (let row = rowStart; row < rowStart + rowCount; row++, rowHandle++) {
for (let col = 0; col < this.colCount; col++) {
const colHandle = this.colHandles.getHandle(col);
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const colHandle = this.colHandles.getHandle(col)!;
const value = this.cells.getCell(rowHandle, colHandle);
if (this.isAttached() && value !== undefined && value !== null) {
this.sendSetCellOp(row, col, value, rowHandle, colHandle);
Expand All @@ -645,7 +647,9 @@ export class SharedMatrix<T = any>
const colCount = inserted.cachedLength;
for (let col = colStart; col < colStart + colCount; col++, colHandle++) {
for (let row = 0; row < this.rowCount; row++) {
const rowHandle = this.rowHandles.getHandle(row);
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const rowHandle = this.rowHandles.getHandle(row)!;
const value = this.cells.getCell(rowHandle, colHandle);
if (this.isAttached() && value !== undefined && value !== null) {
this.sendSetCellOp(row, col, value, rowHandle, colHandle);
Expand Down
4 changes: 3 additions & 1 deletion packages/dds/matrix/src/permutationvector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ export class PermutationVector extends Client {
0x027 /* "Trying to get handle of out-of-bounds position!" */,
);

return this.handleCache.getHandle(pos);
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this.handleCache.getHandle(pos)!;
}

public getAllocatedHandle(pos: number): Handle {
Expand Down
4 changes: 3 additions & 1 deletion packages/dds/matrix/src/sparsearray2d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ const byte3 = (x32: number): number => (x32 << 24) >>> 24;
// Given a uint16 returns the corresponding uint32 integer where 0s are
// interleaved between the original bits. (e.g., 1111... -> 01010101...).
const interlaceBitsX16 = (x16: number): number =>
(x8ToInterlacedX16[byte2(x16)] << 16) | x8ToInterlacedX16[byte3(x16)];
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
(x8ToInterlacedX16[byte2(x16)]! << 16) | x8ToInterlacedX16[byte3(x16)]!;

const r0ToMorton16 = (row: number): number => (interlaceBitsX16(row) << 1) >>> 0;
const c0ToMorton16 = (col: number): number => interlaceBitsX16(col) >>> 0;
Expand Down
8 changes: 6 additions & 2 deletions packages/dds/matrix/src/undoprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ export class VectorUndoProvider {
try {
if (removedTrackingGroup !== undefined) {
while (removedTrackingGroup.size > 0) {
const tracked = removedTrackingGroup.tracked[0];
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const tracked = removedTrackingGroup.tracked[0]!;
removedTrackingGroup.unlink(tracked);
// if there are groups tracked, this in a revert of a remove.
// this means we are about to re-insert the row/column
Expand All @@ -131,7 +133,9 @@ export class VectorUndoProvider {
discard: (): void => {
if (removedTrackingGroup !== undefined) {
while (removedTrackingGroup.size > 0) {
removedTrackingGroup.unlink(removedTrackingGroup.tracked[0]);
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
removedTrackingGroup.unlink(removedTrackingGroup.tracked[0]!);
}
}
discardMergeTreeDeltaRevertible(revertibles);
Expand Down
1 change: 0 additions & 1 deletion packages/dds/matrix/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"rootDir": "./src",
"outDir": "./lib",
"noImplicitAny": true,
"noUncheckedIndexedAccess": false,
"exactOptionalPropertyTypes": false,
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ export class ConsensusRegisterCollection<T>
}

// Remove versions that were known to the remote client at the time of write
while (data.versions.length > 0 && refSeq >= data.versions[0].sequenceNumber) {
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
while (data.versions.length > 0 && refSeq >= data.versions[0]!.sequenceNumber) {
data.versions.shift();
}

Expand All @@ -318,7 +320,9 @@ export class ConsensusRegisterCollection<T>
} else if (data.versions.length > 0) {
assert(
// seqNum should always be increasing, except for the case of grouped batches (seqNum will be the same)
sequenceNumber >= data.versions[data.versions.length - 1].sequenceNumber,
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
sequenceNumber >= data.versions[data.versions.length - 1]!.sequenceNumber,
0x071 /* "Versions should naturally be ordered by sequenceNumber" */,
);
}
Expand Down
1 change: 0 additions & 1 deletion packages/dds/register-collection/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"compilerOptions": {
"rootDir": "./src",
"outDir": "./lib",
"noUncheckedIndexedAccess": false,
"exactOptionalPropertyTypes": false,
},
}
4 changes: 3 additions & 1 deletion packages/dds/sequence/src/intervalCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,9 @@ class IntervalCollectionIterator<TInterval extends ISerializableInterval>
public next(): IteratorResult<TInterval> {
if (this.index < this.results.length) {
return {
value: this.results[this.index++],
// TODO Non null asserting, why is this not null?
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
value: this.results[this.index++]!,
done: false,
};
}
Expand Down
Loading
Loading