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

Updated typescript-eslint to v6 #54693

Merged
merged 13 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
87 changes: 37 additions & 50 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
"node": true,
"es6": true
},
"extends": [
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
"plugin:@typescript-eslint/stylistic"
],
"plugins": [
"@typescript-eslint", "no-null", "import", "eslint-plugin-local", "simple-import-sort"
],
Expand All @@ -25,16 +30,10 @@
"/coverage/**"
],
"rules": {
"max-statements-per-line": ["error", { "max": 1 }],
"simple-import-sort/imports": "error",
"simple-import-sort/exports": "error",

"@typescript-eslint/adjacent-overload-signatures": "error",
"@typescript-eslint/array-type": "error",
"@typescript-eslint/no-array-constructor": "error",

"brace-style": "off",
"@typescript-eslint/brace-style": ["error", "stroustrup", { "allowSingleLine": true }],

"@typescript-eslint/naming-convention": [
"error",
{ "selector": "typeLike", "format": ["PascalCase"], "filter": { "regex": "^(__String|[A-Za-z]+_[A-Za-z]+)$", "match": false } },
Expand All @@ -48,45 +47,44 @@
{ "selector": "property", "format": null }
],

"@typescript-eslint/consistent-type-definitions": ["error", "interface"],
"@typescript-eslint/consistent-type-assertions": ["error", { "assertionStyle": "as" }],

"max-statements-per-line": ["error", { "max": 1 }],

"no-duplicate-imports": "off",
"@typescript-eslint/no-duplicate-imports": "error",

"@typescript-eslint/no-inferrable-types": "error",
"@typescript-eslint/no-misused-new": "error",
"@typescript-eslint/no-this-alias": "error",

"no-unused-expressions": "off",
"@typescript-eslint/no-unused-expressions": ["error", { "allowTernary": true }],

"@typescript-eslint/prefer-for-of": "error",
"@typescript-eslint/prefer-function-type": "error",
"@typescript-eslint/prefer-namespace-keyword": "error",
"@typescript-eslint/prefer-as-const": "error",

"quotes": "off",
"@typescript-eslint/brace-style": ["error", "stroustrup", { "allowSingleLine": true }],
"@typescript-eslint/no-extra-semi": "error",
"@typescript-eslint/quotes": ["error", "double", { "avoidEscape": true, "allowTemplateLiterals": true }],

"semi": "off",
"@typescript-eslint/semi": "error",
"@typescript-eslint/no-extra-semi": "error",

"space-before-function-paren": "off",
"@typescript-eslint/space-before-function-paren": ["error", {
"asyncArrow": "always",
"anonymous": "always",
"named": "never"
}],

"@typescript-eslint/triple-slash-reference": "error",
"@typescript-eslint/type-annotation-spacing": "error",
"@typescript-eslint/unified-signatures": "error",

"@typescript-eslint/no-extra-non-null-assertion": "error",
// Todo: For each of these, investigate whether we want to enable them ✨
"@typescript-eslint/ban-types": "off",
"no-case-declarations": "off",
"no-cond-assign": "off",
"no-constant-condition": "off",
"no-control-regex": "off",
"no-debugger": "off",
"no-extra-boolean-cast": "off",
"no-inner-declarations": "off",
"no-useless-escape": "off",
"prefer-rest-params": "off",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
"prefer-spread": "off",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
"@typescript-eslint/no-empty-function": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-unused-vars": "off",
Copy link
Member

Choose a reason for hiding this comment

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

Is this off because the tsconfig has the equivalent Typescript setting on? If not, what kind and how many unused variables do we have?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's this; honestly I would much prefer if we didn't use TS for this at all and instead made it a lint. So annoying to have to ensure my variables are all used just to run tests or something. At least debug skips typecheck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only 18 failures at the moment. Not bad!

However, typescript-eslint/typescript-eslint#5017 -> gajus/eslint-plugin-jsdoc#858 is impacting the codebase here. Probably best to set up the workarounds in a separate PR IMO.


// Pending https://github.com/typescript-eslint/typescript-eslint/issues/4820
"@typescript-eslint/prefer-optional-chain": "off",
Copy link
Member

Choose a reason for hiding this comment

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

Is this off because it causes lots of churn? If so, I'd like to see it turned on, but maybe in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, and also because the rule has more false positives right now than I'm comfortable with - especially in a codebase like TypeScript's that does a lot of funky stuff with scoping.

Copy link
Member

Choose a reason for hiding this comment

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

I'm honestly shocked we aren't getting lints from prefer-nullish-coalescing too... I was restructuring the lint config of hereby and hit some false positives there too on code like:

declare const a: string | undefined
declare const b: string

const foo = a || b;

Where the actual intent is that the empty string goes away. But maybe that's not a false positive.


// Rules enabled in typescript-eslint configs that are not applicable here
"@typescript-eslint/ban-ts-comment": "off",
"@typescript-eslint/class-literal-property-style": "off",
"@typescript-eslint/consistent-indexed-object-style": "off",
"@typescript-eslint/no-duplicate-enum-values": "off",
"@typescript-eslint/no-namespace": "off",
"@typescript-eslint/no-non-null-asserted-optional-chain": "off",
"@typescript-eslint/no-var-requires": "off",

// scripts/eslint/rules
"local/object-literal-surrounding-space": "error",
Expand All @@ -111,18 +109,14 @@
"no-null/no-null": "error",

// eslint
"constructor-super": "error",
"curly": ["error", "multi-line"],
"dot-notation": "error",
"eqeqeq": "error",
"linebreak-style": ["error", "windows"],
"new-parens": "error",
"no-caller": "error",
"no-duplicate-case": "error",
"no-empty": "error",
"no-eval": "error",
"no-extra-bind": "error",
"no-fallthrough": "error",
"no-new-func": "error",
"no-new-wrappers": "error",
"no-return-await": "error",
Expand All @@ -134,24 +128,17 @@
{ "name": "setImmediate" },
{ "name": "clearImmediate" }
],
"no-sparse-arrays": "error",
"no-template-curly-in-string": "error",
"no-throw-literal": "error",
"no-trailing-spaces": "error",
"no-undef-init": "error",
"no-unsafe-finally": "error",
"no-unused-labels": "error",
"no-var": "error",
"object-shorthand": "error",
"prefer-const": "error",
"prefer-object-spread": "error",
"quote-props": ["error", "consistent-as-needed"],
"space-in-parens": "error",
"unicode-bom": ["error", "never"],
"use-isnan": "error",
"no-prototype-builtins": "error",
"no-self-assign": "error",
"no-dupe-else-if": "error"
"unicode-bom": ["error", "never"]
},
"overrides": [
// By default, the ESLint CLI only looks at .js files. But, it will also look at
Expand Down
Loading