Improve checking of in operator
#50666
Conversation
typescript-bot
added
Author: Team
For Backlog Bug
# Conflicts: # src/compiler/checker.ts
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at d178ea4. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at d178ea4. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at d178ea4. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at d178ea4. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at d178ea4. You can monitor the build here. Update: The results are in! |
ahejlsberg
requested review from
andrewbranch,
DanielRosenwasser and
RyanCavanaugh
|
@ahejlsberg Here are the results of running the user test suite comparing Something interesting changed - please have a look. Details
|
|
@ahejlsberg Here they are:Comparison Report - main..50666
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
|
@ahejlsberg Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
|
Hi, |
|
@marekdedic Sorry, I didn't originally notice the existing PR. Yes, I'd say this one is an improvement in that it tightens the checking of the individual operands (e.g. disallowing unconstrained generic types), it removes dubious exceptions for |
|
The user code test suite has two changes: One is a better error message, the other is an error that goes away because we now reflect an The RWC test suite has a few new errors due to No changes in Definitely Typed tests. The top 100 tests are catching new errors in Performance is unchanged. |
|
One question I'm looking for feedback on is this: function foo1(x: {}) {
if ("a" in x) {
x; // Record<"a", unknown>
}
}The tightened operand validation rules requires the right hand operand to be of a type that is assignable to The issue above is particularly unfortunate in function foo(x: unknown) {
if (x && "a" in x) {
x; // Record<"a", unknown>
}
}The truthy check narrows I'm leaning towards closing this hole, though it may cause additional breaks. EDIT: We now check that the type of the right operand isn't |
|
@typescript-bot pack this |
|
Heya @andrewbranch, I've started to run the tarball bundle task on this PR at 5d31ba8. You can monitor the build here. |
|
Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
For me the problem got fixed when updating to TypeScript 4.9. I also made a video about this improvement: https://www.youtube.com/watch?v=oPkthSFxAHc 🎬 |
This PR improves checking of the
inoperator:key in obj, we now require the type ofkeyto be assignable tostring | number | symbol. Previously we allowedkeyto be of an unconstrained type parameter type.key in obj, we now require the type ofobjto be assignable toobject. Previously we allowedobjto be of an unconstrained generic type (which might be a primitive type, causing an exception to be thrown at runtime).key in obj, wherekeyis of a string literal, numeric literal, or unique symbol type, narrowsobjas follows:keynames a property that exists in some constituent of the type ofobj, the type ofobjis narrowed based on the presence or absence of that property. Previously we only did this for string literal keys.keynames a property that doesn't exist in any constituent of the type ofobj, the type ofobjis narrowed by intersecting withRecord<typeof key, unknown>. Previously we did nothing in this case.Some examples:
I'm marking this PR as fixing #21732 even though it doesn't address the case of
key in objwherekeyis of some generic type. In general, whenkeyis of some non-finite type (likestring), the only effect we can meaningfully reflect following akey in objcheck is thatobj[key]should be a valid expression (of typeunknown), providedkeyandobjare both known not to have been modified since the check. This might be possible to do in the true branch of anifstatement or a ternary operator, but it isn't possible in all control flow scenarios.This PR is a breaking change because of the more accurate checking of the
inoperands and the more consistent narrowing of the right hand operand in control flow analysis.Fixes #21732.
Fixes #50639.