From b7bc713b29344bf94b39b5b8729c0b03fc3f859f Mon Sep 17 00:00:00 2001 From: Rishhi Balakrishnan <107130183+RishhiB@users.noreply.github.com> Date: Fri, 31 Jan 2025 16:29:10 +0000 Subject: [PATCH 1/5] feat(map): Add no-unchecked-record-access rule --- packages/dds/map/.eslintrc.cjs | 3 +-- .../dds/map/src/test/mocha/directory.spec.ts | 20 +++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/dds/map/.eslintrc.cjs b/packages/dds/map/.eslintrc.cjs index c3fd19a9ba22..c4dc95a3abac 100644 --- a/packages/dds/map/.eslintrc.cjs +++ b/packages/dds/map/.eslintrc.cjs @@ -13,8 +13,7 @@ module.exports = { "@typescript-eslint/strict-boolean-expressions": "off", // TODO: consider re-enabling once we have addressed how this rule conflicts with our error codes. - "unicorn/numeric-separators-style": "off", - "@fluid-internal/fluid/no-unchecked-record-access": "warn", + "unicorn/numeric-separators-style": "off" }, overrides: [ { diff --git a/packages/dds/map/src/test/mocha/directory.spec.ts b/packages/dds/map/src/test/mocha/directory.spec.ts index 7b8d46a87b2f..86daac63b7be 100644 --- a/packages/dds/map/src/test/mocha/directory.spec.ts +++ b/packages/dds/map/src/test/mocha/directory.spec.ts @@ -104,12 +104,12 @@ function serialize(directory1: ISharedDirectory): string { assert.strictEqual(summaryObjectKeys.length, 1, "summary tree should only have one blob"); assert.strictEqual(summaryObjectKeys[0], "header", "summary should have a header blob"); assert.strictEqual( - summaryTree.tree.header.type, + summaryTree.tree.header?.type, SummaryType.Blob, "header is not of SummaryType.Blob", ); - const content = summaryTree.tree.header.content as string; + const content = summaryTree.tree.header?.content as string; return JSON.stringify((JSON.parse(content) as IDirectoryNewStorageFormat).content); } @@ -1725,15 +1725,15 @@ describe("Directory", () => { const fooSubDirIterator = fooSubDir.entries(); const fooSubDirResult1 = fooSubDirIterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult1.value[0], "testKey"); + assert.equal(fooSubDirResult1.value?.[0], "testKey"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult1.value[1], "testValue"); + assert.equal(fooSubDirResult1.value?.[1], "testValue"); assert.equal(fooSubDirResult1.done, false); const fooSubDirResult2 = fooSubDirIterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult2.value[0], "testKey2"); + assert.equal(fooSubDirResult2.value?.[0], "testKey2"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult2.value[1], "testValue2"); + assert.equal(fooSubDirResult2.value?.[1], "testValue2"); assert.equal(fooSubDirResult2.done, false); const fooSubDirResult3 = fooSubDirIterator.next(); assert.equal(fooSubDirResult3.value, undefined); @@ -1755,15 +1755,15 @@ describe("Directory", () => { const fooSubDir2Iterator = fooSubDir2.entries(); const fooSubDir2Result1 = fooSubDir2Iterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result1.value[0], "testKey"); + assert.equal(fooSubDir2Result1.value?.[0], "testKey"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result1.value[1], "testValue"); + assert.equal(fooSubDir2Result1.value?.[1], "testValue"); assert.equal(fooSubDir2Result1.done, false); const fooSubDir2Result2 = fooSubDir2Iterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result2.value[0], "testKey2"); + assert.equal(fooSubDir2Result2.value?.[0], "testKey2"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result2.value[1], "testValue2"); + assert.equal(fooSubDir2Result2.value?.[1], "testValue2"); assert.equal(fooSubDir2Result2.done, false); const fooSubDir2Result3 = fooSubDir2Iterator.next(); assert.equal(fooSubDir2Result3.value, undefined); From 92334a053b584a46a513002b673457f28a1c5c6c Mon Sep 17 00:00:00 2001 From: Rishhi Balakrishnan <107130183+RishhiB@users.noreply.github.com> Date: Fri, 31 Jan 2025 16:29:21 +0000 Subject: [PATCH 2/5] feat(merge-tree): Add no-unchecked-record-access rule --- packages/dds/merge-tree/.eslintrc.cjs | 3 +-- packages/dds/merge-tree/src/properties.ts | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/dds/merge-tree/.eslintrc.cjs b/packages/dds/merge-tree/.eslintrc.cjs index fd0c39169dd0..d9eb448e78cb 100644 --- a/packages/dds/merge-tree/.eslintrc.cjs +++ b/packages/dds/merge-tree/.eslintrc.cjs @@ -14,7 +14,6 @@ module.exports = { "keyword-spacing": "off", // Off because it conflicts with typescript-formatter "no-case-declarations": "off", "prefer-arrow/prefer-arrow-functions": "off", - "unicorn/no-useless-spread": "off", // Off because it generates incorrect code in autofixes and cannot distinguish useful copies of arrays from useless ones - "@fluid-internal/fluid/no-unchecked-record-access": "warn", + "unicorn/no-useless-spread": "off" // Off because it generates incorrect code in autofixes and cannot distinguish useful copies of arrays from useless ones }, }; diff --git a/packages/dds/merge-tree/src/properties.ts b/packages/dds/merge-tree/src/properties.ts index 72e382732d75..d3cb1a4e5ef8 100644 --- a/packages/dds/merge-tree/src/properties.ts +++ b/packages/dds/merge-tree/src/properties.ts @@ -122,6 +122,7 @@ export function extendIfUndefined( // eslint-disable-next-line no-restricted-syntax for (const key in extension) { if (base[key] === undefined) { + // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access base[key] = extension[key]; } } From 966bd3417b896baef92fe646e73e04e3ad32977b Mon Sep 17 00:00:00 2001 From: Rishhi Balakrishnan <107130183+RishhiB@users.noreply.github.com> Date: Fri, 31 Jan 2025 16:29:30 +0000 Subject: [PATCH 3/5] feat(driver-utils): Add no-unchecked-record-access rule --- packages/loader/driver-utils/.eslintrc.cjs | 3 +-- packages/loader/driver-utils/src/protocol/gitHelper.ts | 2 +- .../driver-utils/src/test/summaryCompresssionTester.spec.ts | 3 +++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/loader/driver-utils/.eslintrc.cjs b/packages/loader/driver-utils/.eslintrc.cjs index 99682a2f1793..379bb4211526 100644 --- a/packages/loader/driver-utils/.eslintrc.cjs +++ b/packages/loader/driver-utils/.eslintrc.cjs @@ -12,8 +12,7 @@ module.exports = { project: ["./tsconfig.json", "./src/test/tsconfig.json"], }, rules: { - "import/no-nodejs-modules": ["error"], - "@fluid-internal/fluid/no-unchecked-record-access": "warn", + "import/no-nodejs-modules": ["error"] }, overrides: [ { diff --git a/packages/loader/driver-utils/src/protocol/gitHelper.ts b/packages/loader/driver-utils/src/protocol/gitHelper.ts index e7f032a34a81..2f7333e5d3eb 100644 --- a/packages/loader/driver-utils/src/protocol/gitHelper.ts +++ b/packages/loader/driver-utils/src/protocol/gitHelper.ts @@ -79,7 +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 - const node = lookup[entryPathDir]; + const node: ISnapshotTreeEx | undefined = lookup[entryPathDir]; // Add in either the blob or tree if (entry.type === "tree") { diff --git a/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts b/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts index dd62b57e4094..375caf9f9c89 100644 --- a/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts +++ b/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts @@ -63,6 +63,7 @@ function cloneSummary(): ISummaryTree { */ function generateSummaryWithContent(contentSize: number) { const summary = cloneSummary(); + // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access const header = ( ( ((summary.tree[".channels"] as ISummaryTree).tree.rootDOId as ISummaryTree).tree[ @@ -85,6 +86,7 @@ function generateSummaryWithContent(contentSize: number) { function generateSummaryWithBinaryContent(startsWith: number, contentSize: number) { const summary = cloneSummary(); + // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access const header = ( ( ((summary.tree[".channels"] as ISummaryTree).tree.rootDOId as ISummaryTree).tree[ @@ -621,6 +623,7 @@ function getHeaderContent(summary: ISummaryTree) { } function getHeader(summary: ISummaryTree) { + // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access return getHeaderHolder(summary).tree.header; } From 37d95ae4210ec9ed98d144f84377136b35dd074e Mon Sep 17 00:00:00 2001 From: Rishhi Balakrishnan <107130183+RishhiB@users.noreply.github.com> Date: Fri, 31 Jan 2025 16:59:07 +0000 Subject: [PATCH 4/5] policy-check --- packages/dds/map/.eslintrc.cjs | 3 ++- .../dds/map/src/test/mocha/directory.spec.ts | 20 +++++++++---------- packages/dds/merge-tree/.eslintrc.cjs | 3 ++- packages/dds/merge-tree/src/properties.ts | 1 - packages/loader/driver-utils/.eslintrc.cjs | 2 +- 5 files changed, 15 insertions(+), 14 deletions(-) diff --git a/packages/dds/map/.eslintrc.cjs b/packages/dds/map/.eslintrc.cjs index c4dc95a3abac..c3fd19a9ba22 100644 --- a/packages/dds/map/.eslintrc.cjs +++ b/packages/dds/map/.eslintrc.cjs @@ -13,7 +13,8 @@ module.exports = { "@typescript-eslint/strict-boolean-expressions": "off", // TODO: consider re-enabling once we have addressed how this rule conflicts with our error codes. - "unicorn/numeric-separators-style": "off" + "unicorn/numeric-separators-style": "off", + "@fluid-internal/fluid/no-unchecked-record-access": "warn", }, overrides: [ { diff --git a/packages/dds/map/src/test/mocha/directory.spec.ts b/packages/dds/map/src/test/mocha/directory.spec.ts index 86daac63b7be..7b8d46a87b2f 100644 --- a/packages/dds/map/src/test/mocha/directory.spec.ts +++ b/packages/dds/map/src/test/mocha/directory.spec.ts @@ -104,12 +104,12 @@ function serialize(directory1: ISharedDirectory): string { assert.strictEqual(summaryObjectKeys.length, 1, "summary tree should only have one blob"); assert.strictEqual(summaryObjectKeys[0], "header", "summary should have a header blob"); assert.strictEqual( - summaryTree.tree.header?.type, + summaryTree.tree.header.type, SummaryType.Blob, "header is not of SummaryType.Blob", ); - const content = summaryTree.tree.header?.content as string; + const content = summaryTree.tree.header.content as string; return JSON.stringify((JSON.parse(content) as IDirectoryNewStorageFormat).content); } @@ -1725,15 +1725,15 @@ describe("Directory", () => { const fooSubDirIterator = fooSubDir.entries(); const fooSubDirResult1 = fooSubDirIterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult1.value?.[0], "testKey"); + assert.equal(fooSubDirResult1.value[0], "testKey"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult1.value?.[1], "testValue"); + assert.equal(fooSubDirResult1.value[1], "testValue"); assert.equal(fooSubDirResult1.done, false); const fooSubDirResult2 = fooSubDirIterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult2.value?.[0], "testKey2"); + assert.equal(fooSubDirResult2.value[0], "testKey2"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDirResult2.value?.[1], "testValue2"); + assert.equal(fooSubDirResult2.value[1], "testValue2"); assert.equal(fooSubDirResult2.done, false); const fooSubDirResult3 = fooSubDirIterator.next(); assert.equal(fooSubDirResult3.value, undefined); @@ -1755,15 +1755,15 @@ describe("Directory", () => { const fooSubDir2Iterator = fooSubDir2.entries(); const fooSubDir2Result1 = fooSubDir2Iterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result1.value?.[0], "testKey"); + assert.equal(fooSubDir2Result1.value[0], "testKey"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result1.value?.[1], "testValue"); + assert.equal(fooSubDir2Result1.value[1], "testValue"); assert.equal(fooSubDir2Result1.done, false); const fooSubDir2Result2 = fooSubDir2Iterator.next(); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result2.value?.[0], "testKey2"); + assert.equal(fooSubDir2Result2.value[0], "testKey2"); // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - assert.equal(fooSubDir2Result2.value?.[1], "testValue2"); + assert.equal(fooSubDir2Result2.value[1], "testValue2"); assert.equal(fooSubDir2Result2.done, false); const fooSubDir2Result3 = fooSubDir2Iterator.next(); assert.equal(fooSubDir2Result3.value, undefined); diff --git a/packages/dds/merge-tree/.eslintrc.cjs b/packages/dds/merge-tree/.eslintrc.cjs index d9eb448e78cb..fd0c39169dd0 100644 --- a/packages/dds/merge-tree/.eslintrc.cjs +++ b/packages/dds/merge-tree/.eslintrc.cjs @@ -14,6 +14,7 @@ module.exports = { "keyword-spacing": "off", // Off because it conflicts with typescript-formatter "no-case-declarations": "off", "prefer-arrow/prefer-arrow-functions": "off", - "unicorn/no-useless-spread": "off" // Off because it generates incorrect code in autofixes and cannot distinguish useful copies of arrays from useless ones + "unicorn/no-useless-spread": "off", // Off because it generates incorrect code in autofixes and cannot distinguish useful copies of arrays from useless ones + "@fluid-internal/fluid/no-unchecked-record-access": "warn", }, }; diff --git a/packages/dds/merge-tree/src/properties.ts b/packages/dds/merge-tree/src/properties.ts index d3cb1a4e5ef8..72e382732d75 100644 --- a/packages/dds/merge-tree/src/properties.ts +++ b/packages/dds/merge-tree/src/properties.ts @@ -122,7 +122,6 @@ export function extendIfUndefined( // eslint-disable-next-line no-restricted-syntax for (const key in extension) { if (base[key] === undefined) { - // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access base[key] = extension[key]; } } diff --git a/packages/loader/driver-utils/.eslintrc.cjs b/packages/loader/driver-utils/.eslintrc.cjs index 379bb4211526..c27d8e1558b3 100644 --- a/packages/loader/driver-utils/.eslintrc.cjs +++ b/packages/loader/driver-utils/.eslintrc.cjs @@ -12,7 +12,7 @@ module.exports = { project: ["./tsconfig.json", "./src/test/tsconfig.json"], }, rules: { - "import/no-nodejs-modules": ["error"] + "import/no-nodejs-modules": ["error"], }, overrides: [ { From 98bea5d84691f061dab3f86be2211a92eef71d39 Mon Sep 17 00:00:00 2001 From: Rishhi Balakrishnan <107130183+RishhiB@users.noreply.github.com> Date: Fri, 31 Jan 2025 18:03:38 +0000 Subject: [PATCH 5/5] allow error --- packages/loader/driver-utils/src/protocol/gitHelper.ts | 2 +- .../driver-utils/src/test/summaryCompresssionTester.spec.ts | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/loader/driver-utils/src/protocol/gitHelper.ts b/packages/loader/driver-utils/src/protocol/gitHelper.ts index 2f7333e5d3eb..e7f032a34a81 100644 --- a/packages/loader/driver-utils/src/protocol/gitHelper.ts +++ b/packages/loader/driver-utils/src/protocol/gitHelper.ts @@ -79,7 +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 - const node: ISnapshotTreeEx | undefined = lookup[entryPathDir]; + const node = lookup[entryPathDir]; // Add in either the blob or tree if (entry.type === "tree") { diff --git a/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts b/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts index 375caf9f9c89..dd62b57e4094 100644 --- a/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts +++ b/packages/loader/driver-utils/src/test/summaryCompresssionTester.spec.ts @@ -63,7 +63,6 @@ function cloneSummary(): ISummaryTree { */ function generateSummaryWithContent(contentSize: number) { const summary = cloneSummary(); - // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access const header = ( ( ((summary.tree[".channels"] as ISummaryTree).tree.rootDOId as ISummaryTree).tree[ @@ -86,7 +85,6 @@ function generateSummaryWithContent(contentSize: number) { function generateSummaryWithBinaryContent(startsWith: number, contentSize: number) { const summary = cloneSummary(); - // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access const header = ( ( ((summary.tree[".channels"] as ISummaryTree).tree.rootDOId as ISummaryTree).tree[ @@ -623,7 +621,6 @@ function getHeaderContent(summary: ISummaryTree) { } function getHeader(summary: ISummaryTree) { - // eslint-disable-next-line @fluid-internal/fluid/no-unchecked-record-access return getHeaderHolder(summary).tree.header; }