Skip to content

Commit

Permalink
Fix ESLint config
Browse files Browse the repository at this point in the history
The upgrade to ESLint 9 which occurred in a previous commit was not done
correctly. An array must be passed to `createConfig` instead of a series
of arguments. As a result, a lot of lint rules were accidentally
disabled.

This commit fixes the ESLint config and corrects any lint violations.
It turns out that upgrading the ESLint packages created a bunch of new
lint violations, so those have been turned into warnings rather than
errors. To prevent new instances of these violations from occurring, a
script has been added which captures existing warning counts and exits
with a non-zero exit code if the count increases. This script is also
now used for `yarn lint:eslint` so that CI will fail if the count
increases.
  • Loading branch information
mcmire committed Jan 11, 2025
1 parent 3a7e6cb commit 17bcfe5
Show file tree
Hide file tree
Showing 29 changed files with 386 additions and 86 deletions.
28 changes: 28 additions & 0 deletions eslint-warnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"@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": 1,
"@typescript-eslint/no-unsafe-enum-comparison": 59,
"@typescript-eslint/no-unused-vars": 35,
"@typescript-eslint/prefer-promise-reject-errors": 13,
"@typescript-eslint/prefer-readonly": 147,
"@typescript-eslint/switch-exhaustiveness-check": 9,
"import-x/namespace": 189,
"import-x/no-duplicates": 32,
"import-x/no-named-as-default": 1,
"import-x/order": 208,
"jest/no-conditional-in-test": 104,
"jsdoc/check-tag-names": 372,
"jsdoc/require-returns": 22,
"jsdoc/tag-lines": 328,
"n/no-unsupported-features/node-builtins": 18,
"n/prefer-global/text-encoder": 4,
"n/prefer-global/text-decoder": 4,
"prettier/prettier": 115,
"promise/always-return": 3,
"promise/catch-or-return": 2,
"promise/param-names": 8,
"no-shadow": 8,
"no-unused-private-class-members": 6
}
136 changes: 89 additions & 47 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,13 @@ 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-named-as-default-member': '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,54 +44,62 @@ const config = createConfig(
'off',
{ matchDescription: '^[A-Z`\\d_][\\s\\S]*[.?!`>)}]$' },
],
},
settings: {
jsdoc: {
mode: 'typescript',
},

// These rules created more errors after the upgrade to ESLint 9.
// Re-enable these rules and address any lint violations.
'prettier/prettier': 'warn',
},
},
{
files: [
'**/jest.config.js',
'**/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',
// 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}'],
files: ['**/*.test.{js,ts}', '**/tests/**/*.{js,ts}'],
extends: [jest],
rules: {
// 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: {
sourceType: 'script',
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,
},
},
rules: {
// Enable rules that are disabled in `@metamask/eslint-config-typescript`
Expand All @@ -110,6 +109,8 @@ const config = createConfig(
'@typescript-eslint/promise-function-async': 'off',

// TODO: re-enable most of these rules
'@typescript-eslint/explicit-function-return-type': 'off',
'@typescript-eslint/naming-convention': 'off',
'@typescript-eslint/no-unnecessary-type-assertion': 'off',
'@typescript-eslint/unbound-method': 'off',
'@typescript-eslint/prefer-enum-initializers': 'off',
Expand All @@ -118,35 +119,76 @@ const config = createConfig(
'@typescript-eslint/prefer-reduce-type-parameter': 'off',
'no-restricted-syntax': 'off',
'no-restricted-globals': 'off',

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

// 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;
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
"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:misc": "prettier --no-error-on-unmatched-pattern '**/*.json' '**/*.md' '**/*.yml' '!eslint-warnings.json' '!.yarnrc.yml' '!merged-packages/**' --ignore-path .gitignore",
"lint:teams": "ts-node scripts/lint-teams-json.ts",
"prepack": "./scripts/prepack.sh",
"prepare-preview-builds": "./scripts/prepare-preview-builds.sh",
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
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 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
(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
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type { PushNotificationEnv } from '../../types/firebase';
declare const self: ServiceWorkerGlobalScope;

// Exported to help testing
// eslint-disable-next-line import/no-mutable-exports
// eslint-disable-next-line import-x/no-mutable-exports
export let supportedCache: boolean | null = null;

const getPushAvailability = async () => {
Expand Down
1 change: 0 additions & 1 deletion packages/permission-controller/src/PermissionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1982,7 +1982,6 @@ export class PermissionController<
target: string,
): void {
if (!isPlainObject(caveat)) {
// eslint-disable-next-line @typescript-eslint/no-throw-literal
throw new InvalidCaveatError(caveat, origin, target);
}

Expand Down
Loading

0 comments on commit 17bcfe5

Please sign in to comment.