Skip to content

Commit ba3f108

Browse files
GrahamTheCoderGrahamTheCoder
committed
Optional parameters in parameterized properties - fixes #597 
1 parent a392714 commit ba3f108

3 files changed

Lines changed: 121 additions & 33 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
99

1010

1111
### VB -> C#
12-
12+
* Convert parameterized properties with optional parameters [#597](https://github.com/icsharpcode/CodeConverter/issues/597)
1313

1414
### C# -> VB
1515

‎CodeConverter/CSharp/ExpressionNodeVisitor.cs‎

Lines changed: 68 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ public override async Task<CSharpSyntaxNode> VisitMemberAccessExpression(VBasic.
361361
if (left == null) {
362362
if (IsSubPartOfConditionalAccess(node)) {
363363
return isDefaultProperty ? SyntaxFactory.ElementBindingExpression()
364-
: AddEmptyArgumentListIfImplicit(node, SyntaxFactory.MemberBindingExpression(simpleNameSyntax));
364+
: await AdjustForImplicitInvocationAsync(node, SyntaxFactory.MemberBindingExpression(simpleNameSyntax));
365365
}
366366
left = _withBlockLhs.Peek();
367367
}
@@ -381,7 +381,7 @@ public override async Task<CSharpSyntaxNode> VisitMemberAccessExpression(VBasic.
381381
}
382382

383383
var memberAccessExpressionSyntax = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, left, simpleNameSyntax);
384-
return AddEmptyArgumentListIfImplicit(node, memberAccessExpressionSyntax);
384+
return await AdjustForImplicitInvocationAsync(node, memberAccessExpressionSyntax);
385385
}
386386

387387
public override async Task<CSharpSyntaxNode> VisitConditionalAccessExpression(VBasic.Syntax.ConditionalAccessExpressionSyntax node)
@@ -807,19 +807,9 @@ private async Task<CSharpSyntaxNode> ConvertInvocationAsync(VBSyntax.InvocationE
807807
return csEquivalent;
808808
}
809809

810-
var (overrideIdentifier, extraArg) =
811-
await CommonConversions.GetParameterizedPropertyAccessMethodAsync(operation);
812-
if (overrideIdentifier != null) {
813-
var expr = await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor);
814-
var idToken = expr.DescendantTokens().Last(t => t.IsKind(SyntaxKind.IdentifierToken));
815-
expr = ReplaceRightmostIdentifierText(expr, idToken, overrideIdentifier);
816-
817-
var args = await ConvertArgumentListOrEmptyAsync(node, node.ArgumentList);
818-
if (extraArg != null) {
819-
args = args.WithArguments(args.Arguments.Add(SyntaxFactory.Argument(extraArg)));
820-
}
821-
822-
return SyntaxFactory.InvocationExpression((ExpressionSyntax)expr, args);
810+
var expr = await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor);
811+
if (await TryConvertParameterizedPropertyAsync(operation, node, expr, node.ArgumentList) is {} invocation) {
812+
return invocation;
823813
}
824814
//TODO: Decide if the above override should be subject to the rest of this method's adjustments (probably)
825815

@@ -852,8 +842,6 @@ private async Task<CSharpSyntaxNode> ConvertInvocationAsync(VBSyntax.InvocationE
852842
var isElementAccess = operation.IsPropertyElementAccess() ||
853843
operation.IsArrayElementAccess() ||
854844
ProbablyNotAMethodCall(node, expressionSymbol, expressionReturnType);
855-
856-
var expr = await node.Expression.AcceptAsync(TriviaConvertingExpressionVisitor);
857845
return ((ExpressionSyntax)expr, isElementAccess);
858846
}
859847

@@ -872,6 +860,30 @@ async Task<CSharpSyntaxNode> CreateElementAccess()
872860
}
873861
}
874862

863+
private async Task<InvocationExpressionSyntax> TryConvertParameterizedPropertyAsync(IOperation operation,
864+
SyntaxNode node, CSharpSyntaxNode identifier,
865+
VBSyntax.ArgumentListSyntax optionalArgumentList = null)
866+
{
867+
var (overrideIdentifier, extraArg) =
868+
await CommonConversions.GetParameterizedPropertyAccessMethodAsync(operation);
869+
if (overrideIdentifier != null)
870+
{
871+
var expr = identifier;
872+
var idToken = expr.DescendantTokens().Last(t => t.IsKind(SyntaxKind.IdentifierToken));
873+
expr = ReplaceRightmostIdentifierText(expr, idToken, overrideIdentifier);
874+
875+
var args = await ConvertArgumentListOrEmptyAsync(node, optionalArgumentList);
876+
if (extraArg != null)
877+
{
878+
args = args.WithArguments(args.Arguments.Add(SyntaxFactory.Argument(extraArg)));
879+
}
880+
881+
return SyntaxFactory.InvocationExpression((ExpressionSyntax)expr, args);
882+
}
883+
884+
return null;
885+
}
886+
875887

