I'm guessing a fair amount can be pruned from this list.... At a glance,

• default-parameters-last - @typescript-eslint/default-parameters-last should be used instead, which currently has no violations (not sure whether that's a coincidence or because it's already enabled everywhere - hard to tell from flat config 🙃).
• things affecting the website-eslint/src/mock directory (like no-var) - not sure these are really intended to be linted to modern standards... though perhaps it wouldn't hurt. See chore(website): add playground to website #4108
• prefer-destructuring - I am -1 on this rule in general. And especially -1 with the options that report const foo = arr[2] since it could be written const [_unused_1, _unused_2, foo] = arr (which I note is at least the first several of the 454 violations).

But, I'm sure there are some that would make sense to enable!

with the options that report const foo = arr[2] since it could be written const [_unused_1, _unused_2, foo] = arr

It would be const [, , foo] = arr.

It would be const [, , foo] = arr.

🥴

I'm +1 on the following rules:

• array-callback-return
• arrow-body-style
• no-await-in-loop (in principle; it looks like it's not too helpful in practice)
• no-lonely-if
• no-unreachable-loop (not sure if existing ones are bugs)
• no-useless-call
• no-useless-computed-key
• no-useless-concat
• no-useless-return
• no-var
• no-void
• operator-assignment
• prefer-arrow-callback
• prefer-const
• prefer-object-has-own
• prefer-object-spread
• prefer-rest-params
• radix

I'm +0.5 on the following rules:

• no-unused-expressions (with our extended version)
• object-shorthand (but needs to be disabled in some places)
• prefer-template
• prefer-destructuring (need some tweaking so it's less strict)

I'm -1 on the following rules:

• class-methods-use-this
• no-negated-condition
• no-undef-init

I basically agree with @Josh-Cena.

Only notable differences in opinion are

• prefer-destructuring (base or extension), -1 from me
• @typescript-eslint/class-methods-use-this, +0.5 (mostly for dogfooding reasons)

Note that, in my experience, prefer-template is good, but not if you just interpolate arbitrary expressions into the template in order to fix the report. Much better is to create a named variable for the expression and interpolate that.

but not if you just interpolate arbitrary expressions into the template in order to fix the report.

How is that different from using + to join arbitrary expressions?

If it interests you: I have written down my opinion on every ESLint rule here: https://jc-verse.github.io/js-style-guide/eslint-base/ ;) (WIP)

@Josh-Cena very cool! love that

For all the stylistic rules - throw them all away straight away - unless they have a fixer for all cases. Why? Migration effort for starters. Maintenance burden (eg copying in forked code). Contributor pains (having to manually adjust their natural code style to meet our preference).

I'm a -1 on prefer-destructuring. I don't believe that always destructuring is good. I only ever reach for destructuring when I have 2 or more properties, personally.
I dislike the always style cos it doesn't play nice with optional chaining. Eg const bar = foo?.bar; has no destructuring form - so you end up either having these "almost destructurable" cases in the codebase or you have to do like const {bar} = foo ?? {}; (ew).

I'm a -1 on no-undef-init too, personally. I like the explicitness of assigning undefined initially sometimes. It helps the code read better - eg "this variable starts with undefined and then is assigned foo later". It feels better to explicitly initialise the variable.

-1 on array-callback-return as it isn't type-aware so it can't understand exhaustive logic.

-1 on arrow-body-style - I don't believe that "always using implicit return" is a good style, personally. Sometimes you can get cleaner code and / or better formatting by adding the return.

-1000 on no-negated-condition. The premise of that rule is just plain wrong. There's absolutely nothing wrong with if (x != null) {...} else {...} - the code would be no clearer with if (x == null) as the condition -- in fact in many cases it could be less clear.

-0.1 on no-unreachable-loop. It's not great style but it can be a convenient way to write code that is "do this logic on the first element if there is one" as opposed to writing a length check and manually extracting the first element. Also not all iterators can be indexed.

-1 on no-useless-return. It can be a nice style to explicitly have a useless return to document a case. For example the first violation you list is exactly that - a "useless return" that exists to enumerate the switch cases and document why its purposely not handled.

-0.5 on prefer-object-spread - sometimes the Object.assign is better to work around inferred types.

How is that different from using + to join arbitrary expressions?

Good question - took me a minute to remember when and why I formed this opinion. The reason is I was working in a project where we had lots of code roughly like this

/** translates a string to the current lang */
declare function $t(s: string): string;
/** interact with the user in some way */
declare function promptUser(s: string): void;

promptUser($t("This is a string to be translated.") + " " + $t("Do you find this easy to read in a diff without syntax highlighting?"));

// after fix

promptUser(`${$t("This is a string to be translated.")} ${$t("Do you find this easy to read in a diff without syntax highlighting?")}`);

// extracted to variables

const thisIsAStringToBeTranslated = $t("This is a string to be translated.");
const canUReadIt = $t("Do you find this easy to read in a diff without syntax highlighting?");

promptUser(`${thisIsAStringToBeTranslated} ${canUReadIt}`);

IMO the second variant ("after fix") is by far the least readable, and it's really hard to notice the space in the middle compared to the other two forms.

In retrospect, I think this readability pitfall might have been pretty specific to that codebase due to the $t looking similar to ${ and the manually manually inserted space. So, unless something like this comes up when fixing the errors, nothing to stress about.

I dislike the always style cos it doesn't play nice with optional chaining. Eg const bar = foo?.bar; has no destructuring form - so you end up either having these "almost destructurable" cases in the codebase or you have to do like const {bar} = foo ?? {}; (ew).

The rule doesn't report on this. It also doesn't report on const bar: T = foo.bar.

I think the rule works very nicely with object properties and I would recommend. The only reservations I have are with array indices, where the reports are neither always better nor always correct (it transforms code using array-like protocol to iterable protocol).

-1 on array-callback-return as it isn't type-aware so it can't understand exhaustive logic.

Exhaustiveness should be asserted or with a fallback guard; but I agree it's less useful in TS because TS also checks this.

-1 on arrow-body-style - I don't believe that "always using implicit return" is a good style, personally. Sometimes you can get cleaner code and / or better formatting by adding the return.

Disagree—it increases code entropy for no clear reason. I'm in the camp that "if you remove something, you must remove all other extra syntax that was necessitated by it until your code is reduced to absolute minimum".

-0.1 on no-unreachable-loop. It's not great style but it can be a convenient way to write code that is "do this logic on the first element if there is one" as opposed to writing a length check and manually extracting the first element. Also not all iterators can be indexed.

You should definitely do const [first] = iterable then. for (const first of iterable) and array destructuring both use the iterator protocol and they are exactly equivalent. It's not nice to have others question if something is working properly or if it's just a hidden bug.

-1 on no-useless-return. It can be a nice style to explicitly have a useless return to document a case. For example the first violation you list is exactly that - a "useless return" that exists to enumerate the switch cases and document why its purposely not handled.

I agree that sometimes it's good to be defensive and preemptively terminate all possible branches, but I also think it's extra syntax to create confusion.

-0.5 on prefer-object-spread - sometimes the Object.assign is better to work around inferred types.

Well, either fix the types or assert? Object.assign is less ergonomic/sound with types due to it using intersections.

I do see @bradzacher's points, but tend to agree most with @Josh-Cena here.

At any rate, I'll open some draft PRs for some of these so we can start seeing what these individual rule changesets look like.

const bar = foo?.bar;
[prefer-destructuring] doesn't report on this. It also doesn't report on const bar: T = foo.bar.

Which is my point! My point is that if I write this code:

const bar = foo.bar;
const bar = foo?.bar;
const bar: T = foo.bar;

Then the rule would force me to instead write

const { bar } = foo;
const bar = foo?.bar;
const bar: T = foo.bar;

And now I've got inconsistently styled code.
I'd personally rather we have no rule and let people do what they want rather than a rule that enforces an inconsistent style.

Ummm... Sure? People can also opt into const { bar } = foo ?? {} as well. The point is that destructure-by-default is more extensible and less repetitive, and whether you want to use destructuring for other cases is up to you.

You say

It can be a nice style to explicitly have a useless return to document a case.
I agree that sometimes it's good to be defensive and preemptively terminate all possible branches, but I also think it's extra syntax to create confusion.

And you also say

Exhaustiveness should be asserted or with a fallback guard

So is the exhaustive pre-emption confusing extra syntax, or is it required?

The case I specifically mentioned was this, which is the last statement in a function:

https://github.com/typescript-eslint/typescript-eslint/blob/main/packages%2Feslint-plugin%2Fsrc%2Frules%2Fexplicit-module-boundary-types.ts#L220-L241

I think we've got our exhaustive switch rule turned on in our codebase, don't we?

The rule reports on the last case's return - saying its useless. It's not wrong but also the options are:

1. Remove it - bad because it's good to be exhaustive and violates the exhaustive rule.
2. Delete just the return - bad IMO because having an empty case at the end looks like a bug.

It's just an example of where the extra syntax isn't noise and it isn't confusing. TBH it took me a second to realise why the rule reported on it - cos the function decl header is many lines above and wasn't on my screen - so all I could see was the closing curlies.
Having the return made it clear "this case does not do anything ever because it just has a return statement".

It's why I'm a strong -1. I'd even bump it to a -2 on no-useless-return. It's a style that's not harmful and can even help clarify code.

This won't appear like a bug to anyone and is there to be future-proof:

switch (x) {
  case ...:
  default:
    throw new Error("Unreachable");
}

This is unnecessary syntax, is trivially refactorable, but isn't likely a bug:

doSomething(() => {
  return ...
});

This is unnecessary syntax, and is likely a bug:

for (const x of y) {
  ...
  return;
}

This is unnecessary syntax, could be preventing bugs, but the bug vector is unclear and more implicit, because it isn't meant to catch effect-at-a-distance kind of bug like switch exhaustiveness check does.

function x() {
  if (...) {
    ...
    return;
  } else {
    ...
    return;
  }
}

My point is that the rule flags this:

switch (v) {
  case a:
    return;

  case b:
    return; // flagged as unnecessary
}

My argument is that this is technically unnecessary but it:

• provides visual symmetry, which is nice when comparing cases
• provides clear and explicit termination of the branch
• makes code refactor-proof so I needn't add a return if I add a new case.

Eg

 case b:
+  return;
+
+case c:

Vs

 case b:
  return;
+
+case c:
+  return;

Smaller diffs are nicer.

I just don't personally see any positives with deleting a return, personally. But I see negatives.

We can agree to disagree here - I'm still a -1 on the rule.

Practically, in this case, I would suggest changing it to break, but I agree that the rule suggesting to remove return in this case is wrong.

Summary

(Someone please yell if any of these are wrong)

Accepted

Sounds like there is no objection for

• no-await-in-loop
• no-useless-call
• no-useless-computed-key
• no-useless-concat
• no-var
• no-void
• operator-assignment
• prefer-arrow-callback. chore: enable prefer-arrow-callback #9548
• prefer-const
• prefer-object-has-own
• prefer-rest-params
• radix
• @typescript-eslint/no-unused-expressions chore: enable @typescript-eslint/no-unused-expressions #9542
• prefer-template chore: enable prefer-template #9544

And sufficient consensus to proceed if someone wants to champion

• no-unreachable-loop: +1 from Josh Cena, -0.1 from Brad, +1 from me. chore: enable no-unreachable-loop #9540
• prefer-object-spread: +1 from Josh Cena, -0.5 from Brad, +1 from me. chore: enable prefer-object-spread #9541
• no-lonely-if: +1 from Josh Cena, -0.5 from me, +/-0 from Brad. chore: enable no-lonely-if #9547

Rejected/vetoed

• array-callback-return, +1 from Josh Cena, -1 from Brad, -0.5 from me.
• arrow-body-style, +1 from Josh Cena, -1 from Brad, -0.5 from me.
• no-useless-return: +1 from Josh Cena, -1/-2 from Brad, -1 from me.
• prefer-destructuring: +0.5 from Josh Cena, -1 from Brad, -1 from me.
• @typescript-eslint/class-methods-use-this: -1 from Josh Cena, +0.5 from me, -0.5 from Brad

EDIT - moved some around to reflect #9523 (comment)

I'm personally -0.5 on @typescript-eslint/class-methods-use-this.
I don't agree that all class methods must use this. It can be useful to extract functionality into a this-less function on the class for co-location.

The rule bans that pattern and forces you to move the function outside the class entirely as a standalone declararion - which I disagree with, personally.

no-lonely-if I'm indifferent towards.

Doesn't class-methods-use-this just want you to change those methods to static?

Looks like we have a plan of action! 🙂

I would just like to remind implementers of PRs for this issue (i.e. @abrahamguo 🙂) to pls double check for typescript-eslint extension rules for any of these. Might have missed some in the list.

Doesn't class-methods-use-this just want you to change those methods to static?

That's an API change and if you look at the reports I don't think they make sense.