chore: Notification controller eslint cleanup (#7483)

## Explanation

This PR addresses and removes ESLint suppressions from three files
within `packages/notification-services-controller` as part of a larger
cleanup effort (ASSETS-2100).

* **Current state and why it needs to change:** The identified files
contained suppressions for
`@typescript-eslint/explicit-function-return-type`,
`no-restricted-globals`, `require-atomic-updates`, and `id-length`.
These suppressions masked potential code quality issues and reduced code
clarity.
*   **Solution:**
* **`push-utils.test.ts`**: Explicit return types were added to all
functions to satisfy `@typescript-eslint/explicit-function-return-type`.
For `no-restricted-globals` related to `self`, targeted
`eslint-disable-next-line` comments with explanations were added, as
`self` is required for testing service worker functionality.
* **`push-utils.ts`**: Explicit return types were added to functions to
satisfy `@typescript-eslint/explicit-function-return-type`. The
`require-atomic-updates` issue was addressed with a targeted
`eslint-disable-next-line` comment and explanation, as the race
condition for caching a boolean is acceptable.
* **`is-onchain-notification.ts`**: The `id-length` violation was fixed
by renaming the parameter `n` to `notification` for improved
readability.
* All corresponding entries for these files have been removed from
`eslint-suppressions.json`.
* **Changes whose purpose might not be obvious:** The
`eslint-disable-next-line` comments for `no-restricted-globals` and
`require-atomic-updates` are intentional and include inline explanations
in the code, detailing why these specific rules are being ignored at
those points.

## References

*   Fixes ASSETS-2100

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

---
<a
href="https://cursor.com/background-agent?bcId=bc-6ab48784-58dc-44dd-b632-a564eb71d229"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/open-in-cursor-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/open-in-cursor-light.svg"><img alt="Open in
Cursor"
src="https://cursor.com/open-in-cursor.svg"></picture></a>&nbsp;<a
href="https://cursor.com/agents?id=bc-6ab48784-58dc-44dd-b632-a564eb71d229"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/open-in-web-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/open-in-web-light.svg"><img alt="Open in Web"
src="https://cursor.com/open-in-web.svg"></picture></a>

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Adds explicit return types and stricter typings to web push utils and
tests, renames a short param in on-chain notification check, and removes
related entries from eslint-suppressions.json.
> 
> - **Notification Services Push (web)**:
> - **Typing/ESLint**: Add explicit return types to
`getPushAvailability`, `listenToPushNotificationsReceived`,
`listenToPushNotificationsClicked`, and
`createSubscribeToPushNotifications`; ensure handlers return
`Promise<void>`/`void`; type `unsubscribe` functions.
> - **Support caching**: Document acceptable race; add targeted
`eslint-disable-next-line require-atomic-updates`.
> - **Tests (`web/push-utils.test.ts`)**:
> - Refactor helper utilities into typed `function`s; add explicit
return types; type Jest spies; add targeted `eslint-disable-next-line
no-restricted-globals` for `self` usage; simulate `notificationclick`
via `self.dispatchEvent`.
> - **Shared**:
> - Rename `isOnChainRawNotification` param from `n` to `notification`
for clarity.
> - **Tooling**:
> - Remove corresponding entries for these files from
`eslint-suppressions.json`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
ba5e91a. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Cursor Agent <[email protected]>

Author: Prithpal-Sooriya

eslint-suppressions.json

@@ -1638,27 +1638,6 @@
       "count": 9
     }
   },
