Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IDE information diagnostics displayed in the VS Code problems pane do not surface via dotnet build #60620

Closed
johnnyreilly opened this issue Apr 7, 2022 · 17 comments
Labels
Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@johnnyreilly
Copy link

johnnyreilly commented Apr 7, 2022

Follow on from discussion here: #6195 (comment)

6.0 (going by the SDK):

Steps to Reproduce:

  1. have the .NET 6 SDK installed
  2. dotnet new webapi -o AnalyseThis
  3. Update AnalyseThis.csproj to:
<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>

    <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild> 
    <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>
    <AnalysisModeStyle>All</AnalysisModeStyle>
    <!-- <AnalysisMode>All</AnalysisMode> -->
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Swashbuckle.AspNetCore" Version="6.2.3" />
  </ItemGroup>

</Project>

Run dotnet build

Expected Behavior:

Surface IDE messages like this in the build output:

image

Private member 'WeatherForecastController._logger' can be removed as the value assigned to it is never read [AnalyseThis] IDE0052

Actual Behavior:

IDE messages not surfaced in build output:

Microsoft (R) Build Engine version 17.1.0+ae57d105c for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  AnalyseThis -> /workspaces/dapr-devcontainer-debug-and-deploy/AnalyseThis/bin/Debug/net6.0/AnalyseThis.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:01.76

I understand that it was previously by design that this was not surfaced, but if I read the docs correctly, things have changed. And as of .NET 6, the rules should be surfaced by dotnet build. However, I haven't managed to achieve that - it's possible I'm doing it wrong

I took that it was desired behaviour from here: https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#analysislevel

Starting in .NET 6, if you set EnforceCodeStyleInBuild to true, this property affects code-style (IDEXXXX) rules too.

Maybe I'm doing something wrong? or there's a bug?

cc @JohnMarkHowell

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 7, 2022
@Youssef1313
Copy link
Member

@johnnyreilly Does adding dotnet_diagnostic.IDE0052.severity = warning to .editorconfig fixes the issue?

@johnnyreilly
Copy link
Author

That removes the IDE0052 message from the Problems within VS Code. But the issue I'm reporting is that this message is never surfaced from dotnet build even though the docs suggest it ought to be. There's a disparity between what I see in VS Code and what I see in the build output.

Prior to .NET 6 the docs suggest this was intentional. But as of .NET 6 it ought not to be. To be clear: I'm hoping to see IDE0052 surfaced in my build output, but I do not

@Youssef1313
Copy link
Member

@johnnyreilly Ok, I think I understand what happened in your case. Can you try dotnet build --no-incremental?

The problem is very likely that you got the warning in one build, and the build was cached. So the warning doesn't appear again.

You can also delete obj and bin directories and run a regular dotnet build instead of dotnet build --no-incremental.

Please let me know if this fixed your problem.

@JohnMarkHowell
Copy link

@Youssef1313 , does he also need to do a dotnet clean ?

@Youssef1313
Copy link
Member

Youssef1313 commented Apr 7, 2022

For me --no-incremental works alone.

dotnet clean can be an alternative to deleting obj and bin.

So only one of the following should be needed:

  • dotnet clean followed by dotnet build
  • Manually deleting obj and bin followed by dotnet build
  • dotnet build --no-incremental

@timheuer
Copy link
Member

timheuer commented Apr 7, 2022

I'm definitely seeing them as well with these settings. My output:

D:\source\repos\AnalyseThis\Controllers\WeatherForecastController.cs(3,1): warning IDE0160: Convert to block scoped nam
espace [D:\source\repos\AnalyseThis\AnalyseThis.csproj]
D:\source\repos\AnalyseThis\WeatherForecast.cs(1,1): warning IDE0160: Convert to block scoped namespace [D:\source\repo
s\AnalyseThis\AnalyseThis.csproj]
D:\source\repos\AnalyseThis\Program.cs(1,1): warning IDE0008: Use explicit type instead of 'var' [D:\source\repos\Analy
seThis\AnalyseThis.csproj]
D:\source\repos\AnalyseThis\Program.cs(10,1): warning IDE0008: Use explicit type instead of 'var' [D:\source\repos\Anal
yseThis\AnalyseThis.csproj]
D:\source\repos\AnalyseThis\Controllers\WeatherForecastController.cs(14,57): warning IDE0052: Private member 'WeatherFo
recastController._logger' can be removed as the value assigned to it is never read [D:\source\repos\AnalyseThis\Analyse
This.csproj]
D:\source\repos\AnalyseThis\Program.cs(15,5): warning IDE0058: Expression value is never used [D:\source\repos\AnalyseT
his\AnalyseThis.csproj]
D:\source\repos\AnalyseThis\Program.cs(16,5): warning IDE0058: Expression value is never used [D:\source\repos\AnalyseT
his\AnalyseThis.csproj]
    7 Warning(s)

