ESLint: error on missing lint ignore comments (#7829)

* fixing and adding explanations to eslint ignore comments (still 102 problems remaining)
* fixing a new-cap lint error

Author: fungairino

.eslintrc.js

@@ -30,6 +30,12 @@ module.exports = {
     "@shopify/react-hooks-strict-return": "error",
     "@shopify/prefer-module-scope-constants": "error",
     "@shopify/jest/no-snapshots": "warn",
+    "new-cap": [
+      "error",
+      {
+        capIsNewExceptionPattern: "(TEST_|INTERNAL_|HACK_|UNSAFE_)",
+      },
+    ],
     "eslint-comments/require-description": "warn",
     "react/no-array-index-key": "error",
     "react/no-unstable-nested-components": ["error", { allowAsProps: true }],

src/analysis/analysisVisitors/varAnalysis/parseTemplateVariables.ts

@@ -30,7 +30,7 @@ const VARIABLE_START_INDEX = Symbol("#Variable_start_index");
 
 export interface VariableOptions {
   type?: string;
-  // eslint-disable-next-line @typescript-eslint/no-use-before-define
+  // eslint-disable-next-line @typescript-eslint/no-use-before-define -- Cyclic dependency
   parent?: Variable | undefined;
   startIndex: number;
 }

src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts

@@ -114,16 +114,19 @@ async function setIntegrationDependencyVars(
   contextVars: VarMap,
 ): Promise<void> {
   // Loop through all the integrations, so we can set the source for each dependency variable properly
-  for (const integrationDependency of extension.integrationDependencies ?? []) {
-    // eslint-disable-next-line no-await-in-loop
-    const serviceContext = await makeServiceContextFromDependencies([
-      integrationDependency,
-    ]);
-    contextVars.setExistenceFromValues({
-      source: `${KnownSources.SERVICE}:${integrationDependency.integrationId}`,
-      values: serviceContext,
-    });
-  }
+  await Promise.all(
+    (extension.integrationDependencies ?? []).map(
+      async (integrationDependency) => {
+        const serviceContext = await makeServiceContextFromDependencies([
+          integrationDependency,
+        ]);
+        contextVars.setExistenceFromValues({
+          source: `${KnownSources.SERVICE}:${integrationDependency.integrationId}`,
+          values: serviceContext,
+        });
+      },
+    ),
+  );
 }
 
 /**

src/auth/authUtils.ts

@@ -60,8 +60,9 @@ export function selectUserDataUpdate({
 
   return {
     // TODO: Review Required/Partial in Me type
-    // https://github.com/pixiebrix/pixiebrix-extension/issues/7725
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion
+    //  https://github.com/pixiebrix/pixiebrix-extension/issues/7725
+    /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion
+    -- email is always present, pending above type refactoring */
     email: email!,
     organizationId: organization?.id ?? null,
     telemetryOrganizationId: telemetryOrganization?.id ?? null,

src/auth/featureFlagStorage.test.ts

@@ -43,7 +43,6 @@ describe("featureFlags", () => {
   });
 
   it("returns true if flag is present", async () => {
-    // eslint-disable-next-line new-cap
     await TEST_overrideFeatureFlags([
       "test-flag",
       "test-other-flag",
@@ -53,7 +52,6 @@ describe("featureFlags", () => {
   });
 
   it("returns false if flag is not present", async () => {
-    // eslint-disable-next-line new-cap
     await TEST_overrideFeatureFlags(["test-other-flag", "test-other-flag-2"]);
     await expect(flagOn("test-flag")).resolves.toBe(false);
   });
@@ -68,7 +66,6 @@ describe("featureFlags", () => {
   });
 
   it("does not fetch if flags have been updated recently", async () => {
-    // eslint-disable-next-line new-cap
     await TEST_overrideFeatureFlags(["test-flag"]);
     await expect(flagOn("test-flag")).resolves.toBe(true);
     expect(appApiMock.history.get).toHaveLength(0);
@@ -95,9 +92,7 @@ describe("featureFlags", () => {
     expect(appApiMock.history.get).toHaveLength(1);
 
     const authData = tokenAuthDataFactory();
-    // eslint-disable-next-line new-cap
     await TEST_setAuthData(authData);
-    // eslint-disable-next-line new-cap
     TEST_triggerListeners(authData);
 
     // New user doesn't have secret flag

src/background/auth/authStorage.ts

@@ -62,8 +62,7 @@ export async function deleteCachedAuthData(serviceAuthId: UUID): Promise<void> {
     console.debug(
       `deleteCachedAuthData: removed data for auth ${serviceAuthId}`,
     );
-    // OK because we're guarding with hasOwn
-    // eslint-disable-next-line security/detect-object-injection
+    // eslint-disable-next-line security/detect-object-injection -- OK because we're guarding with hasOwn
     delete current[serviceAuthId];
 
     await oauth2Storage.set(current);

src/background/auth/codeGrantFlow.ts

@@ -164,15 +164,13 @@ async function codeGrantFlow(
     }
 
     const json = Object.fromEntries(parsed.entries());
-    // TODO: Fix IntegrationConfig types
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- TODO: Fix IntegrationConfig types
     await setCachedAuthData(auth.id!, json);
     return json as AuthData;
   }
 
   if (typeof data === "object") {
-    // TODO: Fix IntegrationConfig types
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- TODO: Fix IntegrationConfig types
     await setCachedAuthData(auth.id!, data);
     return data as AuthData;
   }

src/background/auth/implicitGrantFlow.ts

@@ -98,8 +98,7 @@ async function implicitGrantFlow(
   }
 
   const data: AuthData = { access_token, ...rest } as unknown as AuthData;
-  // TODO: Fix IntegrationConfig types
-  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion
+  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion -- TODO: Fix IntegrationConfig types
   await setCachedAuthData(auth.id!, data);
   return data;
 }

src/background/restrictUnauthenticatedUrlAccess.test.ts

@@ -64,13 +64,11 @@ describe("enforceAuthentication", () => {
   });
 
   afterEach(async () => {
-    // eslint-disable-next-line new-cap -- used for testing
     await INTERNAL_reset();
     await browser.storage.managed.clear();
     await browser.storage.local.clear();
     axiosMock.reset();
     jest.clearAllMocks();
-    // eslint-disable-next-line new-cap -- used for testing
     TEST_clearListeners();
   });
 
@@ -123,13 +121,11 @@ describe("enforceAuthentication", () => {
     expect(addListenerSpy).toHaveBeenCalledTimes(1);
     expect(removeListenerSpy).toHaveBeenCalledTimes(0);
 
-    // eslint-disable-next-line new-cap -- used for testing
     TEST_triggerListeners({ token: "foo" });
 
     expect(removeListenerSpy).toHaveBeenCalledTimes(1);
     addListenerSpy.mockClear();
 
-    // eslint-disable-next-line new-cap -- used for testing
     TEST_triggerListeners(undefined);
     await waitForEffect();
     expect(addListenerSpy).toHaveBeenCalledTimes(1);
@@ -173,7 +169,6 @@ describe("enforceAuthentication", () => {
       }),
     );
 
-    // eslint-disable-next-line new-cap -- used for testing
     TEST_triggerListeners({ token: "foo" });
 
     await waitForEffect();
@@ -218,7 +213,6 @@ describe("enforceAuthentication", () => {
     // Mock that tab no longer exists
     jest.mocked(browser.tabs.get).mockResolvedValue(null);
 
-    // eslint-disable-next-line new-cap -- used for testing
     TEST_triggerListeners({ token: "foo" });
 
     await waitForEffect();

src/background/setToolbarIconFromTheme.test.ts

@@ -51,8 +51,7 @@ describe("setToolbarIconFromTheme", () => {
   // @ts-expect-error -- No need to mock the whole class for the test
   global.Image = class {
     src = "";
-    // eslint-disable-next-line no-restricted-syntax
-    decode = jest.fn().mockResolvedValue(undefined);
+    decode = jest.fn();
   };
   // @ts-expect-error -- No need to mock the whole class for the test
   global.OffscreenCanvas = class {

src/background/telemetry.test.ts

@@ -27,7 +27,6 @@ beforeEach(async () => {
   appApiMock.reset();
   appApiMock.onPost("/api/events/").reply(201, {});
 
-  // eslint-disable-next-line new-cap -- test file
   await TEST_flushAll();
 });
 

src/bricks/effects/pageState.test.ts

@@ -24,7 +24,6 @@ import { GetPageState, SetPageState } from "@/bricks/effects/pageState";
 import { TEST_resetState } from "@/platform/state/stateController";
 
 beforeEach(() => {
-  // eslint-disable-next-line new-cap -- test method
   TEST_resetState();
 });
 

src/bricks/registry.test.ts

@@ -44,7 +44,6 @@ beforeEach(() => {
   jest.clearAllMocks();
   backgroundGetByKindsMock.mockResolvedValue([]);
 
-  // eslint-disable-next-line new-cap -- test-only method
   bricksRegistry.TEST_reset();
 });
 

src/bricks/transformers/ephemeralForm/formTransformer.test.ts

@@ -34,7 +34,6 @@ const showModalMock = jest.mocked(showModal);
 const brick = new FormTransformer();
 
 afterEach(async () => {
-  // eslint-disable-next-line new-cap -- test method
   await TEST_cancelAll();
   jest.clearAllMocks();
 });
@@ -102,7 +101,6 @@ describe("FormTransformer", () => {
       brickOptionsFactory(),
     );
 
-    // eslint-disable-next-line new-cap -- test method
     await TEST_cancelAll();
 
     await expect(brickPromise).rejects.toThrow(CancelError);
@@ -134,7 +132,6 @@ describe("FormTransformer", () => {
       brickOptionsFactory(),
     );
 
-    // eslint-disable-next-line new-cap -- test method
     await TEST_cancelAll();
 
     await expect(brickPromise).rejects.toThrow(CancelError);

src/bricks/transformers/mapping.ts

@@ -74,8 +74,7 @@ export class MappingTransformer extends TransformerABC {
     }
 
     if (Object.hasOwn(mapping, key)) {
-      // Checking for hasOwn
-      // eslint-disable-next-line security/detect-object-injection
+      // eslint-disable-next-line security/detect-object-injection -- Checking for hasOwn
       return mapping[key];
     }
 

src/bricks/transformers/parseUrl.ts

@@ -127,8 +127,7 @@ export class UrlParser extends TransformerABC {
 
     const searchParams: Record<string, string> = {};
     for (const [key, value] of parsed.searchParams.entries()) {
-      // Fine because value will always be a string
-      // eslint-disable-next-line security/detect-object-injection
+      // eslint-disable-next-line security/detect-object-injection -- Fine because value will always be a string
       searchParams[key] = value;
     }
 

src/components/EmotionShadowRoot.ts

@@ -18,9 +18,9 @@
 // eslint-disable-next-line no-restricted-imports -- All roads lead here
 import EmotionShadowRoot from "react-shadow/emotion";
 
-// "Every property exists" (via Proxy), TypeScript doesn't offer such type
-// Also strictNullChecks config mismatch
-// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion
+/* eslint-disable-next-line @typescript-eslint/no-non-null-assertion, @typescript-eslint/no-unnecessary-type-assertion --
+"Every property exists" (via Proxy), TypeScript doesn't offer such type
+Also strictNullChecks config mismatch */
 const ShadowRoot = EmotionShadowRoot.div!;
 
 export default ShadowRoot;

src/components/documentBuilder/documentTree.tsx

@@ -94,7 +94,7 @@ export const buildDocumentBranch: BuildDocumentBranch = (root, tracePath) => {
   return componentDefinition;
 };
 
-// eslint-disable-next-line complexity
+// eslint-disable-next-line complexity -- We're handling a lot of different element types
 export function getComponentDefinition(
   element: DocumentElement,
   tracePath: DynamicPath,

src/components/documentBuilder/hooks/useMoveWithinParent.ts

@@ -51,12 +51,12 @@ function useMoveWithinParent(documentBodyName: string): MoveWithinParent {
     const newElementsCollection = [...elementsCollection];
     const toIndex = direction === "up" ? elementIndex - 1 : elementIndex + 1;
 
-    /* eslint-disable security/detect-object-injection */
+    /* eslint-disable security/detect-object-injection -- swapping list elements  */
     [newElementsCollection[elementIndex], newElementsCollection[toIndex]] = [
       newElementsCollection[toIndex],
       newElementsCollection[elementIndex],
     ];
-    /* eslint-enable security/detect-object-injection */
+    /* eslint-enable security/detect-object-injection -- re-enable */
 
     await setElementsCollection(newElementsCollection);
     setActiveElement(joinPathParts(collectionName, toIndex));

src/components/documentBuilder/preview/DocumentPreview.test.tsx

@@ -87,43 +87,42 @@ describe("Add new element", () => {
     const listElement = createNewElement("list") as ListDocumentElement;
     listElement.config.element.__value__ = createNewElement("container");
     const containerElement = createNewElement("container");
-    // There's a row in the container and a column in the row.
-    // eslint-disable-next-line testing-library/no-node-access
+    // eslint-disable-next-line testing-library/no-node-access -- There's a row in the container and a column in the row.
     containerElement.children[0].children[0].children[0] = listElement;
 
     const { container } = renderDocumentPreview(containerElement);
 
     // Select a dropdown inside a Col in List and open it
     await userEvent.click(
-      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
       container.querySelector(".col .col .addElement button"),
     );
     expect(
-      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
       container.querySelector(".col .col .addElement button"),
     ).toHaveAttribute("aria-expanded", "true");
 
     // Hover over the Col in the list
-    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
     fireEvent.mouseOver(container.querySelector(".col .col"));
     expect(
-      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
       container.querySelector(".col .col .addElement button"),
     ).toHaveAttribute("aria-expanded", "true");
 
     // Hover over the Container of the List, .root.root - is the Document root element
-    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
     fireEvent.mouseOver(container.querySelector(".root.root > .container"));
     expect(
-      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+      // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
       container.querySelector(".col .col .addElement button"),
     ).toHaveAttribute("aria-expanded", "true");
   });
 
   test("can add an element to a container", async () => {
     const { container } = renderDocumentPreview(createNewElement("container"));
 
-    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
+    // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access -- see test's TODO comment
     await userEvent.click(container.querySelector(".col .addElement button"));
     await userEvent.click(screen.getByText("Header", { selector: "a" }));
 

src/contentScript/lifecycle.test.ts

@@ -1,4 +1,3 @@
-/* eslint-disable new-cap -- using exposed TEST_ methods */
 /*
  * Copyright (C) 2024 PixieBrix, Inc.
  *

src/contentScript/lifecycle.ts

@@ -288,7 +288,6 @@ export function clearEditorExtension(
 
       if (extensionPoint.kind === "actionPanel" && preserveSidebar) {
         const sidebar = extensionPoint as SidebarStarterBrickABC;
-        // eslint-disable-next-line new-cap -- hack for action panels
         sidebar.HACK_uninstallExceptExtension(extensionId);
       } else {
         extensionPoint.uninstall({ global: true });

src/contentScript/loadActivationEnhancementsCore.test.ts

@@ -85,7 +85,6 @@ describe("marketplace enhancements", () => {
 
   afterEach(async () => {
     jest.resetAllMocks();
-    // eslint-disable-next-line new-cap -- test method
     TEST_unloadActivationEnhancements();
   });
 

src/extensionConsole/pages/BrowserBanner.test.tsx

@@ -23,7 +23,6 @@ import { screen } from "@testing-library/react";
 import { INTERNAL_reset } from "@/store/enterprise/managedStorage";
 
 beforeEach(async () => {
-  // eslint-disable-next-line new-cap -- test helper method
   await INTERNAL_reset();
   await browser.storage.managed.clear();
 });

src/hooks/useFlags.test.tsx

@@ -79,7 +79,6 @@ describe("useFlags", () => {
 
     const tokenData = tokenAuthDataFactory();
 
-    // eslint-disable-next-line new-cap
     await TEST_setAuthData(tokenData);
 
     render(
@@ -94,7 +93,6 @@ describe("useFlags", () => {
     expect(appApiMock.history.get).toHaveLength(1);
 
     // Simulate a change in auth data
-    // eslint-disable-next-line new-cap
     TEST_triggerListeners(tokenData);
 
     await waitForEffect();

src/integrations/useSanitizedIntegrationConfigFormikAdapter.test.ts

@@ -60,7 +60,6 @@ const serviceRegistryMock = jest.mocked(serviceRegistry);
 
 describe("useSanitizedIntegrationConfigFormikAdapter", () => {
   beforeEach(() => {
-    // eslint-disable-next-line new-cap -- test helper method
     INTERNAL_reset();
   });