876888
/// <summary>
877889
/// The VB compiler actually just hoists the conditions within the same method, but that leads to the original logic looking very different.
@@ -984,7 +996,8 @@ public override async Task<CSharpSyntaxNode> VisitParameter(VBSyntax.ParameterSy
984996
}
985997

986998
EqualsValueClauseSyntax @default = null;
987-
if (node.Default != null) {
999+
// Parameterized properties get compiled/converted to a methd with non-optional parameters
1000+
if (node.Default != null && node.Parent?.Parent?.IsKind(VBasic.SyntaxKind.PropertyStatement) != true) {
9881001
var defaultValue = node.Default.Value.SkipIntoParens();
9891002
if (_semanticModel.GetTypeInfo(defaultValue).Type?.SpecialType == SpecialType.System_DateTime) {
9901003
var constant = _semanticModel.GetConstantValue(defaultValue);
@@ -1120,8 +1133,8 @@ public override async Task<CSharpSyntaxNode> VisitIdentifierName(VBasic.Syntax.I
11201133
var qualifiedIdentifier = requiresQualification
11211134
? QualifyNode(node, identifier) : identifier;
11221135

1123-
var sym = GetSymbolInfoInDocument<ILocalSymbol>(node);
1124-
if (sym != null) {
1136+
var sym = GetSymbolInfoInDocument<ISymbol>(node);
1137+
if (sym is ILocalSymbol) {
11251138
var vbMethodBlock = node.Ancestors().OfType<VBasic.Syntax.MethodBlockBaseSyntax>().FirstOrDefault();
11261139
if (vbMethodBlock != null &&
11271140
vbMethodBlock.MustReturn() &&
@@ -1134,9 +1147,22 @@ public override async Task<CSharpSyntaxNode> VisitIdentifierName(VBasic.Syntax.I
11341147
}
11351148
}
11361149

1137-
//PERF: AddEmptyArgumentListIfImplicit calls GetOperation, which is expensive, avoid it when it's easy to do so
1138-
var couldBeMethodCall = !(node.Parent is VBSyntax.QualifiedNameSyntax);
1139-
return couldBeMethodCall ? AddEmptyArgumentListIfImplicit(node, qualifiedIdentifier) : qualifiedIdentifier;
1150+
return await AdjustForImplicitInvocationAsync(node, qualifiedIdentifier);
1151+
}
1152+
1153+
private async Task<CSharpSyntaxNode> AdjustForImplicitInvocationAsync(SyntaxNode node, ExpressionSyntax qualifiedIdentifier)
1154+
{
1155+
//PERF: Avoid calling expensive GetOperation when it's easy
1156+
bool nonExecutableNode = node.IsParentKind(VBasic.SyntaxKind.QualifiedName);
1157+
if (nonExecutableNode || _semanticModel.SyntaxTree != node.SyntaxTree) return qualifiedIdentifier;
1158+
1159+
if (await TryConvertParameterizedPropertyAsync(_semanticModel.GetOperation(node), node, qualifiedIdentifier) is {}
1160+
invocation)
1161+
{
1162+
return invocation;
1163+
}
1164+
1165+
return AddEmptyArgumentListIfImplicit(node, qualifiedIdentifier);
11401166
}
11411167

11421168
public override async Task<CSharpSyntaxNode> VisitQualifiedName(VBasic.Syntax.QualifiedNameSyntax node)
@@ -1172,7 +1198,7 @@ public override async Task<CSharpSyntaxNode> VisitGenericName(VBasic.Syntax.Gene
11721198
{
11731199
var symbol = GetSymbolInfoInDocument<ISymbol>(node);
11741200
var genericNameSyntax = await GenericNameAccountingForReducedParametersAsync(node, symbol);
1175-
return AddEmptyArgumentListIfImplicit(node, genericNameSyntax);
1201+
return await AdjustForImplicitInvocationAsync(node, genericNameSyntax);
11761202
}
11771203

11781204
/// <summary>
@@ -1329,13 +1355,21 @@ private IEnumerable<ArgumentSyntax> GetAdditionalRequiredArgs(ISymbol invocation
13291355
var namedArgNames = new HashSet<string>(existingArgs.OfType<VBasic.Syntax.SimpleArgumentSyntax>().Where(a => a.IsNamed).Select(a => a.NameColonEquals.Name.Identifier.Text), StringComparer.OrdinalIgnoreCase);
13301356
if (invocationSymbol != null) {
13311357
var requiredInCs = invocationSymbol.GetParameters()
1332-
.Where((p, i) => p.HasExplicitDefaultValue && p.RefKind != RefKind.None && i >= vbPositionalArgs && !namedArgNames.Contains(p.Name));
1333-
return requiredInCs.Select(CreateOptionalRefArg);
1358+
.Select((p, i) => CreateExtraArgOrNull(invocationSymbol, p, i, vbPositionalArgs, namedArgNames));
1359+
return requiredInCs.Where(x => x != null);
13341360
}
13351361

13361362
return Enumerable.Empty<ArgumentSyntax>();
13371363
}
13381364

1365+
private ArgumentSyntax CreateExtraArgOrNull(ISymbol invocationSymbol, IParameterSymbol p, int i, int vbPositionalArgs, HashSet<string> namedArgNames)
1366+
{
1367+
if (i < vbPositionalArgs || namedArgNames.Contains(p.Name) || !p.HasExplicitDefaultValue) return null;
1368+
if (p.RefKind != RefKind.None) return CreateOptionalRefArg(p);
1369+
if (invocationSymbol is IPropertySymbol) return SyntaxFactory.Argument(CommonConversions.Literal(p.ExplicitDefaultValue));
1370+
return null;
1371+
}
1372+
13391373
private ArgumentSyntax CreateOptionalRefArg(IParameterSymbol p)
13401374
{
13411375
string prefix = $"arg{p.Name}";
@@ -1451,13 +1485,13 @@ private bool ProbablyNotAMethodCall(VBasic.Syntax.InvocationExpressionSyntax nod
14511485

14521486
private async Task<ArgumentListSyntax> ConvertArgumentListOrEmptyAsync(SyntaxNode node, VBSyntax.ArgumentListSyntax argumentList)
14531487
{
1454-
return (ArgumentListSyntax)await argumentList.AcceptAsync(TriviaConvertingExpressionVisitor) ?? CreateArgList(_semanticModel.GetSymbolInfo(node).Symbol as IMethodSymbol);
1488+
return (ArgumentListSyntax)await argumentList.AcceptAsync(TriviaConvertingExpressionVisitor) ?? CreateArgList(_semanticModel.GetSymbolInfo(node).Symbol);
14551489
}
14561490

1457-
private ArgumentListSyntax CreateArgList(IMethodSymbol methodSymbol)
1491+
private ArgumentListSyntax CreateArgList(ISymbol invocationSymbol)
14581492
{
14591493
return SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(
1460-
GetAdditionalRequiredArgs(methodSymbol, Array.Empty<VBSyntax.ArgumentSyntax>()))
1494+
GetAdditionalRequiredArgs(invocationSymbol, Array.Empty<VBSyntax.ArgumentSyntax>()))
14611495
);
14621496
}
14631497

@@ -1477,9 +1511,11 @@ private async Task<CSharpSyntaxNode> SubstituteVisualBasicMethodOrNullAsync(VBas
14771511
private CSharpSyntaxNode AddEmptyArgumentListIfImplicit(SyntaxNode node, ExpressionSyntax id)
14781512
{
14791513
if (_semanticModel.SyntaxTree != node.SyntaxTree) return id;
1480-
return _semanticModel.GetOperation(node) is IInvocationOperation invocation
1481-
? SyntaxFactory.InvocationExpression(id, CreateArgList(invocation.TargetMethod))
1482-
: id;
1514+
return _semanticModel.GetOperation(node) switch {
1515+
IInvocationOperation invocation => SyntaxFactory.InvocationExpression(id, CreateArgList(invocation.TargetMethod)),
1516+
IPropertyReferenceOperation propReference when propReference.Property.Parameters.Any() => SyntaxFactory.InvocationExpression(id, CreateArgList(propReference.Property)),
1517+
_ => id
1518+
};
14831519
}
14841520

14851521
/// <summary>

‎Tests/CSharp/MemberTests/PropertyMemberTests.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,58 @@ public override string ToString()
128128
}", hasLineCommentConversionIssue: true);//TODO: Improve comment mapping for parameterized property
129129
}
130130

131+
[Fact]
132+
public async Task TestOptionalParameterizedPropertyAsync()
133+
{
134+
await TestConversionVisualBasicToCSharpAsync(
135+
@"Class TestClass
136+
Public Property FirstName As String
137+
Public Property LastName As String
138+
139+
Public Property FullName(Optional ByVal isFirst As Boolean = False) As String
140+
Get
141+
Return FirstName & "" "" & LastName
142+
End Get
143+
144+
Friend Set
145+
If isFirst Then FirstName = Value
146+
End Set
147+
End Property
148+
149+
Public Overrides Function ToString() As String
150+
FullName(True) = ""hello2""
151+
FullName() = ""hello3""
152+
FullName = ""hello4""
153+
Return FullName
154+
End Function
155+
End Class", @"
156+
internal partial class TestClass
157+
{
158+
public string FirstName { get; set; }
159+
public string LastName { get; set; }
160+
161+
public string get_FullName(bool isFirst)
162+
{
163+
return FirstName + "" "" + LastName;
164+
}
165+
166+
internal void set_FullName(bool isFirst, string value)
167+
{
168+
if (isFirst)
169+
FirstName = value;
170+
}
171+
172+
public override string ToString()
173+
{
174+
set_FullName(true, ""hello2"");
175+
set_FullName(false, ""hello3"");
176+
set_FullName(false, ""hello4"");
177+
return get_FullName(false);
178+
}
179+
}",
180+
hasLineCommentConversionIssue: true);//TODO: Improve comment mapping for parameterized property
181+
}
182+
131183
[Fact]
132184
public async Task TestParameterizedPropertyAndGenericInvocationAndEnumEdgeCasesAsync()
133185
{

0 commit comments

Comments
 (0)