Issue implicit any in JS files on widened inferred types from initializers in binding patterns #1064
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR ports missing logic from #60083 to improve how JS initializers produce widened types and report implicit any in empty literals.
- Adds
getWidenedLiteralTypeForInitializerto centralize literal widening. - Extends
widenTypeInferredFromInitializerto report and returnany/any[]for empty literals in JS files. - Updates binding-element handling to use the new helper.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/checker/checker.go | Introduces getWidenedLiteralTypeForInitializer and adjusts widenTypeInferredFromInitializer; updates binding-element call site. |
| testdata/baselines/reference/.../*.types | Updates expected baselines to reflect any/any[] instead of never for empty initializers. |
| testdata/baselines/reference/.../*.errors.txt | Adjusts error baselines to include implicit-any diagnostics for empty literals. |
Comments suppressed due to low confidence (1)
internal/checker/checker.go:17022
- This change bypasses
widenTypeInferredFromInitializer, which includes JS-specific implicit-anydiagnostics for empty initializers. Consider callingwidenTypeInferredFromInitializerhere instead ofgetWidenedLiteralTypeForInitializerto preserve those diagnostics.
return c.addOptionality(c.getWidenedLiteralTypeForInitializer(element, c.checkDeclarationInitializer(element, CheckModeNormal, contextualType)))
|
Can you remerge main here? |
…y-widening-binding-patterns
|
done |
| json = [] | ||
| + ~~~~ | ||
| +!!! error TS7031: Binding element 'json' implicitly has an 'any[]' type. |
There was a problem hiding this comment.
I also tried porting this and hit these same issues. I'm pretty sure these are exactly the things that we are aiming to prevent, but now I think the fix doesn't quite work anymore?
There was a problem hiding this comment.
I'll take a look at this tomorrow~ then
There was a problem hiding this comment.
This PR aims to issue implicit any error - and it does that... to some extent 😉 It is overly eager though. From what I can see, this one is still not handled by the reparser - and that's why we are seeing errors here. The compiler thinks it doesn't have a type annotation
There was a problem hiding this comment.
I think @a-tarasyuk had a recent PR that improves handling of destructured parameters. Do things improve if you merge from main?
There was a problem hiding this comment.
yes! destructuringParameterDeclaration9(strict=true).errors.txt.diff is completely gone now
testdata/baselines/reference/submodule/conformance/typeFromJSInitializer.errors.txt
Show resolved
Hide resolved
…y-widening-binding-patterns # Conflicts: # testdata/baselines/reference/submodule/conformance/destructuringParameterDeclaration9(strict=true).types # testdata/baselines/reference/submoduleAccepted/conformance/destructuringParameterDeclaration9(strict=true).types.diff
| @@ -17212,7 +17227,7 @@ func (c *Checker) getTypeFromBindingElement(element *ast.Node, includePatternInT | |||
| if ast.IsBindingPattern(element.Name()) { | |||
| contextualType = c.getTypeFromBindingPattern(element.Name(), true /*includePatternInType*/, false /*reportErrors*/) | |||
| } | |||
| return c.addOptionality(c.widenTypeInferredFromInitializer(element, c.checkDeclarationInitializer(element, CheckModeNormal, contextualType))) | |||
There was a problem hiding this comment.
this new function is called from a couple of other places in Strada. Why not in Corsa too?
There was a problem hiding this comment.
I only see 3 getWidenedLiteralTypeForInitializer hits in both codebases 🤔 And they seem to be in the same places. Same for widenTypeInferredFromInitializer (4 hits)
There was a problem hiding this comment.
How is that possible? This PR is introducing getWidenedLiteralTypeForInitializer and only adds one call
There was a problem hiding this comment.
it splits the old widenTypeInferredFromInitializer into widenTypeInferredFromInitializer and getWidenedLiteralTypeForInitializer. You have the second call to getWidenedLiteralTypeForInitializer at the top of the new version of widenTypeInferredFromInitializer
There was a problem hiding this comment.
Right, but the old code had other calls to the get variant
...nce/submoduleAccepted/conformance/destructuringParameterDeclaration9(strict=true).types.diff
Show resolved
Hide resolved
ports microsoft/TypeScript#60083