paul1956 / CSharpToVB

New version of CSharpToVB converter
MIT License
25 stars 9 forks source link

Fail to assign the right local variable Split from #74 #75

Closed paul1956 closed 3 years ago

paul1956 commented 3 years ago

Fail to assign the right local variable:

Resto = (Total - PagoACta - Efectivo - Tarjeta - Descuento - Devolucion).ToString("#,##0.00");

Converts to:

' TODO: Check, VB does not directly support MemberAccess off a Conditional If Expression
Dim tempVar = Total - PagoACta - Efectivo - Tarjeta - Descuento - Devolucion
Resto = tempVar1.ToString("#,##0.00")

But it uses different variables (declared and used). And I can assign the value directly despite what is stated in the comment:

Resto = (Total - PagoACta - Efectivo - Tarjeta - Descuento - Devolucion).ToString("#,##0.00")

Hope its helps.

Guillermo

elGuille-info commented 3 years ago

I thought I had not put that other bug :-D

paul1956 commented 3 years ago

@elGuille-info Another good catch, I missed the simple case when I tried to handle Tuple deconstruction. This is fixed in 5.0.1.17. Fixing the unneeded tempVar assignment is very difficult given the way the logic works and I am afraid other things will break.

elGuille-info commented 3 years ago

No problem, as you say, it is better to leave that comment and that assignment, than to break it elsewhere ;-)

elGuille-info commented 3 years ago

Only as something strange.

public void AlgoMas()
{
    // falla
    Resto = (Efectivo - Tarjeta).ToString("#,##0.00");
    Resto = (Efectivo * Tarjeta).ToString("#,##0.00");

     // ok
    Resto = (Efectivo + Tarjeta).ToString("#,##0.00");
}

Adding there are no issue.

It's in the code the AddExpression check. I really don't know why, but... as a point ;-)

If TypeOf node.Parent Is CSS.MemberAccessExpressionSyntax OrElse TypeOf node.Parent Is CSS.ElementAccessExpressionSyntax Then
    If node.Expression.IsKind(CS.SyntaxKind.AddExpression) Then
        Return Factory.ParenthesizedExpression(openParenToken.WithConvertedTriviaFrom(node.OpenParenToken), expr, CloseParenT
    End If

Guillermo

paul1956 commented 3 years ago

@elGuille-info don't understand the issue. Do you have a full example what is broken and what you expect?

public void AlgoMas()
{
    // falla
    Resto = (Efectivo - Tarjeta).ToString("#,##0.00");
    Resto = (Efectivo * Tarjeta).ToString("#,##0.00");

     // ok
    Resto = (Efectivo + Tarjeta).ToString("#,##0.00");
}
elGuille-info commented 3 years ago

That the "temp" variable it is not created only when the "add" is used. But the names of the variables works fine.

paul1956 commented 3 years ago

You found where it needed to be fixed, I does special case the (). I have fixed -/ in 5.0.1.18. I might be able to delete the check for other operators but for now I would like to leave that in. Thanks again.