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 rules don't fail builds #6195

Closed
Lukazoid opened this issue Oct 20, 2015 · 20 comments
Closed

IDE rules don't fail builds #6195

Lukazoid opened this issue Oct 20, 2015 · 20 comments
Labels
Area-Analyzers Resolution-By Design The behavior reported in the issue matches the current design Verification Not Required

Comments

@Lukazoid
Copy link

When using code analysis with the IDE rules under Microsoft.CodeAnalysis.CSharp.Features enabled and their action set to Error, if one of the rules is violated, I correctly get the error message, however if I attempt to build the project, the build succeeds.

Is this the intended behavior of these IDE rules? It seems very unintuitive to see 2 Errors and Build Succeeded at the same time.

Steps to reproduce:

  1. New Console Application
  2. Navigate to Project -> Properties -> Code Analysis
  3. Check Enable Code Analysis on Build
  4. Open the rule set
  5. Uncheck Managed Binary Analysis and Microsoft.CodeAnalysis.CSharp
  6. Change Action of Microsoft.CodeAnalysis.CSharp.Features to Error and ensure Microsoft.CodeAnalysis.CSharp.Features is checked
  7. Save the edited ruleset (should be saved as {name of project}.ruleset)
  8. Build project

Observe 2 Errors, 1 Warning in Error List but 0 error(s), 1 warning(s) in output window.

What's even stranger is the error locations take me to a temporary file located at %tmp%\.NETFramework,Version=v4.6.AssemblyAttributes.cs. Should this even be analyzed?

This isn't the issue however, if I put the following in Main:

System.String test = "ABC";

I correctly get the error squiggle and error in the errors window, however the build still succeeds.

@tmeschter
Copy link
Contributor

@Lukazoid This is expected. Those particular rules exist in the IDE, but not in the compiler, so they aren't run as part of the build. You'll get the same behavior with any Roslyn-based analyzers installed as a VS extension (VSIX) rather than as NuGet packages or as manually-added analyzer references.

A couple of other notes:

The Enable Code Analysis on Build check box is a legacy option that has no effect on Roslyn-based diagnostics.

Also, the rule set editor shows all the rules that apply to a given project: legacy rules (e.g., Managed Binary Analysis), rules built into the compiler (Microsoft.CodeAnalysis.CSharp), rules built into the IDE (Microsoft.CodeAnalysis.CSharp.Features), and of course rules added by VSIXs, NuGet packages, and manually-added analyzer references.

@tmeschter tmeschter added the Resolution-By Design The behavior reported in the issue matches the current design label Oct 21, 2015
@davkean
Copy link
Member

davkean commented Oct 21, 2015

@tmeschter You've explained his behavior, but not why this behavior exists. Can you expand on that? Why are they only run in the IDE?

@tmeschter
Copy link
Contributor

@davkean I'm not sure what you're getting at. The rules in question are built into the IDE, not the compiler, and it is the compiler that runs static analysis rules during build.

@sharwell
Copy link
Member

If the IDE-installed rules ran as part of the in-IDE build, it would result in IDE builds and command line builds having potentially very different outputs. For example, a user with code-cracker installed as a VSIX could end up filing a bug report that an open source project does not build due to an analyzer error (or perhaps a warning when the project uses /warnaserror). They would be forced to either uninstall the analyzer extension or modify the rule set used by the project to disable some rule that only exists on one developer's machine.

In contrast, rules that are installed via NuGet become part of the project and part of the build. They run the same way across developer machines, and they run the same way in-IDE, on the command line, and in automated build environments.

@davkean
Copy link
Member

davkean commented Oct 21, 2015

@tmeschter What about these rules make them IDE specific? Why can't they be run as part of the build?

@tmeschter
Copy link
Contributor

@davkean Two reasons:

  1. The code implementing these rules resides in VS-specific assemblies that generally aren't available to the build system.
  2. These rules exist primarily to support IDE features like Simplify Type Name and Remove Unused Usings, so the IDE is the logical place for them. I suppose another option would have been to bundle these rules and the related fixes into NuGet packages (thus making the rules available in the build) but I imagine the IDE team was thinking that developers generally want these features available while editing code, but don't generally want to block their build on unnecessary usings, or type names that can be simplified, or things like that.

@srivatsn
Copy link
Contributor

Closing this as by design per @tmeschter's and @sharwell's explanations

