Skip to content

Conversation

@Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Nov 13, 2025

Explanation

The wrapper script we've used to track warnings has been replaced by the built- in ESLint error suppression feature. This resolves some bugs we had with the wrapper script, and this new approach will be easier to roll out to new repositories.

Here is an example of CI run where there are stale suppressed errors: https://github.com/MetaMask/core/actions/runs/19367196104/job/55413407665
They can be automatically fixed with yarn lint:fix, or by running yarn eslint --prune-suppressions

References

Closes #6790
Fixes #5212

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

Note

Replaces the custom ESLint wrapper/thresholds with ESLint’s built-in suppressions, updating config, scripts, and deps accordingly.

  • Tooling/CI:
    • Replace custom wrapper (scripts/run-eslint.ts) with ESLint built-in suppressions.
    • Add eslint-suppressions.json; remove eslint-warning-thresholds.json.
    • Update npm scripts: lint:eslint now runs eslint directly; lint:fix adds --prune-suppressions.
  • ESLint Config (eslint.config.mjs):
    • Clean up temporary warn-only rules; disable jsdoc/check-tag-names in targeted contexts.
    • Promote key Jest rules to errors (jest/expect-expect, jest/no-alias-methods, jest/no-commented-out-tests, jest/no-disabled-tests).
    • Misc rule/env tweaks for node/jest/test helpers; remove package-specific overrides.
  • Dependencies:
    • Remove chalk (no longer needed by the wrapper script).
  • Codegen:
    • Minor lint directive adjustment in notification API schema.

Written by Cursor Bugbot for commit 2b65be5. This will update automatically on new commits. Configure here.

@Gudahtt Gudahtt force-pushed the replace-warning-threshold-with-error-suppression branch from 9e2f6e5 to b99b5b9 Compare November 13, 2025 13:02
// does not work very well.
'jsdoc/check-tag-names': 'off',

// TODO: re-enable most of these rules
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do another pass later to re-enable rules we had "temporarily" disabled. I just wanted to get rid of the warnings in this pass.

Gudahtt added a commit that referenced this pull request Nov 13, 2025
)

## Explanation

The rule `@typescript-eslint/naming-convention` was already disabled for
most files, but it was set to `warn` in a few places still. It's now
completely disabled instead.

