Scope generated opens to the tagged type's namespace block - #565
Merged
Conversation
Every generator previously emitted every `open` from the whole input file into every generated namespace (via Whippet's AstHelper.extractOpens, which shadows the identically-named dead local helper). A relative open that is valid in one namespace block is not necessarily valid in another, so generated code for a type in a different block could fail to compile. extractOpensForNamespace collects opens only from the namespace blocks whose name is a segment-wise prefix of the tagged type's namespace: prefix rather than exact matching because CataGenerator's type discovery descends into nested modules (a type in module M of namespace N is reported as N.M, and block N's opens are lexically in scope around it). Same-named blocks share a resolution context, so unioning them is sound. The dead local extractOpens is replaced. ConsumePlugin/OpensLeakRegression.fs is a compile-time regression test: its bait block contains a relative `open RelativeOpenBait` which does not resolve in the victim namespace, so the pre-fix generator produces uncompilable output for it (verified against the pre-fix plugin). No checked-in generated code changes: no existing input relied on the leak. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Stacked on #564.
Problem
All six generators that emit opens collected every
openfrom the whole input file (via Whippet'sAstHelper.extractOpens— which, note, shadows the identically-named and therefore dead local helper in this repo'sAstHelper) and emitted them into every generated namespace. A relativeopenthat resolves in one namespace block need not resolve in another, so generated code for a type in a second namespace block could fail to compile.Fix
AstHelper.extractOpensForNamespacecollects opens only from namespace blocks whose name matches the tagged type's namespace. Two blocks with the same name share a resolution context, so unioning them is sound. All six generators (ArgParser, Cata, HttpClient, InterfaceMock, CapturingInterfaceMock, JsonSerialize) now call it per namespace. The dead localextractOpensis deleted.Verification
ConsumePlugin/OpensLeakRegression.fsis a compile-time regression test: a bait namespace block contains a relativeopen Subwhich cannot resolve in the victim namespace. I ran the pre-fix plugin against it manually: it emitsopen Subintonamespace ConsumePlugin.OpensLeakVictim, which does not compile. The fixed plugin emits only the victim block's own opens.🤖 Generated with Claude Code