Skip to content

Commit

Permalink
Treat old TFMs as unsupported by ILLink analyzers (#35837)
Browse files Browse the repository at this point in the history
Up to .NET 7, the analyzers versioned with the SDK (so you would always
get the latest analyzers). This allowed using the analyzers (and related
options which enable the analyzers) even on older TFMs.

#32045 changed to versioning the
analyzers with the target framework, but still allowed use of the analyzers
on target frameworks where the .NET 7 ILLink pack was made available
for backwards compatibility.

This change removes such backwards compat by treating the analyzers
as unsupported for target frameworks before the framework was annotated
with the relevant attributes. This increases the scope of the warning added in
#29441 and #34077 to also warn for these unsupported
TFMs.

The first commit adds testcases to show the old behavior. The second commit
implements the new behavior and shows the changes to these testcases.

PublishTrimmed/PublishSingleFile/PublishAot used to enable the corresponding
analyzers. With this change, we now need to avoid failing
PublishTrimmed/PublishSingleFile for older TFMs that supported these publish
options, but don't support the analyzers. This change handles that by only
defaulting the `Enable*Analyzer` settings based on the publish settings for
TFMs where the analyzers are known to be supported.
  • Loading branch information
sbomer authored Oct 5, 2023
1 parent 269b9eb commit 4a1ffab
Show file tree
Hide file tree
Showing 8 changed files with 149 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
</PropertyGroup>

<Target Name="_AddKnownILLinkPack" BeforeTargets="ProcessFrameworkReferences">
<PropertyGroup>
<!-- Work around a version check that prevents referencing illink pack from a netstandard project. -->
<_FirstTargetFrameworkVersionToSupportTrimAnalyzer>2.0</_FirstTargetFrameworkVersionToSupportTrimAnalyzer>
</PropertyGroup>
<ItemGroup>
<KnownILLinkPack Include="@(KnownILLinkPack)"
Condition="'%(TargetFramework)' == '$(CurrentTargetFramework)'"
Expand Down
22 changes: 22 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ public class ProcessFrameworkReferences : TaskBase

public bool EnableAotAnalyzer { get; set; }

public string FirstTargetFrameworkVersionToSupportAotAnalyzer { get; set; }

public bool PublishTrimmed { get; set; }

public bool IsTrimmable { get; set; }

public string FirstTargetFrameworkVersionToSupportTrimAnalyzer { get; set; }

public bool SilenceIsTrimmableUnsupportedWarning { get; set; }

public string MinNonEolTargetFrameworkForTrimming { get; set; }
Expand All @@ -63,6 +67,8 @@ public class ProcessFrameworkReferences : TaskBase

public bool EnableSingleFileAnalyzer { get; set; }

public string FirstTargetFrameworkVersionToSupportSingleFileAnalyzer { get; set; }

public bool SilenceEnableSingleFileAnalyzerUnsupportedWarning { get; set; }

public string MinNonEolTargetFrameworkForSingleFile { get; set; }
Expand Down Expand Up @@ -828,6 +834,22 @@ private ToolPackSupport AddToolPack(
packagesToDownload.Add(runtimePackToDownload);
}

if (toolPackType is ToolPackType.ILLink)
{
// The ILLink tool pack is available for some TargetFrameworks where we nonetheless consider
// IsTrimmable/IsAotCompatible/EnableSingleFile to be unsupported, because the framework
// was not annotated with the attributes.
var firstTargetFrameworkVersionToSupportAotAnalyzer = NormalizeVersion(new Version(FirstTargetFrameworkVersionToSupportAotAnalyzer));
if ((IsAotCompatible || EnableAotAnalyzer) && normalizedTargetFrameworkVersion < firstTargetFrameworkVersionToSupportAotAnalyzer)
return ToolPackSupport.UnsupportedForTargetFramework;
var firstTargetFrameworkVersionToSupportSingleFileAnalyzer = NormalizeVersion(new Version(FirstTargetFrameworkVersionToSupportSingleFileAnalyzer));
if (EnableSingleFileAnalyzer && normalizedTargetFrameworkVersion < firstTargetFrameworkVersionToSupportSingleFileAnalyzer)
return ToolPackSupport.UnsupportedForTargetFramework;
var firstTargetFrameworkVersionToSupportTrimAnalyzer = NormalizeVersion(new Version(FirstTargetFrameworkVersionToSupportTrimAnalyzer));
if ((IsTrimmable || EnableTrimAnalyzer) && normalizedTargetFrameworkVersion < firstTargetFrameworkVersionToSupportTrimAnalyzer)
return ToolPackSupport.UnsupportedForTargetFramework;
}

// Packs with RID-agnostic build packages that contain MSBuild targets.
if (toolPackType is not ToolPackType.Crossgen2 && EnableRuntimePackDownload)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- PublishAot depends on PublishTrimmed. This must be set early enough for the KnownILLinkPack to be restored. -->
<PublishTrimmed Condition="'$(PublishTrimmed)' == '' And '$(PublishAot)' == 'true'">true</PublishTrimmed>
<IsTrimmable Condition="'$(IsTrimmable)' == '' and '$(IsAotCompatible)' == 'true'">true</IsTrimmable>

<_FirstTargetFrameworkToSupportTrimming>net6.0</_FirstTargetFrameworkToSupportTrimming>
<_FirstTargetFrameworkToSupportAot>net7.0</_FirstTargetFrameworkToSupportAot>
<_FirstTargetFrameworkToSupportSingleFile>net6.0</_FirstTargetFrameworkToSupportSingleFile>

<_FirstTargetFrameworkVersionToSupportTrimAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportTrimming)'))</_FirstTargetFrameworkVersionToSupportTrimAnalyzer>
<_FirstTargetFrameworkVersionToSupportAotAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportAot)'))</_FirstTargetFrameworkVersionToSupportAotAnalyzer>
<_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportSingleFile)'))</_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>
</PropertyGroup>

