Follow identifiers to their declaration in no-constant-condition

[captbaritone]

• What rule do you want to change? no-constant-condition
• Does this change cause the rule to produce more or fewer warnings?: More
  How will the change be implemented? (New option, new default behavior, etc.)?: New default behavior

Please provide some example code that this change will affect:

var foo = true;
if(foo) {}

• What does the rule currently do for this code?: Nothing
• What will the rule do after it's changed?: Warn/error
• Are you willing to submit a pull request to implement this change?: Possibly

This proposal has been split out of #13752

Currently no-constant-condition triggers on if(true){} but not on const foo = true; if(foo){}. In this case we could use the ScopeManager to attempt to follow foo to its declaration/assignment. If the variable is in scope and only assigned once, then we could check if the assigned value is constant.

In addition to assignments, we could also check other types of declarations. For example a function declaration could trigger an error:

function foo() {}

if(foo){} // <= foo is always truthy here

Additionally, @mdjermanovic pointed out that there may be other rules which could be employing a similar technique.

I've done a simple version of this for a rule I wrote (as mentioned in #13752) which could be used as a starting place.

Shout out to @bradzacher who first suggested this approach while we were iterating on my no-useless-null-checks rule.

[nzakas]

An interesting idea for sure. As long as we don’t go too far down this path (evaluating expressions rather than just checking for single truthy values), I’m in favor.

[nzakas]

TSC Summary: This proposal seeks to have no-constant-condition follow the value of identifiers and consider that value as part of it's evaluation. This would allow us to capture more errors when identifiers are assigned a value once and then used in a condition. Optionally, it might make sense to build this into eslint-scope to allow other rules to use the same functionality.

TSC Question: Shall we accept this proposal?

[btmills]

Marking this as accepted after discussion in today's TSC meeting. Since there are some complex parts here, we'd like to discuss the implementation details in a new RFC before starting work on a PR. It should help us decide where we draw the line on the extended checks and whether the implementation belongs in this rule or eslint-scope.

[github-actions]

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

[nzakas]

Just waiting for an RFC here

[captbaritone]

I can work on an RFC for this once I finish making a PR for #13752.

[Gautam-Arora24]

I would like to work on this issue.

[nzakas]

@Gautam-Arora24 awesome! The next step here is to create an RFC.

[Gautam-Arora24]

Sure!

[captbaritone]

@Gautam-Arora24 If you're still working on this, it would be worth including the proposed new no-constant-binary-expression rule (#15296) in the proposal as well since it would similarly benefit from following identifiers to their definition. I made some changes to the proposed rule for our local fork of it and, anecdotally, it detects on the order of twice as many issues.

[sam3k]

@Gautam-Arora24 do you still have interest in contributing a RFC?