Visual Studio, show error at build time for error diagnostics

[dylanvdmerwe]

Product and Version Used:
VS 17.7.0
Roslynator 4.4.0

Steps to Reproduce:
In my .editorconfig I have the following rules set:

dotnet_diagnostic.RCS0031.severity = error
dotnet_diagnostic.RCS1161.severity = error

In a .cs file to test, these rules show up as errors correctly:

However, when the project/solution is built, the build succeeds.

There appears to be a disconnect with Roslynator diagnostics vs the MS ones. If I configure one as an error, it fails the build. For example:
dotnet_diagnostic.CA1841.severity = error

1>C:\Projects\Common\Reporting.Common.Test\Startup.cs(19,13,19,52): error CA1841: Prefer 'ContainsKey' over 'Keys.Contains' for dictionary type 'Dictionary' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1841)
1>Done building project "Reporting.Common.Test.csproj" -- FAILED.

Expected Behavior:
I would expect that, at build time, for analyzers set to warning and error to show up in the build logs. And especially for items marked as error to halt the build.

My .editorconfig file: .editorconfig.txt

[josefpihrt]

I can confirm this behavior in VS 17.7. My suspicion is that there was some change in a way how the analyzers from extension are handled.

Current behavior:

• If the analyzer is part of VS extension then the diagnostics will show up in the IDE but not during build.
• If the analyzer is part of nuget then it will break the build.

I think I would be the best if you file an issue to VS feedback portal to clarify if my assumption is correct. It's possible that it's a bug and it will be fixed in the future. It's also possible that it's a bug in Roslynator but I would say that it's unlikely in this case.

As a side note, I tend to opinion that good practice is to reference analyzers always as nuget, so they are part of the solution and are enforced regardless of the extension installed or not.

[dylanvdmerwe]

Thank you Josef. I have logged with Visual Studio. Let's keep this open until resolved. https://developercommunity.visualstudio.com/t/Analyzers-from-extensions-not-affecting-/10442691

As a side note, is there any documentation around using the analyzers in the nuget package vs extension? Which project must the nuget be referenced in, how do we get it to affect all projects in a solution? This is why we use the extension so it applies everywhere.

[sharwell]

This behavior is by design. See dotnet/roslyn#8068.

[dylanvdmerwe]

This behavior is by design. See dotnet/roslyn#8068.

Thank you for the clarification. What is the point then of the VS extension? Also this means that every single project we have in our solution needs to reference the analyzers nuget package? That's why we install the extension into VS so that it applies globally to all projects.

Our thinking was that even if someone doesn't have the extension installed, or using CLI, they would just miss out on those analyzers.

[sharwell]

What is the point then of the VS extension?

Many analyzer projects do not distribute as a VS extension at all. There are some edge cases where a Roslyn extension might want to be an extension, such as a collection of code refactoring providers (not analyzers) which would never be needed during a build and don't necessarily apply to all users working on the project.

Also this means that every single project we have in our solution needs to reference the analyzers nuget package?

Yes, but it doesn't mean every project file has to have a direct reference. You can place the reference in Directory.Build.props or Directory.Build.targets and it will apply to all projects underneath that folder. For example:
https://github.com/dotnet/roslyn-analyzers/blob/28fbf1107491dd5acafdca1e85a2ad136e30e796/src/Directory.Build.props#L39-L44

Our thinking was that even if someone doesn't have the extension installed, or using CLI, they would just miss out on those analyzers.

Yes, this is exactly why we do not include them. If we included them, we would have to explain to all these users why sometimes a build will break for some users on a project, and other times it will not. Builds are expected to be consistent.

[dylanvdmerwe]

What is the point then of the VS extension?

Many analyzer projects do not distribute as a VS extension at all. There are some edge cases where a Roslyn extension might want to be an extension, such as a collection of code refactoring providers (not analyzers) which would never be needed during a build and don't necessarily apply to all users working on the project.

Also this means that every single project we have in our solution needs to reference the analyzers nuget package?

Yes, but it doesn't mean every project file has to have a direct reference. You can place the reference in Directory.Build.props or Directory.Build.targets and it will apply to all projects underneath that folder. For example: https://github.com/dotnet/roslyn-analyzers/blob/28fbf1107491dd5acafdca1e85a2ad136e30e796/src/Directory.Build.props#L39-L44

Our thinking was that even if someone doesn't have the extension installed, or using CLI, they would just miss out on those analyzers.

Yes, this is exactly why we do not include them. If we included them, we would have to explain to all these users why sometimes a build will break for some users on a project, and other times it will not. Builds are expected to be consistent.

Great input, thank you.

[josefpihrt]

@sharwell was faster then me to answer 😄.

VS extension contains not only analyzers but also refactorings and fixes for compiler diagnostics (CS....). These code analysis tools are distributed only as a part of VS (Code) extension.

Regarding the analyzers: Over the years it turned out that including analyzers in the extension is a bad practice. The intention was good - to have analyzers available for every solution in VS. But the reality is that you want enforce analyzer rules regardless of the extension installed or the IDE.

The only clear solution to this issue is to remove analyzers from the VS (Code) extension. This will be done probably as a part of next major release of Roslynator (when major version of VS is released).

[dylanvdmerwe]

@josefpihrt so to confirm:

1. Keep the Roslynator VS (code) extension installed and enabled.
2. Add the following Nuget packages to the Directory.Build.props:

  <ItemGroup>
    <!-- Rosylnator analyzers -->
    <PackageReference Include="Roslynator.Analyzers" Version="4.4.0" />
    <PackageReference Include="Roslynator.CodeAnalysis.Analyzers" Version="4.4.0" />
    <PackageReference Include="Roslynator.Formatting.Analyzers" Version="4.4.0" />
  </ItemGroup>

And you're good to go?

[josefpihrt]

Yes (assuming that Directory.Build.props is at the repo root directory or src directory).

There is one more thing which is not necessary but it's better to do while the analyzers are part of the extension: You can disable analyzers from the extension to prevent them from running twice by adding following option to your default configuration file:

roslynator_analyzers.enabled_by_default = false

Default configuration is described here.

[josefpihrt]

follow up #1172