Repository navigation
Apply uncalled function checks to boolean expressions (ts2774) #46051
Description
Suggestion
Similar to #36048 (implemented in #36402), it would be great if uncalled function checks were applied to expressions like they are for if statements and ternaries.
export default class Range {
lowerBound: number
upperBound: number
constructor(lowerBound: number, upperBound: number) {
this.lowerBound = lowerBound
this.upperBound = upperBound
}
isEmpty(): boolean {
return this.lowerBound == this.upperBound
}
overlaps(other: Range): boolean {
const isDisjoint = other.upperBound <= this.lowerBound
|| this.upperBound <= other.lowerBound
|| this.isEmpty || other.isEmpty // <-- Always true without an error
return !isDisjoint
}
}🔍 Search Terms
This condition will always return true since this function is always defined. Did you mean to call it instead? ts2774
✅ Viability Checklist
My suggestion meets these guidelines:
- This wouldn't be a breaking change in existing TypeScript/JavaScript code
- This wouldn't change the runtime behavior of existing JavaScript code
- This could be implemented without emitting different JS based on the types of the expressions
- This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
- This feature would agree with the rest of TypeScript's Design Goals.
⭐ Suggestion
It would be great if using an uncalled function in a boolean expression was an error.
📃 Motivating Example
It would be great if this was true:
// Error
const bad = false || new Range().isEmpty
// Works!
const good = false || new Range().isEmpty()💻 Use Cases
Recently spent awhile debugging some code that was always returning true before realizing I was checking the truthyness of an uncalled function.
Activity
[-]Apply uncalled function checks to boolean expressions [/-][+]Apply uncalled function checks to boolean expressions (ts2774)[/+]andrewbranch commentedon Oct 5, 2021
So, we can’t quite do exactly what this issue proposes, because the type of the logical expression ends up including the function itself, which only looks like a mistake if it never gets called. If the variable gets carried forward or passed around, it can be difficult or impossible to decide if it did ever get called. The two things that makes it possible to do these checks for
ifstatements and ternaries arefoo ? foo() : undefined, the user is clearly performing defensive checks, which might appear overly defensive to our type system, but sometimes types are wrong and something can beundefinedwhen we think it can’t. Contrast this to assigning a logical expression to a variable, where we’d have to analyze everything in the same scope as the variable to find its uses. The if/ternary usually presents us a small (and definitely file-local) boundary where we have to check. If a variable for a logical expression appears in the global scope, it could be used anywhere in the whole program.I think the place where you could maybe argue that we go wrong is not at
const isDisjoint = ..., but atreturn !isDisjoint. The type ofisDisjointistrue | (() => boolean), both sides of which are truthy, and so we could make the determination that this function always returns false, and perhaps say that’s suspicious. But on the other hand, it makes total sense for some functions/methods to return constant values, so that analysis alone is insufficient.The example is super compelling, but I’m having trouble coming up with anything we could do that wouldn’t severely break something else.