Initial refactoring of service source generator#4293
Merged
bernardnormier merged 19 commits intoicerpc:mainfrom Mar 11, 2026
Merged
Initial refactoring of service source generator#4293bernardnormier merged 19 commits intoicerpc:mainfrom
bernardnormier merged 19 commits intoicerpc:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates the separate Slice and Ice service source generators into a single generator (IceRpc.ServiceGenerator), standardizes the controlling attribute to [Service], and updates tests/examples/docs and packaging to use the unified generator.
Changes:
- Replaced
[SliceService]/[IceService]with[Service]across tests, examples, templates, and docfx snippets. - Added
IceRpc.ServiceAttributetoIceRpc.dlland migrated generator internals intoIceRpc.ServiceGenerator. - Updated project references/packaging to ship and consume the unified analyzer (
IceRpc.ServiceGenerator.dll).
Reviewed changes
Copilot reviewed 68 out of 69 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/IntegrationTests/ProtocolBridgingTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IntegrationTests/IntegrationTests.csproj | Switches analyzer reference to IceRpc.ServiceGenerator. |
| tests/IntegrationTests/IceObjectTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IntegrationTests/CustomTransportTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Slice.CodeGen.Tests/TypeNameQualificationTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Slice.CodeGen.Tests/ServiceTests.cs | Updates attributes/comments to unified [Service] terminology. |
| tests/IceRpc.Slice.CodeGen.Tests/ProxyTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Slice.CodeGen.Tests/OperationTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Slice.CodeGen.Tests/NamespaceAttributeTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Slice.CodeGen.Tests/IdentifierAttributeTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Slice.CodeGen.Tests/IceRpc.Slice.CodeGen.Tests.csproj | Switches analyzer reference to IceRpc.ServiceGenerator. |
| tests/IceRpc.Ice.CodeGen.Tests/ProxyTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Ice.CodeGen.Tests/IceRpc.Ice.CodeGen.Tests.csproj | Switches analyzer reference to IceRpc.ServiceGenerator. |
| tests/IceRpc.Ice.CodeGen.Tests/ExceptionTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Compressor.Tests/OperationCompressorTests.cs | Updates test service attribute usage to unified [Service]. |
| tests/IceRpc.Compressor.Tests/IceRpc.Compressor.Tests.csproj | Switches analyzer reference to IceRpc.ServiceGenerator. |
| src/IceRpc/ServiceAttribute.cs | Introduces the new unified [Service] attribute in IceRpc.dll. |
| src/IceRpc.Templates/Templates/IceRpc-Slice-Server/Chatbot.cs | Updates template service attribute usage to unified [Service]. |
| src/IceRpc.Templates/Templates/IceRpc-Slice-DI-Server/Chatbot.cs | Updates template service attribute usage to unified [Service]. |
| src/IceRpc.Slice/SliceServiceAttribute.cs | Removes the old Slice-specific service attribute type. |
| src/IceRpc.Slice/README.md | Updates docs to reference [Service] and the unified generator assembly. |
| src/IceRpc.Slice/IceRpc.Slice.csproj | Removes packaging of the Slice generator analyzer from IceRpc.Slice. |
| src/IceRpc.Slice.Generators/ServiceGenerator.cs | Removes the old Slice generator implementation. |
| src/IceRpc.Slice.Generators/Internal/ServiceClass.cs | Removes old Slice generator internal model. |
| src/IceRpc.Slice.Generators/Internal/ContainerDefinition.cs | Removes old Slice generator internal model. |
| src/IceRpc.Slice.Generators/IceRpc.Slice.Generators.csproj | Removes old Slice generator project. |
| src/IceRpc.ServiceGenerator/ServiceGenerator.cs | Rehomes and retargets the unified generator to [Service]. |
| src/IceRpc.ServiceGenerator/Internal/StringExtensions.cs | Moves internal helpers under unified generator namespace. |
| src/IceRpc.ServiceGenerator/Internal/ServiceMethod.cs | Adds IDL classification to support multiple IDLs in one generator. |
| src/IceRpc.ServiceGenerator/Internal/ServiceClass.cs | Updates internal model namespace/docs to unified generator context. |
| src/IceRpc.ServiceGenerator/Internal/Parser.cs | Updates parsing to recognize Ice + Slice operation attributes under [Service]. |
| src/IceRpc.ServiceGenerator/Internal/Idl.cs | Adds IDL enum used by unified generator emission. |
| src/IceRpc.ServiceGenerator/Internal/Emitter.cs | Emits correct using IceRpc.{Idl}; based on the operations encountered. |
| src/IceRpc.ServiceGenerator/Internal/DiagnosticDescriptors.cs | Updates diagnostics messaging/category for unified generator. |
| src/IceRpc.ServiceGenerator/Internal/ContainerDefinition.cs | Moves internal helpers under unified generator namespace. |
| src/IceRpc.Ice/README.md | Updates docs to reference [Service] for Ice integration. |
| src/IceRpc.Ice/IceServiceAttribute.cs | Removes the old Ice-specific service attribute type. |
| src/IceRpc.Ice/IceRpc.Ice.csproj | Packages unified generator analyzer instead of Ice-specific generator DLL. |
| src/IceRpc.Ice.Generators/Internal/StringExtensions.cs | Removes old Ice generator utility. |
| src/IceRpc.Ice.Generators/Internal/ServiceMethod.cs | Removes old Ice generator internal model. |
| src/IceRpc.Ice.Generators/Internal/Parser.cs | Removes old Ice generator parser implementation. |
| src/IceRpc.Ice.Generators/Internal/Emitter.cs | Removes old Ice generator emitter implementation. |
| src/IceRpc.Ice.Generators/Internal/DiagnosticDescriptors.cs | Removes old Ice generator diagnostics. |
| examples/slice/Upload/Server/EarthImageStore.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Thermostat/Server/ThermoFacade.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Thermostat/Server/ThermoBridge.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Thermostat/Device/ThermoBot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Telemetry/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/TcpFallback/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Tcp/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Stream/Server/RandomGenerator.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Retry/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/RequestContext/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/MultipleInterfaces/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Metrics/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Logger/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/InteropGreeter/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Greeter/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/GenericHost/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Download/Server/EarthImageServer.cs | Updates example to use unified [Service] attribute. |
| examples/slice/DiscriminatedUnion/Server/MathWizard.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Deadline/Server/SlowChatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/CustomError/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| examples/slice/Compress/Server/Chatbot.cs | Updates example to use unified [Service] attribute. |
| docfx/examples/IceRpc.RequestContext.Examples/Chatbot.cs | Updates docfx example to use unified [Service] attribute. |
| docfx/examples/Chatbot.cs | Updates docfx example to use unified [Service] attribute. |
| docfx/docfx.json | Updates docfx exclusion list for new generator project path. |
| IceRpc.slnx | Replaces old generator projects with IceRpc.ServiceGenerator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+179
to
+193
| Idl idl = default; | ||
| AttributeData? attribute = null; | ||
|
|
||
| if (_sliceOperationAttribute is not null) | ||
| { | ||
| attribute = GetAttribute(method, _sliceOperationAttribute); | ||
| idl = Idl.Slice; | ||
| } | ||
| if (attribute is null && _iceOperationAttribute is not null) | ||
| { | ||
| attribute = GetAttribute(method, _iceOperationAttribute); | ||
| idl = Idl.Ice; | ||
| } | ||
|
|
||
| if (attribute is null) |
There was a problem hiding this comment.
If a method ends up with both SliceOperationAttribute and IceOperationAttribute, the current logic will silently prefer Slice. To make this deterministic and debuggable, consider explicitly detecting the "both attributes present" case and reporting a diagnostic (or otherwise making the precedence an intentional, documented rule).
Member
Author
There was a problem hiding this comment.
These are generated methods, with a single attribute per method. We could emit a diagnostic if we encounter a method with more than one of these attributes.
Comment on lines
+15
to
+19
| foreach (Idl idl in serviceClass.ServiceMethods.Select(serviceMethod => serviceMethod.Idl).Distinct()) | ||
| { | ||
| codeBlock.WriteLine($"using IceRpc.{idl};"); | ||
| } | ||
| codeBlock.WriteLine("using ZeroC.Slice.Codec;"); |
There was a problem hiding this comment.
Generating using IceRpc.{idl}; relies on Idl.ToString() matching the target namespace exactly. Since Idl is expected to grow (TODO: Protobuf), this is brittle (enum renames or different namespace conventions would break generation). Consider using an explicit mapping (e.g., switch expression from Idl to namespace string) so future IDLs/namespaces can be added safely.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 82 out of 82 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/IceRpc.ServiceGenerator/Internal/Emitter.cs:18
- The generated file conditionally adds
using IceRpc.Slice;and/orusing IceRpc.Ice;. If a service class ends up with both Slice and Ice operations, bringing both namespaces into scope can make calls likerequest.DispatchOperationAsync(...)ambiguous (notably for void-return operations where the overload sets differ only by optional parameters), causing a compile-time error or binding to the wrong extension method. Consider emitting fully-qualified calls to the correctIncomingRequestExtensions.DispatchOperationAsyncbased onserviceMethod.Idl(or emitting a diagnostic that mixed IDLs in one service class are not supported).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pepone
approved these changes
Mar 11, 2026
src/IceRpc.Ice/IceRpc.Ice.csproj
Outdated
| </ItemGroup> | ||
| <ItemGroup> | ||
| <ProjectReference Include="../IceRpc/IceRpc.csproj" ExactVersion="true" /> | ||
| <ProjectReference Include="../IceRpc.ServiceGenerator/IceRpc.ServiceGenerator.csproj" ExactVersion="true" /> |
Member
There was a problem hiding this comment.
Why are we no longer using ReferenceOutputAssembly="false", shouldn't we also use OutputItemType="Analyzer" like the tests?
Member
Author
There was a problem hiding this comment.
I fixed it as you suggested. And the current setup still pulls IceRpc.ServiceGenerator in projects with a reference to the IceRpc.Slice package.
Member
Author
There was a problem hiding this comment.
Actually, we can't use ReferenceOutputAssembly="false" as it prevents this dependency from flowing transitively to consumers of IceRpc.Slice/IceRpc.Ice. See updated project file.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 82 out of 82 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/IceRpc.ServiceGenerator/Internal/Emitter.cs:19
Emitaddsusing IceRpc.{idl};for each IDL used by the service methods. If a service class ends up implementing both Slice and Ice service interfaces, the generated file will import bothIceRpc.SliceandIceRpc.Ice, and calls likerequest.CheckNonIdempotent()/request.DispatchOperationAsync(...)become ambiguous because both namespaces defineIncomingRequestExtensionswith these methods. Consider generating fully-qualified static calls based onserviceMethod.Idl(or adding a diagnostic that forbids mixing IDLs on the same[Service]class) to avoid ambiguous extension method resolution.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
This PR combines the Slice service generator and the Ice service generator into a single service generator, IceRpc.ServiceGenerator.dll.
The controlling class attribute is now IceRpc.ServiceAttribute (in IceRpc.dll), and the source generator is packaged inside the main IceRpc NuGet package.
We want to merge the Protobuf service generator into this source generator in a follow-up PR.
Please note:
With this update, the IceRpc.Slice public API is primarily for use by the code generator / source generator, and not application code. So we have very few "using IceRpc.Slice;" left. We should consider further reducing this public API in a follow-up PR by moving ISliceFeature/SliceFeature to IceRpc.Features, and the ServiceProviderExtensions to IceRpc.Extensions.DependencyInjection.
The IceRpc.ServiceGenerator needs more refactoring to add Protobuf and also make Ice and Slice more separate.