-  "packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.test.ts": {
-    "@typescript-eslint/explicit-function-return-type": {
-      "count": 10
-    },
-    "no-restricted-globals": {
-      "count": 3
-    }
-  },
-  "packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.ts": {
-    "@typescript-eslint/explicit-function-return-type": {
-      "count": 7
-    },
-    "require-atomic-updates": {
-      "count": 1
-    }
-  },
-  "packages/notification-services-controller/src/shared/is-onchain-notification.ts": {
-    "id-length": {
-      "count": 1
-    }
-  },
   "packages/phishing-controller/src/BulkTokenScan.test.ts": {
     "@typescript-eslint/explicit-function-return-type": {
       "count": 2

packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.test.ts

@@ -33,7 +33,10 @@ const firebaseApp: FirebaseAppModule.FirebaseApp = {
   options: mockEnv,
 };
 
-const arrangeFirebaseAppMocks = () => {
+function arrangeFirebaseAppMocks(): {
+  mockGetApp: jest.SpyInstance;
+  mockInitializeApp: jest.SpyInstance;
+} {
   const mockGetApp = jest
     .spyOn(FirebaseAppModule, 'getApp')
     .mockReturnValue(firebaseApp);
@@ -43,9 +46,14 @@ const arrangeFirebaseAppMocks = () => {
     .mockReturnValue(firebaseApp);
 
   return { mockGetApp, mockInitializeApp };
-};
-
-const arrangeFirebaseMessagingSWMocks = () => {
+}
+
+function arrangeFirebaseMessagingSWMocks(): {
+  mockIsSupported: jest.SpyInstance;
+  mockGetMessaging: jest.SpyInstance;
+  mockOnBackgroundMessage: jest.SpyInstance;
+  mockOnBackgroundMessageUnsub: jest.Mock;
+} {
   const mockIsSupported = jest
     .spyOn(FirebaseMessagingSWModule, 'isSupported')
     .mockResolvedValue(true);
@@ -65,9 +73,12 @@ const arrangeFirebaseMessagingSWMocks = () => {
     mockOnBackgroundMessage,
     mockOnBackgroundMessageUnsub,
   };
-};
+}
 
-const arrangeFirebaseMessagingMocks = () => {
+function arrangeFirebaseMessagingMocks(): {
+  mockGetToken: jest.SpyInstance;
+  mockDeleteToken: jest.SpyInstance;
+} {
   const mockGetToken = jest
     .spyOn(FirebaseMessagingModule, 'getToken')
     .mockResolvedValue('test-token');
@@ -77,12 +88,14 @@ const arrangeFirebaseMessagingMocks = () => {
     .mockResolvedValue(true);
 
   return { mockGetToken, mockDeleteToken };
-};
+}
 
 describe('createRegToken() tests', () => {
   const TEST_TOKEN = 'test-token';
 
-  const arrange = () => {
+  function arrange(): ReturnType<typeof arrangeFirebaseAppMocks> &
+    ReturnType<typeof arrangeFirebaseMessagingSWMocks> &
+    ReturnType<typeof arrangeFirebaseMessagingMocks> {
     const firebaseMocks = {
       ...arrangeFirebaseAppMocks(),
       ...arrangeFirebaseMessagingSWMocks(),
@@ -94,7 +107,7 @@ describe('createRegToken() tests', () => {
     return {
       ...firebaseMocks,
     };
-  };
+  }
 
   afterEach(() => {
     jest.clearAllMocks();
@@ -147,13 +160,15 @@ describe('createRegToken() tests', () => {
 });
 
 describe('deleteRegToken() tests', () => {
-  const arrange = () => {
+  function arrange(): ReturnType<typeof arrangeFirebaseAppMocks> &
+    ReturnType<typeof arrangeFirebaseMessagingSWMocks> &
+    ReturnType<typeof arrangeFirebaseMessagingMocks> {
     return {
       ...arrangeFirebaseAppMocks(),
       ...arrangeFirebaseMessagingSWMocks(),
       ...arrangeFirebaseMessagingMocks(),
     };
-  };
+  }
 
   afterEach(() => {
     jest.clearAllMocks();
@@ -193,7 +208,13 @@ describe('deleteRegToken() tests', () => {
 });
 
 describe('createSubscribeToPushNotifications() tests', () => {
-  const arrangeMessengerMocks = () => {
+  function arrangeMessengerMocks(): {
+    messenger: ReturnType<
+      typeof buildPushPlatformNotificationsControllerMessenger
+    >;
+    onNewNotificationsListener: jest.Mock;
+    pushNotificationClickedListener: jest.Mock;
+  } {
     const messenger = buildPushPlatformNotificationsControllerMessenger();
 
     const onNewNotificationsListener = jest.fn();
@@ -213,19 +234,31 @@ describe('createSubscribeToPushNotifications() tests', () => {
       onNewNotificationsListener,
       pushNotificationClickedListener,
     };
-  };
-
-  const arrangeClickListenerMocks = () => {
+  }
+
+  function arrangeClickListenerMocks(): {
+    mockAddEventListener: jest.SpyInstance;
+    mockRemoveEventListener: jest.SpyInstance;
+  } {
+    // Testing service worker functionality requires using the 'self' global
+    // eslint-disable-next-line no-restricted-globals
     const mockAddEventListener = jest.spyOn(self, 'addEventListener');
+    // eslint-disable-next-line no-restricted-globals
     const mockRemoveEventListener = jest.spyOn(self, 'removeEventListener');
 
     return {
       mockAddEventListener,
       mockRemoveEventListener,
     };
-  };
-
-  const arrange = () => {
+  }
+
+  function arrange(): ReturnType<typeof arrangeFirebaseAppMocks> &
+    ReturnType<typeof arrangeFirebaseMessagingSWMocks> &
+    ReturnType<typeof arrangeMessengerMocks> &
+    ReturnType<typeof arrangeClickListenerMocks> & {
+      mockOnReceivedHandler: jest.Mock;
+      mockOnClickHandler: jest.Mock;
+    } {
     const firebaseMocks = {
       ...arrangeFirebaseAppMocks(),
       ...arrangeFirebaseMessagingSWMocks(),
@@ -238,17 +271,19 @@ describe('createSubscribeToPushNotifications() tests', () => {
       mockOnReceivedHandler: jest.fn(),
       mockOnClickHandler: jest.fn(),
     };
-  };
+  }
 
-  const actCreateSubscription = async (mocks: ReturnType<typeof arrange>) => {
+  async function actCreateSubscription(
+    mocks: ReturnType<typeof arrange>,
+  ): Promise<() => void> {
     const unsubscribe = await createSubscribeToPushNotifications({
       messenger: mocks.messenger,
       onReceivedHandler: mocks.mockOnReceivedHandler,
       onClickHandler: mocks.mockOnClickHandler,
     })(mockEnv);
 
     return unsubscribe;
-  };
+  }
 
   afterEach(() => {
     jest.clearAllMocks();
@@ -288,7 +323,9 @@ describe('createSubscribeToPushNotifications() tests', () => {
     expect(mocks.mockRemoveEventListener).toHaveBeenCalled();
   });
 
-  const arrangeActNotificationReceived = async (testData: unknown) => {
+  async function arrangeActNotificationReceived(
+    testData: unknown,
+  ): Promise<ReturnType<typeof arrange>> {
     const mocks = arrange();
     await actCreateSubscription(mocks);
 
@@ -303,7 +340,7 @@ describe('createSubscribeToPushNotifications() tests', () => {
     firebaseCallback(payload);
 
     return mocks;
-  };
+  }
 
   it('should invoke handler when notifications are received', async () => {
     const mocks = await arrangeActNotificationReceived(
@@ -351,7 +388,8 @@ describe('createSubscribeToPushNotifications() tests', () => {
       notification: { data: notificationData },
     });
 
-    // Act
+    // Act - Testing service worker notification click event
+    // eslint-disable-next-line no-restricted-globals
     self.dispatchEvent(mockNotificationEvent);
 
     // Assert Click Notification Event & Handler Calls

packages/notification-services-controller/src/NotificationServicesPushController/web/push-utils.ts

@@ -27,7 +27,10 @@ declare const self: ServiceWorkerGlobalScope;
 // eslint-disable-next-line import-x/no-mutable-exports
 export let supportedCache: boolean | null = null;
 
-const getPushAvailability = async () => {
+const getPushAvailability = async (): Promise<boolean> => {
+  // Race condition is acceptable here - worst case is isSupported() is called
+  // multiple times during initialization, which is harmless for caching a boolean
+  // eslint-disable-next-line require-atomic-updates
   supportedCache ??= await isSupported();
   return supportedCache;
 };
@@ -129,7 +132,7 @@ async function listenToPushNotificationsReceived(
   const unsubscribePushNotifications = onBackgroundMessage(
     messaging,
     // eslint-disable-next-line @typescript-eslint/no-misused-promises
-    async (payload: MessagePayload) => {
+    async (payload: MessagePayload): Promise<void> => {
       try {
         // MessagePayload shapes are not known
         // TODO - provide open-api unfied backend/frontend types
@@ -162,7 +165,7 @@ async function listenToPushNotificationsReceived(
     },
   );
 
-  const unsubscribe = () => unsubscribePushNotifications();
+  const unsubscribe = (): void => unsubscribePushNotifications();
   return unsubscribe;
 }
 
@@ -174,15 +177,15 @@ async function listenToPushNotificationsReceived(
  */
 function listenToPushNotificationsClicked(
   handler: (e: NotificationEvent, notification: Types.INotification) => void,
-) {
-  const clickHandler = (event: NotificationEvent) => {
+): () => void {
+  const clickHandler = (event: NotificationEvent): void => {
     // Get Data
     const data: Types.INotification = event?.notification?.data;
     handler(event, data);
   };
 
   self.addEventListener('notificationclick', clickHandler);
-  const unsubscribe = () =>
+  const unsubscribe = (): void =>
     self.removeEventListener('notificationclick', clickHandler);
   return unsubscribe;
 }
@@ -208,11 +211,11 @@ export function createSubscribeToPushNotifications(props: {
     notification: Types.INotification,
   ) => void;
   messenger: NotificationServicesPushControllerMessenger;
-}) {
-  return async function (env: PushNotificationEnv) {
+}): (env: PushNotificationEnv) => Promise<() => void> {
+  return async function (env: PushNotificationEnv): Promise<() => void> {
     const onBackgroundMessageSub = await listenToPushNotificationsReceived(
       env,
-      async (notification) => {
+      async (notification): Promise<void> => {
         props.messenger.publish(
           'NotificationServicesPushController:onNewNotifications',
           notification,
@@ -221,7 +224,7 @@ export function createSubscribeToPushNotifications(props: {
       },
     );
     const onClickSub = listenToPushNotificationsClicked(
-      (event, notification) => {
+      (event, notification): void => {
         props.messenger.publish(
           'NotificationServicesPushController:pushNotificationClicked',
           notification,
@@ -230,7 +233,7 @@ export function createSubscribeToPushNotifications(props: {
       },
     );
 
-    const unsubscribe = () => {
+    const unsubscribe = (): void => {
       onBackgroundMessageSub?.();
       onClickSub();
     };

packages/notification-services-controller/src/shared/is-onchain-notification.ts

@@ -3,13 +3,13 @@ import type { OnChainRawNotification } from '../NotificationServicesController';
 /**
  * Checks if the given value is an OnChainRawNotification object.
  *
- * @param n - The value to check.
+ * @param notification - The value to check.
  * @returns True if the value is an OnChainRawNotification object, false otherwise.
  */
 export function isOnChainRawNotification(
-  n: unknown,
-): n is OnChainRawNotification {
-  const assumed = n as OnChainRawNotification;
+  notification: unknown,
+): notification is OnChainRawNotification {
+  const assumed = notification as OnChainRawNotification;
 
   // We don't have a validation/parsing library to check all possible types of an on chain notification
   // It is safe enough just to check "some" fields, and catch any errors down the line if the shape is bad.