Skip execution of analyzers reporting info and hidden diagnostics dur…#43546
Merged
mavasani merged 7 commits intodotnet:masterfrom Apr 24, 2020
Merged
Skip execution of analyzers reporting info and hidden diagnostics dur…#43546mavasani merged 7 commits intodotnet:masterfrom
mavasani merged 7 commits intodotnet:masterfrom
Conversation
mavasani
commented
Apr 21, 2020
Contributor
Author
There was a problem hiding this comment.
All members moved to AnalyzerOptionsExtensions.cs
mavasani
commented
Apr 21, 2020
Contributor
Author
There was a problem hiding this comment.
All deleted code here is moved to a shared extension method on AnalyzerOptions in AnalyzerOptionsExtensions.cs
…ing build Closes dotnet#42166 This PR is aimed towards improving the build performance in presence of analyzers that report Info (Suggestion) and/or Hidden diagnostics. Hidden diagnostics are never reported in build and Info diagnostics are only officially supported in presence of /errorlog, we will retain this support. The command line compilers do report Info diagnostics on command line today, but these are filtered away by MSBuild and are effectively never visible to users. This makes this a non-breaking scenario in .NET core build environment and an extreme corner case breaking change on desktop (clients relying on info diagnostics from raw csc/vbc output). Note that such clients would still have a workaround to get the prior behavior by enabling /errorlog. This optimization should improve build performance when users install the new IDE CodeStyle NuGet package (mostly hidden diagnostics) and the upcoming .NET5 analyzers package (has large number of info/hidden diagnostics).
cc9d220 to
ffa7ab4
Compare
jaredpar
reviewed
Apr 22, 2020
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs
Outdated
Show resolved
Hide resolved
mavasani
added a commit
to mavasani/roslyn-analyzers
that referenced
this pull request
Apr 22, 2020
1. Do not generate different global analyzer config files for build and live analysis. dotnet/roslyn#43546 will ensure that we do not need special logic in each analyzer package to disable hidden/suggestion analyzers in build - this will be baked into the compiler. 2. Update the contents of generated targets and editorconfig file as per the new design for global analyzer config: Everything for global analyzer config files will match regular analyzer config files (file name should be .editorconfig, existing MSBuild property "EditorConfigFiles" will be used, etc.), except for a special entry "is_global = true" to indicate global analyzer config.
This was referenced Apr 22, 2020
jaredpar
reviewed
Apr 23, 2020
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerManager.cs
Outdated
Show resolved
Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerOptionsExtensions.cs
Show resolved
Hide resolved
…hout /errorlog) are skipped during build
Contributor
Author
|
FYI: 021468a Adds unit tests to explicitly guard the core performance optimization in this PR |
jaredpar
reviewed
Apr 23, 2020
jaredpar
approved these changes
Apr 23, 2020
Contributor
Author
|
Thanks @jaredpar. @dotnet/roslyn-compiler for an additional review. |
cston
reviewed
Apr 23, 2020
cston
reviewed
Apr 23, 2020
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs
Outdated
Show resolved
Hide resolved
Contributor
cston
reviewed
Apr 23, 2020
cston
reviewed
Apr 23, 2020
Contributor
Author
@cston I avoided that change to retain the ordering of comments. |
cston
approved these changes
Apr 23, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ing build
Closes #42166. Implements the design from #42166 (comment).
This PR is aimed towards improving the build performance in presence of analyzers that report Info (Suggestion) and/or Hidden diagnostics. Hidden diagnostics are never reported in build and Info diagnostics are only officially supported in presence of /errorlog, we will retain this support. The command line compilers do report Info diagnostics on command line today, but these are filtered away by MSBuild and are effectively never visible to users. This makes this a non-breaking scenario in .NET core build environment and an extreme corner case breaking change on desktop (clients relying on info diagnostics from raw csc/vbc output). Note that such clients would still have a workaround to get the prior behavior by enabling /errorlog. This change has been discussed with Jared and Andy, who are fine with the new semantics.
This optimization should improve build performance when users install the new IDE CodeStyle NuGet package (mostly hidden diagnostics) and the upcoming .NET5 analyzers package (has large number of info/hidden diagnostics).