Add ILLink Analyzers #15239
Add ILLink Analyzers #15239
Conversation
|
Just a confirmation - the target (master) branch of this PR targets .NET 6.0 SDK, correct? |
Yes. |
| @@ -73,6 +73,7 @@ | |||
| <PropertyGroup> | |||
| <!-- Dependencies from https://github.com/mono/linker --> | |||
| <MicrosoftNETILLinkTasksPackageVersion>6.0.0-alpha.1.21057.1</MicrosoftNETILLinkTasksPackageVersion> | |||
| <MicrosoftNETILLinkAnalyzerPackageVersion>$(MicrosoftNETILLinkTasksPackageVersion)</MicrosoftNETILLinkAnalyzerPackageVersion> | |||
There was a problem hiding this comment.
Is this sufficient to register the package in darc? Don't we also need to add an entry to Version.details.xml?
There was a problem hiding this comment.
I think it is - since the MicrosoftNETILLinkTasksPackageVersion is already flowing. I find it a bit odd to tie the versions together, but this seems to be the pattern used elsewhere in sdk.
| @@ -500,6 +499,45 @@ public void It_rewrites_the_apphost_for_non_single_file_publish() | |||
| appHostSize.Should().BeLessThan(singleFileSize); | |||
| } | |||
|
|
|||
| [RequiresMSBuildVersionTheory("16.8.0")] | |||
| [InlineData("net6.0")] | |||
| public void ILLink_analyzer_warnings_are_produced(string targetFramework) | |||
There was a problem hiding this comment.
Note: if this succeeds it's probably because we still have a reference to the old package, as opposed to the new analyzers working. I think dotnet/linker#1745 is needed to fix the new analyzers.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Analyzers.targets
Outdated
Show resolved
Hide resolved
mateoatr
force-pushed
the
illink-analyzer
branch
from
5ef4314 to
f19f79d
Compare
mateoatr
force-pushed
the
illink-analyzer
branch
from
f19f79d to
9d1c602
Compare
|
@marcpopMSFT Do you know if this made the snap for 6.0 P1? I'd like to get it in if possible as it prevents a regression from 5.0. |
|
@agocke I see it in the list of commits in the preview 1 branch so we should be good: https://github.com/dotnet/sdk/commits/release/6.0.1xx-preview1 |
Fixes: #5943 Context: https://github.com/dotnet/sdk/blob/c1b8e80be1f7cca767ed9f3c90a45d6830b166a0/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Analyzers.targets#L92-L96 Context: https://docs.microsoft.com/en-us/dotnet/core/deploying/single-file Context: dotnet/sdk#15239 [.NET Single File Apps][0] are a .NET deployment construct in which "all application-dependent files [are built] into a single binary." It's conceptually similar to Xamarin.Android `.apk` files, and there is some overlap in the "semantic [API incompatibilities][1]" between "normal" desktop .NET apps and Xamarin.Android/single-file apps, such as the (lack of) utility in `Assembly.Location`, which is the empty string in Xamarin.Android apps when `$(EmbedAssembliesIntoApk)`=True, which is required for "app store" distribution. For .NET 6+ app builds, enable analyzers which check for usage of these "semantically changed" APIs, by setting `$(EnableSingleFileAnalyzer)`=True. Update `Mono.Android.csproj` and `Mono.Android.Export.csproj` to also enable the single-file analyzers. After enabling this analyzer, no warnings appeared. But I do see the appropriate analyzers setup in projects now: Analyzer ~\android-toolchain\dotnet\sdk\6.0.100-preview.6.21280.2\Sdks\Microsoft.NET.Sdk\targets\..\analyzers\ILLink.CodeFixProvider.dll IsImplicitlyDefined = true ~\android-toolchain\dotnet\sdk\6.0.100-preview.6.21280.2\Sdks\Microsoft.NET.Sdk\targets\..\analyzers\ILLink.RoslynAnalyzer.dll IsImplicitlyDefined = true [0]: https://docs.microsoft.com/en-us/dotnet/core/deploying/single-file [1]: https://docs.microsoft.com/en-us/dotnet/core/deploying/single-file#api-incompatibility
Adds ILLinkAnalyzers. These help us report diagnostics about the usage of APIs that are potentially dangerous in the context of single-file applications as well as warnings related to the utilization of
RequiresUnreferencedCodeattribute.The single-file analyzer was originally located in dotnet/roslyn-analyzers; it was recently moved to mono/linker. With this change we can fully migrate its ownership to the linker repo (and remove it from roslyn-analyzers).