@davkean
Copy link
Member

davkean commented Oct 21, 2015

@tmeschter Thanks.

@Lukazoid
Copy link
Author

If enable code analysis on build is legacy and has no effect, why does it still exist? Is there a blog post explaining not to use this any more?

I guess I wont be using the IDE rules or vsix based analyzers, if I mark something as an error it is because I want it as an error, regardless of whether it's built through an IDE or command line. I'll just have to try and get an analyzer from nuget working at compile time, not having any luck yet!

-----Original Message-----
From: "Tom Meschter" notifications@github.com
Sent: ‎21/‎10/‎2015 17:13
To: "dotnet/roslyn" roslyn@noreply.github.com
Cc: "Luke Horsley" luke.horsley@offset1337.co.uk
Subject: Re: [roslyn] IDE rules don't fail builds (#6195)

@Lukazoid This is expected. Those particular rules exist in the IDE, but not in the compiler, so they aren't run as part of the build. You'll get the same behavior with any Roslyn-based analyzers installed as a VS extension (VSIX) rather than as NuGet packages or as manually-added analyzer references.
A couple of other notes:
The Enable Code Analysis on Build check box is a legacy option that has no effect on Roslyn-based diagnostics.
Also, the rule set editor shows all the rules that apply to a given project: legacy rules (e.g., Managed Binary Analysis), rules built into the compiler (Microsoft.CodeAnalysis.CSharp), rules built into the IDE (Microsoft.CodeAnalysis.CSharp.Features), and of course rules added by VSIXs, NuGet packages, and manually-added analyzer references.

Reply to this email directly or view it on GitHub.

@jmarolf
Copy link
Contributor

jmarolf commented Oct 21, 2015

@Lukazoid what problems are you having getting an analyzer through nuget?

@Lukazoid
Copy link
Author

@jmarolf attempting to use https://www.nuget.org/packages/Microsoft.CodeAnalysis.FxCopAnalyzers/1.1.0-beta1-20150812-01 and I get an error about unable to load analyzer, unable to find Microsoft.CodeAnalysis. This is the best I can remember off the top of my head, I will paste the full error when I get a chance.

@JobaDiniz
Copy link

Is there a Roslyn analyzer nuget package that analyze IDExxx stuff?

@lmgchristian
Copy link

lmgchristian commented Jul 4, 2021

I'm still experiencing this issue. For example, I turned on CS8019 and IDE0005 as errors. I have all the RunAnalyzers set to true and I have an unused using. I see the error in the error window and there is squiggly red text over the offending line, but the project builds without a problem. I can build at the CLI, in Visual Studio (Mac) and Rider.

I recently experienced the same problem in another codebase. I can't consistently make the code analysis rules break the build.

NuGet is included

    <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="5.0.3">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>

@JohnMarkHowell
Copy link

Has anyone gotten Roslynator Analyzer rule violations to fail from MSBuild?

@johnnyreilly
Copy link

The docs here suggest that having

<PropertyGroup>
  <AnalysisModeStyle>All</AnalysisModeStyle>
</PropertyGroup>

Should trigger this behavior - it doesn't, alas

@JohnMarkHowell
Copy link

Thx @johnnyreilly for confirming that it is still broken. And the issue is closed for some reason.

@JohnMarkHowell
Copy link

Ah, resolved as "By Design" meaning they are intended to show in the IDE.
I don't agree with that resolution, because if that's all they are intended for, why expose them?
😣

@johnnyreilly
Copy link

johnnyreilly commented Apr 7, 2022

resolved as "By Design" meaning they are intended to show in the IDE.

So I think that was correct at the point this issue was closed.

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

Have a read of this: 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've the wrong end of the stick. Perhaps we should open a fresh issue?

@johnnyreilly
Copy link

Okay - for my money this isn't broken at all! It's just not obvious how to do things. See conversation here: #60620

And I've taken a stab at making it easier for folk to make use of this by writing a blog post: https://blog.johnnyreilly.com/2022/04/06/eslint-your-csharp-in-vs-code-with-roslyn-analyzers

@fdalsotto
Copy link

awesome article, @johnnyreilly. After reading your article for a few minutes I was able to resolve an issue I was chasing for some time. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Analyzers Resolution-By Design The behavior reported in the issue matches the current design Verification Not Required
Projects
None yet
Development

No branches or pull requests