Roslyn: Refactoring to convert to static member and vice versa

[jnm2]

There is no 'make static' refactoring for methods, so I have to fix up the call sites by hand if I ever want to do that.

It also seems like there could be a code fix for "CS0176 Member cannot be accessed with an instance reference; qualify it with a type name instead."

@mavasani Cyrus said roslyn-analyzers has this and that I should ask you to port it.

[alrz]

Current make static is not that interesting. I think it only works if you missed static on a method in a static class?

what I find very helpful is to suggest to make static if there's no this reference.

I think there exists such feature in roslyn-analyzers. Not sure if you're referring to that one or something else.

[mavasani]

Yes, as @alrz mentioned, FxCopAnalyzers implements CA1822 to flag members that do not reference any instance members, but are not marked static. It also provides a code fix that makes the member static and also fixes up all call sites.

Our divide between whether something belongs to roslyn repo versus roslyn-analyzers repo is as follows:

• Roslyn repo:
  
  
  
  • Code style analyzers
  
  
  • Code fixes for compiler diagnostics
  
  
  • Pure code refactorings.
  
  

This would be VS-built in functionality. Code style analyzers are also available via CodeStyle NuGet package for build enforcement.

• Roslyn Analyzers repo:
  
  
  
  • Code quality analyzers performing semantic analysis, that are extremely likely to be enforced in build.
  
  

Available as CA rules in FxCopAnalyzers NuGet package.

So, if your request is for a code quality enforcement for marking members as static, whenever they could be, CA1822 already exists.
If your request is about any refactorings/code fixes that is just a helpful tool to mark something static without any semantic analysis, this should be written as a new refactoring in Roslyn repo.

[jnm2]

I'm not interested in enforcement or warnings, strictly refactoring help.

[mavasani]

I'm not interested in enforcement or warnings, strictly refactoring help.

Thanks @jnm2. We should be able to port MarkMembersAsStatic.Fixer.cs as a code refactoring to Roslyn repo. The core design/PM question is would it be too noisy to show this refactoring on every instance member?

[mavasani]

Marked this for next design review meeting so we can iron out when this refactoring should be shown.

[alrz]

would it be too noisy to show this refactoring on every instance member?

I don't think so. Rider has set this to "suggestion" by default and most of the time static was preferable for me. that way I can know if I want to move the method to another type or make local function, since it doesn't depend on the instance.

[alrz]

every instance member?

it wouldn't be on every instance member though, only those that doesn't have an implicit or explicit reference to this.

[mavasani]

@alrz - what you are requesting is already available as CA1822 in FxCopAnalyzers package - you can easily configure the severity for CA1822 as a Suggestion/Info and get the exact same behavior.
Duplicating the exact same functionality in Roslyn does not provide much benefit. Additionally, with .NET5 the CA analyzers will soon be shipping with the .NET SDK (majority either disabled by default or enabled as suggestions). CA1822 will ship as a suggestion as part of this, so you will get it by default for all .NET5 projects and can enable CA rules in non-.NET5 projects via some MSBuild property (still being designed).

To summarize, if the intent here is to see a suggestion/refactoring for Make Static only for instance members that can be safely made static, you can already do so currently by installing FxCopAnalyzers package, and soon this analyzer suggestion will be available with .NET SDK itself.
However, if the intent is to get a refactoring for cases where you are working towards converting an instance member into static member, where this instance member does reference other instance members and would need follow-up fixups to delete or refactor code that touches instance references to make sure it compiles, then this can be added as a new Roslyn refactoring.

[alrz]

you can already do so currently by installing FxCopAnalyzers package

For instance, when I work on roslyn, I don't really want to install packages to get a refactoring suggestion. I just want it to be available in the IDE - that shouldn't depend on the project settings.

Right now, it seems like VS is lacking this feature, while it is bound to some code style package. IMO we shouldn't offer a (lightweight and generally useful) productivity feature only as a nuget package, rather it should be bundled within the IDE for the maximum accessibility.

[mavasani]

when I work on roslyn

We should be able to enable CA1822 as a suggestion in Roslyn.sln. We just need to change the below to "Info": https://github.com/dotnet/roslyn/blob/d7d0c22e12f7684adf75b92e205b6b5bb7391a1d/eng/config/rulesets/Shipping.ruleset#L54

Right now, it seems like VS is lacking this feature, while it is bound to some code style package. IMO we shouldn't offer a (lightweight and generally useful) productivity feature only as a nuget package, rather it should be bundled within the IDE for the maximum accessibility.

This is potentially the core feature request that needs to be discussed at the design meeting.

[mavasani]

Right now, it seems like VS is lacking this feature, while it is bound to some code style package. IMO we shouldn't offer a (lightweight and generally useful) productivity feature only as a nuget package, rather it should be bundled within the IDE for the maximum accessibility.

Tagging @tmat as well. Given his recent refactoring in Roslyn to treat VS/host analyzer references as solution references that can execute in OOP, I think we can add a feature where analyzer NuGet packages are installed to a VS instance rather then a specific project to act similar to VSIX packages.

[JoeRobich]

Design Review

• This is not really a code-style analyzer. If it were to move it would be as a refactoring.
• Any project can reference the Roslyn-Analyzers today, but there is friction if it isn't enabled for a project.
• Could package and ship this as a VSIX.

Proposal:

• Keep the analyzer in Roslyn-Analyzers, but enable it in the Roslyn repo.
• Consider enhancing the Change Signature dialog with the ability to make members static.

[mavasani]

Keep the analyzer in Roslyn-Analyzers, but enable it in the Roslyn repo.

FYI: This part is complete. CA1822 is available as an IDE suggestion in entire Roslyn repo and enforced as a build warning in certain IDE layers (Workspaces, Features and CodeStyle).

[alrz]

Could package and ship this as a VSIX.

Is this part going to happen? I think that is exactly what I've been asking for in any project out there that is not roslyn.