Skip to content

Commit 2df8e04

Browse files
GrahamTheCoderGrahamTheCoder
authored
Merge pull request #1227 from GrahamTheCoder/fix-1158-ref-extension-property-5310240367378084052
Fix 1158 ref extension property
2 parents 708eec7 + 6847a13 commit 2df8e04

File tree

4 files changed

+111
-22
lines changed

4 files changed

+111
-22
lines changed

‎CodeConverter/CSharp/ArgumentConverter.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public async Task<CSharpSyntaxNode> ConvertSimpleArgumentAsync(VBSyntax.SimpleAr
3535
var refType = CommonConversions.GetRefConversionType(node, argList, possibleParameters.Value, out var argName, out var refKind);
3636
token = CommonConversions.GetRefToken(refKind);
3737
if (refType != SemanticModelExtensions.RefConversion.Inline) {
38-
convertedArgExpression = HoistByRefDeclaration(node, convertedArgExpression, refType, argName, refKind);
38+
convertedArgExpression = HoistByRefDeclaration(node.Expression, convertedArgExpression, refType, argName, refKind);
3939
} else {
4040
convertedArgExpression = typeConversionAnalyzer.AddExplicitConversion(node.Expression, convertedArgExpression, defaultToCast: refKind != RefKind.None);
4141
}
@@ -119,11 +119,11 @@ CSSyntax.ArgumentSyntax ConvertOmittedArgument(IParameterSymbol parameter)
119119
}
120120

121121

122-
private CSSyntax.ExpressionSyntax HoistByRefDeclaration(VBSyntax.SimpleArgumentSyntax node, CSSyntax.ExpressionSyntax refLValue, SemanticModelExtensions.RefConversion refType, string argName, RefKind refKind)
122+
internal CSSyntax.ExpressionSyntax HoistByRefDeclaration(VBSyntax.ExpressionSyntax node, CSSyntax.ExpressionSyntax refLValue, SemanticModelExtensions.RefConversion refType, string argName, RefKind refKind)
123123
{
124124
string prefix = $"arg{argName}";
125-
var expressionTypeInfo = _semanticModel.GetTypeInfo(node.Expression);
126-
bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability) == true && !CommonConversions.ShouldPreferExplicitType(node.Expression, expressionTypeInfo.ConvertedType, out var _);
125+
var expressionTypeInfo = _semanticModel.GetTypeInfo(node);
126+
bool useVar = expressionTypeInfo.Type?.Equals(expressionTypeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability) == true && !CommonConversions.ShouldPreferExplicitType(node, expressionTypeInfo.ConvertedType, out var _);
127127
var typeSyntax = CommonConversions.GetTypeSyntax(expressionTypeInfo.ConvertedType, useVar);
128128

129129
if (refLValue is CSSyntax.ElementAccessExpressionSyntax eae) {
@@ -132,12 +132,12 @@ private CSSyntax.ExpressionSyntax HoistByRefDeclaration(VBSyntax.SimpleArgumentS
132132
refLValue = eae.WithExpression(tmpContainer.IdentifierName);
133133
}
134134

135-
var withCast = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, refLValue, defaultToCast: refKind != RefKind.None);
135+
var withCast = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node, refLValue, defaultToCast: refKind != RefKind.None);
136136

137137
var local = _typeContext.PerScopeState.Hoist(new AdditionalDeclaration(prefix, withCast, typeSyntax));
138138

139139
if (refType == SemanticModelExtensions.RefConversion.PreAndPostAssignment) {
140-
var convertedLocalIdentifier = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node.Expression, local.IdentifierName, forceSourceType: expressionTypeInfo.ConvertedType, forceTargetType: expressionTypeInfo.Type);
140+
var convertedLocalIdentifier = CommonConversions.TypeConversionAnalyzer.AddExplicitConversion(node, local.IdentifierName, forceSourceType: expressionTypeInfo.ConvertedType, forceTargetType: expressionTypeInfo.Type);
141141
_typeContext.PerScopeState.Hoist(new AdditionalAssignment(refLValue, convertedLocalIdentifier));
142142
}
143143

