Replace var with explicit type

[tugberkugurlu]

When var is used in some cases, it should be suggested to replace it with the explicit type. For example:

var foo = bar.GetSomething();

It should be suggested here that var can be replaced with the explicit type. After the conversion it should look like this:

Bar foo = bar.GetSomething();

In some cases, this cannot be the case though. For example:

var foo = new { Foo = "Foo", Bar = "Bar" };

As this is an anonymous type, it shouldn't suggest a code fix for this. The warning also can be suppressed for new statements. For example:

var foo = new Foo();

It's too obvious here what the type is and it's unnecessary to use the explicit type.

I'm not sure how it works but I'm happy to implement this and put it inside the analyzers repository. I actually had this implemented before 😄 https://twitter.com/tourismgeek/status/454020174706798592 but I deleted the VM and its data files on Azure (forgot that I had anything useful there) and all code is basically gone now 😞

[tugberkugurlu]

I implemented this and put it under my account. Not sure how contributions work here but I'm happy to host it here.

[sharwell]

Well you didn't waste any time at all! Thanks for volunteering to help build this organization. I'm out right now, but I'll help you do the transfer later today plus make sure you still have all the permissions to work on it.

[tugberkugurlu]

Great! How do you intend the projects to stand out here?

• separate project and NuGet package per each diagnostic?
• bundled diagnostics into one project / nuget package?

[sharwell]

tl;dr If you author a package, you can bundle them however you want. But we do have some recommendations to which may help your users more easily incorporate your work into their projects.

As far as rules, we're currently very relaxed. Primary authors of individual packages are free to set their own licensing and distribution policies provided they are OSS licenses and do not interfere with other repositories in the organization.

However, I do have more specific recommendations. The key to remember is it's much easier to aggregate multiple individually packaged analyzers into a single package than it is to separate out analyzers that only appear within a larger package. For example, the following would be straightforward:

1. You create analyzer A.
2. I create analyzer B.
3. We decide A and B make sense together for many users, so we get together and release OurAnalyzers as a bundle containing both A and B. Users who only need A are still free to download A by itself.

In practice, I find that multiple diagnostics do make sense to be packaged together. Here are several examples of how I handled specific cases.

1. For my ANTLR analyzer (which isn't moved to this organization yet), several diagnostics are included because they represent different types of errors which can occur when working with ANTLR grammars. ANTLR users would have no reason to selectively enable these diagnostics because they have a virtual non-existent rate of false positives.
2. For my Default Value Diagnostic, several diagnostics are provided which are specific to users who define methods with default values.
3. StyleCop and FxCop rules implemented using the .NET Compiler Platform are a gray area; while it makes sense to implement these as a group because they are modeled after a previously defined group, not all of the rules are desired by everyone who might incorporate them.

[AdamSpeight2008]

They can be independent and DLLs incorporated into a single VSIX. If you look at my pull request you'll see how I combined both the C# and VB diagnostic + code-fixes into a single VSIX. Or we can maintain a VSIX repository and place them in there.

[tugberkugurlu]

@sharwell great info! thanks!

I do think that separate projects and VSIX/nuget packages for each domain (e.g. ASP.NET 5 analyzers, ASP.NET Web API analyzers, ANTLR analyzers, var analyzers, etc.). Possibly bundled packages/VSIX for the ones that make sense (e.g. language level analyzers could go under a bundled package which essentially refers to all other nuget packages)

Let me know how I can get my project here and I will modify it accordingly.

[sharwell]

@tugberkugurlu You can move the repository using the following steps.

1. Rename the repository to better reflect that the repository holds analyzers specific to var analysis (steps 1-4 on that page)
2. Transfer the repository to the DotNetAnalyzers organization (steps 1-5 on that page)

I'll approve the transfer and close this issue.

[tugberkugurlu]

@sharwell thanks, I'll complete the process very shortly.

Is there a defined standard on NuGet package names? Should the package names be prefixed with DotNetAnalyzers.? E.g.: DotNetAnalyzers.VarKeywordDiagnostic.

[sharwell]

At this time there is not, but it seems like VarUsageAnalyzer would be a good name for the repository. You can worry about refactoring the code within the repository later (if you want).

[tugberkugurlu]

@sharwell Getting this while trying to transfer the repo

[sharwell]

@tugberkugurlu Looks like it needs to go the other way around. Can you add me as a collaborator to that repository (on the left side of that screenshot)? It seems as though I start the transfer and you approve it, as opposed to the other way around like we were trying before.

[tugberkugurlu]

@sharwell added.

[MarkMichaelis]

IMPORTANT: This is a controversial rule. There are strongly opinionated folks that insist is should be the other way around. (In fact at the MVP Summit the opposite rule was written.) Please be sure to set this up as off by default in the ruleset.

[tugberkugurlu]

@MarkMichaelis IMO, it should be on by default because if you don't want the behavior, you wouldn't have this analyzer for your project. Am I missing something?

Or, should we make this a refactoring thing instead of an diagnostic analyzer?

[sharwell]

This proposal is now in progress in the following repository:
https://github.com/DotNetAnalyzers/VarUsageAnalyzer

[sharwell]

@tugberkugurlu If you find that you don't have permission to do something that you expected, let me know since blocking it was unintentional.

[tugberkugurlu]

@sharwell thanks! I'm not the repo admin I guess which should be fine for most of the cases.

As I'm not the repo admin, can you change the repo description to something more fitting for the context rather than the vague one I have put there: "Some Useful Analyzers for .NET Compiler Platform" 😄

[sharwell]

@tugberkugurlu Fixed (the admin part, not the description)

[MarkMichaelis]

@tugberkugurlu: Continued discussion analyzer (set) nuget/vsix with multiple analyzers #7