Skip to content

Commit

Permalink
Restore ESLint warnings as errors (ignoring them for now) (#4382)
Browse files Browse the repository at this point in the history
Currently, `yarn lint` produces a bunch of warnings from ESLint. These
warnings were added in a previous commit that upgraded ESLint packages
as a way to avoid fixing lint violations created by the upgrade.
However, this causes two problems:

1. They produce a lot of noise that makes it very difficult to find true
errors, especially when looking at a CI run.
2. They allow new instances of these violations to show up (because
warnings don't cause `eslint` to fail).

This commit removes the overrides from the ESLint config for these rules
so that they go back to being errors and adds `eslint-disable`
directives above the lines that cause the violations. Note that this is
not intended to be a long-term solution, and in the future, we should
dedicate time to either fixing the violations or explaining why ignoring
them is necessary.
  • Loading branch information
mcmire authored and legobeat committed Jun 11, 2024
1 parent bbb4c10 commit 4e03732
Show file tree
Hide file tree
Showing 119 changed files with 1,568 additions and 86 deletions.
8 changes: 1 addition & 7 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,13 @@ module.exports = {
// TODO: auto-fix breaks stuff
'@typescript-eslint/promise-function-async': 'off',

// TODO: re-enble most of these rules
'@typescript-eslint/await-thenable': 'warn',
'@typescript-eslint/naming-convention': 'off',
'@typescript-eslint/no-floating-promises': 'warn',
'@typescript-eslint/no-misused-promises': 'warn',
// TODO: re-enable most of these rules
'@typescript-eslint/no-unnecessary-type-assertion': 'off',
'@typescript-eslint/unbound-method': 'off',
'@typescript-eslint/prefer-enum-initializers': 'off',
'@typescript-eslint/prefer-nullish-coalescing': 'off',
'@typescript-eslint/prefer-optional-chain': 'off',
'@typescript-eslint/prefer-reduce-type-parameter': 'off',
'@typescript-eslint/restrict-plus-operands': 'warn',
'@typescript-eslint/restrict-template-expressions': 'warn',
'no-restricted-syntax': 'off',
'no-restricted-globals': 'off',
},
Expand Down
11 changes: 5 additions & 6 deletions packages/accounts-controller/src/AccountsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
BtcAccountType,
BtcMethod,
EthAccountType,
EthErc4337Method,
EthMethod,
} from '@metamask/keyring-api';
import { KeyringTypes } from '@metamask/keyring-controller';
Expand Down Expand Up @@ -184,7 +183,7 @@ function createExpectedInternalAccount({
}): InternalAccount {
const accountTypeToMethods = {
[`${EthAccountType.Eoa}`]: [...Object.values(EthMethod)],
[`${EthAccountType.Erc4337}`]: [...Object.values(EthErc4337Method)],
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
[`${BtcAccountType.P2wpkh}`]: [...Object.values(BtcMethod)],
};

Expand Down Expand Up @@ -2519,8 +2518,7 @@ describe('AccountsController', () => {
it('gets the next account name', async () => {
const messenger = buildMessenger();

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const accountsController = setupAccountsController({
const { messenger: controllerMessenger } = setupAccountsController({
initialState: {
internalAccounts: {
accounts: {
Expand All @@ -2532,6 +2530,7 @@ describe('AccountsController', () => {
},
messenger,
});
expect(controllerMessenger).toBe(messenger);

const accountName = messenger.call(
'AccountsController:getNextAvailableAccountName',
Expand All @@ -2542,8 +2541,7 @@ describe('AccountsController', () => {
it('gets the next account name with a gap', async () => {
const messenger = buildMessenger();

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const accountsController = setupAccountsController({
const { messenger: controllerMessenger } = setupAccountsController({
initialState: {
internalAccounts: {
accounts: {
Expand All @@ -2564,6 +2562,7 @@ describe('AccountsController', () => {
},
messenger,
});
expect(controllerMessenger).toBe(messenger);

const accountName = messenger.call(
'AccountsController:getNextAvailableAccountName',
Expand Down
2 changes: 2 additions & 0 deletions packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,8 @@ export class AccountsController extends BaseController<
* @param metadataKey - The key of the metadata to retrieve.
* @returns The value of the specified metadata key, or undefined if the account or metadata key does not exist.
*/
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
#populateExistingMetadata<T extends keyof InternalAccount['metadata']>(
accountId: string,
metadataKey: T,
Expand Down
6 changes: 6 additions & 0 deletions packages/address-book-controller/src/AddressBookController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ export interface ContactEntry {
}

export enum AddressType {
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
externallyOwnedAccounts = 'EXTERNALLY_OWNED_ACCOUNTS',
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
contractAccounts = 'CONTRACT_ACCOUNTS',
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
nonAccounts = 'NON_ACCOUNTS',
}

Expand Down
Loading

0 comments on commit 4e03732

Please sign in to comment.