‎CodeConverter/CSharp/NameExpressionNodeVisitor.cs‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public async Task<CSharpSyntaxNode> ConvertMemberAccessExpressionAsync(VBasic.Sy
8787
if (IsSubPartOfConditionalAccess(node)) {
8888
return isDefaultProperty ? SyntaxFactory.ElementBindingExpression()
8989
: await AdjustForImplicitInvocationAsync(node, SyntaxFactory.MemberBindingExpression(simpleNameSyntax));
90-
} else if (node.IsParentKind(Microsoft.CodeAnalysis.VisualBasic.SyntaxKind.NamedFieldInitializer)) {
90+
} else if (node.IsParentKind(VBasic.SyntaxKind.NamedFieldInitializer)) {
9191
return ValidSyntaxFactory.IdentifierName(_tempNameForAnonymousScope[node.Name.Identifier.Text].Peek().TempName);
9292
}
9393
left = _withBlockLhs.Peek();
@@ -285,14 +285,36 @@ private async Task<ExpressionSyntax> ConvertInvocationAsync(VBSyntax.InvocationE
285285
}
286286

287287
if (invocationSymbol.IsReducedExtension() && invocationSymbol is IMethodSymbol { ReducedFrom: { Parameters: var parameters } } &&
288-
!parameters.FirstOrDefault().ValidCSharpExtensionMethodParameter() &&
289288
node.Expression is VBSyntax.MemberAccessExpressionSyntax maes) {
290-
var thisArgExpression = await maes.Expression.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);
291-
var thisArg = SyntaxFactory.Argument(thisArgExpression).WithRefKindKeyword(CommonConversions.GetRefToken(RefKind.Ref));
292-
convertedArgumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(convertedArgumentList.Arguments.Prepend(thisArg)));
293-
var containingType = (ExpressionSyntax)CommonConversions.CsSyntaxGenerator.TypeExpression(invocationSymbol.ContainingType);
294-
convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, containingType,
295-
ValidSyntaxFactory.IdentifierName((invocationSymbol.Name)));
289+
290+
var thisParam = parameters.FirstOrDefault();
291+
bool requiresStaticInvocation = !thisParam.ValidCSharpExtensionMethodParameter();
292+
var refKind = CommonConversions.GetCsRefKind(thisParam);
293+
294+
bool requiresHoist = false;
295+
RefConversion refConversion = RefConversion.Inline;
296+
if (refKind != RefKind.None) {
297+
refConversion = _semanticModel.GetRefConversionForExpression(maes.Expression);
298+
requiresHoist = refConversion != RefConversion.Inline;
299+
}
300+
301+
if (requiresStaticInvocation || requiresHoist) {
302+
var thisArgExpression = await maes.Expression.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor);
303+
304+
if (requiresHoist) {
305+
thisArgExpression = _argumentConverter.HoistByRefDeclaration(maes.Expression, thisArgExpression, refConversion, thisParam.Name, refKind);
306+
}
307+
308+
if (requiresStaticInvocation) {
309+
var thisArg = SyntaxFactory.Argument(thisArgExpression).WithRefKindKeyword(CommonConversions.GetRefToken(refKind));
310+
convertedArgumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(convertedArgumentList.Arguments.Prepend(thisArg)));
311+
var containingType = (ExpressionSyntax)CommonConversions.CsSyntaxGenerator.TypeExpression(invocationSymbol.ContainingType);
312+
convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, containingType,
313+
ValidSyntaxFactory.IdentifierName((invocationSymbol.Name)));
314+
} else {
315+
convertedExpression = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, thisArgExpression, ValidSyntaxFactory.IdentifierName((invocationSymbol.Name)));
316+
}
317+
}
296318
}
297319

