From eacdcef511494bdaf98a1ee0b0c21ba08c82a7fd Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 29 Apr 2026 11:22:40 +0200 Subject: [PATCH] Escape reserved names injected by the IceRpc Slice generator User-defined Slice names could collide with synthetic identifiers the generator injects (cancellationToken, encodeOptions, Payload), producing uncompilable C#. Generalize the existing features_ rename to cover cancellationToken on proxy/service public methods, encodeOptions on Request.* and Response.* helpers, and the Payload tuple element on cs::encodedReturn streamed returns. --- src/IceRpc.Slice.Generator/EncodeHelper.cs | 6 ++-- .../OperationExtensions.cs | 32 ++++++++++++++++--- src/IceRpc.Slice.Generator/ProxyGenerator.cs | 28 +++++++++------- .../ServiceGenerator.cs | 11 ++++--- .../OperationTests.cs | 32 +++++++++++++++++++ .../OperationTests.slice | 12 +++++++ 6 files changed, 98 insertions(+), 23 deletions(-) diff --git a/src/IceRpc.Slice.Generator/EncodeHelper.cs b/src/IceRpc.Slice.Generator/EncodeHelper.cs index 05a4a66b7e..74607285f2 100644 --- a/src/IceRpc.Slice.Generator/EncodeHelper.cs +++ b/src/IceRpc.Slice.Generator/EncodeHelper.cs @@ -8,11 +8,13 @@ namespace IceRpc.Slice.Generator; internal static class EncodeHelper { /// Wraps an encode body in the standard pipe creation/size-placeholder boilerplate. - internal static CodeBlock BuildEncodeBody(CodeBlock encodeBody) + /// The encode body. + /// The escaped name of the encodeOptions parameter referenced by the body. + internal static CodeBlock BuildEncodeBody(CodeBlock encodeBody, string encodeOptionsName) { return new CodeBlock($$""" var pipe_ = new global::System.IO.Pipelines.Pipe( - encodeOptions?.PipeOptions ?? SliceEncodeOptions.Default.PipeOptions); + {{encodeOptionsName}}?.PipeOptions ?? SliceEncodeOptions.Default.PipeOptions); var encoder_ = new SliceEncoder(pipe_.Writer); Span sizePlaceholder_ = encoder_.GetPlaceholderSpan(4); diff --git a/src/IceRpc.Slice.Generator/OperationExtensions.cs b/src/IceRpc.Slice.Generator/OperationExtensions.cs index 3b2183c8cb..1b9393943d 100644 --- a/src/IceRpc.Slice.Generator/OperationExtensions.cs +++ b/src/IceRpc.Slice.Generator/OperationExtensions.cs @@ -38,6 +38,26 @@ internal static class OperationExtensions internal string FeaturesParamName => op.Parameters.Any(p => p.ParameterName == "features") ? "features_" : "features"; + /// Gets the escaped name for the injected "cancellationToken" parameter, appending "_" if any + /// operation parameter uses the name "cancellationToken". + internal string CancellationTokenParamName => + op.Parameters.Any(p => p.ParameterName == "cancellationToken") ? "cancellationToken_" : "cancellationToken"; + + /// Gets the escaped name for the injected "encodeOptions" parameter on Request.* helpers, + /// appending "_" if any operation parameter uses the name "encodeOptions". + internal string RequestEncodeOptionsParamName => + op.Parameters.Any(p => p.ParameterName == "encodeOptions") ? "encodeOptions_" : "encodeOptions"; + + /// Gets the escaped name for the injected "encodeOptions" parameter on Response.* helpers, + /// appending "_" if any operation return field uses the name "encodeOptions". + internal string ResponseEncodeOptionsParamName => + op.ReturnType.Any(f => f.ParameterName == "encodeOptions") ? "encodeOptions_" : "encodeOptions"; + + /// Gets the escaped name for the synthetic "Payload" tuple element used by cs::encodedReturn + /// operations with a streamed return, appending "_" if the streamed return field uses the name "Payload". + /// + internal string EncodedReturnPayloadName => op.StreamedReturn?.Name == "Payload" ? "Payload_" : "Payload"; + /// Returns the C# type string for a streamed field (parameter or return). /// Non-optional stream uint8 maps to PipeReader, all others to IAsyncEnumerable<T>. internal static string GetStreamTypeString(Field streamField, string currentNamespace) @@ -51,7 +71,10 @@ internal static string GetStreamTypeString(Field streamField, string currentName } /// Builds the EncodeStreamOf{Op} method for a streamed field (parameter or return value). - internal CodeBlock BuildEncodeStreamMethod(Field streamParam, string currentNamespace) + /// The streamed field. + /// The current C# namespace. + /// The escaped name to use for the injected encodeOptions parameter. + internal CodeBlock BuildEncodeStreamMethod(Field streamParam, string currentNamespace, string encodeOptionsName) { string opName = op.Name; string streamType = GetStreamTypeString(streamParam, currentNamespace); @@ -65,7 +88,7 @@ internal CodeBlock BuildEncodeStreamMethod(Field streamParam, string currentName "summary", $"Encodes the stream argument of operation {op.Name} into a request payload continuation.") .AddParameter(streamType, streamParam.ParameterName) - .AddParameter("SliceEncodeOptions?", "encodeOptions", "null", "The Slice encode options.") + .AddParameter("SliceEncodeOptions?", encodeOptionsName, "null", "The Slice encode options.") .AddComment("returns", "A new request payload continuation."); if (streamParam.IsByteStream) @@ -92,7 +115,7 @@ internal CodeBlock BuildEncodeStreamMethod(Field streamParam, string currentName {{streamParam.ParameterName}}.ToPipeReader( {{encodeLambda}}, {{(useSegments ? "true" : "false")}}, - encodeOptions) + {{encodeOptionsName}}) """); } @@ -114,7 +137,8 @@ internal string GetServiceReturnType(string currentNamespace) if (op.StreamedReturn is Field streamReturn) { string streamType = GetStreamTypeString(streamReturn, currentNamespace); - return $"global::System.Threading.Tasks.ValueTask<(global::System.IO.Pipelines.PipeReader Payload, {streamType} {streamReturn.Name})>"; + string payloadName = op.EncodedReturnPayloadName; + return $"global::System.Threading.Tasks.ValueTask<(global::System.IO.Pipelines.PipeReader {payloadName}, {streamType} {streamReturn.Name})>"; } return op.NonStreamedReturns.Count > 0 ? "global::System.Threading.Tasks.ValueTask" diff --git a/src/IceRpc.Slice.Generator/ProxyGenerator.cs b/src/IceRpc.Slice.Generator/ProxyGenerator.cs index 4fd41774e9..73b2e6fbcf 100644 --- a/src/IceRpc.Slice.Generator/ProxyGenerator.cs +++ b/src/IceRpc.Slice.Generator/ProxyGenerator.cs @@ -95,7 +95,7 @@ private static CodeBlock BuildClientOperationDeclaration(Operation op, string cu builder.AddParameter("IceRpc.Features.IFeatureCollection?", featuresParam, "null", "The invocation features."); builder.AddParameter( "global::System.Threading.CancellationToken", - "cancellationToken", + op.CancellationTokenParamName, "default", "A cancellation token that receives the cancellation requests."); @@ -193,6 +193,7 @@ private static CodeBlock BuildProxyRequestClass( ImmutableList nonStreamedParams = op.NonStreamedParameters; string opName = op.Name; + string encodeOptionsName = op.RequestEncodeOptionsParamName; CodeBlock? encodeBody = nonStreamedParams.GenerateEncodeBody(currentNamespace); @@ -213,7 +214,7 @@ private static CodeBlock BuildProxyRequestClass( .AddComment( "summary", $"Encodes the arguments of operation {op.Identifier} into a request payload.") - .AddParameter("SliceEncodeOptions?", "encodeOptions", "null", "The Slice encode options.") + .AddParameter("SliceEncodeOptions?", encodeOptionsName, "null", "The Slice encode options.") .AddComment("returns", "The Slice-encoded payload.") .SetBody(emptyPayload) .Build()); @@ -237,16 +238,16 @@ private static CodeBlock BuildProxyRequestClass( } request.AddBlock( - fn.AddParameter("SliceEncodeOptions?", "encodeOptions", "null", "The Slice encode options.") + fn.AddParameter("SliceEncodeOptions?", encodeOptionsName, "null", "The Slice encode options.") .AddComment("returns", "The Slice-encoded payload.") - .SetBody(EncodeHelper.BuildEncodeBody(encodeBody)) + .SetBody(EncodeHelper.BuildEncodeBody(encodeBody, encodeOptionsName)) .Build()); } // Add EncodeStreamOf{Op} method for streamed parameters if (op.StreamedParameter is Field streamParam) { - request.AddBlock(op.BuildEncodeStreamMethod(streamParam, currentNamespace)); + request.AddBlock(op.BuildEncodeStreamMethod(streamParam, currentNamespace, encodeOptionsName)); } } @@ -440,16 +441,18 @@ private static CodeBlock BuildProxyOperationCore(Operation op, string currentNam } string featuresParam = op.FeaturesParamName; + string cancellationTokenParam = op.CancellationTokenParamName; + string requestEncodeOptionsParam = op.RequestEncodeOptionsParamName; builder.AddParameter("IceRpc.Features.IFeatureCollection?", featuresParam, "null"); - builder.AddParameter("global::System.Threading.CancellationToken", "cancellationToken", "default"); + builder.AddParameter("global::System.Threading.CancellationToken", cancellationTokenParam, "default"); string encodeArgs = nonStreamedParams.Count > 0 - ? string.Join(", ", nonStreamedParams.Select(p => p.ParameterName)) + ", encodeOptions: EncodeOptions" - : "encodeOptions: EncodeOptions"; + ? string.Join(", ", nonStreamedParams.Select(p => p.ParameterName)) + $", {requestEncodeOptionsParam}: EncodeOptions" + : $"{requestEncodeOptionsParam}: EncodeOptions"; // payloadContinuation: either null or the stream encoder string payloadContinuation = streamParam is not null - ? $"Request.EncodeStreamOf{opName}({streamParam.ParameterName}, encodeOptions: EncodeOptions)" + ? $"Request.EncodeStreamOf{opName}({streamParam.ParameterName}, {requestEncodeOptionsParam}: EncodeOptions)" : "null"; FunctionCallBuilder bodyBuilder = new FunctionCallBuilder("this.InvokeOperationAsync") @@ -462,7 +465,7 @@ private static CodeBlock BuildProxyOperationCore(Operation op, string currentNam .AddArgument(featuresParam) .AddArgumentIf(op.IsIdempotent, "idempotent: true") .AddArgumentIf(op.Attributes.HasAttribute("oneway"), "oneway: true") - .AddArgument("cancellationToken: cancellationToken"); + .AddArgument($"cancellationToken: {cancellationTokenParam}"); if (compressArgs) { @@ -506,8 +509,9 @@ private static CodeBlock BuildProxyBaseOperationDelegation( paramNames.Add(streamParam.ParameterName); } string featuresParam = op.FeaturesParamName; + string cancellationTokenParam = op.CancellationTokenParamName; paramNames.Add(featuresParam); - paramNames.Add("cancellationToken"); + paramNames.Add(cancellationTokenParam); FunctionBuilder builder = new FunctionBuilder( "public", returnType, $"{opName}Async", FunctionType.ExpressionBody) @@ -526,7 +530,7 @@ private static CodeBlock BuildProxyBaseOperationDelegation( } builder.AddParameter("IceRpc.Features.IFeatureCollection?", featuresParam, "null"); - builder.AddParameter("global::System.Threading.CancellationToken", "cancellationToken", "default"); + builder.AddParameter("global::System.Threading.CancellationToken", cancellationTokenParam, "default"); builder.SetBody($"(({baseProxyName})this).{opName}Async({string.Join(", ", paramNames)})"); diff --git a/src/IceRpc.Slice.Generator/ServiceGenerator.cs b/src/IceRpc.Slice.Generator/ServiceGenerator.cs index 0f8c4a9186..0ce8bbe527 100644 --- a/src/IceRpc.Slice.Generator/ServiceGenerator.cs +++ b/src/IceRpc.Slice.Generator/ServiceGenerator.cs @@ -214,6 +214,7 @@ private static CodeBlock BuildServiceResponseClass( string opName = op.Name; ImmutableList nonStreamedReturns = op.NonStreamedReturns; + string encodeOptionsName = op.ResponseEncodeOptionsParamName; CodeBlock? encodeBody = nonStreamedReturns.GenerateEncodeBody(currentNamespace); if (encodeBody is null) @@ -231,7 +232,7 @@ private static CodeBlock BuildServiceResponseClass( .AddComment( "summary", $"Encodes the return value of operation {op.Identifier} into a response payload.") - .AddParameter("SliceEncodeOptions?", "encodeOptions", "null", "The Slice encode options.") + .AddParameter("SliceEncodeOptions?", encodeOptionsName, "null", "The Slice encode options.") .AddComment("returns", "A new response payload.") .SetBody(emptyPayload) .Build()); @@ -256,16 +257,16 @@ private static CodeBlock BuildServiceResponseClass( response.AddBlock( encodeBuilder - .AddParameter("SliceEncodeOptions?", "encodeOptions", "null", "The Slice encode options.") + .AddParameter("SliceEncodeOptions?", encodeOptionsName, "null", "The Slice encode options.") .AddComment("returns", "A new response payload.") - .SetBody(EncodeHelper.BuildEncodeBody(encodeBody)) + .SetBody(EncodeHelper.BuildEncodeBody(encodeBody, encodeOptionsName)) .Build()); } // Add EncodeStreamOf method for streamed returns if (op.StreamedReturn is Field streamReturn) { - response.AddBlock(op.BuildEncodeStreamMethod(streamReturn, currentNamespace)); + response.AddBlock(op.BuildEncodeStreamMethod(streamReturn, currentNamespace, encodeOptionsName)); } } @@ -305,7 +306,7 @@ private static CodeBlock BuildServiceOperationDeclaration(Operation op, string c docComment: "The dispatch features."); operationBuilder.AddParameter( "global::System.Threading.CancellationToken", - "cancellationToken", + op.CancellationTokenParamName, docComment: "A cancellation token that receives the cancellation requests."); if (op.NonStreamedReturns.Count == 0 && !op.HasStreamedReturn) diff --git a/tests/IceRpc.Slice.Generator.Tests/OperationTests.cs b/tests/IceRpc.Slice.Generator.Tests/OperationTests.cs index 1fa259a11d..3a38b1d22b 100644 --- a/tests/IceRpc.Slice.Generator.Tests/OperationTests.cs +++ b/tests/IceRpc.Slice.Generator.Tests/OperationTests.cs @@ -789,6 +789,38 @@ public ValueTask OpWithSpecialParameterNamesAsync( IFeatureCollection features_, CancellationToken cancellationToken) => default; + public ValueTask OpWithCancellationTokenParameterAsync( + int cancellationToken, + IFeatureCollection features, + CancellationToken cancellationToken_) => default; + + public ValueTask OpWithEncodeOptionsParameterAsync( + int encodeOptions, + IFeatureCollection features, + CancellationToken cancellationToken) => default; + + public ValueTask<(int R1, int EncodeOptions)> OpWithEncodeOptionsReturnAsync( + IFeatureCollection features, + CancellationToken cancellationToken) => new((1, 42)); + + public ValueTask<(PipeReader Payload_, IAsyncEnumerable Payload)> OpWithPayloadStreamReturnAsync( + IFeatureCollection features, + CancellationToken cancellationToken) + { + var payload = IMyOperationsAService.Response.EncodeOpWithPayloadStreamReturn( + new int[] { 1, 2, 3 }, + features.Get()?.EncodeOptions); + return new((payload, GetDataAsync())); + + static async IAsyncEnumerable GetDataAsync() + { + await Task.Yield(); + yield return 1; + yield return 2; + yield return 3; + } + } + public ValueTask OpWithSingleReturnValueAndEncodedReturnAttributeAsync( IFeatureCollection features, CancellationToken cancellationToken) => diff --git a/tests/IceRpc.Slice.Generator.Tests/OperationTests.slice b/tests/IceRpc.Slice.Generator.Tests/OperationTests.slice index a15988c106..b2d4e6ee3a 100644 --- a/tests/IceRpc.Slice.Generator.Tests/OperationTests.slice +++ b/tests/IceRpc.Slice.Generator.Tests/OperationTests.slice @@ -38,6 +38,18 @@ interface MyOperationsA { // cancel and features as regular parameter names opWithSpecialParameterNames(cancel: int32, features: int32) + // cancellationToken as regular parameter name (collides with the injected cancellationToken parameter) + opWithCancellationTokenParameter(cancellationToken: int32) + + // encodeOptions as regular parameter name (collides with the injected encodeOptions parameter on Request.Encode*) + opWithEncodeOptionsParameter(encodeOptions: int32) + + // encodeOptions as regular return name (collides with the injected encodeOptions parameter on Response.Encode*) + opWithEncodeOptionsReturn() -> (r1: int32, encodeOptions: int32) + + // payload as streamed return name with cs::encodedReturn (collides with the synthetic Payload tuple element) + [cs::encodedReturn] opWithPayloadStreamReturn() -> (r1: Sequence, payload: stream int32) + // Encoded return [cs::encodedReturn] opWithSingleReturnValueAndEncodedReturnAttribute() -> Sequence [cs::encodedReturn] opWithMultipleReturnValuesAndEncodedReturnAttribute() -> (r1: Sequence, r2: Sequence)