Roslyn-analyzers: Very Inconsistent Behavior With Rules

[JasonBock]

Analyzer package

Microsoft.CodeQuality.Analyzers, version 2.6.0

Analyzer

CA1801 (though it could be happening with others)

Repro steps

1. I create a new console app (tried this with both .NET Core and .NET Framework)
2. I add the Microsoft.CodeAnalysis.FxCopAnalyzers NuGet package.

Expected behavior

I would see CA1801 show up not only in the Error List but as a squiggle under the args argument that's automatically created for the Main() method. I would expect to see the squiggle as soon as the analyzer package was added, and if I add the unused argument back in.

Actual behavior

This is where it gets inconsistent. The build always fails, but sometimes the rule shows up in the Error List, sometimes it doesn't. I rarely if ever see the squiggle under args. I tried another Nuget package that I know has analyzers contained within (CSLA-Core) and they work just fine in both the .NET Core and .NET Framework package.

Note that this error seems to show up in the build output though.

[mavasani]

@JasonBock Have you turned on full solution analysis https://msdn.microsoft.com/en-us/library/mt709421.aspx? Tagging @heejaechang as well.

[sharwell]

This sounds like the issues where true build errors never make their way to the Errors window.

/cc @Pilchie

[JasonBock]

@mavasani that seems to fix things. Funny that the page says, "By default, full solution analysis is enabled for Visual Basic and disabled for Visual C#." and it was disabled for me (and I don't remember ever turning it off). As a side note, on this GitHub site, it may be good to reference this so devs know to turn this on if they're getting this weird "things aren't lighting up as expected" behavior.

[sharwell]

so devs know to turn this on

We have a few more performance issues to address before making a general recommendation along these lines. 😄 Most of my pull requests for 15.6 were improvements in this area there are a few more big use cases to smooth out.

[mavasani]

Tagging @jinujoseph

This is the same issue as https://github.com/dotnet/roslyn/issues/11750. By default "Full solution analysis" is turned off for C# (for performance reasons), which means we analyze only open file during live analysis in IDE and never call the compilation end actions, which reports diagnostics for compilation analyzers. Note that the errors from build are indeed added to the error list, but the IDE live/build de-duping removes them as it expects results from live analysis to produce the same diagnostics, which it doesn't.
For an end user, the experience is super confusing, and they feel the analyzer is misbehaving, without knowing how to fix it. Personally, I feel we should change the semantics of full solution analysis to mean:

1. OFF: Open files and active project.
2. ON: Entire solution.

I don't think it will incur any overhead to ensure that we always execute compilation end actions for active project, and theoretically, these analyzers cannot function properly unless we do so. I believe @CyrusNajmabadi has also been bitten by this multiple times, and we all agree something must be done here - current experience is not good.

[sharwell]

Note that the errors from build are indeed added to the error list, but the IDE live/build de-duping removes them as it expects results from live analysis to produce the same diagnostics, which it doesn't.

This is a nasty bug, for sure

[mavasani]

Unfortunately, the IDE engine is pretty much left in a very helpless state due to the current semantics of "Full solution analysis". I don't think it is okay to leave partial live and partial build diagnostics in error list, especially as latter can get stale soon. Consider this very case that @JasonBock mentioned. If we leave the CA1801 (remove unused parameter) diagnostics from build in the error list, then fixing the code will still leave these diagnostics lingering in the error list, until the end user does another explicit build. This will be super confusing if you combine with non-compilation end diagnostics, which will correctly appear and disappear as you type. Additionally, I am not sure if it is easy for the IDE diagnostic engine to detect whether a build diagnostic is from compilation end action or not.

I believe the IDE engine design that was chosen by @heejaechang is correct - we show all build diagnostics in error list when user does an explicit build, but as soon live diagnostics start kicking in, we remove all build diagnostics from error list (except certain emit diagnostics which can never be reported during live analysis). If semantics for full solution analysis are as per my previous comment, live analysis will ensure that all analyzer actions always execute for the active project and error list always has them (regardless of whether Full solution analysis is on or off). This also ensures you always get all the required code fixes for compilation end diagnostics. "Full solution analysis" then becomes primarily a control over diagnostics for non-active project, where it is fine to have partial diagnostics in error list.

[JasonBock]

This all sounds interesting, but I have to admit, I'm having a hard time following all this. Is it safe to say that if I turn on full solution analaysis then things will "work as expected", which to me means I'll always see the errors/warnings in the error list? But this is a "perf issue" - just how much of an "issue" is it?

All I really want is that the errors always show up when I do a build. But it seemed like it was not happening, or inconsistently, when I didn't have full solution analysis enabled. Is there a "right" answer here?

[mavasani]

@JasonBock Enabling full solution analysis should ensure you get very consistent experience - you always get the expected diagnostics from build and intellisense. My comments were mainly about how to improve the user experience for the default behavior (full solution analysis turned off). As @sharwell mentioned, we are still working on improving the VS performance with Full solution analysis enabled, and if you do indeed see any performance degradation with this option turned on, please do submit a performance trace to us for investigation.

[JasonBock]

Thanks @mavasani, that helps.

[sharwell]

But this is a "perf issue" - just how much of an "issue" is it?

You may not even notice, depending on hardware, configuration, and the project you are working on. If you observe any issues related to performance, please file a performance feedback item and reach out to me.
https://github.com/dotnet/roslyn/wiki/Reporting-Visual-Studio-crashes-and-performance-issues#performance-issues

[JasonBock]

I'll keep that in mind. My hope is that the projects/solutions should be fairly small so we shouldn't really notice a hit, and it's far more important to catch the things these analysis rules as soon as possible.

[mavasani]

The bad user experience with compilation analyzers when Full solution analysis is OFF is tracked by https://github.com/dotnet/roslyn/issues/11750.