Carry the source file's using directives into the generated file - #153
Merged
Conversation
Two cases, both recorded here as the generator currently handles them so that the fix shows what changes: A cref is resolved against the file it lands in, not the file it was written in. The generated file carries neither the using directives from the top of the source file nor those inside its namespace, so a cref which relied on one goes unresolved and the compiler reports CS1574 - a build failure for the many projects which treat warnings as errors. A method which returned a task returns nothing once synchronized, so a <returns> element describing that task describes something which no longer exists. The element is copied across regardless. Generated with Claude Code
Documentation is copied into the generated file verbatim, and a cref in it is resolved against the file it lands in. Whatever the crefs relied on being in scope has to land there too, or the compiler reports CS1574 for each one and a project which treats warnings as errors fails to build. The directives are emitted on the side of the namespace they were declared on: one written inside a namespace may name another only relatively, and would not resolve from above it. The generated code itself is fully qualified and does not need any of them, so nothing here changes what the code means. Also drop a <returns> element when the synchronized method returns nothing, since the task it describes is no longer returned. A <returns> describing a value which survives is kept: it is imprecise about the task it mentions, but dropping it would say less than saying it imprecisely. Generated with Claude Code
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #153 +/- ##
==========================================
+ Coverage 93.82% 93.92% +0.09%
==========================================
Files 9 9
Lines 1522 1563 +41
Branches 352 360 +8
==========================================
+ Hits 1428 1468 +40
Misses 25 25
- Partials 69 70 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Problem
Documentation comments are copied into the generated file verbatim, but a cref in one is resolved against the file it lands in, not the file it was written in. The generated file carries none of the source file's using directives, so every cref which relied on one goes unresolved:
For a project which treats warnings as errors - which is most of the ones worth adopting this - that is a build failure, not a nit. Found while converting SSH.NET, where a single converted method produced seven of them.
Fix
Emit the directives the source file declared, on the side of the namespace they were declared on:
The distinction matters: a directive written inside a namespace may name another namespace relatively, and would not resolve if hoisted above it. The generated code itself is fully qualified and needs none of them, so nothing here changes what the code means - only what the documentation can refer to.
Also in this change: a
<returns>element is dropped when the synchronized method returns nothing, because the task it describes is no longer returned. A<returns>describing a value which survives is kept - it is imprecise about the task it mentions, but dropping it would say less than saying it imprecisely. Rewriting the prose is not something the generator can do.Verification
First commit records both behaviours as they are today, so the second commit's diff is exactly what changed. 19 existing snapshots pick up the directives their sources declared; no generated code changed.
Full suite passes on both target frameworks (338 and 297) and all three Roslyn variants build.
🤖 Generated with Claude Code