dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.95k stars 4.65k forks source link

Trim analyzer: Implement type name parsing #95118

Open vitek-karas opened 9 months ago

vitek-karas commented 9 months ago

For example:

// Should produce warning because we should be able to parse the type name, resolve it
// and find the method with RUC and thus issue a warning due to potential access to that method.
Type.GetType("TypeWithRUCMethod").GetMethods();

This is also happening in places with annotated string values.

There's a TODO in the code about this: https://github.com/dotnet/runtime/blob/2096625a41b8284f4388a5a42137b675df4685da/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs#L26-L30

ghost commented 9 months ago

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas See info in area-owners.md if you want to be subscribed.

Issue Details
For example: ```csharp // Should produce warning because we should be able to parse the type name, resolve it // and find the method with RUC and thus issue a warning due to potential access to that method. Type.GetType("TypeWithRUCMethod").GetMethods(); ``` This is also happening in places with annotated string values. There's a TODO in the code about this: https://github.com/dotnet/runtime/blob/2096625a41b8284f4388a5a42137b675df4685da/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/RequireDynamicallyAccessedMembersAction.cs#L26-L30
Author: vitek-karas
Assignees: -
Labels: `area-Tools-ILLink`
Milestone: -
sbomer commented 1 month ago

The note about IL2105 is out of date with https://github.com/dotnet/runtime/pull/104060 - analyzer should produce the new IL2122 whenever a non-assembly-qualified type name is passed to a location with annotations, same as the other tools.

sbomer commented 1 month ago

Reopening since the fix was reverted: https://github.com/dotnet/runtime/pull/106343

sbomer commented 3 weeks ago

I think this needs Roslyn to update the version of System.Reflection.Metadata here: https://github.com/dotnet/roslyn/blame/d3ba1144d3d8dd41f1c0a5d84729ba46c5625a54/eng/Versions.props#L32C3-L32C3. I believe that will update the binding redirects so that we can use the 9.0 version of TypeNameParseOptions. See the relevant comments https://github.com/dotnet/runtime/pull/106209#discussion_r1715946109 and https://github.com/dotnet/runtime/issues/106321#issuecomment-2286867577.