Skip to content

Commit

Permalink
tree: enable strict-boolean-expressions (#14487)
Browse files Browse the repository at this point in the history
## Description

Enable strict-boolean-expressions to make code more readable when
dealing with cases like optional booleans and strings that might be
empty.

Also migrate some test code which was failing these checks to newer apis
(Map and "undefined")
  • Loading branch information
CraigMacomber authored Mar 9, 2023
1 parent 42f38ae commit 41b095a
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 20 deletions.
1 change: 0 additions & 1 deletion packages/dds/tree/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ module.exports = {
rules: {
"@typescript-eslint/no-namespace": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/strict-boolean-expressions": "off",
"@typescript-eslint/explicit-member-accessibility": "error",
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function composeMarkLists<TNodeChange>(
);
while (!queue.isEmpty()) {
const popped = queue.pop();
if (popped.areInverses) {
if (popped.areInverses === true) {
factory.pushOffset(getInputLength(popped.baseMark));
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ function applyMoveEffectsToDest<T>(

assert(effect.modifyAfter === undefined, 0x566 /* Cannot modify move destination */);

if (!effect.shouldRemove) {
if (effect.shouldRemove !== true) {
const newMark: MoveIn | ReturnTo = {
...mark,
count: effect.count ?? mark.count,
Expand Down Expand Up @@ -277,7 +277,7 @@ function applyMoveEffectsToSource<T>(
mark.id,
);
const result: Mark<T>[] = [];
if (!effect.shouldRemove) {
if (effect.shouldRemove !== true) {
const newMark = cloneMark(mark);
newMark.count = effect.count ?? newMark.count;
if (effect.modifyAfter !== undefined) {
Expand Down
4 changes: 2 additions & 2 deletions packages/dds/tree/src/test/domains/json/twitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -637,13 +637,13 @@ export function parseSentencesIntoWords(inputSentences: string[]) {
const spaceSeparatedWords: string[] = inputSentence.split(" ");
spaceSeparatedWords.forEach((potentialWord) => {
const innerWords: string[] = [];
let previousChar: string | null = null;
let previousChar: string | undefined;
let currentWord = "";
for (let i = 0; i < potentialWord.length; i++) {
const currentChar = potentialWord.charAt(i);
if (isEscapeChar(currentChar) || isJapaneseSymbolOrPunctuation(currentChar)) {
if (
(previousChar && !isEscapeChar(previousChar)) ||
(previousChar !== undefined && !isEscapeChar(previousChar)) ||
isJapaneseSymbolOrPunctuation(currentChar)
) {
innerWords.push(`${currentWord}`);
Expand Down
23 changes: 10 additions & 13 deletions packages/dds/tree/src/test/shared-tree/opSize.bench.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { isInPerformanceTestingMode } from "@fluid-tools/benchmark";
import { ISequencedDocumentMessage } from "@fluidframework/protocol-definitions";
import { emptyField, FieldKinds, namedTreeSchema, singleTextCursor } from "../../feature-libraries";
import { ISharedTree } from "../../shared-tree";
import { brand } from "../../util";
import { brand, getOrAddEmptyToMap } from "../../util";
import { ITestTreeProvider, TestTreeProvider } from "../utils";
import {
FieldKey,
Expand Down Expand Up @@ -349,7 +349,7 @@ const getSuccessfulOpByteSize = (
const BENCHMARK_NODE_COUNT = 100;

describe("SharedTree Op Size Benchmarks", () => {
const opsByBenchmarkName: Record<string, ISequencedDocumentMessage[]> = {};
const opsByBenchmarkName: Map<string, ISequencedDocumentMessage[]> = new Map();
let currentBenchmarkName = "";
const currentTestOps: ISequencedDocumentMessage[] = [];

Expand Down Expand Up @@ -412,10 +412,9 @@ describe("SharedTree Op Size Benchmarks", () => {
};

const saveAndResetCurrentOps = () => {
if (!opsByBenchmarkName[currentBenchmarkName]) {
opsByBenchmarkName[currentBenchmarkName] = [];
}
currentTestOps.forEach((op) => opsByBenchmarkName[currentBenchmarkName].push(op));
currentTestOps.forEach((op) =>
getOrAddEmptyToMap(opsByBenchmarkName, currentBenchmarkName).push(op),
);
currentTestOps.length = 0;
};

Expand All @@ -424,22 +423,20 @@ describe("SharedTree Op Size Benchmarks", () => {
};

afterEach(() => {
if (!opsByBenchmarkName[currentBenchmarkName]) {
opsByBenchmarkName[currentBenchmarkName] = [];
}
currentTestOps.forEach((op) => opsByBenchmarkName[currentBenchmarkName].push(op));
currentTestOps.forEach((op) =>
getOrAddEmptyToMap(opsByBenchmarkName, currentBenchmarkName).push(op),
);
currentTestOps.length = 0;
});

after(() => {
const allBenchmarkOpStats: any[] = [];
Object.entries(opsByBenchmarkName).forEach((entry) => {
const [benchmarkName, ops] = entry;
for (const [benchmarkName, ops] of opsByBenchmarkName) {
allBenchmarkOpStats.push({
"Test name": benchmarkName,
...getOperationsStats(ops),
});
});
}
const table = new Table();
allBenchmarkOpStats.forEach((data) => {
Object.keys(data).forEach((key) => table.cell(key, data[key]));
Expand Down
2 changes: 1 addition & 1 deletion packages/dds/tree/src/util/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export function* zipIterables<T, U>(
const iteratorB = iterableB[Symbol.iterator]();
for (
let nextA = iteratorA.next(), nextB = iteratorB.next();
!nextA.done && !nextB.done;
nextA.done !== true && nextB.done !== true;
nextA = iteratorA.next(), nextB = iteratorB.next()
) {
yield [nextA.value, nextB.value];
Expand Down

0 comments on commit 41b095a

Please sign in to comment.