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.

Sign up for GitHub

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

Jump to bottom

Add analyzer action for reporting diagnostics in additional files #45076

Merged
16 commits merged into from
Jul 15, 2020
Merged

Add analyzer action for reporting diagnostics in additional files #45076

16 commits merged into from
Jul 15, 2020

Conversation

mavasani
Copy link
Contributor

@mavasani mavasani commented Jun 11, 2020 •
edited
Loading

Addresses #44131
The added analyzer APIs are similar to the existing APIs for syntax tree callback.

I have split the changes into separate commits for compiler and IDE changes for easier review.

  1. Compiler changes in 450cf6a: Adds analyzer API + driver implementation + tests.

  2. IDE changes in d2d197f: IDE support for analyzing and reporting diagnostics in non-source files

Future enhancement: Add analyzer action for reporting diagnostics in analyzer config files.

@mavasani mavasani added this to the 16.8 milestone Jun 11, 2020
@mavasani mavasani changed the title [WIP] Add analyzer action for reporting diagnostics in additional files Add analyzer action for reporting diagnostics in additional files Jun 11, 2020
@mavasani mavasani requested review from sharwell, cston, agocke and a team June 11, 2020 21:00
@mavasani mavasani marked this pull request as ready for review June 11, 2020 21:01
@mavasani mavasani requested a review from a team as a code owner June 11, 2020 21:01
@mavasani
Copy link
Contributor Author

@dotnet/roslyn-compiler Ping for reviews.

@mavasani
Add analyzer action for reporting diagnostics in additional files
450cf6a 
Addresses dotnet#44131
The added analyzer APIs are similar to the existing APIs for syntax tree callback. This commit adds the compiler APIs + tests

Future enhancement: Add analyzer action for reporting diagnostics in analyzer config files
@mavasani
IDE support for analyzing and reporting diagnostics in non-source files
d2d197f 
Fix tests
@mavasani mavasani force-pushed the RegisterAdditionalFileAction branch from 09f2fb4 to d2d197f Compare June 22, 2020 14:38
@mavasani mavasani mentioned this pull request Jun 22, 2020
Merged
@mavasani
Copy link
Contributor Author

mavasani commented Jul 2, 2020

Ping @dotnet/roslyn-compiler for reviews

@mavasani
Copy link
Contributor Author

Ping @dotnet/roslyn-compiler @dotnet/roslyn-ide for reviews.

jaredpar
jaredpar reviewed Jul 10, 2020
View reviewed changes
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisContextInfo.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisContextInfo.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisResult.cs Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisResultBuilder.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisResultBuilder.cs Show resolved Hide resolved
src/Compilers/CSharp/Test/Semantic/Diagnostics/DiagnosticAnalyzerTests.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisScope.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisScope.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisScope.cs Outdated Show resolved Hide resolved
src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalysisScope.cs Outdated Show resolved Hide resolved
@mavasani
Copy link
Contributor Author

Thanks @cston @jaredpar. Jared, any further feedback?

jmarolf
jmarolf approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@jmarolf jmarolf left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: looks good to me, just a few nits. Would like to understand the plan for partners moving to the new interface / combining the interfaces

@mavasani mavasani mentioned this pull request Jul 13, 2020
Closed
@mavasani
Copy link
Contributor Author

Thank you everyone!

@mavasani mavasani modified the milestones: 16.8, 16.8.P1 Jul 15, 2020
ghost
ghost approved these changes Jul 15, 2020
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approval

@ghost ghost merged commit 507c81a into dotnet:master Jul 15, 2020
@mavasani mavasani deleted the RegisterAdditionalFileAction branch July 15, 2020 17:06
@mavasani mavasani mentioned this pull request Jul 15, 2020
Closed
jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this pull request Aug 7, 2020
@jasonmalinowski
Fix crash in the preview of a code action that modified an .editorconfig
0dae2e9 
For reasons that aren't entirely clear, when we create a preview of
a code fix, we remove and re-add documents that are being changed when
we produce the PreviewWorkspace. This process had an bug which meant
that the file path of an .editorconfig got dropped, so the resultant
.editorconfig document path was null. This later caused a crash if
the diagnostic engine, when processing the PreviewWorkspace's
new solution, tried asking for a Compilation, since the null path would
get passed to AnalyzerConfig.Parse(), and it would throw.

This was exposed by dotnet#45076; until then if you had a preview of just
an .editorconfig file, nothing would actually be analyzed and so
nothing asked for the Compilation. The crash is "fixed" by dotnet#44331 in
that asking for a Compilation no longer results in a call to
AnalyzerConfig.Parse(); you have to ask for a Compilation and then do
something with a semantic model. By luck, that doesn't happen in the
common repro, but more obscure things absolutely could still fail.

This is being tested by the integration test being added in dotnet#46639. We
don't really have a good unit tests here as far as I can tell, but even
then a unit test really doesn't validate any of the end-to-end here.

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1162464
@jasonmalinowski jasonmalinowski mentioned this pull request Aug 7, 2020
Merged
mavasani added a commit to mavasani/roslyn that referenced this pull request Sep 22, 2022
@mavasani
Add support for reporting analyzer diagnostics in editorconfig files
3596caa 
Closes dotnet#44131

dotnet#45076 added support for reporting analyzer diagnostics in additional files. This PR addresses pending work from dotnet#44131 (comment) by adding similar support for editorconfig/globalconfig files in the project.

There are two primary public API additions
@mavasani mavasani mentioned this pull request Sep 22, 2022
Open
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cosifne Cosifne Cosifne left review comments

@cston cston cston approved these changes

@jmarolf jmarolf jmarolf approved these changes

@jaredpar jaredpar jaredpar approved these changes

@sharwell sharwell Awaiting requested review from sharwell

@agocke agocke Awaiting requested review from agocke

Assignees
No one assigned
Labels
Area-Analyzers Area-Compilers auto-merge
Projects
None yet
Milestone
16.8.P1
Development

Successfully merging this pull request may close these issues.
5 participants
@mavasani @jaredpar @jmarolf @cston @Cosifne