Workaround: Fixing analyzer build errors on VS2019 v16.9

[stebet]

Proposed Changes

VS 2019 automatically enables nullable checks and some analyzers for projects. To avoid builds failing when developing locally I disabled the .NET analyzers for the time being until they can be reviewed and fixed properly.

Would be nice if someone can confirm this behavior before and after: @bollhals @danielmarbach

Types of Changes

• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)

Checklist

• I have read the CONTRIBUTING.md document
• I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
• All tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• Any dependent changes have been merged and published in related repositories

[danielmarbach]

@bording has much more context in this area. I defer to him

[bollhals]

haven't updated yet, but I plan to, I can report back once I have.

How many failures does it raise? Maybe it's quicker to analyse and either fix or change the rule.

[stebet]

haven't updated yet, but I plan to, I can report back once I have.

How many failures does it raise? Maybe it's quicker to analyse and either fix or change the rule.

Almost 200. Ranging from implementing IDisposable properly to proper exception constructors etc.. Not an easy fix.

[stebet]

B.t.w, I'm not saying we shouldn't fix the issues. Just workaround it for now and create PRs later with proper fixes for them. These analysers are also automatically enabled on net5.0 projects, so it'd be a good preparation to fix them properly.

[bording]

This is the most extreme mode that the analyzers can be set to. Only a handful of rules are enabled by default, so this is opting in to all of them.

[stebet]

Thanks for this. I'll take a closer look tomorrow and see if I can get it back to the previous behaviour.

[bollhals]

If you change it to "Default" then only a hand full of warnings are left that you've already fixed anyway.

[bording]

This would a way to globally disable the analyzers, though since this project isn't targeting net5.0, they are already disabled by default. Seems like this is redundant at the moment.

[stebet]

I removed this line so they aren't all disabled now.

[stebet]

Updated to not completely remove the analyzers and only remove the feature enabling all of them at the same time. Thanks for taking a look @bording.

[bording]

Do we know what version of the .NET SDK we're using to build against in CI? If we can just update to a .NET 5 SDK, I'd prefer to do that over explicitly adding a package reference to the analyzer.

Then we just need to have <EnableNETAnalyzers>true</EnableNETAnalyzers> in the project file to enable the default set of analyzers to run against this project.

[michaelklishin]

RabbitMQ's Concourse uses

dotnet-sdk-3.1
dotnet-runtime-2.1

I could not get .NET 5 build 3.1 apps without surprising failures (I no longer remember the details). I'd be all for moving to use .NET 5 (but target .NET Core 3.1+ for now) but perhaps it should be a separate PR.

[michaelklishin]

@stebet @bording @bollhals I'm OK with merging this and perhaps investigating how/why we should switch to using .NET 5 for development and CI next. It was not straightforward to use .NET 5 and target 3.1 on a Mac last time I tried it but it easily could have been my lack of experience with .NET.

[stebet]

@stebet @bording @bollhals I'm OK with merging this and perhaps investigating how/why we should switch to using .NET 5 for development and CI next. It was not straightforward to use .NET 5 and target 3.1 on a Mac last time I tried it but it easily could have been my lack of experience with .NET.

Sounds good. I'll rebase to fix the merge errors.

[stebet]

@michaelklishin rebase done :)

[michaelklishin]

Thank you!