-
Notifications
You must be signed in to change notification settings - Fork 256
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
Extend AnalyzerTest to support tests for DiagnosticSuppressor analyzers #916
Conversation
<Testing_MicrosoftCodeAnalysisWorkspaces_Version>3.8.0</Testing_MicrosoftCodeAnalysisWorkspaces_Version> | ||
<Testing_MicrosoftCodeAnalysisAnalyzers_Version>3.3.2</Testing_MicrosoftCodeAnalysisAnalyzers_Version> | ||
<DefineConstants Condition="'$(Language)' == 'C#'">$(DefineConstants);DIAG_SUPPORTS_SUPPRESSION</DefineConstants> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❗ These can't change. Light-up/reflection must be used to support the scenario in cases where the APIs do not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, then I will do another PR using reflection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a few places where it's already used. See CodeActionExtensions
for an example:
Lines 15 to 24 in 27fe3d6
public static ImmutableArray<CodeAction> GetNestedActions(this CodeAction action) | |
{ | |
var property = typeof(CodeAction).GetTypeInfo().DeclaredProperties.SingleOrDefault(property => property.Name == "NestedCodeActions"); | |
if (property is null) | |
{ | |
return ImmutableArray<CodeAction>.Empty; | |
} | |
return (ImmutableArray<CodeAction>)(property.GetValue(action) ?? throw new NotSupportedException()); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the support for AnalyzerConfigDocuments #734
ProjectSection(SolutionItems) = preProject | ||
src\Microsoft.CodeAnalysis.Testing\Directory.Build.props = src\Microsoft.CodeAnalysis.Testing\Directory.Build.props | ||
src\Microsoft.CodeAnalysis.Testing\Directory.Build.targets = src\Microsoft.CodeAnalysis.Testing\Directory.Build.targets | ||
EndProjectSection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not about viewing the file, it's about getting FindInFiles with scope solution working as expected. (for guys like me being not so used to all the features and tweaks in an unknown solution)
However I won't insist in adding this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it's a losing battle to keep solution files synchronized with the solution. The only way it's always consistent is to omit them all and point users to the solution filter button. 😄
The button used to work much better in Visual Studio 2017 (one click would toggle show/hide). Now it's tedious but still technically works.
Replaced by #917 |
This implements scenario 1 from #915 by conditionally selecting dependencies and extend the analyzer validation code.