Hermetic ambient walks, node:* declaration imports, and a warning-free build - #1310
Merged
Conversation
added 3 commits
July 27, 2026 20:56
FileDiscovery already refused to let a stray tsconfig.json in %TEMP% reconfigure whatever happens to compile below it, but six other upward walks never got that policy: ModuleResolver's node_modules probe, subpath-import and self-reference package.json walks, and @types roots; CommonJsDetector's package.json type walk; and TsConfigLoader's bare "extends" walk. Any of them could pick up debris that unrelated tooling left in the global temp folder or the user profile. Share the ceiling policy through FileDiscovery.AmbientParent: the start directory is still probed even when it is itself a ceiling, but no walk ascends into one. Validated by running the full suite with a hostile %TEMP% (a commonjs package.json, a strict tsconfig.json, a sharpts.json, and node_modules/@types/node all present).
The declaration-file facade guard skipped a node:* import only when the stdlib chain could serve the bare name, so @types/node's own web-globals/console.d.ts — "import * as console from `node:console`", a module SharpTS's stdlib does not provide — fell through to bare-specifier file resolution and threw. The CLI path hid this by deferring resolution failures to the checker; embedding callers using TypeScriptProgramOptions.Disabled crashed outright. A node:* specifier in a declaration file can only mean an ambient module declaration or the stdlib facade, never a loadable source file, so the guard now skips every node:* import unconditionally. Regression test mirrors the real @types/node shape (verified against @types/node 25.9.1 on disk).
The debug-scope registration indexed a "normalized spelling" that NormalizeToEmissionPath can never produce: it either returns the path unchanged or folds a script path to null, so the insert was a duplicate key write today and a null-key ArgumentNullException if the phases ever reorder (CS8604). Registration now keys by raw path only and documents that null-path emission is served by the entry-point fallback. MockHttpServer assigned its accept-loop task but never observed it, so Dispose could tear down the listener and token source while the loop was still touching them during parallel test cleanup (IDE0052). Dispose now joins the loop with a 2s bound before closing either. The rest is removal of never-called members — the Parser span-recording wrappers left over from the span-table work (transforms use SpanTable directly), an unused REPL-completion test helper overload, an unused diagnostic assertion helper — and the xUnit2031 Assert.Single predicate overload in DebugSymbolsTests.
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.
Three independent fixes that shipped on this branch together.
Stop ambient upward walks at the temp and user-profile ceilings
FileDiscovery already refused to let a stray
tsconfig.jsonin%TEMP%reconfigure whatever compiles below it, but six other upward walks never got that policy: ModuleResolver''snode_modulesprobe, subpath-import and self-referencepackage.jsonwalks, and@typesroots; CommonJsDetector''spackage.jsontype walk; and TsConfigLoader''s bareextendswalk. Any of them could pick up debris that unrelated tooling left in the global temp folder or the user profile.The ceiling policy is now shared through
FileDiscovery.AmbientParent: the start directory is still probed even when it is itself a ceiling, but no walk ascends into one. Validated by running the full suite with a hostile%TEMP%(a commonjspackage.json, a stricttsconfig.json, asharpts.json, andnode_modules/@types/nodeall present).Bind declaration-file node:* imports ambiently, not as files
The declaration-file facade guard skipped a
node:*import only when the stdlib chain could serve the bare name, so @types/node''s ownweb-globals/console.d.ts—import * as console from "node:console", a module SharpTS''s stdlib does not provide — fell through to bare-specifier file resolution and threw. The CLI path hid this by deferring resolution failures to the checker; embedding callers usingTypeScriptProgramOptions.Disabledcrashed outright.A
node:*specifier in a declaration file can only mean an ambient module declaration or the stdlib facade, never a loadable source file, so the guard now skips everynode:*import unconditionally. Regression test mirrors the real @types/node shape (verified against @types/node 25.9.1 on disk).Clear all build warnings (11 → 0)
Two were substantive:
ILCompiler.RegisterModuleDocuments— the debug-scope table indexed a "normalized spelling" thatNormalizeToEmissionPathcan never produce: it either returns the path unchanged or folds a script path tonull, so the insert was a duplicate key write today and a null-keyArgumentNullExceptionif the phases ever reorder. Registration now keys by raw path only and documents that null-path emission is served by the entry-point fallback.MockHttpServer— the accept-loop task was assigned but never observed, soDisposecould tear down the listener and token source while the loop was still touching them during parallel test cleanup.Disposenow joins the loop (2s bound) before closing either.The rest: removed never-called members (the Parser span-recording wrappers left over from the span-table work, an unused REPL-completion test helper overload, an unused diagnostic assertion helper) and switched
DebugSymbolsTeststo theAssert.Singlepredicate overload (xUnit2031).Testing
%TEMP%as described above