From 66f305ab07aa63286805641c9e35c4f1dd39d49b Mon Sep 17 00:00:00 2001
From: Tyler Butler <tyler.butler@microsoft.com>
Date: Tue, 17 May 2022 16:58:14 -0700
Subject: [PATCH] Enable prefer-includes eslint rule and pre-apply fixes
 (#10272)

From the documentation on the rule:

Until ES5, we were using String#indexOf method to check whether a string contains an arbitrary substring or not. Until ES2015, we were using Array#indexOf method to check whether an array contains an arbitrary value or not.

ES2015 has added String#includes and ES2016 has added Array#includes. It makes code more understandable if we use those includes methods for the purpose.
---
 common/build/eslint-config-fluid/minimal.js                    | 1 +
 common/build/eslint-config-fluid/printed-configs/default.json  | 3 +++
 common/build/eslint-config-fluid/printed-configs/test.json     | 3 +++
 .../PropertyDDS/packages/property-changeset/.eslintrc.js       | 2 ++
 .../packages/property-common/src/datastructures/collection.ts  | 2 +-
 .../packages/property-common/src/error_objects/httpError.ts    | 2 +-
 .../packages/property-inspector-table/src/NameCell.tsx         | 2 +-
 .../packages/property-inspector-table/src/NewDataForm.tsx      | 2 +-
 .../src/PropertyDataCreationHandlers.ts                        | 2 +-
 .../PropertyDDS/packages/property-properties/.eslintrc.js      | 1 +
 .../property-properties/src/propertyTemplateWrapper.js         | 2 +-
 experimental/PropertyDDS/packages/property-proxy/.eslintrc.js  | 2 +-
 experimental/dds/tree/src/Forest.ts                            | 2 +-
 packages/drivers/odsp-driver/src/odspUtils.ts                  | 2 +-
 packages/loader/driver-utils/src/blobAggregationStorage.ts     | 2 +-
 packages/runtime/container-runtime/src/dataStoreContext.ts     | 2 +-
 packages/test/test-service-load/src/utils.ts                   | 2 +-
 packages/utils/telemetry-utils/src/test/errorLogging.spec.ts   | 2 +-
 18 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/common/build/eslint-config-fluid/minimal.js b/common/build/eslint-config-fluid/minimal.js
index f80b2c8b7f3b..fe681699bb4c 100644
--- a/common/build/eslint-config-fluid/minimal.js
+++ b/common/build/eslint-config-fluid/minimal.js
@@ -189,6 +189,7 @@ module.exports = {
         // This rule ensures that our Intellisense looks good by verifying the TSDoc syntax.
         "tsdoc/syntax": "error",
 
+        "@typescript-eslint/prefer-includes": "error",
         "@typescript-eslint/prefer-optional-chain": "error",
     },
     "overrides": [
diff --git a/common/build/eslint-config-fluid/printed-configs/default.json b/common/build/eslint-config-fluid/printed-configs/default.json
index 3ee1bea67781..7f86273372cf 100644
--- a/common/build/eslint-config-fluid/printed-configs/default.json
+++ b/common/build/eslint-config-fluid/printed-configs/default.json
@@ -266,6 +266,9 @@
         "@typescript-eslint/prefer-function-type": [
             "error"
         ],
+        "@typescript-eslint/prefer-includes": [
+            "error"
+        ],
         "@typescript-eslint/prefer-namespace-keyword": [
             "error"
         ],
diff --git a/common/build/eslint-config-fluid/printed-configs/test.json b/common/build/eslint-config-fluid/printed-configs/test.json
index 96717b89c03a..7299cdceabdf 100644
--- a/common/build/eslint-config-fluid/printed-configs/test.json
+++ b/common/build/eslint-config-fluid/printed-configs/test.json
@@ -266,6 +266,9 @@
         "@typescript-eslint/prefer-function-type": [
             "error"
         ],
+        "@typescript-eslint/prefer-includes": [
+            "error"
+        ],
         "@typescript-eslint/prefer-namespace-keyword": [
             "error"
         ],
diff --git a/experimental/PropertyDDS/packages/property-changeset/.eslintrc.js b/experimental/PropertyDDS/packages/property-changeset/.eslintrc.js
index e51edc6050f9..081089b7e254 100644
--- a/experimental/PropertyDDS/packages/property-changeset/.eslintrc.js
+++ b/experimental/PropertyDDS/packages/property-changeset/.eslintrc.js
@@ -11,6 +11,7 @@ module.exports = {
         "project": ["./tsconfig.json", "./src/test/tsconfig.json"]
     },
     "rules": {
+        // Many rules are disabled in PropertyDDS projects. See https://github.com/microsoft/FluidFramework/pull/10272
         "@typescript-eslint/ban-ts-comment": "off",
         "@typescript-eslint/ban-types": "off",
         "@typescript-eslint/consistent-type-definitions": "off",
@@ -27,6 +28,7 @@ module.exports = {
         "@typescript-eslint/no-unused-expressions": "off",
         "@typescript-eslint/no-var-requires": "off",
         "@typescript-eslint/prefer-for-of": "off",
+        "@typescript-eslint/prefer-includes": "off",
         "@typescript-eslint/prefer-optional-chain": "off",
         "@typescript-eslint/quotes": "off",
         "@typescript-eslint/restrict-plus-operands": "off",
diff --git a/experimental/PropertyDDS/packages/property-common/src/datastructures/collection.ts b/experimental/PropertyDDS/packages/property-common/src/datastructures/collection.ts
index a9af8c9fde64..0f6578937eeb 100644
--- a/experimental/PropertyDDS/packages/property-common/src/datastructures/collection.ts
+++ b/experimental/PropertyDDS/packages/property-common/src/datastructures/collection.ts
@@ -175,7 +175,7 @@ export class Collection<T> {
 
         if (_.isArray(in_filterKey)) {
             filterCb = function(in_key, in_item) {
-                if (in_filterKey.indexOf(in_key) >= 0) {
+                if (in_filterKey.includes(in_key)) {
                     rtn.add(in_key, in_item);
                 }
             };
diff --git a/experimental/PropertyDDS/packages/property-common/src/error_objects/httpError.ts b/experimental/PropertyDDS/packages/property-common/src/error_objects/httpError.ts
index 0dafaf224d17..0ba1f7346fc8 100644
--- a/experimental/PropertyDDS/packages/property-common/src/error_objects/httpError.ts
+++ b/experimental/PropertyDDS/packages/property-common/src/error_objects/httpError.ts
@@ -60,7 +60,7 @@ export class HTTPError extends Error {
         const isFirefox = typeof window !== "undefined" &&
             typeof window.navigator !== "undefined" &&
             typeof window.navigator.userAgent !== "undefined" &&
-            window.navigator.userAgent.toLowerCase().indexOf("firefox") > -1;
+            window.navigator.userAgent.toLowerCase().includes("firefox");
 
         return isFirefox ? `${this.message}, stack:${stack}` : `stack:${stack}`;
     }
diff --git a/experimental/PropertyDDS/packages/property-inspector-table/src/NameCell.tsx b/experimental/PropertyDDS/packages/property-inspector-table/src/NameCell.tsx
index e2302b85506e..8a016e53ef62 100644
--- a/experimental/PropertyDDS/packages/property-inspector-table/src/NameCell.tsx
+++ b/experimental/PropertyDDS/packages/property-inspector-table/src/NameCell.tsx
@@ -85,7 +85,7 @@ const copyHandler = (rowData: IInspectorRow, ref: React.MutableRefObject<HTMLTex
 const isStaticProperty = (parent: BaseProperty, rowName: string) => {
   if (typeof (parent as NodeProperty).getDynamicIds === "function") {
     const dynamicIds = (parent as NodeProperty).getDynamicIds();
-    if (dynamicIds.indexOf(rowName) > -1) {
+    if (dynamicIds.includes(rowName)) {
       return false;
     }
   } else if (parent.getContext() !== "single") {
diff --git a/experimental/PropertyDDS/packages/property-inspector-table/src/NewDataForm.tsx b/experimental/PropertyDDS/packages/property-inspector-table/src/NewDataForm.tsx
index d11bc8d8052b..a0a090bc0830 100644
--- a/experimental/PropertyDDS/packages/property-inspector-table/src/NewDataForm.tsx
+++ b/experimental/PropertyDDS/packages/property-inspector-table/src/NewDataForm.tsx
@@ -243,7 +243,7 @@ export const NewDataForm: React.FunctionComponent<INewDataFormProps> = (props) =
     </Button>
   );
 
-  const isSiblingFound = siblingIds.indexOf(inputName) >= 0;
+  const isSiblingFound = siblingIds.includes(inputName);
   const createBtn = (
     <Button
       id="createDataButton"
diff --git a/experimental/PropertyDDS/packages/property-inspector-table/src/PropertyDataCreationHandlers.ts b/experimental/PropertyDDS/packages/property-inspector-table/src/PropertyDataCreationHandlers.ts
index de2e4f1fc3a3..c70e77dcf4ff 100644
--- a/experimental/PropertyDDS/packages/property-inspector-table/src/PropertyDataCreationHandlers.ts
+++ b/experimental/PropertyDDS/packages/property-inspector-table/src/PropertyDataCreationHandlers.ts
@@ -19,7 +19,7 @@ export const fetchRegisteredTemplates = () => {
   const primitiveLocalTemplates: string[] = [];
   const customLocalTemplates: string[] = [];
   Object.keys(localTemplates).forEach((item) => {
-    if (!TypeIdHelper.isTemplateTypeid(item) && EXCLUDE_PROPS.indexOf(item) === -1) {
+      if (!TypeIdHelper.isTemplateTypeid(item) && !EXCLUDE_PROPS.includes(item)) {
       primitiveLocalTemplates.push(item);
     } else if (TypeIdHelper.isTemplateTypeid(item)) {
       customLocalTemplates.push(item);
diff --git a/experimental/PropertyDDS/packages/property-properties/.eslintrc.js b/experimental/PropertyDDS/packages/property-properties/.eslintrc.js
index 50dd67a06031..a139a43aca24 100644
--- a/experimental/PropertyDDS/packages/property-properties/.eslintrc.js
+++ b/experimental/PropertyDDS/packages/property-properties/.eslintrc.js
@@ -11,6 +11,7 @@ module.exports = {
         "project": ["./tsconfig.json", "./src/test/tsconfig.json"]
     },
     "rules": {
+        // Many rules are disabled in PropertyDDS projects. See https://github.com/microsoft/FluidFramework/pull/10272
         "@typescript-eslint/ban-types": "off",
         "@typescript-eslint/comma-spacing": "off",
         "@typescript-eslint/dot-notation": "off",
diff --git a/experimental/PropertyDDS/packages/property-properties/src/propertyTemplateWrapper.js b/experimental/PropertyDDS/packages/property-properties/src/propertyTemplateWrapper.js
index 6db2fcd63b3b..ce6af3ba47eb 100644
--- a/experimental/PropertyDDS/packages/property-properties/src/propertyTemplateWrapper.js
+++ b/experimental/PropertyDDS/packages/property-properties/src/propertyTemplateWrapper.js
@@ -108,7 +108,7 @@ export class PropertyTemplateWrapper {
         const isCreationType = (typeid) => {
             const creationTypes =
                 ['ContainerProperty', 'NodeProperty', 'Enum', 'BinaryProperty', 'Binary', 'RepositoryReferenceProperty'];
-            return creationTypes.indexOf(typeid) !== -1;
+            return creationTypes.includes(typeid);
         };
 
         if (isCreationType(in_typeid)) {
diff --git a/experimental/PropertyDDS/packages/property-proxy/.eslintrc.js b/experimental/PropertyDDS/packages/property-proxy/.eslintrc.js
index a6324691547f..ae6666f99320 100644
--- a/experimental/PropertyDDS/packages/property-proxy/.eslintrc.js
+++ b/experimental/PropertyDDS/packages/property-proxy/.eslintrc.js
@@ -11,7 +11,7 @@ module.exports = {
         "project": ["./tsconfig.json", "./src/test/tsconfig.json"]
     },
     "rules": {
-        // TODO(marcus): remove the linting issues
+        // Many rules are disabled in PropertyDDS projects. See https://github.com/microsoft/FluidFramework/pull/10272
         "@typescript-eslint/no-non-null-assertion": "off",
         "@typescript-eslint/no-unsafe-return": "off",
         "@typescript-eslint/strict-boolean-expressions": "off",
diff --git a/experimental/dds/tree/src/Forest.ts b/experimental/dds/tree/src/Forest.ts
index d008b140e4a6..123cea8753ae 100644
--- a/experimental/dds/tree/src/Forest.ts
+++ b/experimental/dds/tree/src/Forest.ts
@@ -401,7 +401,7 @@ export class Forest {
 				const parent = this.get(node.parentId);
 				const trait = parent.traits.get(node.traitParent);
 				assert(trait !== undefined);
-				assert(trait.indexOf(node.identifier) >= 0, 'node is parented incorrectly');
+				assert(trait.includes(node.identifier), 'node is parented incorrectly');
 			}
 
 			for (const trait of node.traits.values()) {
diff --git a/packages/drivers/odsp-driver/src/odspUtils.ts b/packages/drivers/odsp-driver/src/odspUtils.ts
index 9881128154b4..db63f94834e8 100644
--- a/packages/drivers/odsp-driver/src/odspUtils.ts
+++ b/packages/drivers/odsp-driver/src/odspUtils.ts
@@ -143,7 +143,7 @@ export async function fetchHelper(
                 "Fetch Timeout (AbortError)", OdspErrorType.fetchTimeout, { driverVersion });
         }
         // TCP/IP timeout
-        if (errorText.indexOf("ETIMEDOUT") !== -1) {
+        if (errorText.includes("ETIMEDOUT")) {
             throw new RetryableError(
                 "Fetch Timeout (ETIMEDOUT)", OdspErrorType.fetchTimeout, { driverVersion });
         }
diff --git a/packages/loader/driver-utils/src/blobAggregationStorage.ts b/packages/loader/driver-utils/src/blobAggregationStorage.ts
index 4dd61c49a20d..e256ef872255 100644
--- a/packages/loader/driver-utils/src/blobAggregationStorage.ts
+++ b/packages/loader/driver-utils/src/blobAggregationStorage.ts
@@ -343,7 +343,7 @@ export class BlobAggregationStorage extends SnapshotExtractor implements IDocume
                     }
                     // Ensure only whole data stores can be reused, no reusing at deeper level!
                     assert(level === 0, 0x0fc /* "tree reuse at lower level" */);
-                    assert(handlePath.indexOf("/") === -1,
+                    assert(!handlePath.includes("/"),
                         0x0fd /* "data stores are writing incremental summaries!" */);
                     break;
                 }
diff --git a/packages/runtime/container-runtime/src/dataStoreContext.ts b/packages/runtime/container-runtime/src/dataStoreContext.ts
index f8527e2c1216..8a4fa7cc6525 100644
--- a/packages/runtime/container-runtime/src/dataStoreContext.ts
+++ b/packages/runtime/container-runtime/src/dataStoreContext.ts
@@ -275,7 +275,7 @@ export abstract class FluidDataStoreContext extends TypedEventEmitter<IFluidData
 
         // URIs use slashes as delimiters. Handles use URIs.
         // Thus having slashes in types almost guarantees trouble down the road!
-        assert(this.id.indexOf("/") === -1, 0x13a /* `Data store ID contains slash: ${id}` */);
+        assert(!this.id.includes("/"), 0x13a /* `Data store ID contains slash: ${id}` */);
 
         this._attachState = this.containerRuntime.attachState !== AttachState.Detached && this.existing ?
             this.containerRuntime.attachState : AttachState.Detached;
diff --git a/packages/test/test-service-load/src/utils.ts b/packages/test/test-service-load/src/utils.ts
index 8c4269d7b3c3..b4492e40217c 100644
--- a/packages/test/test-service-load/src/utils.ts
+++ b/packages/test/test-service-load/src/utils.ts
@@ -69,7 +69,7 @@ class FileLogger extends TelemetryLogger implements ITelemetryBufferedLogger {
     send(event: ITelemetryBaseEvent): void {
         if (typeof event.testCategoryOverride === "string") {
             event.category = event.testCategoryOverride;
-        } else if (typeof event.message === "string" && event.message.indexOf("FaultInjectionNack") > -1) {
+        } else if (typeof event.message === "string" && event.message.includes("FaultInjectionNack")) {
             event.category = "generic";
         }
         this.baseLogger?.send({ ...event, hostName: pkgName });
diff --git a/packages/utils/telemetry-utils/src/test/errorLogging.spec.ts b/packages/utils/telemetry-utils/src/test/errorLogging.spec.ts
index 7cb892c67725..7b38addc7342 100644
--- a/packages/utils/telemetry-utils/src/test/errorLogging.spec.ts
+++ b/packages/utils/telemetry-utils/src/test/errorLogging.spec.ts
@@ -620,7 +620,7 @@ describe("normalizeError", () => {
             assert.equal(actual.message, expected.message, "message should match");
             const actualStack = actual.stack;
             assert(actualStack !== undefined, "stack should be present as a string");
-            if (actualStack.indexOf("at normalizeError") >= 0) { // This indicates the stack was populated naturally by new SimpleFluidError
+            if (actualStack.includes("at normalizeError")) { // This indicates the stack was populated naturally by new SimpleFluidError
                 assert.equal(expected.stack, "<<natural stack>>", "<<natural stack>> hint should be used if not overwritten");
                 expected.withExpectedTelemetryProps({ stack: actualStack });
             } else {