Roslyn: [Analyzer Suggestion] VB - Add Parentheses to method calls

[Youssef1313]

Unlike C#, the VB language allows a method to be called without parentheses if it doesn't take parameters. However, it's always a good practice to add the parentheses to differentiate method calls from property accessing.

[jrmoreno1]

Why is that a good practice? I prefer the reverse.

[Youssef1313]

Why is that a good practice? I prefer the reverse.

Given the following code:

Dim x As Integer = someObj.SomeThing

If SomeThing is a method, it will be confusing and anyone who reads the code may think it's a property. On the other hand, when you say:

Dim x As Integer = someObj.SomeThing()

Things now are clear that SomeThing is a method.

I'm not sure why would you prefer not adding parentheses? Adding them makes things more clear.

Why would you prefer not adding parentheses? Adding them makes things more clear.

[Youssef1313]

I even noticed that VB allows for using parentheses with properties:

So, I suggest the analyzer also remove parentheses if the member is a property.

[jrmoreno1]

The get method is a method, you can even get it’s method info and call it directly, ditto for the set method. It’s created and generally hidden by the compiler, but is otherwise unremarkable.

Why do you think it makes it more clear?

[Youssef1313]

The getter and setter are generated by the compiler behind the scenes. I know that property is just a syntactic sugar for these methods. But still, adding parentheses to method calls makes it clear that it's a real method. and removing parentheses from properties makes it clear that it's a property (that is later will be a converted to methods by the compiler). The code will be easier to understand that way, and will be more consistent. I'll wait for others' opinions, and anyways if this is approved and implemented, you can disable it if you prefer not adding parentheses to methods and/or adding parentheses to properties.

[jrmoreno1]

@Youssef1313 : my point is that IF this was implemented by an analyzer (or the language was changed to require/forbid) parentheses, you would know that one was a method that the compiler will generate using a bit of sugar and the code you wrote, and one will be a method that the compiler generates using code you wrote. How does knowing that sugar was/was not used make things clearer or better for the programmer? It exposes an implementation detail, what I am trying to get at is what that implementation detail does for you? I am not arguing for or against this at the moment, I am trying to understand the point of what you are asking. For me, it makes the code clearer precisely because I am not exposed to the implementation detail. And AddressOf is clearer than () or not () at least I my opinion

[paul1956]

I assume this would be a VB .editorConfig option, so you can select Add/Remove/Do Nothing but this is going to cause a lot of issues with noise in source control. I don't understand the comment about AddressOf. Personally I don't see any benefit of adding () when it is not needed or why I care if something is a property or method except possibly to remind me to do Null checks (which already mostly has an analyzer). I would be more likely to support removing unnecessary ().

[CyrusNajmabadi]

I don't see any issue here. However, i think it would make sense to have two options, one for properties and one for methods. They would have two values: if default parens are preferred or should be eschewed. Both options should be off by default.

I imagine we might take a community PR on this. If you're interested, i can bring to design meeting to see if we'd take it.

[jrmoreno1]

@paul1956 : C# uses the lack of parentheses to indicate a method group for a method. Which, other than the fact that the language requires it, is the only useful use of parentheses for methods that I can think of, and I think VB’s keyword approach is better.

[Youssef1313]

I don't see any issue here. However, i think it would make sense to have two options, one for properties and one for methods. They would have two values: if default parens are preferred or should be eschewed. Both options should be off by default.

I imagine we might take a community PR on this. If you're interested, i can bring to design meeting to see if we'd take it.

Certainly, I'm interested.

[CyrusNajmabadi]

Bringing to next meeting to get team check on if this would be something we'd take.

[CyrusNajmabadi]

Design review conclusion: We will take a community PR here.

Speclet:

1. There should be two editor config options to control this. Names TBD durign PR, but something akin to visual_basic_style_parentheses_for_invocations and visual_basic_style_parentheses_for_property_accesses.
2. The options should be disabled by default.
3. The invocation option should be true by default, the property-access option should be false by default.
4. An analyzer/fixer should be provided for this.
5. The fixer should support fix-all
6. The VB codestyle UI should be updated to include options for this, including preview examples.

[Youssef1313]

@CyrusNajmabadi Great! I'll try to work on this soon :-)

[NiKiZe]

Something I have observed (but not verified)
If you have a property with the same name as an extension, in which case will it call the property and in which case the extension? I would think that the extension would be used when using (), but maybe the property is always used, even with ()?

[paul1956]

Does this include option to match the .editorConfig option if it means removing the ()? If so the name of this suggestion/feature is confusing.

[Youssef1313]

@paul1956, There will be two different analyzers, one for methods and the other for properties. Each analyzer is configurable using editorconfig. The defaults are to include parentheses for methods, but not for properties. Regarding the name, it won't be ParenthesesStyle as this issue title. It'll be more descriptive.

[jrmoreno1]

@NiKiZe : by design, looking for an extension is the last thing that is done. It should never conflict with anything else.

[sharwell]

@CyrusNajmabadi I propose that in addition to https://github.com/dotnet/roslyn/issues/45170#issuecomment-644392752, the pretty-listing should apply this preference automatically if/when enabled.

[jmarolf]

Design Meeting Notes:

• Do not add to pretty-lister at this time. Wait for feedback

[Youssef1313]

@sharwell @jmarolf Is pretty-lister that formatter which runs on saving?
If so, it currently already handle half the scenario (or I'd say quarter):

https://github.com/dotnet/roslyn/pull/45223#issuecomment-699564406

[jmarolf]

@Youssef1313 Yes the pretty-lister formats the line on enter. The conclusion of the design meeting was that having this setting auto-applied by the pretty-lister would be annoying.

[Youssef1313]

@jmarolf It is already auto-applied by the pretty-lister. But not in all cases.

For example:

Public Module Module1
    Public Sub Main()
        Console.WriteLine // Press CTRL+S, the parentheses are added automatically.
    End Sub
End Module

In that case, if the analyzer is configured with visual_basic_style_parentheses_for_invocations = false, it will go into this circle:

1. Pretty-lister adds parentheses.
2. Analyzer wants to remove parentheses.
3. User applies the codefix to remove parentheses.
4. Go to 1