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

Fix DiagnosticAnalyzerFieldsAnalyzer for delegate fields #6724

Merged
merged 5 commits into from
Jul 5, 2023

Conversation

CollinAlpert
Copy link
Contributor

This PR removes the warning for storing per-compilation data inside an analyzer field, when that field is a delegate type.

@CollinAlpert CollinAlpert requested a review from a team as a code owner July 1, 2023 16:16
@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Merging #6724 (73a985c) into main (bdcbcac) will decrease coverage by 0.01%.
The diff coverage is 99.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6724      +/-   ##
==========================================
- Coverage   96.32%   96.32%   -0.01%     
==========================================
  Files        1389     1389              
  Lines      325167   325396     +229     
  Branches    10724    10727       +3     
==========================================
+ Hits       313232   313450     +218     
- Misses       9225     9234       +9     
- Partials     2710     2712       +2     

@mavasani
Copy link
Member

mavasani commented Jul 3, 2023

@CollinAlpert Is there a github issue tracking this bug or was this identified by you through dogfooding?

@@ -8,6 +9,25 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Analyzers.MetaAnalyzers
<DiagnosticAnalyzer(LanguageNames.VisualBasic)>
Public Class BasicDiagnosticAnalyzerFieldsAnalyzer
Inherits DiagnosticAnalyzerFieldsAnalyzer(Of ClassBlockSyntax, StructureBlockSyntax, FieldDeclarationSyntax, TypeSyntax, SimpleAsClauseSyntax)

Protected Overrides Function IsContainedInFuncOrAction(typeSyntax As TypeSyntax, model As SemanticModel, funcs As ImmutableArray(Of INamedTypeSymbol), actions As ImmutableArray(Of INamedTypeSymbol)) As Boolean
Copy link
Member

@mavasani mavasani Jul 3, 2023

Choose a reason for hiding this comment

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

Can we add VB tests for this scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I simplified the check to use DelegateInvokeMethod.

@CollinAlpert
Copy link
Contributor Author

@mavasani I noticed it while working on #6727, specifically here.

Copy link
Member

@mavasani mavasani left a comment

Choose a reason for hiding this comment

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

LGTM. I can merge once you add VB tests. Thanks!

Copy link
Member

@mavasani mavasani left a comment

Choose a reason for hiding this comment

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

Thanks!

@mavasani mavasani merged commit bbb6b89 into dotnet:main Jul 5, 2023
11 checks passed
@CollinAlpert CollinAlpert deleted the DiagnosticAnalyzerFieldsAnalyzer branch July 5, 2023 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants