Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ports TypeScript’s --importHelpers / tslib helper validation into the Go checker so missing or incompatible helpers are consistently diagnosed, and updates the submodule baseline/triage lists accordingly (Fixes #3476).
Changes:
- Add per-source-file caching and expanded helper-bit tracking for external emit helper checks in the checker.
- Add new helper checks across async/for-await/destructuring/decorators/private names/module transforms to match emitted-helper requirements.
- Update accepted/triaged lists and rebaseline affected submodule test outputs now that helper diagnostics are produced.
Reviewed changes
Copilot reviewed 86 out of 86 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/submoduleTriaged.txt | Removes the tslib-helper issue block now that behavior is implemented. |
| testdata/submoduleAccepted.txt | Drops outdated accepted baselines and adds new accepted cases for per-file helper checking. |
| testdata/baselines/reference/submoduleTriaged/conformance/usingDeclarationsWithImportHelpers.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/privateNameStaticEmitHelpers.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/privateNameEmitHelpers.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions3(module=nodenext,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions3(module=node20,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions3(module=node18,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions3(module=node16,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions2(module=nodenext).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions2(module=node20).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions2(module=node18).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions2(module=node16).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions(module=nodenext).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions(module=node20).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions(module=node18).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesImportHelpersCollisions(module=node16).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=nodenext,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=node20,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=node18,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=node16,target=es2015).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=nodenext).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=node20).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=node18).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=node16).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=nodenext).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=node20).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=node18).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=node16).errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/exportAsNamespace_missingEmitHelpers.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/conformance/awaitUsingDeclarationsWithImportHelpers.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/compiler/tslibNotFoundDifferentModules.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/compiler/tslibMultipleMissingHelper.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleTriaged/compiler/tslibMissingHelper.errors.txt.diff | Removes triaged diff (now matches reference). |
| testdata/baselines/reference/submoduleAccepted/compiler/tslibMultipleMissingHelper.errors.txt.diff | New accepted diff reflecting per-file helper diagnostics. |
| testdata/baselines/reference/submoduleAccepted/compiler/tslibMissingHelper.errors.txt.diff | New accepted diff reflecting per-file helper diagnostics. |
| testdata/baselines/reference/submoduleAccepted/compiler/importHelpersWithImportOrExportDefaultNoTslib.3(esmoduleinterop=true,module=commonjs).errors.txt.diff | Removes outdated accepted diff. |
| testdata/baselines/reference/submoduleAccepted/compiler/importHelpersWithImportOrExportDefaultNoTslib.2(esmoduleinterop=true,module=commonjs).errors.txt.diff | Removes outdated accepted diff. |
| testdata/baselines/reference/submoduleAccepted/compiler/importHelpersWithImportOrExportDefaultNoTslib.1(esmoduleinterop=true,module=commonjs).errors.txt.diff | Removes outdated accepted diff. |
| testdata/baselines/reference/submoduleAccepted/compiler/importHelpersES6.errors.txt.diff | Removes outdated accepted diff. |
| testdata/baselines/reference/submoduleAccepted/compiler/esModuleInteropTslibHelpers.errors.txt.diff | Removes outdated accepted diff. |
| testdata/baselines/reference/submoduleAccepted/compiler/ctsFileInEsnextHelpers.errors.txt.diff | Removes outdated accepted diff. |
| testdata/baselines/reference/submodule/conformance/usingDeclarationsWithImportHelpers.errors.txt | Adds updated reference baseline with tslib missing-helper error. |
| testdata/baselines/reference/submodule/conformance/privateNameStaticEmitHelpers.errors.txt | Adds updated reference baseline with tslib arity mismatch errors. |
| testdata/baselines/reference/submodule/conformance/privateNameEmitHelpers.errors.txt | Adds updated reference baseline with tslib arity mismatch errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions3(module=nodenext,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions3(module=node20,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions3(module=node18,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions3(module=node16,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions2(module=nodenext).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions2(module=node20).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions2(module=node18).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions2(module=node16).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions(module=nodenext).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions(module=node20).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions(module=node18).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesImportHelpersCollisions(module=node16).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=nodenext,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=node20,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=node18,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions3(module=node16,target=es2015).errors.txt | Adds updated reference baseline with tslib helper missing error. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=nodenext).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=node20).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=node18).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions2(module=node16).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=nodenext).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=node20).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=node18).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/nodeModulesAllowJsImportHelpersCollisions1(module=node16).errors.txt | Adds updated reference baseline with tslib helper missing errors. |
| testdata/baselines/reference/submodule/conformance/exportAsNamespace_missingEmitHelpers.errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/conformance/awaitUsingDeclarationsWithImportHelpers.errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/compiler/tslibNotFoundDifferentModules.errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/compiler/tslibMultipleMissingHelper.errors.txt | Adds updated reference baseline with multiple missing helper errors. |
| testdata/baselines/reference/submodule/compiler/tslibMissingHelper.errors.txt | Adds updated reference baseline with per-file missing helper errors. |
| testdata/baselines/reference/submodule/compiler/importHelpersWithImportOrExportDefaultNoTslib.3(esmoduleinterop=true,module=commonjs).errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/compiler/importHelpersWithImportOrExportDefaultNoTslib.2(esmoduleinterop=true,module=commonjs).errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/compiler/importHelpersWithImportOrExportDefaultNoTslib.1(esmoduleinterop=true,module=commonjs).errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/compiler/importHelpersES6.errors.txt | Adds updated reference baseline with tslib missing-module error. |
| testdata/baselines/reference/submodule/compiler/esModuleInteropTslibHelpers.errors.txt | Adds updated reference baseline with tslib missing-module errors. |
| testdata/baselines/reference/submodule/compiler/ctsFileInEsnextHelpers.errors.txt | Adds updated reference baseline with tslib missing-module error. |
| internal/transformers/estransforms/namedevaluation.go | Switches named-evaluation source detection to shared AST utility functions. |
| internal/printer/helpers.go | Removes outdated section comment near helper definitions. |
| internal/checker/utilities.go | Adds outer-expression parent walk helper for named evaluation checks. |
| internal/checker/types.go | Introduces ExternalEmitHelpers bitmask and adds per-file helper/module caches to SourceFileLinks. |
| internal/checker/checker.go | Implements and wires helper checks across relevant syntax/features and adds module/helper resolution logic. |
| internal/ast/utilities.go | Adds IsNamedEvaluationSource and IsProtoSetter utilities for reuse across packages. |
Comment on lines
5203
to
+5208
| if ast.IsNamespaceImport(namedBindings) { | ||
| c.checkImportBinding(namedBindings) | ||
| if c.program.GetEmitModuleFormatOfFile(ast.GetSourceFileOfNode(node)) == core.ModuleKindCommonJS { | ||
| // import * as ns from "foo"; | ||
| c.checkExternalEmitHelpers(node, ExternalEmitHelpersImportStar) | ||
| } |
Member
Author
There was a problem hiding this comment.
This was a bug in Strada which is now fixed.
Comment on lines
+12569
to
+12572
| if ast.IsPropertyAccessExpression(target) && ast.IsPrivateIdentifier(target.AsPropertyAccessExpression().Name()) { | ||
| // NOTE: we do not limit this to LanguageFeatureTargets.PrivateNames as some other feature downleveling still requires this. | ||
| c.checkExternalEmitHelpers(target, ExternalEmitHelpersClassPrivateFieldSet) | ||
| } |
Member
Author
There was a problem hiding this comment.
I'm convinced this was dead code. I removed it and added a test and everything still works.
Member
|
This seems 1:1 but I think the copilot comments are worth addresssing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3476.
We also had some accepted baselines that are no longer accepted, because as far as I could tell we do still use the emit helpers. That check used to be gated on
esModuleInteropbeing true, but now it's always true so we always do the checks.As far as I could verify, we still use all of the helpers checked in this PR in emit.
I had to change
requestedExternalEmitHelpersto be cached per-file instead of per module symbol, since caching it on the module symbol meant you'd get less diagnostics if a checker happened to check two files with the error, but more diagnostics if two different checkers (and therefore module symbols) checked those two files. The accepted baselines are for this reason.