Rule Change: [no-extra-parens] assume unknown node types need to be parenthesised if they are parenthesised

[bradzacher]

What rule do you want to change?

no-extra-parens

What change to do you want to make?

Generate fewer warnings

How do you think the change should be implemented?

A new default behavior

Example code

const x = (1 satisfies number).toFixed();
const y = (1 as number).toFixed();

What does the rule currently do for this code?

The rule currently errors on this TypeScript code because it doesn't understand that TSAsExpression and TSSatisfiesExpression have a lower precedence than MemberExpression.

This occurs because the precedence of "unknown" node types is automatically set to the maximum precedence:

eslint/lib/rules/utils/ast-utils.js

Lines 1444 to 1445 in 8bf5505

	default:
	return 20;

So whenever the rule compares precedence it assumes the unknown node doesn't need parens

What will the rule do after it's changed?

For existing JS code - no change at all.
For non-JS code (eg TS) that has new AST node types - the rule will assume that these new nodes must be wrapped in parentheses and will not report.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

We do have an extension rule, but some users do use the base rule (either because they choose to or accidentally).
This change would stop annoying false positives which will make users' lives better.

[sam3k]

Considering that const z = (2).toFixed(); is perfectly okay and it passes; I think this is a good rule change. You got my +1. 👍

[mdjermanovic]

assume unknown node types need to be parenthesised if they are parenthesised

Makes sense to me 👍

@nzakas what do you think?

[bradzacher]

My proposal for the change is essentially this:

 default:
-  return 20;
+  return node.type in eslintVisitorKeys ? 20 : -1;

WDYT?

[mdjermanovic]

My proposal for the change is essentially this:

 default:
-  return 20;
+  return node.type in eslintVisitorKeys ? 20 : -1;

WDYT?

I think this will make sense for most places where astUtils.getPrecedence is used, but we should check each to see if the current assumption that unknown nodes have higher precedence seems better and in that case handle the -1 return value separately. For example, in prefer-exponentiation-operator here where we check if a ** b should be parenthesized based on its parent.

[bradzacher]

Maybe instead parameterise it so the usecases can choose?

function getPrecedence(node, useLowestForUnknownNodes = true) {
  switch (node.type) {
    // ...

    default:
      return !useLowestForUnknownNodes || node.type in eslintVisitorKeys ? 20 : -1;
  }
}

[mdjermanovic]

Maybe instead parameterise it so the usecases can choose?

function getPrecedence(node, useLowestForUnknownNodes = true) {
  case (node.type) {
    // ...

    default:
     return !useLowestForUnknownNodes || node.type in eslintVisitorKeys ? 20 : -1;
  }
}

I think it's simpler and more flexible to always return -1 for unknown nodes and let the caller handle it as a special case if needed for the use case.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[fasttime]

@nzakas I think your feedback is wanted: #17173 (comment)

[nzakas]

assume unknown node types need to be parenthesised if they are parenthesised

Makes sense to me. 👍

Thanks for the ping @fasttime -- still catching up after missing a couple weeks.

[fasttime]

This change request has been accepted. @bradzacher would you like to submit a PR?

[bradzacher]

Sure - I'll look into it!