<ItemDefinitionGroup>
Expand All @@ -43,7 +51,6 @@ Copyright (c) .NET Foundation. All rights reserved.
<_RequiresILLinkPack Condition="'$(_RequiresILLinkPack)' == ''">false</_RequiresILLinkPack>
</PropertyGroup>


<!-- ProcessFrameworkReferences warns when the project settings introduce a dependency on a
tool pack that's not available for the target framework. For trimming, AOT, and single-file, the recommendation
is to multitarget the project to include a more recent TargetFramework. For correctly multitargeted
Expand All @@ -60,9 +67,6 @@ Copyright (c) .NET Foundation. All rights reserved.
and silences the warning if it is. -->

<PropertyGroup>
<_FirstTargetFrameworkToSupportTrimming>net6.0</_FirstTargetFrameworkToSupportTrimming>
<_FirstTargetFrameworkToSupportAot>net7.0</_FirstTargetFrameworkToSupportAot>
<_FirstTargetFrameworkToSupportSingleFile>net6.0</_FirstTargetFrameworkToSupportSingleFile>
<!-- The min non-EOL TFM has already caught up with the first TFM to support trimming/singlefile. No need to compare against it. -->
<_MinNonEolTargetFrameworkForTrimming>$(_MinimumNonEolSupportedNetCoreTargetFramework)</_MinNonEolTargetFrameworkForTrimming>
<_MinNonEolTargetFrameworkForSingleFile>$(_MinimumNonEolSupportedNetCoreTargetFramework)</_MinNonEolTargetFrameworkForSingleFile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,26 @@ Copyright (c) .NET Foundation. All rights reserved.

<!-- Intermediate step to enable ILLink.Analyzers so ILLink, Blazor, Xamarin, AOT, etc. can enable the same flags -->
<EnableSingleFileAnalyzer Condition="'$(EnableSingleFileAnalyzer)' == '' And
('$(PublishSingleFile)' == 'true' Or '$(PublishAot)' == 'true' Or '$(IsAotCompatible)' == 'true')">true</EnableSingleFileAnalyzer>
(
('$(PublishSingleFile)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer)'))) Or
('$(PublishAot)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)'))) Or
'$(IsAotCompatible)' == 'true'
)">true</EnableSingleFileAnalyzer>

<!-- Enable the trim analyzer when any trimming settings are enabled. Warnings may suppressed based on other settings. -->
<EnableTrimAnalyzer Condition="'$(EnableTrimAnalyzer)' == '' And
('$(PublishTrimmed)' == 'true' Or '$(IsTrimmable)' == 'true' Or '$(PublishAot)' == 'true')">true</EnableTrimAnalyzer>
(
('$(PublishTrimmed)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportTrimAnalyzer)'))) Or
('$(PublishAot)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)'))) Or
'$(IsTrimmable)' == 'true'
)">true</EnableTrimAnalyzer>

<!-- Enable the AOT analyzer when AOT is enabled. Warnings may suppressed based on other settings. -->
<EnableAotAnalyzer Condition="'$(EnableAotAnalyzer)' == '' And ('$(PublishAot)' == 'true' or '$(IsAotCompatible)' == 'true')">true</EnableAotAnalyzer>
<EnableAotAnalyzer Condition="'$(EnableAotAnalyzer)' == '' And
(
('$(PublishAot)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)'))) Or
'$(IsAotCompatible)' == 'true'
)">true</EnableAotAnalyzer>

