Preserve pseudotype when serializing autoType declarations#4551
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
This PR fixes .d.ts emission for var/let declarations whose checker type is autoType due to null/undefined initializers, ensuring the declaration serializer prefers the syntactic pseudotype (e.g. null) instead of emitting any.
Changes:
- Prefer pseudotype-based serialization for
autoTypevariable declarations with an initializer, preservingnull/undefinedin.d.tsoutput. - Update the submodule reference baseline to expect
nullfor the affectedletdeclaration. - Remove the previously-accepted baseline diff now that output matches the reference.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/checker/nodebuilderimpl.go | Special-cases autoType var/let declarations with initializers to serialize from pseudotypes for .d.ts output. |
| testdata/baselines/reference/submodule/compiler/letDeclarations.js | Updates expected .d.ts output to emit null for let l11 = null. |
| testdata/baselines/reference/submoduleAccepted/compiler/letDeclarations.js.diff | Removes the accepted diff because the implementation now matches the reference baseline. |
| reportErrors := !b.ctx.suppressReportInferenceFallback | ||
| if b.pseudoTypeEquivalentToType(pt, t, !requiresAddingUndefined && (ast.IsParameterDeclaration(declaration) || ast.IsPropertySignatureDeclaration(declaration) || ast.IsPropertyDeclaration(declaration)) && isOptionalDeclaration(declaration), reportErrors) { | ||
| if ast.IsVariableDeclaration(declaration) && !ast.IsVarConstLike(declaration) && ast.HasInitializer(declaration) && t == b.ch.autoType { | ||
| // In the case of an autoType variable declaration (let v = null), we should still prefer the pseudotype derived from the AST for the .d.ts purposes. |
| declare let l7: boolean; | ||
| declare let l8: number; | ||
| declare let l9: number, l10: string, l11: any; | ||
| declare let l9: number, l10: string, l11: null; |
There was a problem hiding this comment.
This seems to be a relatively rare scenario. It requires a specific combination of options: declaration: true, strictNullChecks: true, and let v = null.
However, since the current coverage is only indirect through a single test, would it make sense to add a dedicated test case for this scenario? Alternatively, we could extend an existing test to cover this combination.
| } | ||
| reportErrors := !b.ctx.suppressReportInferenceFallback | ||
| if b.pseudoTypeEquivalentToType(pt, t, !requiresAddingUndefined && (ast.IsParameterDeclaration(declaration) || ast.IsPropertySignatureDeclaration(declaration) || ast.IsPropertyDeclaration(declaration)) && isOptionalDeclaration(declaration), reportErrors) { | ||
| if ast.IsVariableDeclaration(declaration) && !ast.IsVarConstLike(declaration) && ast.HasInitializer(declaration) && t == b.ch.autoType { |
There was a problem hiding this comment.
Unrelated. I noticed that ast.IsVarConstLike is duplicated in the checker. Do you think this duplication is worth removing? If so, would it make sense to include it in this PR, or would it be better as a separate change?
This PR fixes declaration generation for variables with inferred
autoTypeinitialized withnullorundefined.Previously, declaration serialization preferred the inferred checker type over the pseudotype derived from the declaration syntax. This affects cases like
where the inferred type is
autoType, while the pseudotype preserves the originalnull/undefinedinitializer. As a result, the declaration transform always emittedanyinstead of respecting the Strada'sstrictNullChecksbehavior.