feat(utilities): TypeDeclarationInfo.GetFullyQualifiedMetadataName + GetFullyQualifiedName - #156
Conversation
GetFullyQualifiedMetadataName() reconstructs the CLR metadata name (Deep.Outer`1+Middle+Inner`1) from the cached snapshot so later pipeline stages can re-resolve the live symbol via Compilation.GetTypeByMetadataName without retaining the original ISymbol. GetFullyQualifiedName() yields the global::-qualified C# reference with type parameter names (global::Deep.Outer<T>.Middle.Inner<U>) for emitting self-references inside the generated partial, where those parameters are in scope. Note: the cachability-assertion feature originally planned for this arc already exists as GeneratorResult.IsCached() — scope reduced to what was actually missing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting Summary by CodeRabbit
WalkthroughAdds a shared ChangesFully-qualified name generation
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)Not applicable — this change is a self-contained refactor/implementation within a single class and its tests, without multi-component orchestration. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4053a38179
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| builder.Append(Namespace).Append('.'); | ||
|
|
||
| foreach (var containing in ContainingTypes.AsImmutableArray()) | ||
| builder.Append(containing.DisplayName).Append('.'); |
There was a problem hiding this comment.
For a legal nested generic such as partial class Outer<T> { partial class Inner<T> { } } (CS0693 warning only), this appends the containing clause as Outer<T>, so GetFullyQualifiedName() returns global::N.Outer<T>.Inner<T>. Inside the generated Inner<T> partial, T resolves to the inner parameter, making the reference Outer<inner T>.Inner<inner T> rather than the current Outer<outer T>.Inner<inner T>; the documented public {info.GetFullyQualifiedName()} Self() => this; pattern then fails to compile. The helper needs to avoid qualifying through shadowed generic containers or explicitly reject/document that case.
Useful? React with 👍 / 👎.
| if (Namespace is not null) | ||
| builder.Append(Namespace).Append('.'); |
There was a problem hiding this comment.
When the source namespace needs escaping, such as namespace @class { partial class C { } }, Namespace is stored as the C# display text so BeginDeclaration can re-emit it. Reusing that value here produces @class.C, but Compilation.GetTypeByMetadataName expects the CLR metadata namespace class.C, so the advertised round-trip returns null for these legal declarations. Build the metadata name from namespace metadata segments rather than the display string.
Useful? React with 👍 / 👎.
| builder.Append('+'); | ||
| } | ||
|
|
||
| AppendMetadataNameLevel(builder, Name, GenericParameterClause); |
There was a problem hiding this comment.
For file-local declarations such as file class Hidden, Roslyn gives the symbol a synthesized metadata name while Name remains only Hidden. Reconstructing the lookup key from the source name here makes GetFullyQualifiedMetadataName() return Hidden/N.Hidden, so Compilation.GetTypeByMetadataName(...) cannot resolve the original file-local symbol despite the new round-trip contract. Store and append INamedTypeSymbol.MetadataName separately from the source name used for emitted declarations.
Useful? React with 👍 / 👎.
….2.35 round-trip surface) (#157) Arc A (#155 → 2.2.34): warnings-as-errors gate armed, zero-warning build. Arc B (#156 → 2.2.35): GetFullyQualifiedMetadataName + GetFullyQualifiedName. Both indexed on nuget.org; qyl bumped 2.2.33 → 2.2.35 in qyl#494. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Completes the
TypeDeclarationInforound-trip surface (From → emit → re-resolve):GetFullyQualifiedMetadataName()Reconstructs the CLR metadata name from the cached snapshot —
Deep.Outer\1+Middle+Inner`1— so a later pipeline stage can re-resolve the liveINamedTypeSymbolviaCompilation.GetTypeByMetadataNamewithout ever retaining the original symbol (which would break incremental caching). Round-trip test: the returned name resolves back to the original symbol underSymbolEqualityComparer`.GetFullyQualifiedName()The
global::Deep.Outer<T>.Middle.Inner<U>reference for emitting self-references from generated code. Compile-test proves the reference is valid inside the partial wrapper opened byBeginDeclaration(where the type parameters are in scope).Scope note (honest reduction)
The cachability-assertion helper originally pitched for this arc already exists —
GeneratorResult.IsCached()in the Testing package is the run-twice, all-steps-cached, forbidden-types one-liner, more complete than what was proposed. Nothing to add there.Verification
Status: complete-and-verified. Publishes as 2.2.35 via auto-bump; qyl gets one consolidated bump 2.2.33 → 2.2.35 after indexing.
🤖 Generated with Claude Code