@johnnyreilly
Copy link
Author

johnnyreilly commented Apr 7, 2022

So I got this working! And I turned it into a blog post - as I found it quite hard to discover how to do this and was keen to make it easier for one and all!

https://blog.johnnyreilly.com/2022/04/06/eslint-your-csharp-in-vs-code-with-roslyn-analyzers

@Youssef1313 your point here was useful:

@johnnyreilly Does adding dotnet_diagnostic.IDE0052.severity = warning to .editorconfig fixes the issue?

There is one thing that still foxes me. The Roslyn Analyzers in VS Code surface information messages like IDE0052 by default, along with a number of other (useful) information diagnostics. These are information - they do not show up in the build by default.

Do these information diagnostics exist in some kind of category? So in the same way that I can put this in an .editorconfig:

dotnet_analyzer_diagnostic.category-Style.severity = error

And dial up style diagnostics to errors, can I do a blanket upgrade to the information diagnostics, the likes of IDE0052, IDE0090 etc to be warning or error?

image

Or is there a list somewhere of the information lints that Roslyn Analyzers surface which I could use as a reference?

@timheuer
Copy link
Member

timheuer commented Apr 7, 2022

Ahh, so it was working in build, but not in the context of VSCode/OmniSharp without the OmniSharp setting. Or are you still not seeing it without any VS Code/OmniSharp in the workflow?

@johnnyreilly
Copy link
Author

johnnyreilly commented Apr 7, 2022

So with the OmniSharp settings set, I see information diagnostics in the "problems" panel of VS Code. And I really value the information diagnostics that show up. I'd like them to show up in the build output too.

However, the only way to do that now, is to spy on the information diagnostics showing up in VS Code, make a note of their code, and manually add it to the .editorconfig as a warning or error.

I'd like to find a way to either:

  • surface those IDE information diagnostics in the build directly - so the build would have the same information as the problems panel
  • or upgrade all those information diagnostics to warnings - or errors - so I can use them to fail a build.
  • or at least see a reference somewhere of what IDE information diagnostics that the Roslyn Analysers are looking for

@timheuer
Copy link
Member

timheuer commented Apr 7, 2022

2022-04-07_11-28-49
Yeah my experience is that it does...

@johnnyreilly
Copy link
Author

That is very interesting.

If you look at the output in my blog post, you'll see that the IDE0052 information diagnostic does not show up in the way it does for you:

https://blog.johnnyreilly.com/2022/04/06/eslint-your-csharp-in-vs-code-with-roslyn-analyzers

I'll put together a reproduction repo with a devcontainer and share it when I get a moment

@johnnyreilly
Copy link
Author

johnnyreilly commented Apr 8, 2022

Actually - that's incorrect. I'm having exactly the same experience as you, but your animated GIF moved so swiftly I misread it as surfacing the information diagnostics by default in the build. But actually you've got it dialed up to a warning as we were discussing earlier.

Here's a repo repro which should allow anyone to replicate my experience: https://github.com/johnnyreilly/roslyn-analyzers-reproduction (though as I say @timheuer - it seems like we're having the same experience already)

Let me restate the problem to solve as it's evolved with my understanding over the course of this thread. (and thanks all for the pointers - it's been super helpful ❤️ )

Restating the problem

When using VS Code I experience informational diagnostics in the problems pane of VS Code.

screenshot of the problems pane in VS Code displaying "Private member 'WeatherForecastController._logger' can be removed as the value assigned to it is never read [AnalyseThis, AnalyseThis] csharp(IDE0052) [Ln 14, Col 57]"

But if we run dotnet build:

Microsoft (R) Build Engine version 17.1.0+ae57d105c for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
  AnalyseThis -> /workspaces/roslyn-analyzers-reproduction/bin/Debug/net6.0/AnalyseThis.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:08.20

This information is not present. Informational diagnostics are not surfaced in the build by default (or maybe ever?)

If I added the following to my .editorconfig:

dotnet_diagnostic.IDE0052.severity = warning

I could dial up each individual IDE**** to a warning or an error, but I wouldn't get the VS Code problems pane experience of just seeing that information by default. I have to opt in to each one.

So there's this manual syncing effort require to spy on the information diagnostics showing up in VS Code, make a note of their codes, and manually add them to the .editorconfig as a warning or error.

