Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes a bug that caused an anyFunctionType leak in the type checker. The changes update the contextual function checking logic and refine the checkMode usage to prevent caching of unintended return types.
- Default checkMode is now explicitly provided in the function signature.
- Simplified conditional checks and modified the return type computation to mask out CheckMode.SkipContextSensitive.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/cases/compiler/contextualTypingGenericFunction2.ts | Added tests to verify that inappropriate type inference in callback functions is now caught. |
| tests/baselines/reference/contextualTypingGenericFunction2.* | Updated baselines to match the expected errors from the fixed type checking logic. |
| src/compiler/checker.ts | Updated contextuallyCheckFunctionExpressionOrObjectLiteralMethod to use default checkMode and fixed return type computation to avoid caching anyFunctionType leaks. |
Comments suppressed due to low confidence (4)
src/compiler/checker.ts:9115
- Removing the redundant existence check for checkMode improves readability now that a default is provided.
) {
src/compiler/checker.ts:9115
- The explicit default value for checkMode enhances clarity; please ensure that existing call sites behave correctly with this default.
) {
src/compiler/checker.ts:9115
- The simplified condition using checkMode enhances consistency with previous changes; ensure the logical behavior remains as expected.
) {
|
@typescript-bot test it |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
This feels plausible, but I find it surprising that we haven't used this trick elsewhere? |
|
This leak is quite specific to generic functions containing context-sensitive parts when other parts of the argument(s) are context-sensitive too. The problem is that the generic function itself is not treated as context-sensitive per-se (despite it containing context-sensitive functions inside it) so without other context-sensitive parts of the arguments the initial But in the other situation, the |
fixes #61979