VB -> C#: Null coalesce missing brackets

[jmeijrink]

Input code

Public Class VisualBasicClass
   Public Sub New()
       Dim y As String = If(("xxx")?.InnerText, String.Empty).ToUpper()
   End Sub
End Class

Erroneous output

using System;

public partial class VisualBasicClass
{
    public VisualBasicClass()
    {
        string y = "xxx"?.InnerText ?? string.Empty.ToUpper();
    }
}

Expected output

using System;

public partial class VisualBasicClass
{
    public VisualBasicClass()
    {
        string y = ("xxx"?.InnerText ?? string.Empty).ToUpper();
    }
}

Details

• Because the parentheses are missing in the Erroneous output the value of .InnerText is not uppercased. But left in lowercase.
• Current online code converter at https://codeconverter.icsharpcode.net/ can be used to reproduce this issue.

[GrahamTheCoder]

Should be a very straightforward fix. Let me know if you're interested in having a go and want some hints.

[SuperRembo]

Actually for the ternary operator parenthesis are added when needed, but not for the null-coalescing operator.

I've added a test to describe the issue, but don't have a clue how to fix it. It would be nice to get some hints.

[GrahamTheCoder]

Nice, always good to start with the test. OK, so the first thing is to figure out which syntax type the expression is. SharpLab has a nice syntax tree viewer:

Then you'll usually find there's a "Visit" method for that syntax construct (in this case within the CSharp namespace since that's the target language)

CodeConverter/CodeConverter/CSharp/ExpressionNodeVisitor.cs

Lines 612 to 619 in 1bb96ee

	public override async Task<CSharpSyntaxNode> VisitBinaryConditionalExpression(VBasic.Syntax.BinaryConditionalExpressionSyntax node)
	{
	return SyntaxFactory.BinaryExpression(
	SyntaxKind.CoalesceExpression,
	await node.FirstExpression.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor),
	await node.SecondExpression.AcceptAsync<ExpressionSyntax>(TriviaConvertingExpressionVisitor)
	);
	}

So if you breakpoint that, your test should hit it

You may notice that right below that method is the ternary case which you mentioned works, so see if you can make this one work using a similar mechanism.

[SuperRembo]

I applied the same mechanism as used for the ternary operator. That makes the test pass.

I'm not sure what the purpose of node.Parent.IsKind(VBasic.SyntaxKind.Interpolation) is. It seems to be intended for when the operator is used in string interpolation, but the tests pass without this check.

[SuperRembo]

In other words: would node.ParenthesizeIfPrecedenceCouldChange(expr) be sufficient?

[GrahamTheCoder]

Yeah sounds right. I think it's a special case: within string interpolation, ternary statements need parentheses in c# to avoid a compile error. Should have really had a comment.

Could write a test to check this works within string interpolation to be sure.

[GrahamTheCoder]

Ah yes, just saw you did a test for it, nice