From dde12bffde7c260cf965255226cd0663fae3566f Mon Sep 17 00:00:00 2001 From: Jonathan Cardenas Date: Fri, 29 May 2026 17:01:06 -0700 Subject: [PATCH 1/4] feat(http-client-csharp): add ArgumentExpression for ref/out argument support Add a new ArgumentExpression class that wraps a ValueExpression with optional ref/out modifiers, separating argument-passing semantics from variable reference semantics. Changes: - Create ArgumentExpression to handle ref/out keywords in argument context - Remove IsRef/IsOut from VariableExpression (now a pure variable reference) - Simplify ParameterProvider by removing the dual _asVariable/_asArgument pattern - Update AsArgument() to return ArgumentExpression wrapping the variable - Update call sites: ClientProvider, MrwSerializationTypeDefinition, JsonPatchSnippets, ScmModelProvider - Add VisitArgumentExpression to LibraryVisitor - Add comprehensive ArgumentExpressionTests Closes microsoft/typespec#8399 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Providers/ClientProvider.cs | 2 +- .../MrwSerializationTypeDefinition.Dynamic.cs | 2 +- .../src/Providers/ScmModelProvider.cs | 2 +- .../src/Snippets/JsonPatchSnippets.cs | 2 +- .../src/Expressions/ArgumentExpression.cs | 60 +++++++++ .../src/Expressions/VariableExpression.cs | 20 +-- .../src/LibraryVisitor.cs | 5 + .../src/Providers/ParameterProvider.cs | 74 +++------- .../src/Snippets/Snippet.cs | 4 +- .../Expressions/ArgumentExpressionTests.cs | 127 ++++++++++++++++++ .../Expressions/VariableExpressionTests.cs | 12 +- 11 files changed, 222 insertions(+), 88 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/ArgumentExpression.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ArgumentExpressionTests.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index a0e522fb0ef..b9e72b3d10b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1171,7 +1171,7 @@ protected override ScmMethodProvider[] BuildMethods() continue; } - var cachedClientFieldVar = new VariableExpression(subClient.Type, subClient._clientCachingField.Declaration, IsRef: true); + var cachedClientFieldVar = new ArgumentExpression(new VariableExpression(subClient.Type, subClient._clientCachingField.Declaration), IsRef: true); List subClientConstructorArgs = new(3); List accessorMethodParams = []; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs index ebb69144805..cada3e97f84 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs @@ -418,7 +418,7 @@ private MethodBodyStatement[] BuildCollectionIfStatements( statements.Add(new IfStatement(ReadOnlySpanSnippets.IsEmpty(currentSlice)) { Return(new InvokeMethodExpression(null, tryResolveMethodName, - [new VariableExpression(valueVar.Type, valueVar.Declaration, IsOut: true)])) + [new ArgumentExpression(valueVar, IsOut: true)])) }); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs index 31c2de56eb1..75a931e1629 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs @@ -242,7 +242,7 @@ protected override ConstructorProvider[] BuildConstructors() type: _jsonPatchFieldType, name: JsonPatchPropertyName, isRef: isRef, - body: new ExpressionPropertyBody(new VariableExpression(JsonPatchField.Type, JsonPatchField.Declaration, IsRef: isRef)), + body: new ExpressionPropertyBody(isRef ? ByRef(new VariableExpression(JsonPatchField.Type, JsonPatchField.Declaration)) : new VariableExpression(JsonPatchField.Type, JsonPatchField.Declaration)), attributes: [ new AttributeStatement(typeof(JsonIgnoreAttribute)), diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Snippets/JsonPatchSnippets.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Snippets/JsonPatchSnippets.cs index 241700ece53..725be9ca310 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Snippets/JsonPatchSnippets.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Snippets/JsonPatchSnippets.cs @@ -34,7 +34,7 @@ public static ScopedApi TryGetEncodedValue( this ScopedApi patch, ValueExpression jsonPath, VariableExpression encodedValue) - => patch.Invoke(nameof(JsonPatch.TryGetEncodedValue), [jsonPath, new VariableExpression(encodedValue.Type, encodedValue.Declaration, IsOut: true)]).As(); + => patch.Invoke(nameof(JsonPatch.TryGetEncodedValue), [jsonPath, new ArgumentExpression(encodedValue, IsOut: true)]).As(); public static ScopedApi Contains( this ScopedApi patch, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/ArgumentExpression.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/ArgumentExpression.cs new file mode 100644 index 00000000000..29aea969eca --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/ArgumentExpression.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.TypeSpec.Generator.Providers; + +namespace Microsoft.TypeSpec.Generator.Expressions +{ + /// + /// Represents a method argument that wraps a with optional ref or out modifiers. + /// + public sealed record ArgumentExpression(ValueExpression Expression, bool IsRef = false, bool IsOut = false) : ValueExpression + { + public ValueExpression Expression { get; private set; } = Expression; + public bool IsRef { get; private set; } = IsRef; + public bool IsOut { get; private set; } = IsOut; + + internal override void Write(CodeWriter writer) + { + writer.AppendRawIf("ref ", IsRef); + writer.AppendRawIf("out ", IsOut); + Expression.Write(writer); + } + + internal override ValueExpression? Accept(LibraryVisitor visitor, MethodProvider method) + { + var expr = visitor.VisitArgumentExpression(this, method); + + if (expr is not ArgumentExpression argumentExpression) + { + return expr?.Accept(visitor, method); + } + + var newExpression = argumentExpression.Expression.Accept(visitor, method); + if (newExpression != null) + { + argumentExpression.Expression = newExpression; + } + + return argumentExpression; + } + + public void Update(ValueExpression? expression = null, bool? isRef = null, bool? isOut = null) + { + if (expression != null) + { + Expression = expression; + } + + if (isRef != null) + { + IsRef = isRef.Value; + } + + if (isOut != null) + { + IsOut = isOut.Value; + } + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs index 430b3fbd6eb..eb41209b652 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs @@ -6,21 +6,17 @@ namespace Microsoft.TypeSpec.Generator.Expressions { - public record VariableExpression(CSharpType Type, CodeWriterDeclaration Declaration, bool IsRef = false, bool IsOut = false) : ValueExpression + public record VariableExpression(CSharpType Type, CodeWriterDeclaration Declaration) : ValueExpression { public CSharpType Type { get; private set; } = Type; public CodeWriterDeclaration Declaration { get; private set; } = Declaration; - public bool IsRef { get; private set; } = IsRef; - public bool IsOut { get; private set; } = IsOut; - public VariableExpression(CSharpType type, string name, bool isRef = false, bool isOut = false) - : this(type, new CodeWriterDeclaration(name), isRef, isOut) + public VariableExpression(CSharpType type, string name) + : this(type, new CodeWriterDeclaration(name)) { } internal override void Write(CodeWriter writer) { - writer.AppendRawIf("ref ", IsRef); - writer.AppendRawIf("out ", IsOut); writer.Append(Declaration); } @@ -29,7 +25,7 @@ internal override VariableExpression Accept(LibraryVisitor visitor, MethodProvid return visitor.VisitVariableExpression(this, method); } - public void Update(CSharpType? type = null, string? name = null, bool? isRef = null, bool? isOut = null) + public void Update(CSharpType? type = null, string? name = null) { if (type != null) { @@ -39,14 +35,6 @@ public void Update(CSharpType? type = null, string? name = null, bool? isRef = n { Declaration = new CodeWriterDeclaration(name); } - if (isRef != null) - { - IsRef = isRef.Value; - } - if (isOut != null) - { - IsOut = isOut.Value; - } } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs index 6df7ccc9758..949f05f5bf3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs @@ -265,6 +265,11 @@ protected internal virtual void VisitLibrary(OutputLibrary library) return expression; } + protected internal virtual ValueExpression? VisitArgumentExpression(ArgumentExpression expression, MethodProvider method) + { + return expression; + } + protected internal virtual VariableExpression VisitVariableExpression(VariableExpression expression, MethodProvider method) { return expression; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ParameterProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ParameterProvider.cs index e8387fc0be3..1a4121e5367 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ParameterProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ParameterProvider.cs @@ -191,65 +191,35 @@ private string GetDebuggerDisplay() // TODO test case for changing the parameter name via the visitor to see if the variable expression is updated // Same for properties and fields // https://github.com/microsoft/typespec/issues/3813 - public static implicit operator VariableExpression(ParameterProvider parameter) => GetVariableExpression(parameter, includeModifiers: false); + public static implicit operator VariableExpression(ParameterProvider parameter) => GetVariableExpression(parameter); - internal static VariableExpression GetVariableExpression(ParameterProvider parameter, bool includeModifiers) + internal static VariableExpression GetVariableExpression(ParameterProvider parameter) { - CodeWriterDeclaration? declaration = parameter._asVariable?.Declaration ?? parameter._asArgument?.Declaration; - - var variableName = parameter.InputParameter?.IsExactName == true - ? parameter.Name - : parameter.Name.ToVariableName(); - - if (includeModifiers) - { - if (parameter._asArgument == null) - { - if (declaration != null) - { - parameter._asArgument = new VariableExpression( - parameter.Type, - declaration, - parameter.IsRef, - parameter.IsOut); - } - else - { - parameter._asArgument = new VariableExpression( - parameter.Type, - variableName, - parameter.IsRef, - parameter.IsOut); - } - } - return parameter._asArgument; - } - if (parameter._asVariable == null) { - if (declaration != null) - { - parameter._asVariable = new VariableExpression( - parameter.Type, - declaration, - parameter.IsRef, - parameter.IsOut); - } - else - { - parameter._asVariable = new VariableExpression( - parameter.Type, - variableName, - includeModifiers && parameter.IsRef, - includeModifiers && parameter.IsOut); - } + var variableName = parameter.InputParameter?.IsExactName == true + ? parameter.Name + : parameter.Name.ToVariableName(); + + parameter._asVariable = new VariableExpression( + parameter.Type, + variableName); } return parameter._asVariable; } + internal static ValueExpression GetArgumentExpression(ParameterProvider parameter) + { + var variable = GetVariableExpression(parameter); + if (parameter.IsRef || parameter.IsOut) + { + return new ArgumentExpression(variable, parameter.IsRef, parameter.IsOut); + } + return variable; + } + private VariableExpression? _asVariable; - private VariableExpression? _asArgument; public TypeProvider? SpreadSource { get; set; } @@ -300,7 +270,6 @@ internal ParameterProvider WithRef() validation: Validation) { _asVariable = _asVariable, - _asArgument = _asArgument, }; } @@ -328,7 +297,6 @@ public void Update( { Name = name; _asVariable?.Update(name: name); - _asArgument?.Update(name: name); } if (description is not null) @@ -350,15 +318,11 @@ public void Update( if (isRef is not null) { IsRef = isRef.Value; - _asVariable?.Update(isRef: IsRef); - _asArgument?.Update(isRef: IsRef); } if (isOut is not null) { IsOut = isOut.Value; - _asVariable?.Update(isOut: IsOut); - _asArgument?.Update(isOut: IsOut); } if (isIn is not null) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Snippets/Snippet.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Snippets/Snippet.cs index 48172113ffc..4853ed48329 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Snippets/Snippet.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Snippets/Snippet.cs @@ -179,7 +179,7 @@ public static ValueExpression Invoke(this PropertyProvider property, public static ScopedApi NotEqual(this ParameterProvider parameter, ValueExpression other) => new BinaryOperatorExpression("!=", parameter, other).As(); - public static VariableExpression AsVariable(this ParameterProvider parameter) => ParameterProvider.GetVariableExpression(parameter, includeModifiers: false); - public static VariableExpression AsArgument(this ParameterProvider property) => ParameterProvider.GetVariableExpression(property, includeModifiers: true); + public static VariableExpression AsVariable(this ParameterProvider parameter) => ParameterProvider.GetVariableExpression(parameter); + public static ValueExpression AsArgument(this ParameterProvider parameter) => ParameterProvider.GetArgumentExpression(parameter); } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ArgumentExpressionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ArgumentExpressionTests.cs new file mode 100644 index 00000000000..1db3e378ced --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ArgumentExpressionTests.cs @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.TypeSpec.Generator.Expressions; +using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Snippets; +using NUnit.Framework; + +namespace Microsoft.TypeSpec.Generator.Tests.Expressions +{ + public class ArgumentExpressionTests + { + [Test] + public void ArgumentExpressionWithoutModifiers() + { + var variable = new VariableExpression(typeof(int), "foo"); + var argument = new ArgumentExpression(variable); + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("foo", writer.ToString(false)); + } + + [Test] + public void ArgumentExpressionWithRef() + { + var variable = new VariableExpression(typeof(int), "foo"); + var argument = new ArgumentExpression(variable, IsRef: true); + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("ref foo", writer.ToString(false)); + } + + [Test] + public void ArgumentExpressionWithOut() + { + var variable = new VariableExpression(typeof(int), "foo"); + var argument = new ArgumentExpression(variable, IsOut: true); + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("out foo", writer.ToString(false)); + } + + [Test] + public void ArgumentExpressionUpdateChangesModifiers() + { + var variable = new VariableExpression(typeof(int), "foo"); + var argument = new ArgumentExpression(variable); + + argument.Update(isRef: true); + + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("ref foo", writer.ToString(false)); + } + + [Test] + public void ArgumentExpressionUpdateChangesExpression() + { + var variable1 = new VariableExpression(typeof(int), "foo"); + var variable2 = new VariableExpression(typeof(int), "bar"); + var argument = new ArgumentExpression(variable1, IsRef: true); + + argument.Update(expression: variable2); + + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("ref bar", writer.ToString(false)); + } + + [Test] + public void ParameterProviderAsArgumentWithRef() + { + MockHelpers.LoadMockGenerator(); + var parameter = new ParameterProvider("myParam", $"", typeof(int), isRef: true); + var argument = Snippet.AsArgument(parameter); + + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("ref myParam", writer.ToString(false)); + } + + [Test] + public void ParameterProviderAsArgumentWithOut() + { + MockHelpers.LoadMockGenerator(); + var parameter = new ParameterProvider("myParam", $"", typeof(int), isOut: true); + var argument = Snippet.AsArgument(parameter); + + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("out myParam", writer.ToString(false)); + } + + [Test] + public void ParameterProviderAsArgumentWithoutModifiers() + { + MockHelpers.LoadMockGenerator(); + var parameter = new ParameterProvider("myParam", $"", typeof(int)); + var argument = Snippet.AsArgument(parameter); + + using CodeWriter writer = new CodeWriter(); + argument.Write(writer); + + Assert.AreEqual("myParam", writer.ToString(false)); + } + + [Test] + public void ParameterProviderAsVariableDoesNotIncludeModifiers() + { + MockHelpers.LoadMockGenerator(); + var parameter = new ParameterProvider("myParam", $"", typeof(int), isRef: true); + VariableExpression variable = parameter; + + using CodeWriter writer = new CodeWriter(); + variable.Write(writer); + + Assert.AreEqual("myParam", writer.ToString(false)); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs index b65c269f1b9..15ab9ea0e97 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs @@ -10,7 +10,7 @@ namespace Microsoft.TypeSpec.Generator.Tests.Expressions public class VariableExpressionTests { [Test] - public void VariableExpressionWithoutRef() + public void VariableExpressionWritesName() { var variableExpression = new VariableExpression(typeof(int), "foo"); using CodeWriter writer = new CodeWriter(); @@ -18,15 +18,5 @@ public void VariableExpressionWithoutRef() Assert.AreEqual("foo", writer.ToString(false)); } - - [Test] - public void VariableExpressionWithRef() - { - var variableExpression = new VariableExpression(typeof(int), "foo", true); - using CodeWriter writer = new CodeWriter(); - variableExpression.Write(writer); - - Assert.AreEqual("ref foo", writer.ToString(false)); - } } } From 8cffdbda74f3046fbc3a1e3cf0d49d0ce2385a17 Mon Sep 17 00:00:00 2001 From: Jonathan Cardenas Date: Tue, 2 Jun 2026 15:09:04 -0700 Subject: [PATCH 2/4] fix(generator): preserve VariableExpression compatibility Restore the legacy VariableExpression ref/out constructor and add regression tests for ArgumentExpression visitor handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Expressions/VariableExpression.cs | 28 ++++++++++- .../Expressions/ExpressionVisitorTests.cs | 49 +++++++++++++++++++ .../Expressions/VariableExpressionTests.cs | 9 ++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs index eb41209b652..29471ab274d 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs @@ -8,13 +8,29 @@ namespace Microsoft.TypeSpec.Generator.Expressions { public record VariableExpression(CSharpType Type, CodeWriterDeclaration Declaration) : ValueExpression { + // Kept for backward compatibility with existing callers that still construct + // a variable with ref/out modifiers; use ArgumentExpression for argument semantics. + public bool IsRef { get; private set; } + + // Kept for backward compatibility with existing callers that still construct + // a variable with ref/out modifiers; use ArgumentExpression for argument semantics. + public bool IsOut { get; private set; } + public CSharpType Type { get; private set; } = Type; public CodeWriterDeclaration Declaration { get; private set; } = Declaration; + public VariableExpression(CSharpType type, string name) : this(type, new CodeWriterDeclaration(name)) { } + public VariableExpression(CSharpType type, string name, bool isRef = false, bool isOut = false) + : this(type, new CodeWriterDeclaration(name)) + { + IsRef = isRef; + IsOut = isOut; + } + internal override void Write(CodeWriter writer) { writer.Append(Declaration); @@ -25,7 +41,7 @@ internal override VariableExpression Accept(LibraryVisitor visitor, MethodProvid return visitor.VisitVariableExpression(this, method); } - public void Update(CSharpType? type = null, string? name = null) + public void Update(CSharpType? type = null, string? name = null, bool? isRef = null, bool? isOut = null) { if (type != null) { @@ -35,6 +51,16 @@ public void Update(CSharpType? type = null, string? name = null) { Declaration = new CodeWriterDeclaration(name); } + + if (isRef != null) + { + IsRef = isRef.Value; + } + + if (isOut != null) + { + IsOut = isOut.Value; + } } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs index 9cb47fad7ee..f5bdc03a0d9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; @@ -97,6 +98,37 @@ private static void ValidateVariableExpression(string variableName) Assert.AreEqual("replacedVariable = \"foo\";\n", updatedMethod!.BodyStatements!.ToDisplayString()); } + [Test] + public void CanChangeArgumentExpression() + { + ValidateArgumentExpression("change"); + } + + [Test] + public void CanUpdateArgumentExpression() + { + ValidateArgumentExpression("update"); + } + + private static void ValidateArgumentExpression(string variableName) + { + MockHelpers.LoadMockGenerator(); + var type = new TestTypeProvider(); + var method = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, type.Type, $"", []), + Array.Empty(), + type); + var argument = new ArgumentExpression(new VariableExpression(type.Type, variableName), IsRef: true); + var visitor = new TestLibraryVisitor(); + var updatedExpression = argument.Accept(visitor, method); + + Assert.IsNotNull(updatedExpression); + Assert.IsInstanceOf(updatedExpression); + var updatedArgument = (ArgumentExpression)updatedExpression!; + Assert.AreEqual(variableName == "change" ? "replacedVariable" : "replacedVariable", ((VariableExpression)updatedArgument.Expression).Declaration.RequestedName); + Assert.IsTrue(updatedArgument.IsRef); + } + [Test] public void CanChangeMemberExpression() { @@ -210,6 +242,23 @@ protected internal override VariableExpression VisitVariableExpression(VariableE return expression; } + protected internal override ValueExpression? VisitArgumentExpression(ArgumentExpression expression, + MethodProvider methodProvider) + { + if (((VariableExpression)expression.Expression).Declaration.RequestedName == "change") + { + return new ArgumentExpression(new VariableExpression(typeof(string), "replacedVariable"), IsRef: true); + } + + if (((VariableExpression)expression.Expression).Declaration.RequestedName == "update") + { + expression.Update(expression: new VariableExpression(typeof(string), "replacedVariable"), isRef: true); + return expression; + } + + return expression; + } + protected internal override ValueExpression VisitMemberExpression(MemberExpression expression, MethodProvider methodProvider) { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs index 15ab9ea0e97..c70584ad1cd 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs @@ -18,5 +18,14 @@ public void VariableExpressionWritesName() Assert.AreEqual("foo", writer.ToString(false)); } + + [Test] + public void VariableExpressionSupportsLegacyRefAndOutConstructor() + { + var variableExpression = new VariableExpression(typeof(int), "foo", isRef: true, isOut: true); + + Assert.IsTrue(variableExpression.IsRef); + Assert.IsTrue(variableExpression.IsOut); + } } } From 56c20d4689d54e95f7410562c8199be9682cdaa5 Mon Sep 17 00:00:00 2001 From: Jonathan Cardenas Date: Tue, 2 Jun 2026 16:14:38 -0700 Subject: [PATCH 3/4] fix(http-client-csharp): bypass engine-strict for workspace setup in CI The pnpm setup:min step in RegenCheck CI fails on Linux runners where NodeTool resolves Node.js 24.x to v24.14.1, which does not satisfy the which@7.0.0 engine requirement (^24.15.0). Temporarily relax engine-strict during workspace setup to unblock CI while the runner images are updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/http-client-csharp/eng/scripts/Test-Packages.ps1 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/http-client-csharp/eng/scripts/Test-Packages.ps1 b/packages/http-client-csharp/eng/scripts/Test-Packages.ps1 index f859bbd44eb..27b1a18a795 100644 --- a/packages/http-client-csharp/eng/scripts/Test-Packages.ps1 +++ b/packages/http-client-csharp/eng/scripts/Test-Packages.ps1 @@ -37,7 +37,12 @@ try { Invoke-LoggedCommand "npm install -g pnpm" -GroupOutput Write-Host "Setting up workspace" -ForegroundColor Cyan + # Temporarily relax engine-strict so that pnpm install succeeds even when + # the CI runner's Node version doesn't match every transitive dependency's + # engines field (e.g. which@7 requiring a newer Node patch release). + $env:npm_config_engine_strict = "false" Invoke-LoggedCommand "pnpm setup:min" $packageRoot/../.. + Remove-Item Env:\npm_config_engine_strict -ErrorAction SilentlyContinue Write-Host "Regenerating extern signatures" -ForegroundColor Cyan Invoke-LoggedCommand "npm run gen-extern-signature" -GroupOutput From e7255fb056b3df400527f754e4745921707eec89 Mon Sep 17 00:00:00 2001 From: Jonathan Cardenas Date: Tue, 2 Jun 2026 16:29:34 -0700 Subject: [PATCH 4/4] refactor(http-client-csharp): improve ArgumentExpression adoption - Mark VariableExpression.IsRef/IsOut and legacy constructor as [Obsolete] to guide callers toward ArgumentExpression - Replace ByRef() with ArgumentExpression in MrwSerializationTypeDefinition for method argument semantics (ref return in ScmModelProvider is correct) - Fix dead ternary in ExpressionVisitorTests assertion - Keep redundant 2-param VariableExpression(type, name) constructor since it is part of the public API surface and used widely Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../MrwSerializationTypeDefinition.cs | 4 ++-- .../src/Expressions/VariableExpression.cs | 20 +++++++++++-------- .../Expressions/ExpressionVisitorTests.cs | 2 +- .../Expressions/VariableExpressionTests.cs | 2 ++ 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs index 43688790768..c7304f739e0 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs @@ -2597,7 +2597,7 @@ private IReadOnlyList GetDeserializationHookArguments( if (hookMethod == null) { // Fall back: no method found, use previous behavior (first known arg, ref variable, no options) - return [knownArguments[0].Argument, ByRef(refVariable)]; + return [knownArguments[0].Argument, new ArgumentExpression(refVariable, IsRef: true)]; } var args = new List(); @@ -2605,7 +2605,7 @@ private IReadOnlyList GetDeserializationHookArguments( { if (param.IsRef) { - args.Add(ByRef(refVariable)); + args.Add(new ArgumentExpression(refVariable, IsRef: true)); } else { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs index 29471ab274d..0f52afbd52c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; @@ -8,27 +9,26 @@ namespace Microsoft.TypeSpec.Generator.Expressions { public record VariableExpression(CSharpType Type, CodeWriterDeclaration Declaration) : ValueExpression { - // Kept for backward compatibility with existing callers that still construct - // a variable with ref/out modifiers; use ArgumentExpression for argument semantics. + [Obsolete("Use ArgumentExpression for ref/out argument semantics.")] public bool IsRef { get; private set; } - // Kept for backward compatibility with existing callers that still construct - // a variable with ref/out modifiers; use ArgumentExpression for argument semantics. + [Obsolete("Use ArgumentExpression for ref/out argument semantics.")] public bool IsOut { get; private set; } public CSharpType Type { get; private set; } = Type; public CodeWriterDeclaration Declaration { get; private set; } = Declaration; - public VariableExpression(CSharpType type, string name) + [Obsolete("Use ArgumentExpression for ref/out argument semantics. Use VariableExpression(CSharpType, string) instead.")] + public VariableExpression(CSharpType type, string name, bool isRef = false, bool isOut = false) : this(type, new CodeWriterDeclaration(name)) { + IsRef = isRef; + IsOut = isOut; } - public VariableExpression(CSharpType type, string name, bool isRef = false, bool isOut = false) + public VariableExpression(CSharpType type, string name) : this(type, new CodeWriterDeclaration(name)) { - IsRef = isRef; - IsOut = isOut; } internal override void Write(CodeWriter writer) @@ -54,12 +54,16 @@ public void Update(CSharpType? type = null, string? name = null, bool? isRef = n if (isRef != null) { +#pragma warning disable CS0618 // Obsolete IsRef = isRef.Value; +#pragma warning restore CS0618 } if (isOut != null) { +#pragma warning disable CS0618 // Obsolete IsOut = isOut.Value; +#pragma warning restore CS0618 } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs index f5bdc03a0d9..1a0bd58f4e4 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/ExpressionVisitorTests.cs @@ -125,7 +125,7 @@ private static void ValidateArgumentExpression(string variableName) Assert.IsNotNull(updatedExpression); Assert.IsInstanceOf(updatedExpression); var updatedArgument = (ArgumentExpression)updatedExpression!; - Assert.AreEqual(variableName == "change" ? "replacedVariable" : "replacedVariable", ((VariableExpression)updatedArgument.Expression).Declaration.RequestedName); + Assert.AreEqual("replacedVariable", ((VariableExpression)updatedArgument.Expression).Declaration.RequestedName); Assert.IsTrue(updatedArgument.IsRef); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs index c70584ad1cd..7dc4fb64d23 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Expressions/VariableExpressionTests.cs @@ -22,10 +22,12 @@ public void VariableExpressionWritesName() [Test] public void VariableExpressionSupportsLegacyRefAndOutConstructor() { +#pragma warning disable CS0618 // Obsolete var variableExpression = new VariableExpression(typeof(int), "foo", isRef: true, isOut: true); Assert.IsTrue(variableExpression.IsRef); Assert.IsTrue(variableExpression.IsOut); +#pragma warning restore CS0618 } } }