298320
if (invocationSymbol is IMethodSymbol m && convertedExpression is LambdaExpressionSyntax) {

‎CodeConverter/CSharp/SemanticModelExtensions.cs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,15 @@ public static RefConversion NeedsVariableForArgument(this SemanticModel semantic
7070
if (!(node is VBSyntax.SimpleArgumentSyntax sas) || sas is { Expression: VBSyntax.ParenthesizedExpressionSyntax }) return RefConversion.PreAssigment;
7171
var expression = sas.Expression;
7272

73-
return GetRefConversion(expression);
73+
return semanticModel.GetRefConversionForExpression(expression);
7474

75-
RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression)
75+
}
76+
77+
public static RefConversion GetRefConversionForExpression(this SemanticModel semanticModel, VBasic.Syntax.ExpressionSyntax expression)
78+
{
79+
RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expr)
7680
{
77-
var symbolInfo = semanticModel.GetSymbolInfoInDocument<ISymbol>(expression);
81+
var symbolInfo = semanticModel.GetSymbolInfoInDocument<ISymbol>(expr);
7882
if (symbolInfo is IPropertySymbol { ReturnsByRef: false, ReturnsByRefReadonly: false } propertySymbol) {
7983
// a property in VB.NET code can be ReturnsByRef if it's defined in a C# assembly the VB.NET code references
8084
return propertySymbol.IsReadOnly ? RefConversion.PreAssigment : RefConversion.PreAndPostAssignment;
@@ -87,10 +91,10 @@ RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression)
8791

8892
if (DeclaredInUsing(symbolInfo)) return RefConversion.PreAssigment;
8993

90-
if (expression is VBasic.Syntax.IdentifierNameSyntax || expression is VBSyntax.MemberAccessExpressionSyntax ||
91-
IsRefArrayAcces(expression)) {
94+
if (expr is VBasic.Syntax.IdentifierNameSyntax || expr is VBSyntax.MemberAccessExpressionSyntax ||
95+
IsRefArrayAcces(expr)) {
9296

93-
var typeInfo = semanticModel.GetTypeInfo(expression);
97+
var typeInfo = semanticModel.GetTypeInfo(expr);
9498
bool isTypeMismatch = typeInfo.Type == null || !typeInfo.Type.Equals(typeInfo.ConvertedType, SymbolEqualityComparer.IncludeNullability);
9599

96100
if (isTypeMismatch) {
@@ -103,9 +107,9 @@ RefConversion GetRefConversion(VBSyntax.ExpressionSyntax expression)
103107
return RefConversion.PreAssigment;
104108
}
105109

106-
bool IsRefArrayAcces(VBSyntax.ExpressionSyntax expression)
110+
bool IsRefArrayAcces(VBSyntax.ExpressionSyntax expr)
107111
{
108-
if (!(expression is VBSyntax.InvocationExpressionSyntax ies)) return false;
112+
if (!(expr is VBSyntax.InvocationExpressionSyntax ies)) return false;
109113
var op = semanticModel.GetOperation(ies);
110114
return (op.IsArrayElementAccess() || IsReturnsByRefPropertyElementAccess(op))
111115
&& GetRefConversion(ies.Expression) == RefConversion.Inline;
@@ -117,6 +121,8 @@ static bool IsReturnsByRefPropertyElementAccess(IOperation op)
117121
&& (prop.ReturnsByRef || prop.ReturnsByRefReadonly);
118122
}
119123
}
124+
125+
return GetRefConversion((VBSyntax.ExpressionSyntax)expression);
120126
}
121127

122128
private static bool DeclaredInUsing(ISymbol symbolInfo)

‎Tests/VB/ExtensionMethodRefPropertyTests.cs‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
using System.Threading.Tasks;
2+
using Xunit;
3+
4+
namespace ICSharpCode.CodeConverter.Tests.VB;
5+
6+
public class ExtensionMethodRefPropertyTests : TestRunners.ConverterTestBase
7+
{
8+
[Fact]
9+
public async Task TestExtensionMethodRefPropertyAsync()
10+
{
11+
await TestConversionVisualBasicToCSharpAsync(
12+
@"Imports System.Runtime.CompilerServices
13+
Public Class ExtensionMethodsRefPropertyParameter
14+
Public Property Number As Integer = 3
15+
Public Sub WithExtensionMethod()
16+
Number.NegEx()
17+
End Sub
18+
Public Sub WithMethod()
19+
Neg(Number)
20+
End Sub
21+
End Class
22+
23+
Public Module MathEx
24+
<Extension()>
25+
Public Sub NegEx(ByRef num As Integer)
26+
num = -num
27+
End Sub
28+
Public Sub Neg(ByRef num As Integer)
29+
num = -num
30+
End Sub
31+
End Module", @"
32+
public partial class ExtensionMethodsRefPropertyParameter
33+
{
34+
public int Number { get; set; } = 3;
35+
public void WithExtensionMethod()
36+
{
37+
int argnum = Number;
38+
argnum.NegEx();
39+
Number = argnum;
40+
}
41+
public void WithMethod()
42+
{
43+
int argnum = Number;
44+
MathEx.Neg(ref argnum);
45+
Number = argnum;
46+
}
47+
}
48+
49+
public static partial class MathEx
50+
{
51+
public static void NegEx(this ref int num)
52+
{
53+
num = -num;
54+
}
55+
public static void Neg(ref int num)
56+
{
57+
num = -num;
58+
}
59+
}", incompatibleWithAutomatedCommentTesting: true);
60+
}
61+
}

0 commit comments

Comments
 (0)