Enable a Roslyn analyzer for a subset of trim analysis warnings. #12598
Conversation
Yeah well not my fault :) lock files are failing as CI has updated .NET version |
It contains trimming warning about reflection usage
|
Can you explain a bit more this PR? Are we going to see fewer warnings (only important ones)? |
We are actually will be seeing more warning as it shows where we use reflection that is marked as not trim compatible. So this is first step, next step that can be done gradually is to eliminate those warning by removing reflection usage. |
|
cACK. Wait to merge this until we have a clear picture of how to implement the UI part of Mobile. We might not use anything from the Desktop version and implement everything from scratch for Mobile. Which would make it unnecessary to refactor and avoid Reflection for the sake of AOT in Fluent. |
Agree but this PR is also useful for backend part which also uses reflection and it affects the mobile and desktop targets. Something to keep in mind while evaluating this PR |
| @@ -12,7 +12,7 @@ jobs: | |||
| - task: UseDotNet@2 | |||
| displayName: 'Install .NET' | |||
| inputs: | |||
| version: 8.0.x | |||
| useGlobalJson: true | |||
There was a problem hiding this comment.
This is nice.
However, I wonder what it leads to in practice. If I understand correctly, it can happen that, for example, we would test against https://github.com/dotnet/runtime/releases/tag/v8.0.2 and not against 8.0.3 because 8.0.3 can be on a newer SDK (e.g. 8.0.300).
Is it how it works or not?
There was a problem hiding this comment.
| @@ -10,6 +10,7 @@ | |||
| <PathMap>$(MSBuildProjectDirectory)\=WalletWasabi.Fluent.Generators</PathMap> | |||
| <IsRoslynComponent>true</IsRoslynComponent> | |||
| <EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules> | |||
| <EnableTrimAnalyzer>false</EnableTrimAnalyzer> | |||
There was a problem hiding this comment.
Is this necessary? Directory.Build.props contains this line.
|
I would say that the warnings can be disabled for the test project and for the backend project. |
Thie PR sets PublishTrimmed to enable a Roslyn analyzer that shows a limited set of analysis warnings.
This will make easier to diagnose issues with code that uses Reflection and future proof code for Native AOT compatibility.
https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/trimming-options?pivots=dotnet-8-0#roslyn-analyzer
Related #11625