Fix: Handle empty namespace in CodeWriter to prevent 'global::.' output#9882
Closed
live1206 wants to merge 1 commit intomicrosoft:mainfrom
Closed
Fix: Handle empty namespace in CodeWriter to prevent 'global::.' output#9882live1206 wants to merge 1 commit intomicrosoft:mainfrom
live1206 wants to merge 1 commit intomicrosoft:mainfrom
Conversation
live1206
requested review from
JoshLove-msft,
ShivangiReja,
christothes,
jorgerangel-msft,
joseharriaga,
jsquire and
m-nash
as code owners
microsoft-github-policy-service
bot
added
the
emitter:client:csharp
commit: |
Contributor
|
No changes needing a change description found. |
When a CSharpType has an empty namespace (which can occur when Roslyn cannot resolve a type reference, such as in Custom code that inherits from generated types), the CodeWriter was outputting 'global::.' followed by the type name, resulting in invalid C# syntax. This fix adds a check to only append the namespace and dot separator when the namespace is not empty. Fixes microsoft#9880 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
live1206
force-pushed
the
fix/csharp-custom-type-alias-namespace
branch
from
a86d4a0 to
6df22b1
Compare
| AppendRaw("global::"); | ||
| AppendRaw(type.Namespace); | ||
| AppendRaw("."); | ||
| if (!string.IsNullOrEmpty(type.Namespace)) |
Contributor
There was a problem hiding this comment.
I don't think this is the right approach. If the actual namespace is not included in the global reference, we may not end up adding the correct using as part of Roslyn reduce. The actual issue would be somewhere in here -
https://github.com/microsoft/typespec/blob/main/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs#L294
We already have handling for this so we would need to debug to figure out why it isn't working.
Contributor
There was a problem hiding this comment.
My guess is that we are not properly handling when the generated base type is renamed somehow.
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
2 participants
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.
Summary
When a CSharpType has an empty namespace (which can occur when Roslyn cannot resolve a type reference, such as in Custom code that inherits from generated types), the CodeWriter was outputting
global::.followed by the type name, resulting in invalid C# syntax.Before:
After:
Root Cause
The issue occurs when:
PublisherResource : HciClusterPublisherResource)HciClusterPublisherResourceis a generated type that exists in theGenerated/folderHciClusterPublisherResourcebecause generated files are excluded from the compilationglobal::{empty_namespace}.{name}which becomesglobal::.{name}Fix
Added a check in
CodeWriter.AppendTypeto only append the namespace and dot separator when the namespace is not empty.Tests
Added two unit tests:
TypeWithEmptyNamespaceDoesNotOutputGlobalDot: Verifies the fixTypeWithNamespaceOutputsCorrectly: Regression test for normal caseAll existing tests pass (1263 generator tests + 1171 ClientModel tests).
Fixes #9880