Flag non-any/number/string operands on either side of comparison as errors #27926
Conversation
|
|
src/compiler/checker.ts
Outdated
| (t === TypeFlags.Any) || | ||
| (t === TypeFlags.TypeParameter) || | ||
| (t === TypeFlags.Enum) || | ||
| (t === TypeFlags.Object) || |
There was a problem hiding this comment.
This differs from the PR description and the proposed change in the linked issue. It allows basically any structured type. That means all objects are allowed. That's awesome if you need to compare Date objects but may not be desired in all other cases.
There was a problem hiding this comment.
I have added some more types than the ones done in the proposed change:
- Object,
- Intersection,
- TypeFlags.EnumLiteral | TypeFlags.Union | TypeFlags.UnionOfPrimitiveType
- TypeFlags.Union | TypeFlags.UnionOfPrimitiveTypes
for TypeScript to continue compiling.
Without the Object type selection, the compilation of TypeScript will fail because Dates are compared somewhere in the code.
|
@typescript-bot test this |
|
Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at a9939ab. You can monitor the build here. It should now contribute to this PR's status checks. |
|
I see that the extended tests fail, but I'm not able to reproduce the failure locally, and cannot find specific messages of the errors. |
|
It's marked as failing, but the contents of the (small) change look benign - likely just a small diff between what commits from |
|
Thanks for working on this, it would be great to understand what went wrong with the tests. |
|
Well, at this point it needs to be synced with |
|
Ok, I'll work on this asap |
|
@feloy will you been able to sync this? |
|
Hi @weswigham I will try to work on this this week. If without news before next monday, you can consider I've dropped |
feloy
force-pushed
the
feloy-15506
branch
3 times, most recently
from
2b43e5b to
3559a93
Compare
|
Hi @weswigham I hope I've made all the necessary changes |
| @@ -22760,6 +22762,19 @@ namespace ts { | |||
| return Debug.fail(); | |||
| } | |||
|
|
|||
| function checkTypeComparableWithLessOrGreaterThanOperator(valueType: Type): boolean { | |||
| const t = valueType.flags; | |||
| return (t === TypeFlags.String) || | |||
There was a problem hiding this comment.
I'm pretty sure all these comparisons should be using & and not ===. I'm also curious why NumberLiteral (and BigIntLiteral) is missing - I'd assume comparing 0 and 2 (and 0n and 2n) is probably OK.
There was a problem hiding this comment.
I'd definitely like to see some tests comparing numeric literal types of all sorts among one another and with their bases (number, bigint).
There was a problem hiding this comment.
I have lots of troubles adapting the tests. I prefer to give up on this PR :(
Fixes #15506