Roslyn: [New Analyzer Suggestion] Remove Unnecessary ByVal

[Youssef1313]

The ByVal modifier is the default and unnecessary to include.

A new analyzer to detect the usages of ByVal in Visual Basic and a CodeFix to remove it will help cleaning the code and make it less noisy.

Example:

Given the following code:

Public Sub SayHello(ByVal name As String)
    Console.WriteLine($"Hello, {name}!")
End Sub

The analyzer should report unnecessary ByVal and the code fix turns the code to:

Public Sub SayHello(name As String)
    Console.WriteLine($"Hello, {name}!")
End Sub

The removal of ByVal, as mentioned before, makes the code much less noiser.
It also helps identifying ByRef easily. For example, it's easy to spot that param3 is ByRef in the following snippet:

Public Sub DoSomething(param1, param2, ByRef param3, param4, param5)
    ' Do something.
End Sub

But it's harder to spot that in the following snippet.

Public Sub DoSomething(ByVal param1, ByVal param2, ByRef param3, ByVal param4, ByVal param5)
    ' Do something.
End Sub

[Youssef1313]

If this suggestion is accepted, I'd like to try implementing it. Is there a guide on that?

[mavasani]

This is more of a code style analyzer, that belongs to Roslyn repo. This repo targets semantic/code quality analyzers. I will move it over.

[CyrusNajmabadi]

Design meeting conclusion:

We would take a community provided analyzer/fixer here. Important design points:

1. there would be no editorconfig setting for this.
2. the analyzer should be set to ReportDiagnostic.Hidden mode (i.e. not 'warning/error/info').
3. the analyzer should mark the ByVal as unnecessary so it gets faded out.
4. the analyzer should support fixall (trivial with roslyn fixall infrastructure).

[Youssef1313]

@CyrusNajmabadi That was too fast!

Can you guide me to a similar analyzer (Any analyzer that is 'refactoring only') so I can see how it's written and try to write this one?

[CyrusNajmabadi]

"refactoring only" just means that this analyzer will report things with the value ReportDiagnostic.Hidden :) That just means that the lightbulb will show, but no squiggles will be included.

[jrmoreno1]

Reshaper has a similar feature, which misses the ByVal when it it is preceded by Optional.

[Youssef1313]

The issue wasn't closed for some reason after the PR is merged, although the Fixes #IssueNumber exist.
Manually closing this.

[jrmoreno1]

PR for this is https://github.com/dotnet/roslyn/pull/44824