The problem I'm trying to solve is this; I'd like to find a way to either:

  • surface those IDE information diagnostics in the build directly - so the build would have the same information as the problems panel OR
  • upgrade all those information diagnostics to warnings - or errors - so I can use them to fail a build. A category upgrade working the same way that dotnet_analyzer_diagnostic.category-Style.severity = error does OR
  • failing that it would be great if there was a reference somewhere of what IDE information diagnostics that the Roslyn Analysers are looking for and surfacing by default. If we had that we could use it as the basis for creating a .editorconfig with all the informational diagnostics in.

@johnnyreilly johnnyreilly changed the title IDE rules don't fail builds - docs say it should as of .NET 6 IDE information diagnostics that show up in VS Code problems pane do not surface in the build Apr 8, 2022
@johnnyreilly johnnyreilly changed the title IDE information diagnostics that show up in VS Code problems pane do not surface in the build IDE information diagnostics displayed in the VS Code problems pane do not surface via dotnet build Apr 8, 2022
@timheuer
Copy link
Member

timheuer commented Apr 8, 2022

Thanks @johnnyreilly I think this is a matter of how OmniSharp is surfacing diagnostics irrespective of dotnet build /cc @333fred

@JoeRobich
Copy link
Member

JoeRobich commented Apr 8, 2022

I could dial up each individual IDE**** to a warning or an error, but I wouldn't get the VS Code problems pane experience of just seeing that information by default. I have to opt in to each one.

@johnnyreilly These analyzers are not one-size fits all and it really should take some thought to turn the severity up and make it a requirement for your team. With that said, dotnet-format exists and is truly intended to be the formatter/linter for C# code. dotnet-format style -v detailed --severity info --verify-no-changes may be a better fit for your scenario. If you've used dotnet-format and found it lacking, please consider leaving feedback because we want to see it improve and be more successful for our users.

upgrade all those information diagnostics to warnings - or errors - so I can use them to fail a build.

Are you proposing a dotnet_analyzer_diagnostic.prefix-IDE.severity = error configuration (something similar was requested in #52051)? The reason IDE0052 is unaffected by setting the Style category's severity is because it belongs to the Quality category of analyzers (see IDE0052 for more details).

it would be great if there was a reference somewhere of what IDE information diagnostics that the Roslyn Analysers are looking for and surfacing by default.

I believe that dotnet new editorconfig will generate an .editorconfig file containing all the default code style options with default severity (@jmarolf to correct me if wrong). You can also see a version of this in the docs under 'Example EditorConfig file'. Also in the docs, you can also find a listing of CodeStyle IDExxxx diagnostic ids under the 'Code Style rules > Overview' section.

@johnnyreilly
Copy link
Author

Oh wow - there's so much good information here! Thank you @JoeRobich!

@johnnyreilly
Copy link
Author

@johnnyreilly These analyzers are not one-size fits all and it really should take some thought to turn the severity up and make it a requirement for your team.

Completely agree @JoeRobich - this is more about learning what's possible. So much more than I realised it turns out!

With that said, dotnet-format exists and is truly intended to be the formatter/linter for C# code. dotnet-format style -v detailed --severity info --verify-no-changes may be a better fit for your scenario.

Thanks for this. I was surprised that dotnet-format is considered to be a linting tool. The name of dotnet-format and in fact the description in the repo suggests that it's only for formatting:

dotnet-format is a code formatter for dotnet that applies style preferences to a project or solution.

Upon experimenting the command is:

dotnet format style -v detailed --severity info --verify-no-changes

Essentially losing the hyphen. I've updated the blog post to include this.

Are you proposing a dotnet_analyzer_diagnostic.prefix-IDE.severity = error configuration (something similar was requested in #52051 (comment))? The reason IDE0052 is unaffected by setting the Style category's severity is because it belongs to the Quality category of analyzers (see IDE0052 for more details).

Ah this is great. Essentially I didn't know about the categories of analyzers. They're documented here and I've updated the blog post to reflect this

Given that IDE0052 is part of CodeQuality I can dial the category up with:

# Default severity for analyzer diagnostics with category 'CodeQuality' (escalated to build errors)
dotnet_analyzer_diagnostic.category-CodeQuality.severity = error

I probably don't want to do this in general - but it's good to know what's possible.

I believe that dotnet new editorconfig will generate an .editorconfig file containing all the default code style options with default severity

Totally does - this is gold dust! Again I've updated the blog post to reflect. I've also credited folk in here who've provided some very useful assistance; thank you! ❤️ 🌻

@johnnyreilly
Copy link
Author

Closing as resolved - thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

5 participants