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
Improve (and actually use) "always truthy promise" error #43023
Conversation
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
tests/cases/compiler/truthinessCallExpressionCoercion.ts(66,13): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? | ||
tests/cases/compiler/truthinessCallExpressionCoercion.ts(76,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? | ||
tests/cases/compiler/truthinessCallExpressionCoercion.ts(82,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? | ||
tests/cases/compiler/truthinessCallExpressionCoercion.ts(2,9): error TS2774: This condition will always return true since this function appears to always be defined. Did you mean to call it instead? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 I think I first introduced this error message back when I contributed the first uncalled function check. I admit I didn't give it in-depth thought and it's kind of bothered me since. Since it's being adjusted, can I suggest one additional tweak?
tests/cases/compiler/truthinessCallExpressionCoercion.ts(2,9): error TS2774: This condition will always return true since this function appears to always be defined. Did you mean to call it instead? | |
tests/cases/compiler/truthinessCallExpressionCoercion.ts(2,9): error TS2774: This condition will always evaluate to true since this function appears to always be defined. Did you mean to call it instead? |
Rationale being that return
is a function-level concern, but these are almost always expressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That also kind of bugged me - what about "This condition will always be true"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even better!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like swapping "appears to always be" with "is". The wiggle room that the words give you isn't worth the extra reading, and the associated drop in number of people who will read to the end.
Just giving an incorrect error, regardless of wording, drops the compiler's credibility to zero, so you might as well be confident and clear in your wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, except back out the "appears to" addition.
tests/cases/compiler/truthinessCallExpressionCoercion.ts(66,13): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? | ||
tests/cases/compiler/truthinessCallExpressionCoercion.ts(76,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? | ||
tests/cases/compiler/truthinessCallExpressionCoercion.ts(82,9): error TS2774: This condition will always return true since the function is always defined. Did you mean to call it instead? | ||
tests/cases/compiler/truthinessCallExpressionCoercion.ts(2,9): error TS2774: This condition will always return true since this function appears to always be defined. Did you mean to call it instead? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like swapping "appears to always be" with "is". The wiggle room that the words give you isn't worth the extra reading, and the associated drop in number of people who will read to the end.
Just giving an incorrect error, regardless of wording, drops the compiler's credibility to zero, so you might as well be confident and clear in your wording.
Addresses some of the concerns in #43004 (comment)