<!-- EnforceCodeStyleInBuild Allows code style analyzers to be disabled in bulk via msbuild if the user wants to -->
<EnforceCodeStyleInBuild Condition="'$(EnforceCodeStyleInBuild)' == ''">false</EnforceCodeStyleInBuild>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ Copyright (c) .NET Foundation. All rights reserved.
SilenceIsAotCompatibleUnsupportedWarning="$(_SilenceIsAotCompatibleUnsupportedWarning)"
MinNonEolTargetFrameworkForAot="$(_MinNonEolTargetFrameworkForAot)"
EnableAotAnalyzer="$(EnableAotAnalyzer)"
FirstTargetFrameworkVersionToSupportAotAnalyzer="$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)"
PublishTrimmed="$(PublishTrimmed)"
IsTrimmable="$(IsTrimmable)"
FirstTargetFrameworkVersionToSupportTrimAnalyzer="$(_FirstTargetFrameworkVersionToSupportTrimAnalyzer)"
SilenceIsTrimmableUnsupportedWarning="$(_SilenceIsTrimmableUnsupportedWarning)"
MinNonEolTargetFrameworkForTrimming="$(_MinNonEolTargetFrameworkForTrimming)"
EnableTrimAnalyzer="$(EnableTrimAnalyzer)"
EnableSingleFileAnalyzer="$(EnableSingleFileAnalyzer)"
FirstTargetFrameworkVersionToSupportSingleFileAnalyzer="$(_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer)"
SilenceEnableSingleFileAnalyzerUnsupportedWarning="$(_SilenceEnableSingleFileAnalyzerUnsupportedWarning)"
MinNonEolTargetFrameworkForSingleFile="$(_MinNonEolTargetFrameworkForSingleFile)"
AotUseKnownRuntimePackForTarget="$(PublishAotUsingRuntimePack)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,45 @@ public void ILLink_analyzer_warnings_are_produced_using_EnableSingleFileAnalyzer
.And.HaveStdOutContaining("(10,13): warning IL3001");
}


[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("netcoreapp2.1", true)]
[InlineData("netcoreapp3.0", false)]
[InlineData("netcoreapp3.1", false)]
[InlineData("net5.0", false)]
[InlineData("net6.0", false)]
[InlineData("net7.0", false)]
public void PublishSingleFile_fails_for_unsupported_target_framework(string targetFramework, bool shouldFail)
{
var testProject = new TestProject()
{
Name = "HelloWorld",
IsExe = true,
TargetFrameworks = targetFramework
};
testProject.AdditionalProperties["PublishSingleFile"] = "true";
testProject.AdditionalProperties["SelfContained"] = "true";
testProject.AdditionalProperties["NoWarn"] = "NETSDK1138"; // Silence warning about targeting EOL TFMs
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);

var publishCommand = new PublishCommand(testAsset);
var result = publishCommand.Execute(RuntimeIdentifier);
if (shouldFail)
{
result.Should().Fail()
.And.HaveStdOutContaining(Strings.PublishSingleFileRequiresVersion30);
}
else
{
result.Should().Pass()
.And.NotHaveStdOutContaining("warning");
}
}

[RequiresMSBuildVersionTheory("17.8.0")]
[InlineData("netstandard2.0", true)]
[InlineData("net5.0", true)]
[InlineData("net6.0", false)]
[InlineData("netstandard2.0;net5.0", true)] // None of these TFMs are supported for single-file
[InlineData("netstandard2.0;net6.0", false)] // Net6.0 is the min TFM supported for single-file and targeting.
[InlineData("netstandard2.0;net8.0", true)] // Net8.0 is supported for single-file, but leaves a "gap" for the supported net6./net7.0 TFMs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Reflection.PortableExecutable;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.NET.Build.Tasks;
using Newtonsoft.Json.Linq;
using static Microsoft.NET.Publish.Tests.PublishTestUtils;

Expand Down Expand Up @@ -590,13 +591,49 @@ public void IsAotCompatible_implies_enable_analyzers(string targetFramework)
.And.NotHaveStdOutContaining("warning IL3002");
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("net5.0", true)]
[InlineData("net6.0", true)]
[InlineData("net7.0", false)]
public void PublishAot_fails_for_unsupported_target_framework(string targetFramework, bool shouldFail)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
// OSX wasn't supported before net8
return;
}

var rid = EnvironmentInfo.GetCompatibleRid(targetFramework);

