Add DynamicallyAccessedMembers analyzer #2184
Conversation
Add GetDiagnosticDescriptor
Update test
Remove unused files
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| static DiagnosticId GetDiagnosticId (SymbolKind source, SymbolKind target, bool targetsType = false) |
There was a problem hiding this comment.
It's not 100% clear to me what targetsType means. Does it just mean that a SymbolKind.Method target should be treated like it represents the method's return value as opposed to its this parameter?
If so I don't understand the case for (SymbolKind.Method, SymbolKind.Parameter).
There was a problem hiding this comment.
Yes, that's the intention. I think I'd better use another parameter name for this to make it clearer, and maybe add a comment. The logic for (Method, Parameter) was a bit off because of the targetsType was passed in ProcessArgument, I already changed that.
There was a problem hiding this comment.
How about targetIsMethodReturnType? Then the (Method, Parameter) case still doesn't make sense because there the source represents a method return type - I think it should instead be (NamedType, Parameter) for consistency with the other "ThisParameterTargets*" cases.
Eventually I think we'll want to avoid using NamedType to represent a "this" parameter source - we may need sourceIsMethodReturnType or similar, but this could be fixed later.
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
Use DiagnosticId names in for-loop Remove unnecessary if-def Add TryGetAttribute method Add Annotations to shared code in preparation for sharing the logic to retrieve unmatched DAM annotations between linker and analyzer Rename DiagnosticIds Add license headers where missing
Share code between linker and analyzer for retrieving the missing type annotations
| } | ||
| } | ||
|
|
||
| static DiagnosticId GetDiagnosticId (SymbolKind source, SymbolKind target, bool targetsType = false) |
There was a problem hiding this comment.
How about targetIsMethodReturnType? Then the (Method, Parameter) case still doesn't make sense because there the source represents a method return type - I think it should instead be (NamedType, Parameter) for consistency with the other "ThisParameterTargets*" cases.
Eventually I think we'll want to avoid using NamedType to represent a "this" parameter source - we may need sourceIsMethodReturnType or similar, but this could be fixed later.
src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
Outdated
Show resolved
Hide resolved
|
@mateoatr I created a branch |
* Add DiagnosticId enum Add GetDiagnosticDescriptor * Check that the diagnostic id is in the range of the supported linker warnings * Lint * Share DiagnosticString * Noisy whitespace * Warnings go up to 6000 inclusive * PR feedback * Get diagnostic string Update test * Lint * Tests * Add warnings * Display members using the linker's format * Remove unused Xunit theory * Check for PublicParameterlessCtor Remove unused files * Refactor analyzer * Delete SemanticModelExtensions * Update DiagnosticString * Lint * Rename methods Use DiagnosticId names in for-loop Remove unnecessary if-def Add TryGetAttribute method Add Annotations to shared code in preparation for sharing the logic to retrieve unmatched DAM annotations between linker and analyzer Rename DiagnosticIds Add license headers where missing * Don't warn when the method is RUC annotated Share code between linker and analyzer for retrieving the missing type annotations * Add comments, add source/targetIsMethodReturnType parameters * Match with NamedType for consistency * Remove GetMembersTypesString Commit migrated from dotnet/linker@f2cb5fe
This PR supercedes #2038, the main differences are that this one uses an operation context where previous one used to work with syntax nodes (harder to reason about and error prone), and that this PR uses the new mechanism for grabbing diagnostic descriptors (it also updates how we generate
DAMrelated warnings on the linker side -- this has been previously proposed but the main reason not to do it was that it would make it harder to customize the produced messages; since we now rely on grabbing the correct string format from the shared strings given a diagnostic id, this is no longer a problem, we can edit these strings in the resource file whenever we want).Just like previous draft, this focuses on diagnostics concerning mismatched
DynamicallyAccessedMemberannotations (warnings IL2067 to IL2091) and the idea is still to have a feature branch with this analyzer until we are comfortable merging it with main (mainly, be 100% sure that only valid warning scenarios will produce diagnostics, and handle the scenarios where code flow analysis is needed).