Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VB -> C#: Method call with unknown class uses square brackets #266

Open
mrmonday opened this Issue Apr 5, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@mrmonday
Copy link
Contributor

mrmonday commented Apr 5, 2019

Input code

Public Class Class1
    Sub Foo()
        Bar(Nothing)
    End Sub

    Private Function Bar(x As SomeClass) As SomeClass
        Return x
    End Function

End Class

Erroneous output

public class Class1
{
    public void Foo()
    {
        Bar[null/* TODO Change to default(_) if this is not a reference type */];
    }

    private SomeClass Bar(SomeClass x)
    {
        return x;
    }
}

Expected output

public class Class1
{
    public void Foo()
    {
        Bar(null);
    }

    private SomeClass Bar(SomeClass x)
    {
        return x;
    }
}

Details

Product in use: VS extension

Version in use: 6.6.0

Even without SomeClass being available, it should still treat Bar() as a method call.

@GrahamTheCoder

This comment has been minimized.

Copy link
Member

GrahamTheCoder commented Apr 8, 2019

Yep, agreed. There is already some attempt to handle missing information for this, but this case should take precedence over the guesswork done at this point:

// Chances of having an unknown delegate stored as a field/local seem lower than having an unknown non-delegate type with an indexer stored, so for a standalone identifier err on the side of assuming it's an indexer

I'm also tempted to also change that "TODO" to just always say default({Type}) since it only appears when semantic info is missing

@mrmonday

This comment has been minimized.

Copy link
Contributor Author

mrmonday commented Apr 18, 2019

We could actually use C# 7.1 syntax for that, which allows for default, without a type.

@GrahamTheCoder

This comment has been minimized.

Copy link
Member

GrahamTheCoder commented Apr 19, 2019

As a very general rule I basically aim to support the current and previous major versions of things where it's convenient to do so. I think here it's pretty easily possible to stay backwards compatible, so probably worth doing since it's quite a common case.
Within this codebase of course we're free to use the latest stable version (i.e. whatever we can reasonably expect contributors to upgrade to).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.