var testProject = new TestProject()
{
Name = "HelloWorld",
TargetFrameworks = targetFramework
};
testProject.AdditionalProperties["PublishAot"] = "true";
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);

var publishCommand = new PublishCommand(testAsset);
var result = publishCommand.Execute($"/p:RuntimeIdentifier={rid}");
if (shouldFail) {
result.Should().Fail()
.And.HaveStdOutContaining(Strings.AotUnsupportedTargetFramework);
} else {
result.Should().Pass()
.And.NotHaveStdOutContaining("warning");
}
}

[RequiresMSBuildVersionTheory("17.8.0")]
[InlineData("netstandard2.0", true)]
[InlineData("net6.0", true)]
[InlineData("net7.0", false)]
[InlineData("netstandard2.0;net5.0", true)] // None of these TFMs are supported for AOT
[InlineData("netstandard2.0;net7.0", false)] // Net7.0 is the min TFM supported for AOT and targeting.
[InlineData("netstandard2.0;net8.0", true)] // Net8.0 is supported for AOT, but leaves a "gap" for the supported net7.0 TFMs.
[InlineData("alias-ns2", true)]
[InlineData("alias-n6", false)]
[InlineData("alias-n6", true)]
[InlineData("alias-n7", false)]
[InlineData("alias-n7;alias-n8", false)] // If all TFMs are supported, there's no warning even though the project uses aliases.
[InlineData("alias-ns2;alias-n7", true)] // This is correctly multi-targeted, but the logic can't detect this due to the alias so it still warns.
public void IsAotCompatible_warns_when_expected_for_not_correctly_multitarget_libraries(string targetFrameworks, bool shouldWarn)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,25 +148,39 @@ public void PublishTrimmed_fails_when_no_matching_pack_is_found(string targetFra
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("netcoreapp2.0")]
[InlineData("netcoreapp2.1")]
[InlineData("netstandard2.1")]
public void PublishTrimmed_fails_for_unsupported_target_framework(string targetFramework)
[InlineData("netcoreapp2.0", true)]
[InlineData("netcoreapp2.1", true)]
[InlineData("netstandard2.1", true)]
[InlineData("netcoreapp3.0", false)]
[InlineData("netcoreapp3.1", false)]
[InlineData("net5.0", false)]
[InlineData("net6.0", false)]
public void PublishTrimmed_fails_for_unsupported_target_framework(string targetFramework, bool shouldFail)
{
var projectName = "HelloWorld";
var rid = EnvironmentInfo.GetCompatibleRid(targetFramework);

var testProject = CreateTestProjectForILLinkTesting(targetFramework, projectName);
testProject.AdditionalProperties["PublishTrimmed"] = "true";
testProject.AdditionalProperties["NoWarn"] = "NETSDK1138"; // Silence warning about targeting EOL TFMs
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);
var publishCommand = new PublishCommand(testAsset);
publishCommand.Execute($"/p:RuntimeIdentifier={rid}", "/p:PublishTrimmed=true")
.Should().Fail()
.And.HaveStdOutContaining($"error {Strings.PublishTrimmedRequiresVersion30}");
var result = publishCommand.Execute($"/p:RuntimeIdentifier={rid}");
if (shouldFail) {
result.Should().Fail()
.And.HaveStdOutContaining($"error {Strings.PublishTrimmedRequiresVersion30}");
} else {
result.Should().Pass()
.And.NotHaveStdOutContaining("warning");
}
}

[RequiresMSBuildVersionTheory("17.8.0")]
[InlineData("netstandard2.0", true)]
[InlineData("netstandard2.1", true)]
[InlineData("netcoreapp3.1", true)]
[InlineData("net5.0", true)]
[InlineData("net6.0", false)]
[InlineData("netstandard2.0;net5.0", true)] // None of these TFMs are supported for trimming
[InlineData("netstandard2.0;net6.0", false)] // Net6.0 is the min TFM supported for trimming and targeting.
[InlineData("netstandard2.0;net8.0", true)] // Net8.0 is supported for trimming, but leaves a "gap" for the supported net6.0/net7.0 TFMs.
Expand All @@ -181,6 +195,7 @@ public void IsTrimmable_warns_when_expected_for_not_correctly_multitargeted_libr

var testProject = CreateTestProjectForILLinkTesting(targetFrameworks, projectName);
testProject.AdditionalProperties["IsTrimmable"] = "true";
testProject.AdditionalProperties["NoWarn"] = "NETSDK1138"; // Silence warning about targeting EOL TFMs
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFrameworks)
.WithProjectChanges(AddTargetFrameworkAliases);

Expand Down

0 comments on commit 4a1ffab

Please sign in to comment.