This was done to reduce the set of changes that are part of the upcoming
switch to error suppression (#6790), and also for consistency. We still
want to use this rule, but we can gradually begin enforcing it in a more
controlled manner using error suppression, across the entire repository
at once rather than selectively.

## References

Relates to #6790

These changes were extracted from the draft PR #7148

## 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),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Disables `@typescript-eslint/naming-convention` everywhere and removes
related thresholds/config and an inline disable comment.
> 
> - **ESLint config**
> - Remove `@typescript-eslint/naming-convention: 'warn'` from
`**/*.d.ts` and `packages/eth-block-tracker/**/*.ts` overrides in
`eslint.config.mjs`.
> - **Warning thresholds**
> - Delete thresholds for
`packages/eth-block-tracker/tests/buildDeferred.ts` and remove
`@typescript-eslint/naming-convention` entry from
`tests/setupAfterEnv.ts` in `eslint-warning-thresholds.json`.
> - **Types**
> - Drop inline disable for `@typescript-eslint/naming-convention` in
`types/global.d.ts` comment.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
4bc7432. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Gudahtt added a commit that referenced this pull request Nov 13, 2025
## Explanation

The ESLint rule `jsdoc/check-tag-names` mangles a lot of comment blocks
we have for types. The types are currently written in a way that doesn't
comply with the TSDoc spec, and we should fix them, but leaving the rule
enabled (even with error suppression!) prevents us from using `--fix`.

The rule has been selectively disabled in each file that has malformed
doc blocks impacted by this problem. We can remove these files
one-by-one as we fix the inline docs.

## References

Relates to #6790

These changes were extracted from the draft PR #7148

## 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),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Disables `jsdoc/check-tag-names` for specific files via ESLint
overrides and updates warning thresholds accordingly.
> 
> - **ESLint Configuration**:
> - Remove global `jsdoc/check-tag-names` warning from TypeScript rules.
> - Add targeted override disabling `jsdoc/check-tag-names` for specific
files (e.g., various controllers, message-manager files, tests).
> - **Warning Thresholds**:
> - Update `eslint-warning-thresholds.json` to drop
`jsdoc/check-tag-names` entries for affected files.
> - Minor cleanup to reflect current active rules (e.g.,
`TokensController.ts`, `siwe.ts`, `tests/fake-provider.ts`).
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
bc83018. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Gudahtt added a commit that referenced this pull request Nov 13, 2025
## Explanation



## References

Relates to #6790

These changes were extracted from the draft PR #7148

## 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),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Removes redundant ESLint ignore comments (e.g.,
`jest/no-conditional-in-test`, related enable/disable blocks, and a
stray `no-var`) across numerous test files without functional changes.
> 
> - **Tests cleanup**:
> - Strip unused ESLint disable/enable comments across many test files
in packages like `account-tree-controller`, `assets-controllers`,
`bridge-controller`, `bridge-status-controller`,
`chain-agnostic-permission`, `controller-utils`, `earn-controller`,
`eth-json-rpc-*`, `json-rpc-engine`, `network-controller`,
`network-enablement-controller`, `profile-sync-controller`,
`sample-controllers`, and `shield-controller`.
> - Primarily removes `/* eslint-disable jest/no-conditional-in-test */`
and matching enable blocks; also removes a few other unused directives
(e.g., `no-var`).
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
e38e3e7. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Guillaume Roux <guillaumeroux123@gmail.com>
@Gudahtt Gudahtt force-pushed the replace-warning-threshold-with-error-suppression branch from b99b5b9 to b6d4632 Compare November 14, 2025 13:13
@Gudahtt Gudahtt marked this pull request as ready for review November 14, 2025 13:28
@Gudahtt Gudahtt requested a review from a team as a code owner November 14, 2025 13:28
@Gudahtt Gudahtt marked this pull request as draft November 14, 2025 13:36
Gudahtt added a commit that referenced this pull request Nov 14, 2025
ESLint ignore directives about the rule `no-unusafe-enum-comparison`
have been removed. This rule was disabled in the most recent ESLint
config update.

This change helps to unblock #7148
Gudahtt added a commit that referenced this pull request Nov 14, 2025
## Explanation

ESLint ignore directives about the rule `no-unusafe-enum-comparison`
have been removed. This rule was disabled in the most recent ESLint
config update.

## References

This change helps to unblock #7148

## 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),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Removes now-unnecessary ESLint disable directives for enum comparisons
across permission, keyring, and seedless onboarding modules.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
df5da81. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@Gudahtt Gudahtt force-pushed the replace-warning-threshold-with-error-suppression branch 2 times, most recently from 56b6c85 to a6955f7 Compare November 14, 2025 14:05
The wrapper script we've used to track warnings has been replaced by the built-
in ESLint error suppression feature. This resolves some bugs we had with the
wrapper script, and this new approach will be easier to roll out to new
repositories.
@Gudahtt Gudahtt force-pushed the replace-warning-threshold-with-error-suppression branch from a6955f7 to 49f700f Compare November 14, 2025 14:13
@Gudahtt Gudahtt marked this pull request as ready for review November 14, 2025 14:21
Copy link
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Gudahtt Gudahtt enabled auto-merge (squash) November 14, 2025 16:44
@Gudahtt Gudahtt merged commit a7a17b6 into main Nov 14, 2025
272 checks passed
@Gudahtt Gudahtt deleted the replace-warning-threshold-with-error-suppression branch November 14, 2025 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace custom ESLint warning tracking with bulk suppressions It's now impossible to autofix new lint violations introduced on the current branch

5 participants