icsharpcode / CodeConverter

Convert code from C# to VB.NET and vice versa using Roslyn
https://icsharpcode.github.io/CodeConverter/
MIT License
795 stars 207 forks source link

VB -> C#: comments can be misplaced in the `else if`-simplification #1098

Closed TymurGubayev closed 2 months ago

TymurGubayev commented 2 months ago

VB.Net input code

Sub S()
    If A Then
        'deal with A
    Else
        'here be cases B and C
        If B Then
            'handle B
        Else
            'handle C
        End If
        'cases B and C should be logically grouped together
    End If
End Sub

Erroneous output

public void S()
{
    if (A)
    {
    }
    // deal with A
    // here be cases B and C
    else if (B)
    {
    }
    // handle B
    else
    {
        // handle C
        // cases B and C should be logically grouped together
    }
}

Expected output

if (A)
{
    // deal with A
}
else
{
    // here be cases B and C
    if (B)
    {
        // handle B
    }
    else
    {
        // handle C
    }
    // cases B and C should be logically grouped together
}

This is somewhat similar to issue #1095: while the resulting code is correct, there is a subtle loss of information because of how the comments are moved around.

Details

GrahamTheCoder commented 2 months ago

Thanks for the report. There will be an extremely large number of bugs relating to comments/trivia with this level of subtlety. Unless you are intending to tackle any of them (in which case let me know and I can help get you started), I'd suggest adding any further examples together on the existing ticket for now. For full transparency, I currently have no plans to improve comment/trivia conversion accuracy since it's impossible to get exactly right and many cases are ambiguous as to whether the comment refers to the thing above or below.

TymurGubayev commented 2 months ago

this one is less bad than the #1095, and I kind of hoped the fix would be just to disable the simplification. I'll try to tackle both at some point, I just can't currently say when that would be (probably in a month or so).

GrahamTheCoder commented 2 months ago

Great, let me know when you're getting started if you need any tips.

In general, the thing to get right is the calls to WithSourceMapping (and WithoutSourceMapping). It's automatically called when the visitor is used, but the more custom a structural transformation becomes, the more manual calls it needs to get things in the right place (essentially the end of each line).

Sometimes just making use of the visitor more will improve things, and that would be preferable to littering the code with lots of logic around trivia.