From 679eb44ed60217e79b59281f8caab745decc63b3 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 8 Mar 2026 12:16:47 +0000 Subject: [PATCH 1/5] Fix VB to C# conversion for extension methods on properties passed by ref * Extract `HoistByRefDeclaration` in `ArgumentConverter` to accept `ExpressionSyntax` instead of `SimpleArgumentSyntax` so it can be reused for the `this` parameter of extension methods. * Update `NameExpressionNodeVisitor` to apply ref-conversion hoisting rules to the `this` expression of extension methods using `ref` parameters. * Add a unit test verifying that `Number.NegEx()` where `NegEx` takes `ByRef num as Integer` correctly generates a temporary variable assignment before and after the call. Co-authored-by: GrahamTheCoder <2490482+GrahamTheCoder@users.noreply.github.com> --- CodeConverter/CSharp/ArgumentConverter.cs | 12 ++-- .../CSharp/NameExpressionNodeVisitor.cs | 47 +++++++++++--- Tests/VB/ExtensionMethodRefPropertyTests.cs | 62 +++++++++++++++++++ 3 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 Tests/VB/ExtensionMethodRefPropertyTests.cs diff --git a/CodeConverter/CSharp/ArgumentConverter.cs b/CodeConverter/CSharp/ArgumentConverter.cs index 68bebdac0..083801d95 100644 --- a/CodeConverter/CSharp/ArgumentConverter.cs +++ b/CodeConverter/CSharp/ArgumentConverter.cs @@ -35,7 +35,7 @@ public async Task ConvertSimpleArgumentAsync(VBSyntax.SimpleAr var refType = CommonConversions.GetRefConversionType(node, argList, possibleParameters.Value, out var argName, out var refKind); token = CommonConversions.GetRefToken(refKind); if (refType != SemanticModelExtensions.RefConversion.Inline) { - convertedArgExpression = HoistByRefDeclaration(node, convertedArgExpression, refType, argName, refKind); + convertedArgExpression = HoistByRefDeclaration(node.Expression, convertedArgExpression, refType, argName, refKind); } else { convertedArgExpression = typeConversionAnalyzer.AddExplicitConversion(node.Expression, convertedArgExpression, defaultToCast: refKind != RefKind.None); } @@ -119,11 +119,11 @@ CSSyntax.ArgumentSyntax ConvertOmittedArgument(IParameterSymbol parameter) } - private CSSyntax.ExpressionSyntax HoistByRefDeclaration(VBSyntax.SimpleArgumentSyntax node, CSSyntax.ExpressionSyntax refLValue, SemanticModelExtensions.RefConversion refType, string argName, RefKind refKind) + internal CSSyntax.ExpressionSyntax HoistByRefDeclaration(VBSyntax.ExpressionSyntax node, CSSyntax.ExpressionSyntax refLValue, SemanticModelExtensions.RefConversion refType, string argName, RefKind refKind) { string prefix = $"arg{argName}"; - var expressionTypeInfo = _semanticModel.GetTypeInfo(node.Expression); - bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability) == true && !CommonConversions.ShouldPreferExplicitType(node.Expression, expressionTypeInfo.ConvertedType, out var _); + var expressionTypeInfo = _semanticModel.GetTypeInfo(node); + bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability) == true && !CommonConversions.ShouldPreferExplicitType(node, expressionTypeInfo.ConvertedType, out var _); var typeSyntax = CommonConversions.GetTypeSyntax(expressionTypeInfo.ConvertedType, useVar); if (refLValue is CSSyntax.ElementAccessExpressionSyntax eae) { @@ -132,12 +132,12 @@ private CSSyntax.ExpressionSyntax HoistByRefDeclaration(VBSyntax.SimpleArgumentS refLValue = eae.WithExpression(tmpContainer.IdentifierName); } - var withCast = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, refLValue, defaultToCast: refKind != RefKind.None); + var withCast = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node, refLValue, defaultToCast: refKind != RefKind.None); var local = _typeContext.PerScopeState.Hoist(new AdditionalDeclaration(prefix, withCast, typeSyntax)); if (refType == SemanticModelExtensions.RefConversion.PreAndPostAssignment) { - var convertedLocalIdentifier = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, local.IdentifierName, forceSourceType: expressionTypeInfo.ConvertedType, forceTargetType: expressionTypeInfo.Type); + var convertedLocalIdentifier = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node, local.IdentifierName, forceSourceType: expressionTypeInfo.ConvertedType, forceTargetType: expressionTypeInfo.Type); _typeContext.PerScopeState.Hoist(new AdditionalAssignment(refLValue, convertedLocalIdentifier)); } diff --git a/CodeConverter/CSharp/NameExpressionNodeVisitor.cs b/CodeConverter/CSharp/NameExpressionNodeVisitor.cs index 1b39251f5..513e3227f 100644 --- a/CodeConverter/CSharp/NameExpressionNodeVisitor.cs +++ b/CodeConverter/CSharp/NameExpressionNodeVisitor.cs @@ -285,14 +285,47 @@ private async Task ConvertInvocationAsync(VBSyntax.InvocationE } if (invocationSymbol.IsReducedExtension() && invocationSymbol is IMethodSymbol { ReducedFrom: { Parameters: var parameters } } && - !parameters.FirstOrDefault().ValidCSharpExtensionMethodParameter() && node.Expression is VBSyntax.MemberAccessExpressionSyntax maes) { - var thisArgExpression = await maes.Expression.AcceptAsync(TriviaConvertingExpressionVisitor); - var thisArg = SyntaxFactory.Argument(thisArgExpression).WithRefKindKeyword(CommonConversions.GetRefToken(RefKind.Ref)); - convertedArgumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(convertedArgumentList.Arguments.Prepend(thisArg))); - var containingType = (ExpressionSyntax)CommonConversions.CsSyntaxGenerator.TypeExpression(invocationSymbol.ContainingType); - convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, containingType, - ValidSyntaxFactory.IdentifierName((invocationSymbol.Name))); + + var thisParam = parameters.FirstOrDefault(); + bool requiresStaticInvocation = !thisParam.ValidCSharpExtensionMethodParameter(); + var refKind = CommonConversions.GetCsRefKind(thisParam); + + bool requiresHoist = false; + SemanticModelExtensions.RefConversion refConversion = SemanticModelExtensions.RefConversion.Inline; + if (refKind != RefKind.None) { + var symbolInfo = _semanticModel.GetSymbolInfoInDocument(maes.Expression); + if (symbolInfo is IPropertySymbol { ReturnsByRef: false, ReturnsByRefReadonly: false } propertySymbol) { + refConversion = propertySymbol.IsReadOnly ? SemanticModelExtensions.RefConversion.PreAssigment : SemanticModelExtensions.RefConversion.PreAndPostAssignment; + } else if (symbolInfo is IFieldSymbol { IsConst: true } or ILocalSymbol { IsConst: true }) { + refConversion = SemanticModelExtensions.RefConversion.PreAssigment; + } else if (symbolInfo is IMethodSymbol { ReturnsByRef: false, ReturnsByRefReadonly: false }) { + refConversion = SemanticModelExtensions.RefConversion.PreAssigment; + } else { + var typeInfo = _semanticModel.GetTypeInfo(maes.Expression); + bool isTypeMismatch = typeInfo.Type == null || !typeInfo.Type.Equals(typeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability); + if (isTypeMismatch) refConversion = SemanticModelExtensions.RefConversion.PreAndPostAssignment; + } + requiresHoist = refConversion != SemanticModelExtensions.RefConversion.Inline; + } + + if (requiresStaticInvocation || requiresHoist) { + var thisArgExpression = await maes.Expression.AcceptAsync(TriviaConvertingExpressionVisitor); + + if (requiresHoist) { + thisArgExpression = _argumentConverter.HoistByRefDeclaration(maes.Expression, thisArgExpression, refConversion, thisParam.Name, refKind); + } + + if (requiresStaticInvocation) { + var thisArg = SyntaxFactory.Argument(thisArgExpression).WithRefKindKeyword(CommonConversions.GetRefToken(refKind)); + convertedArgumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(convertedArgumentList.Arguments.Prepend(thisArg))); + var containingType = (ExpressionSyntax)CommonConversions.CsSyntaxGenerator.TypeExpression(invocationSymbol.ContainingType); + convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, containingType, + ValidSyntaxFactory.IdentifierName((invocationSymbol.Name))); + } else { + convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, thisArgExpression, ValidSyntaxFactory.IdentifierName((invocationSymbol.Name))); + } + } } if (invocationSymbol is IMethodSymbol m && convertedExpression is LambdaExpressionSyntax) { diff --git a/Tests/VB/ExtensionMethodRefPropertyTests.cs b/Tests/VB/ExtensionMethodRefPropertyTests.cs new file mode 100644 index 000000000..dc5d8d982 --- /dev/null +++ b/Tests/VB/ExtensionMethodRefPropertyTests.cs @@ -0,0 +1,62 @@ +using System.Threading.Tasks; +using Xunit; + +namespace ICSharpCode.CodeConverter.Tests.VB; + +public class ExtensionMethodRefPropertyTests : TestRunners.ConverterTestBase +{ + [Fact] + public async Task TestExtensionMethodRefPropertyAsync() + { + await TestConversionVisualBasicToCSharpAsync( + @"Imports System.Runtime.CompilerServices +Public Class ExtensionMethodsRefPropertyParameter + Public Property Number As Integer = 3 + Public Sub WithExtensionMethod() + Number.NegEx() + End Sub + Public Sub WithMethod() + Neg(Number) + End Sub +End Class + +Public Module MathEx + + Public Sub NegEx(ByRef num As Integer) + num = -num + End Sub + Public Sub Neg(ByRef num As Integer) + num = -num + End Sub +End Module", @" +public partial class ExtensionMethodsRefPropertyParameter +{ + public int Number { get; set; } = 3; + public void WithExtensionMethod() + { + int argnum = Number; + argnum.NegEx(); + Number = argnum; + } + public void WithMethod() + { + int argnum = Number; + MathEx.Neg(ref argnum); + Number = argnum; + } +} + +public static partial class MathEx +{ + public static void NegEx(this ref int num) + + { + num = -num; + } + public static void Neg(ref int num) + { + num = -num; + } +}", incompatibleWithAutomatedCommentTesting: true); + } +} \ No newline at end of file From 11903a9a3dc0c1b84033055c56a250c4af6cca5c Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 8 Mar 2026 15:20:45 +0000 Subject: [PATCH 2/5] Fix VB to C# conversion for extension methods on properties passed by ref * Add `HoistByRefDeclaration` overload in `ArgumentConverter` accepting `ExpressionSyntax` instead of `SimpleArgumentSyntax` to support generic expression hoisting. * Update `NameExpressionNodeVisitor` to correctly detect when the receiver expression of an extension method needs to be passed by reference (e.g. properties, fields), and reuse the variable hoisting logic to correctly generate temporary variables for property modification before and after the method call. * Add comprehensive test `TestExtensionMethodRefPropertyAsync` to ensure the generated C# code successfully extracts the property to a temporary variable for method calls. Co-authored-by: GrahamTheCoder <2490482+GrahamTheCoder@users.noreply.github.com> From 10f2e68a3fe00d4144a5f900419e87130952e0fb Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2026 00:11:28 +0000 Subject: [PATCH 3/5] Fix VB to C# conversion for extension methods on properties passed by ref * Generalize `HoistByRefDeclaration` in `ArgumentConverter` to accept `ExpressionSyntax`. * Update `NameExpressionNodeVisitor` to correctly process extension method receiver ref-conversions, hoisting the receiver expression (e.g., properties) into a temporary variable when required by `ByRef`. * Introduce `TestExtensionMethodRefPropertyAsync` to explicitly verify property ref assignments to extension methods generate a valid C# state. Co-authored-by: GrahamTheCoder <2490482+GrahamTheCoder@users.noreply.github.com> From d453053039c477f25667f700967fd94111c86a13 Mon Sep 17 00:00:00 2001 From: Graham Date: Tue, 10 Mar 2026 23:34:50 +0000 Subject: [PATCH 4/5] Fix newline --- Tests/VB/ExtensionMethodRefPropertyTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/VB/ExtensionMethodRefPropertyTests.cs b/Tests/VB/ExtensionMethodRefPropertyTests.cs index dc5d8d982..53db5e8e6 100644 --- a/Tests/VB/ExtensionMethodRefPropertyTests.cs +++ b/Tests/VB/ExtensionMethodRefPropertyTests.cs @@ -49,7 +49,6 @@ public void WithMethod() public static partial class MathEx { public static void NegEx(this ref int num) - { num = -num; } From 6847a135eef8f68fac2c394bfd371ac734dc5e73 Mon Sep 17 00:00:00 2001 From: Graham Date: Tue, 10 Mar 2026 23:43:16 +0000 Subject: [PATCH 5/5] Dedupe logic --- .../CSharp/NameExpressionNodeVisitor.cs | 19 ++++------------ .../CSharp/SemanticModelExtensions.cs | 22 ++++++++++++------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/CodeConverter/CSharp/NameExpressionNodeVisitor.cs b/CodeConverter/CSharp/NameExpressionNodeVisitor.cs index 513e3227f..666fbe4ed 100644 --- a/CodeConverter/CSharp/NameExpressionNodeVisitor.cs +++ b/CodeConverter/CSharp/NameExpressionNodeVisitor.cs @@ -87,7 +87,7 @@ public async Task ConvertMemberAccessExpressionAsync(VBasic.Sy if (IsSubPartOfConditionalAccess(node)) { return isDefaultProperty ? SyntaxFactory.ElementBindingExpression() : await AdjustForImplicitInvocationAsync(node, SyntaxFactory.MemberBindingExpression(simpleNameSyntax)); - } else if (node.IsParentKind(Microsoft.CodeAnalysis.VisualBasic.SyntaxKind.NamedFieldInitializer)) { + } else if (node.IsParentKind(VBasic.SyntaxKind.NamedFieldInitializer)) { return ValidSyntaxFactory.IdentifierName(_tempNameForAnonymousScope[node.Name.Identifier.Text].Peek().TempName); } left = _withBlockLhs.Peek(); @@ -292,21 +292,10 @@ private async Task ConvertInvocationAsync(VBSyntax.InvocationE var refKind = CommonConversions.GetCsRefKind(thisParam); bool requiresHoist = false; - SemanticModelExtensions.RefConversion refConversion = SemanticModelExtensions.RefConversion.Inline; + RefConversion refConversion = RefConversion.Inline; if (refKind != RefKind.None) { - var symbolInfo = _semanticModel.GetSymbolInfoInDocument(maes.Expression); - if (symbolInfo is IPropertySymbol { ReturnsByRef: false, ReturnsByRefReadonly: false } propertySymbol) { - refConversion = propertySymbol.IsReadOnly ? SemanticModelExtensions.RefConversion.PreAssigment : SemanticModelExtensions.RefConversion.PreAndPostAssignment; - } else if (symbolInfo is IFieldSymbol { IsConst: true } or ILocalSymbol { IsConst: true }) { - refConversion = SemanticModelExtensions.RefConversion.PreAssigment; - } else if (symbolInfo is IMethodSymbol { ReturnsByRef: false, ReturnsByRefReadonly: false }) { - refConversion = SemanticModelExtensions.RefConversion.PreAssigment; - } else { - var typeInfo = _semanticModel.GetTypeInfo(maes.Expression); - bool isTypeMismatch = typeInfo.Type == null || !typeInfo.Type.Equals(typeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability); - if (isTypeMismatch) refConversion = SemanticModelExtensions.RefConversion.PreAndPostAssignment; - } - requiresHoist = refConversion != SemanticModelExtensions.RefConversion.Inline; + refConversion = _semanticModel.GetRefConversionForExpression(maes.Expression); + requiresHoist = refConversion != RefConversion.Inline; } if (requiresStaticInvocation || requiresHoist) { diff --git a/CodeConverter/CSharp/SemanticModelExtensions.cs b/CodeConverter/CSharp/SemanticModelExtensions.cs index 238b001bc..669641a7a 100644 --- a/CodeConverter/CSharp/SemanticModelExtensions.cs +++ b/CodeConverter/CSharp/SemanticModelExtensions.cs @@ -70,11 +70,15 @@ public static RefConversion NeedsVariableForArgument(this SemanticModel semantic if (!(node is VBSyntax.SimpleArgumentSyntax sas) || sas is { Expression: VBSyntax.ParenthesizedExpressionSyntax }) return RefConversion.PreAssigment; var expression = sas.Expression; - return GetRefConversion(expression); + return semanticModel.GetRefConversionForExpression(expression); - RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression) + } + + public static RefConversion GetRefConversionForExpression(this SemanticModel semanticModel, VBasic.Syntax.ExpressionSyntax expression) + { + RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expr) { - var symbolInfo = semanticModel.GetSymbolInfoInDocument(expression); + var symbolInfo = semanticModel.GetSymbolInfoInDocument(expr); if (symbolInfo is IPropertySymbol { ReturnsByRef: false, ReturnsByRefReadonly: false } propertySymbol) { // a property in VB.NET code can be ReturnsByRef if it's defined in a C# assembly the VB.NET code references return propertySymbol.IsReadOnly ? RefConversion.PreAssigment : RefConversion.PreAndPostAssignment; @@ -87,10 +91,10 @@ RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression) if (DeclaredInUsing(symbolInfo)) return RefConversion.PreAssigment; - if (expression is VBasic.Syntax.IdentifierNameSyntax || expression is VBSyntax.MemberAccessExpressionSyntax || - IsRefArrayAcces(expression)) { + if (expr is VBasic.Syntax.IdentifierNameSyntax || expr is VBSyntax.MemberAccessExpressionSyntax || + IsRefArrayAcces(expr)) { - var typeInfo = semanticModel.GetTypeInfo(expression); + var typeInfo = semanticModel.GetTypeInfo(expr); bool isTypeMismatch = typeInfo.Type == null || !typeInfo.Type.Equals(typeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability); if (isTypeMismatch) { @@ -103,9 +107,9 @@ RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression) return RefConversion.PreAssigment; } - bool IsRefArrayAcces(VBSyntax.ExpressionSyntax expression) + bool IsRefArrayAcces(VBSyntax.ExpressionSyntax expr) { - if (!(expression is VBSyntax.InvocationExpressionSyntax ies)) return false; + if (!(expr is VBSyntax.InvocationExpressionSyntax ies)) return false; var op = semanticModel.GetOperation(ies); return (op.IsArrayElementAccess() || IsReturnsByRefPropertyElementAccess(op)) && GetRefConversion(ies.Expression) == RefConversion.Inline; @@ -117,6 +121,8 @@ static bool IsReturnsByRefPropertyElementAccess(IOperation op) && (prop.ReturnsByRef || prop.ReturnsByRefReadonly); } } + + return GetRefConversion((VBSyntax.ExpressionSyntax)expression); } private static bool DeclaredInUsing(ISymbol symbolInfo)