Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ESLint config #5132

Merged
merged 4 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ If you need to customize the behavior of Jest for a package, see `jest.config.js

## Linting

[ESLint](https://eslint.org/docs/v8.x/) v8 (via [MetaMask's shared ESLint configurations](https://github.com/MetaMask/eslint-config)) is used to check for code quality issues, and [Prettier](https://prettier.io/docs/en/) is used to format files.
[ESLint](https://eslint.org) v9 (via [MetaMask's shared ESLint configurations](https://github.com/MetaMask/eslint-config)) is used to check for code quality issues, and [Prettier](https://prettier.io/docs/en/) is used to format files.

If you need to customize the behavior of ESLint, see `.eslintrc.js` in the root.
If you need to customize the behavior of ESLint, see `eslint.config.mjs` in the root.

- Run `yarn lint` to lint all files and show possible violations across the monorepo.
- Run `yarn lint:fix` to fix any automatically fixable violations.
Expand Down
29 changes: 29 additions & 0 deletions eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"@typescript-eslint/consistent-type-exports": 19,
"@typescript-eslint/no-base-to-string": 3,
"@typescript-eslint/no-duplicate-enum-values": 2,
"@typescript-eslint/no-misused-promises": 3,
"@typescript-eslint/no-unsafe-enum-comparison": 59,
"@typescript-eslint/no-unused-vars": 36,
"@typescript-eslint/prefer-promise-reject-errors": 13,
"@typescript-eslint/prefer-readonly": 152,
"@typescript-eslint/switch-exhaustiveness-check": 10,
"import-x/namespace": 189,
"import-x/no-named-as-default": 1,
"import-x/no-named-as-default-member": 8,
"import-x/order": 209,
"jest/no-conditional-in-test": 104,
"jsdoc/check-tag-names": 372,
"jsdoc/require-returns": 22,
"jsdoc/tag-lines": 329,
"n/no-unsupported-features/node-builtins": 18,
"n/prefer-global/text-encoder": 4,
"n/prefer-global/text-decoder": 4,
"prettier/prettier": 116,
"promise/always-return": 3,
"promise/catch-or-return": 2,
"promise/param-names": 8,
"no-empty-function": 2,
"no-shadow": 8,
"no-unused-private-class-members": 6
}
129 changes: 92 additions & 37 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
@@ -1,19 +1,12 @@
import base, { createConfig } from '@metamask/eslint-config';
import nodejs from '@metamask/eslint-config-nodejs';
import jest from '@metamask/eslint-config-jest';
import nodejs from '@metamask/eslint-config-nodejs';
import typescript from '@metamask/eslint-config-typescript';

const config = createConfig(
const config = createConfig([
...base,
{
ignores: [
'yarn.lock',
'**/**.map',
'**/**.tsbuildinfo',
'**/*.json',
'**/*.md',
'**/LICENSE',
'**/*.sh',
'**/.DS_Store',
'**/dist/**',
'**/docs/**',
'**/coverage/**',
Expand All @@ -22,7 +15,6 @@ const config = createConfig(
'scripts/create-package/package-template/**',
],
},
...base,
{
rules: {
// Left disabled because various properties throughough this repo are snake_case because the
Expand All @@ -32,14 +24,12 @@ const config = createConfig(
'id-length': 'off',

// TODO: re-enble most of these rules
'@typescript-eslint/naming-convention': 'off',
'function-paren-newline': 'off',
'id-denylist': 'off',
'implicit-arrow-linebreak': 'off',
'import/no-anonymous-default-export': 'off',
'import/no-unassigned-import': 'off',
'import-x/no-anonymous-default-export': 'off',
'import-x/no-unassigned-import': 'off',
'lines-around-comment': 'off',
'n/no-sync': 'off',
'no-async-promise-executor': 'off',
'no-case-declarations': 'off',
'no-invalid-this': 'off',
Expand All @@ -53,6 +43,12 @@ const config = createConfig(
'off',
{ matchDescription: '^[A-Z`\\d_][\\s\\S]*[.?!`>)}]$' },
],

// TODO: These rules created more errors after the upgrade to ESLint 9.
// Re-enable these rules and address any lint violations.
'import-x/no-named-as-default-member': 'warn',
'prettier/prettier': 'warn',
'no-empty-function': 'warn',
},
settings: {
jsdoc: {
Expand All @@ -62,54 +58,76 @@ const config = createConfig(
},
{
files: [
'**/jest.config.js',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This list of globs should be functionally equivalent as we had some overlap previously.

'**/jest.environment.js',
'**/tests/**/*.{ts,js}',
'*.js',
'*.test.{ts,js}',
'**/*.{js,cjs,mjs}',
'**/*.test.{js,ts}',
'**/tests/**/*.{js,ts}',
'scripts/*.ts',
'scripts/create-package/*.ts',
'yarn.config.cjs',
'scripts/create-package/**/*.ts',
],
extends: [nodejs],
rules: {
// TODO: Re-enable this
'n/no-sync': 'off',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This rule was previously disabled earlier in this file but the Node rules may re-enable this rule so we here we explicitly disable it.

// TODO: These rules created more errors after the upgrade to ESLint 9.
// Re-enable these rules and address any lint violations.
'n/no-unsupported-features/node-builtins': 'warn',
},
},
{
files: ['*.test.{ts,js}', '**/tests/**/*.{ts,js}'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Globs changed in ESLint 9. You need to prepend the glob with **/ if you want to look for files in any directory, not just the root.

files: ['**/*.test.{js,ts}', '**/tests/**/*.{js,ts}'],
extends: [jest],
rules: {
// TODO: These rules created more errors after the upgrade to ESLint 9.
// Re-enable these rules and address any lint violations.
'jest/no-conditional-in-test': 'warn',
},
},
{
// These files are test helpers, not tests. We still use the Jest ESLint
// config here to ensure that ESLint expects a test-like environment, but
// various rules meant just to apply to tests have been disabled.
files: ['**/tests/**/*.{ts,js}', '!*.test.{ts,js}'],
files: ['**/tests/**/*.{js,ts}'],
ignores: ['**/*.test.{js,ts}'],
rules: {
'jest/no-export': 'off',
'jest/require-top-level-describe': 'off',
'jest/no-if': 'off',
},
},
{
files: ['*.js', '*.cjs'],
parserOptions: {
files: ['**/*.{js,cjs}'],
languageOptions: {
sourceType: 'script',
ecmaVersion: '2020',
ecmaVersion: 2020,
},
},
{
files: ['*.ts'],
files: ['**/*.ts'],
extends: [typescript],
parserOptions: {
tsconfigRootDir: import.meta.dirname,
project: ['./tsconfig.packages.json'],
languageOptions: {
parserOptions: {
tsconfigRootDir: import.meta.dirname,
project: './tsconfig.packages.json',
// Disable `projectService` because we run into out-of-memory issues.
// See this ticket for inspiration out how to solve this:
// <https://github.com/typescript-eslint/typescript-eslint/issues/1192>
projectService: false,
Copy link
Contributor Author

@mcmire mcmire Jan 13, 2025

Choose a reason for hiding this comment

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

Disabling projectService is inconsistent with the module template, but I had to do this in order to even run eslint. Disabling projectService does not seem to make eslint run more slowly.

},
},
rules: {
// This rule does not detect multiple imports of the same file where types
// are being imported in one case and runtime values are being imported in
// another
'import-x/no-duplicates': 'off',

// Enable rules that are disabled in `@metamask/eslint-config-typescript`
'@typescript-eslint/no-explicit-any': 'error',

// TODO: auto-fix breaks stuff
'@typescript-eslint/promise-function-async': 'off',

// TODO: re-enable most of these rules
'@typescript-eslint/naming-convention': 'off',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This rule was previously disabled earlier in this file but the TypeScript rules may re-enable this rule so we here we explicitly disable it.

'@typescript-eslint/no-unnecessary-type-assertion': 'off',
'@typescript-eslint/unbound-method': 'off',
'@typescript-eslint/prefer-enum-initializers': 'off',
Expand All @@ -118,35 +136,72 @@ const config = createConfig(
'@typescript-eslint/prefer-reduce-type-parameter': 'off',
'no-restricted-syntax': 'off',
'no-restricted-globals': 'off',

// TODO: These rules created more errors after the upgrade to ESLint 9.
// Re-enable these rules and address any lint violations.
'@typescript-eslint/consistent-type-exports': 'warn',
'@typescript-eslint/explicit-function-return-type': 'off',
'@typescript-eslint/no-base-to-string': 'warn',
'@typescript-eslint/no-duplicate-enum-values': 'warn',
'@typescript-eslint/no-misused-promises': 'warn',
'@typescript-eslint/no-unsafe-enum-comparison': 'warn',
'@typescript-eslint/no-unused-vars': 'warn',
'@typescript-eslint/only-throw-error': 'warn',
'@typescript-eslint/prefer-promise-reject-errors': 'warn',
'@typescript-eslint/prefer-readonly': 'warn',
'@typescript-eslint/switch-exhaustiveness-check': 'warn',
'import-x/namespace': 'warn',
'import-x/no-named-as-default': 'warn',
'import-x/order': 'warn',
'jsdoc/check-tag-names': 'warn',
'jsdoc/require-returns': 'warn',
'jsdoc/tag-lines': 'warn',
'no-unused-private-class-members': 'warn',
'promise/always-return': 'warn',
'promise/catch-or-return': 'warn',
'promise/param-names': 'warn',
},
},
{
files: ['tests/setupAfterEnv/matchers.ts'],
parserOptions: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

languageOptions: {
sourceType: 'script',
},
},
// This should really be in `@metamask/eslint-config-typescript`
{
files: ['*.d.ts'],
files: ['**/*.d.ts'],
rules: {
'@typescript-eslint/naming-convention': 'warn',
'import/unambiguous': 'off',
'import-x/unambiguous': 'off',
},
},
{
files: ['scripts/*.ts'],
rules: {
// All scripts will have shebangs.
'n/shebang': 'off',
// Scripts may be self-executable and thus have hashbangs.
'n/hashbang': 'off',
},
},
{
files: ['**/jest.environment.js'],
rules: {
// These files run under Node, and thus `require(...)` is expected.
'n/global-require': 'off',

// TODO: These rules created more errors after the upgrade to ESLint 9.
// Re-enable these rules and address any lint violations.
'n/prefer-global/text-encoder': 'warn',
'n/prefer-global/text-decoder': 'warn',
'no-shadow': 'warn',
},
},
{
files: ['**/*.mjs'],
languageOptions: {
sourceType: 'module',
},
},
);
]);

export default config;
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"lint": "yarn lint:eslint && yarn lint:misc --check && yarn constraints && yarn lint:dependencies && yarn lint:teams",
"lint:dependencies": "depcheck && yarn dedupe --check",
"lint:dependencies:fix": "depcheck && yarn dedupe",
"lint:eslint": "eslint . --cache",
"lint:eslint": "yarn ts-node ./scripts/run-eslint.ts --cache",
"lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies:fix",
"lint:misc": "prettier --no-error-on-unmatched-pattern '**/*.json' '**/*.md' '**/*.yml' '!.yarnrc.yml' '!merged-packages/**' --ignore-path .gitignore",
"lint:teams": "ts-node scripts/lint-teams-json.ts",
Expand Down
1 change: 1 addition & 0 deletions packages/controller-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"@types/jest": "^27.4.1",
"deepmerge": "^4.2.2",
"jest": "^27.5.1",
"jest-environment-jsdom": "^27.5.1",
"nock": "^13.3.1",
"ts-jest": "^27.1.4",
"typedoc": "^0.24.8",
Expand Down
2 changes: 1 addition & 1 deletion packages/controller-utils/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ function logOrRethrowError(error: unknown, codesToCatch: number[] = []) {
throw error;
}
} else {
// eslint-disable-next-line @typescript-eslint/no-throw-literal
// eslint-disable-next-line @typescript-eslint/only-throw-error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The no-throw-literal rule was essentially renamed to only-throw-error: typescript-eslint/typescript-eslint#8701

throw error;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/json-rpc-engine/src/JsonRpcEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ export class JsonRpcEngine extends SafeEventEmitter {
// Now we re-throw the middleware processing error, if any, to catch it
// further up the call chain.
if (error) {
// eslint-disable-next-line @typescript-eslint/no-throw-literal
// eslint-disable-next-line @typescript-eslint/only-throw-error
throw error;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/json-rpc-middleware-stream/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { createStreamMiddleware, createEngineStream } from '.';
const artificialDelay = async (time = 0) =>
new Promise((resolve) => setTimeout(resolve, time));
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-explicit-any
// eslint-disable-next-line @typescript-eslint/no-empty-function,@typescript-eslint/no-explicit-any
const noop = function (_a: any) {};

const jsonrpc = '2.0' as const;
Expand Down
1 change: 0 additions & 1 deletion packages/keyring-controller/src/KeyringController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1580,7 +1580,6 @@ export class KeyringController extends BaseController<
* @deprecated Use `withKeyring` instead.
*/
async cancelQRSynchronization(): Promise<void> {
// eslint-disable-next-line n/no-sync
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unsure why the Node rules were getting applied to these files. ESLint was reporting that this rule no longer applied, so I removed it. (Similar for other cases.)

(await this.getOrAddQRKeyring()).cancelSync();
}

Expand Down
2 changes: 1 addition & 1 deletion packages/message-manager/src/AbstractMessageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { BaseControllerV1 } from '@metamask/base-controller';
import type { ApprovalType } from '@metamask/controller-utils';
import type { Hex, Json } from '@metamask/utils';
// This package purposefully relies on Node's EventEmitter module.
// eslint-disable-next-line import/no-nodejs-modules
// eslint-disable-next-line import-x/no-nodejs-modules
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are now using eslint-plugin-import-x, a fork of eslint-plugin-import.

import { EventEmitter } from 'events';
import { v1 as random } from 'uuid';

Expand Down
2 changes: 0 additions & 2 deletions packages/network-controller/src/create-network-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ export function createNetworkClient(
const rpcProvider = providerFromMiddleware(rpcApiMiddleware);

const blockTrackerOpts =
// eslint-disable-next-line n/no-process-env
process.env.IN_TEST && networkConfig.type === 'custom'
? { pollingInterval: SECOND }
: {};
Expand Down Expand Up @@ -190,7 +189,6 @@ function createCustomNetworkMiddleware({
chainId: Hex;
rpcApiMiddleware: JsonRpcMiddleware<JsonRpcParams, Json>;
}): JsonRpcMiddleware<JsonRpcParams, Json> {
// eslint-disable-next-line n/no-process-env
const testMiddlewares = process.env.IN_TEST
? [createEstimateGasDelayTestMiddleware()]
: [];
Expand Down
7 changes: 5 additions & 2 deletions packages/network-controller/tests/NetworkController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13069,6 +13069,7 @@ function refreshNetworkTests({
initialState?: Partial<NetworkState>;
operation: (controller: NetworkController) => Promise<void>;
}) {
// eslint-disable-next-line jest/require-top-level-describe
it('emits networkWillChange with state payload', async () => {
await withController(
{
Expand Down Expand Up @@ -13097,6 +13098,7 @@ function refreshNetworkTests({
);
});

// eslint-disable-next-line jest/require-top-level-describe
it('emits networkDidChange with state payload', async () => {
await withController(
{
Expand Down Expand Up @@ -13126,6 +13128,7 @@ function refreshNetworkTests({
});

if (expectedNetworkClientConfiguration.type === NetworkClientType.Custom) {
// eslint-disable-next-line jest/require-top-level-describe
it('sets the provider to a custom RPC provider initialized with the RPC target and chain ID', async () => {
await withController(
{
Expand Down Expand Up @@ -13167,8 +13170,7 @@ function refreshNetworkTests({
);
});
} else {
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
// eslint-disable-next-line jest/require-top-level-describe
it(`sets the provider to an Infura provider pointed to ${expectedNetworkClientConfiguration.network}`, async () => {
await withController(
{
Expand Down Expand Up @@ -13209,6 +13211,7 @@ function refreshNetworkTests({
});
}

// eslint-disable-next-line jest/require-top-level-describe
it('replaces the provider object underlying the provider proxy without creating a new instance of the proxy itself', async () => {
await withController(
{
Expand Down
Loading
Loading