Skip to content

Commit

Permalink
Enable strict null in Merge Tree Tests (#9955)
Browse files Browse the repository at this point in the history 
Merge Tree tests didn't have strict nulls enabled, which kept biting me when making changes. This PR enabled them, but adds lots of non-null assertions, and a couple any casts to avoid issues in existing cases. While the workarounds are not always great it seems better to have strict nulls enabled for future work.
  • Loading branch information
@anthony-murphy
anthony-murphy authored Apr 19, 2022
1 parent ebe4ef9 commit ae93fdb
Show file tree
Hide file tree
Showing 30 changed files with 320 additions and 304 deletions.
4 changes: 2 additions & 2 deletions api-report/merge-tree.api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class Client {
addLocalReference(lref: LocalReference): void;
// (undocumented)
addLongClientId(longClientId: string): void;
annotateMarker(marker: Marker, props: PropertySet, combiningOp: ICombiningOp): IMergeTreeAnnotateMsg | undefined;
annotateMarker(marker: Marker, props: PropertySet, combiningOp?: ICombiningOp): IMergeTreeAnnotateMsg | undefined;
annotateMarkerNotifyConsensus(marker: Marker, props: PropertySet, consensusCallback: (m: Marker) => void): IMergeTreeAnnotateMsg | undefined;
annotateRangeLocal(start: number, end: number, props: PropertySet, combiningOp: ICombiningOp | undefined): IMergeTreeAnnotateMsg | undefined;
// (undocumented)
Expand Down Expand Up @@ -253,7 +253,7 @@ export const compareStrings: (a: string, b: string) => number;
export type ConflictAction<TKey, TData> = (key: TKey, currentKey: TKey, data: TData, currentData: TData) => QProperty<TKey, TData>;

// @public
export function createAnnotateMarkerOp(marker: Marker, props: PropertySet, combiningOp: ICombiningOp): IMergeTreeAnnotateMsg | undefined;
export function createAnnotateMarkerOp(marker: Marker, props: PropertySet, combiningOp?: ICombiningOp): IMergeTreeAnnotateMsg | undefined;

// @public
export function createAnnotateRangeOp(start: number, end: number, props: PropertySet, combiningOp: ICombiningOp | undefined): IMergeTreeAnnotateMsg;
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class Client {
public annotateMarker(
marker: Marker,
props: PropertySet,
combiningOp: ICombiningOp): IMergeTreeAnnotateMsg | undefined {
combiningOp?: ICombiningOp): IMergeTreeAnnotateMsg | undefined {
const annotateOp =
createAnnotateMarkerOp(marker, props, combiningOp)!;

Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/localReference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export class LocalReferenceCollection {
private readonly segment: ISegment,
initialRefsByfOffset = new Array<IRefsAtOffset | undefined>(segment.cachedLength)) {
// Since javascript arrays are sparse the above won't populate any of the
// indicies, but it will ensure the length property of the array matches
// indices, but it will ensure the length property of the array matches
// the length of the segment.
this.refsByOffset = initialRefsByfOffset;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/mergeTree.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/
/* eslint-disable max-lines */

/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* eslint-disable @typescript-eslint/consistent-type-assertions */

Expand Down
2 changes: 1 addition & 1 deletion packages/dds/merge-tree/src/opBuilder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { PropertySet } from "./properties";
* @returns The annotate op
*/
export function createAnnotateMarkerOp(
marker: Marker, props: PropertySet, combiningOp: ICombiningOp): IMergeTreeAnnotateMsg | undefined {
marker: Marker, props: PropertySet, combiningOp?: ICombiningOp): IMergeTreeAnnotateMsg | undefined {
const id = marker.getId();
if (!id) {
return undefined;
Expand Down
60 changes: 30 additions & 30 deletions packages/dds/merge-tree/src/test/beastTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/

/* eslint-disable @typescript-eslint/consistent-type-assertions, max-len, no-bitwise */
/* eslint-disable @typescript-eslint/no-unnecessary-type-assertion */
/* eslint-disable @typescript-eslint/no-non-null-assertion */

import { strict as assert } from "assert";
import fs from "fs";
Expand Down Expand Up @@ -63,11 +65,9 @@ function LinearDictionary<TKey, TData>(compareKeys: KeyComparer<TKey>): SortedDi
if (props.length !== 0) { return; }

if (_start === undefined) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
_start = min()!.key;
}
if (_end === undefined) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
_end = max()!.key;
}
for (let i = 0, len = props.length; i < len; i++) {
Expand Down Expand Up @@ -151,8 +151,8 @@ const compareStrings = (a: string, b: string) => a.localeCompare(b);

const compareNumbers = (a: number, b: number) => a - b;

function printStringProperty(p: Property<string, string>) {
log(`[${p.key}, ${p.data}]`);
function printStringProperty(p?: Property<string, string>) {
log(`[${p?.key}, ${p?.data}]`);
return true;
}

Expand Down Expand Up @@ -327,7 +327,7 @@ function checkMarkRemoveMergeTree(mergeTree: MergeTree, start: number, end: numb
const origText = helper.getText(UniversalSequenceNumber, LocalClientId);
const checkText = editFlat(origText, start, end - start);
const clockStart = clock();
mergeTree.markRangeRemoved(start, end, UniversalSequenceNumber, LocalClientId, UniversalSequenceNumber, false, undefined);
mergeTree.markRangeRemoved(start, end, UniversalSequenceNumber, LocalClientId, UniversalSequenceNumber, false, {op: createRemoveRangeOp(start, end)});
accumTime += elapsedMicroseconds(clockStart);
const updatedText = helper.getText(UniversalSequenceNumber, LocalClientId);
const result = (checkText === updatedText);
Expand All @@ -352,7 +352,7 @@ export function mergeTreeTest1() {
checkInsertMergeTree(mergeTree, 4, makeCollabTextSegment("fi"));
mergeTree.map({ leaf: printTextSegment }, UniversalSequenceNumber, LocalClientId, undefined);
const segoff = mergeTree.getContainingSegment(4, UniversalSequenceNumber, LocalClientId);
log(mergeTree.getPosition(segoff.segment, UniversalSequenceNumber, LocalClientId));
log(mergeTree.getPosition(segoff.segment!, UniversalSequenceNumber, LocalClientId));
log(new MergeTreeTextHelper(mergeTree).getText(UniversalSequenceNumber, LocalClientId));
log(mergeTree.toString());
TestPack().firstTest();
Expand Down Expand Up @@ -406,7 +406,7 @@ export function mergeTreeLargeTest() {
const pos = random.integer(0, preLen)(mt);
// Log(itree.toString());
const clockStart = clock();
mergeTree.markRangeRemoved(pos, pos + dlen, UniversalSequenceNumber, LocalClientId, UniversalSequenceNumber, false, undefined);
mergeTree.markRangeRemoved(pos, pos + dlen, UniversalSequenceNumber, LocalClientId, UniversalSequenceNumber, false, undefined as any);
accumTime += elapsedMicroseconds(clockStart);

if ((i > 0) && (0 === (i % 50000))) {
Expand Down Expand Up @@ -625,7 +625,7 @@ export function TestPack(verbose = true) {
const aveTime = (client.accumTime / client.accumOps).toFixed(1);
const aveLocalTime = (client.localTime / client.localOps).toFixed(1);
const stats = client.mergeTree.getStats();
const windowTime = stats.windowTime;
const windowTime = stats.windowTime!;
const packTime = stats.packTime;
const aveWindowTime = ((windowTime || 0) / (client.accumOps)).toFixed(1);
const avePackTime = ((packTime || 0) / (client.accumOps)).toFixed(1);
Expand Down Expand Up @@ -765,10 +765,10 @@ export function TestPack(verbose = true) {
if (includeMarkers) {
const insertMarkerOp = client.insertMarkerLocal(pos, ReferenceType.Tile,
{ [reservedTileLabelsKey]: "test" });
server.enqueueMsg(client.makeOpMessage(insertMarkerOp, UnassignedSequenceNumber));
server.enqueueMsg(client.makeOpMessage(insertMarkerOp!, UnassignedSequenceNumber));
}
const insertTextOp = client.insertTextLocal(pos, text);
server.enqueueMsg(client.makeOpMessage(insertTextOp, UnassignedSequenceNumber));
server.enqueueMsg(client.makeOpMessage(insertTextOp!, UnassignedSequenceNumber));

if (TestClient.useCheckQ) {
client.enqueueTestString();
Expand All @@ -780,7 +780,7 @@ export function TestPack(verbose = true) {
const preLen = client.getLength();
const pos = random.integer(0, preLen)(mt);
const op = client.removeRangeLocal(pos, pos + dlen);
server.enqueueMsg(client.makeOpMessage(op));
server.enqueueMsg(client.makeOpMessage(op!));
if (TestClient.useCheckQ) {
client.enqueueTestString();
}
Expand All @@ -792,7 +792,7 @@ export function TestPack(verbose = true) {
const removeStart = word1.pos;
const removeEnd = removeStart + word1.text.length;
const removeOp = client.removeRangeLocal(removeStart, removeEnd);
server.enqueueMsg(client.makeOpMessage(removeOp, UnassignedSequenceNumber));
server.enqueueMsg(client.makeOpMessage(removeOp!, UnassignedSequenceNumber));
if (TestClient.useCheckQ) {
client.enqueueTestString();
}
Expand All @@ -802,7 +802,7 @@ export function TestPack(verbose = true) {
}
const pos = word2.pos + word2.text.length;
const insertOp = client.insertTextLocal(pos, word1.text);
server.enqueueMsg(client.makeOpMessage(insertOp, UnassignedSequenceNumber));
server.enqueueMsg(client.makeOpMessage(insertOp!, UnassignedSequenceNumber));

if (TestClient.useCheckQ) {
client.enqueueTestString();
Expand Down Expand Up @@ -1065,13 +1065,13 @@ export function TestPack(verbose = true) {
const preLen = cliA.getLength();
const pos = random.integer(0, preLen)(mt);

const msg = cliA.makeOpMessage(cliA.insertTextLocal(pos, text), sequenceNumber++);
const msg = cliA.makeOpMessage(cliA.insertTextLocal(pos, text)!, sequenceNumber++);
msg.minimumSequenceNumber = min;
cliAMsgs.push(msg);
cliB.applyMsg(msg);
}
for (let k = firstSeq; k < sequenceNumber; k++) {
cliA.applyMsg(cliAMsgs.shift());
cliA.applyMsg(cliAMsgs.shift()!);
}
if (checkTextMatch(sequenceNumber - 1)) {
return true;
Expand All @@ -1088,13 +1088,13 @@ export function TestPack(verbose = true) {
const text = randomString(textLen, String.fromCharCode(zedCode + (sequenceNumber % 50)));
const preLen = cliB.getLength();
const pos = random.integer(0, preLen)(mt);
const msg = cliB.makeOpMessage(cliB.insertTextLocal(pos, text), sequenceNumber++);
const msg = cliB.makeOpMessage(cliB.insertTextLocal(pos, text)!, sequenceNumber++);
msg.minimumSequenceNumber = min;
cliBMsgs.push(msg);
cliA.applyMsg(msg);
}
for (let k = firstSeq; k < sequenceNumber; k++) {
cliB.applyMsg(cliBMsgs.shift());
cliB.applyMsg(cliBMsgs.shift()!);
}
if (checkTextMatch(sequenceNumber - 1)) {
return true;
Expand All @@ -1115,13 +1115,13 @@ export function TestPack(verbose = true) {
const dlen = randTextLength();
const preLen = cliA.getLength();
const pos = random.integer(0, preLen)(mt);
const msg = cliA.makeOpMessage(cliA.removeRangeLocal(pos, pos + dlen), sequenceNumber++);
const msg = cliA.makeOpMessage(cliA.removeRangeLocal(pos, pos + dlen)!, sequenceNumber++);
msg.minimumSequenceNumber = min;
cliAMsgs.push(msg);
cliB.applyMsg(msg);
}
for (let k = firstSeq; k < sequenceNumber; k++) {
cliA.applyMsg(cliAMsgs.shift());
cliA.applyMsg(cliAMsgs.shift()!);
}
if (checkTextMatch(sequenceNumber - 1)) {
return true;
Expand All @@ -1137,13 +1137,13 @@ export function TestPack(verbose = true) {
const dlen = randTextLength();
const preLen = cliB.getLength() - 1;
const pos = random.integer(0, preLen)(mt);
const msg = cliB.makeOpMessage(cliB.removeRangeLocal(pos, pos + dlen), sequenceNumber++);
const msg = cliB.makeOpMessage(cliB.removeRangeLocal(pos, pos + dlen)!, sequenceNumber++);
msg.minimumSequenceNumber = min;
cliBMsgs.push(msg);
cliA.applyMsg(msg);
}
for (let k = firstSeq; k < sequenceNumber; k++) {
cliB.applyMsg(cliBMsgs.shift());
cliB.applyMsg(cliBMsgs.shift()!);
}
if (checkTextMatch(sequenceNumber - 1)) {
return true;
Expand Down Expand Up @@ -1326,7 +1326,7 @@ export function TestPack(verbose = true) {
}
const removeOp = cli.removeRangeLocal(3, 5);
cli.applyMsg(cli.makeOpMessage(createRemoveRangeOp(3, 6), 10, 9, "2"));
cli.applyMsg(cli.makeOpMessage(removeOp, 11));
cli.applyMsg(cli.makeOpMessage(removeOp!, 11));
if (verbose) {
log(cli.mergeTree.toString());
for (let clientId = 0; clientId < 4; clientId++) {
Expand Down Expand Up @@ -1402,7 +1402,7 @@ function tst() {
function addCorpus(_corpusContent: string, _corpusTree: TST<number>) {
let count = 0;
const re = /\b\w+\b/g;
let result: RegExpExecArray;
let result: RegExpExecArray | null;
do {
result = re.exec(_corpusContent);
if (result) {
Expand Down Expand Up @@ -1496,7 +1496,7 @@ export type DocumentNode = string | DocumentTree;
export class DocumentTree {
pos = 0;
ids = { box: 0, row: 0 };
id: string;
id: string | undefined;
static randPack = new RandomPack();

constructor(public name: string, public children: DocumentNode[]) {
Expand All @@ -1508,7 +1508,7 @@ export class DocumentTree {
client.insertTextLocal(this.pos, text);
this.pos += text.length;
} else {
let id: number;
let id: number | undefined;
if (docNode.name === "pg") {
client.insertMarkerLocal(this.pos, ReferenceType.Tile,
{
Expand Down Expand Up @@ -1538,7 +1538,7 @@ export class DocumentTree {
this.addToMergeTree(client, child);
}
if (docNode.name !== "pg") {
const etrid = `end-${docNode.name}${id.toString()}`;
const etrid = `end-${docNode.name}${id?.toString()}`;
client.insertMarkerLocal(this.pos, ReferenceType.NestEnd,
{
[reservedMarkerIdKey]: etrid,
Expand Down Expand Up @@ -1633,12 +1633,12 @@ export class DocumentTree {
};

let prevPos = -1;
let prevChild: DocumentNode;
let prevChild: DocumentNode | undefined;

// log(client.mergeTree.toString());
for (const rootChild of this.children) {
if (prevPos >= 0) {
if ((typeof prevChild !== "string") && (prevChild.name === "row")) {
if ((typeof prevChild !== "string") && (prevChild?.name === "row")) {
const id = prevChild.id;
const endId = `end-${id}`;
const endRowMarker = <Marker>client.getMarkerFromId(endId);
Expand Down Expand Up @@ -1754,7 +1754,7 @@ function findReplacePerf(filename: string) {
client.getClientId(),
1,
false,
undefined);
undefined as any);
insertText(
client.mergeTree,
pos + i,
Expand All @@ -1767,7 +1767,7 @@ function findReplacePerf(filename: string) {
pos = pos + i + 3;
cReplaces++;
} else {
pos += (curSeg.cachedLength - curSegOff.offset);
pos += (curSeg!.cachedLength - curSegOff!.offset!);
}
}
}
Expand Down
Loading

0 comments on commit ae93fdb

Please sign in to comment.