Generate valid C# for .proto files with no package or csharp_namespace#4562
Generate valid C# for .proto files with no package or csharp_namespace#4562pepone wants to merge 1 commit intoicerpc:mainfrom
Conversation
When neither `package` nor `option csharp_namespace = "..."` is set,
GetCsharpNamespace returned the empty string. The generator then emitted
"namespace ;" (rejected by the C# compiler) at the top of the file and
"global::.MyMessage" for any cross-namespace reference. Both are
invalid C#.
Skip the namespace declaration when the namespace is empty, and emit
"global::MyMessage" instead of "global::.MyMessage" for cross-namespace
references when the target file has no namespace. This matches Google's
protoc-gen-csharp behavior of placing top-level types in the global
namespace.
Add a regression .proto file with neither package nor csharp_namespace;
the test project's compile-time success is the regression check.
Verified empirically that without the fix the project fails to compile
with CS1001 ("Identifier expected") on line 16 of the generated file.
There was a problem hiding this comment.
Pull request overview
This PR fixes protoc-gen-icerpc-csharp output for valid .proto files that omit both package and option csharp_namespace, ensuring generated C# compiles and matches protoc-gen-csharp behavior for global-namespace types.
Changes:
- Skip emitting a file-scoped
namespace ...;declaration when the computed C# namespace is empty. - Fix cross-namespace message type qualification to avoid emitting invalid
global::.Typewhen targeting a no-namespace.proto. - Add a regression
.protowith nopackageand nocsharp_namespaceto ensure the test project compiles.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/IceRpc.Protobuf.Tests/NoNamespaceTests.proto | Adds a .proto schema with no namespace metadata to catch invalid generated namespace ; output during compilation. |
| src/IceRpc.Protobuf.Generator/Program.cs | Conditionally emits the namespace declaration only when non-empty. |
| src/IceRpc.Protobuf.Generator/MessageDescriptorExtensions.cs | Centralizes message type qualification and handles the empty-namespace case to produce valid global::Type references. |
| // Intentionally has no `package` and no `option csharp_namespace`. The generator must emit valid C# in | ||
| // this case (no `namespace ;` declaration; cross-namespace references use `global::Type` rather than | ||
| // `global::.Type`). |
There was a problem hiding this comment.
This regression .proto doesn’t currently exercise the cross-namespace reference scenario described in the header comment: nothing imports this file from a non-empty package/csharp_namespace, so the generator’s global::Type (vs global::.Type) path won’t be compiled/tested. Consider adding a second .proto with a non-empty package/csharp_namespace that imports this file and uses NoNamespaceMessage as an RPC input/output to ensure the generated C# contains global::NoNamespaceMessage and stays valid.
Summary
.protohas neitherpackagenoroption csharp_namespace = ...,GetCsharpNamespacereturned""and the generator emittednamespace ;(CS1001 from the C# compiler) andglobal::.MyMessagefor any cross-namespace reference.global::MyMessageinstead ofglobal::.MyMessagefor cross-namespace references targeting a file with no namespace. Matches Google'sprotoc-gen-csharpbehavior of placing top-level types in the global namespace..protoregression file with neitherpackagenorcsharp_namespace. Without the fix the test project fails to compile withCS1001: Identifier expectedon the generatednamespace ;line; with the fix it builds cleanly and all 48 Protobuf tests pass.Fixes #4447