From 95cbdfa41ab6833edac97daaeae4d0632cd85e90 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Mon, 10 Apr 2023 12:49:00 -0700 Subject: [PATCH 1/5] First test --- .../RequiresCapabilityTests.g.cs | 17 ++++++++ .../RequiresInLibraryAssembly.cs | 42 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs create mode 100644 src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs new file mode 100644 index 00000000000000..3644357c1cfe47 --- /dev/null +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs @@ -0,0 +1,17 @@ +using System; +using System.Threading.Tasks; +using Xunit; + +namespace ILLink.RoslynAnalyzer.Tests +{ + public sealed partial class RequiresCapabilityTests : LinkerTestBase + { + + [Fact] + public Task RequiresInLibraryAssembly () + { + return RunTest (allowMissingWarnings: true); + } + + } +} \ No newline at end of file diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs new file mode 100644 index 00000000000000..8b229be00feb3d --- /dev/null +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs @@ -0,0 +1,42 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Linq.Expressions; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using Mono.Linker.Tests.Cases.Expectations.Assertions; +using Mono.Linker.Tests.Cases.Expectations.Metadata; + +[assembly: InternalsVisibleTo ("Microsoft.AspNetCore.Http.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] + +namespace Mono.Linker.Tests.Cases.RequiresCapability +{ + [SetupLinkerArgument ("-a", "test.exe", "library")] + + [SkipKeptItemsValidation] + [ExpectedNoWarnings] + public class RequiresInLibraryAssembly + { + public static void Main () + { + } + + [RequiresDynamicCode ("--MethodWhichRequires--")] + public static void MethodWhichRequires () { } + } + + [RequiresUnreferencedCode ("--ClassWithRequires--")] + internal sealed class ClassWithRequires + { + public static int Field; + + internal static readonly ParameterExpression HttpContextExpr = Expression.Parameter (typeof (ParameterExpression), "httpContext"); + + public static void Method () { } + } +} From f2681674fa8b19083e2c7ec105b2fcc065b30615 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 11 Apr 2023 03:49:51 -0700 Subject: [PATCH 2/5] Implementation --- .../src/linker/Linker.Steps/MarkStep.cs | 40 +++++++++++++++---- .../RequiresInLibraryAssembly.cs | 38 +++++++++++++----- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index fbe2ee3439aabf..8e41afef8d6ffd 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1775,9 +1775,17 @@ void ProcessAnalysisAnnotationsForField (FieldDefinition field, DependencyKind d (origin.Provider is MethodDefinition originMethod && CompilerGeneratedState.IsNestedFunctionOrStateMachineMember (originMethod))); } - if (dependencyKind == DependencyKind.DynamicallyAccessedMemberOnType) { + switch (dependencyKind) { + // Marked through things like descriptor - don't want to warn as it's intentional choice + case DependencyKind.TypePreserve: + return; + + case DependencyKind.DynamicallyAccessedMemberOnType: ReportWarningsForTypeHierarchyReflectionAccess (field, origin); return; + + default: + break; } if (Annotations.ShouldSuppressAnalysisWarningsForRequiresUnreferencedCode (origin.Provider, out _)) @@ -3189,9 +3197,10 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo if (!Annotations.ProcessSatelliteAssemblies && KnownMembers.IsSatelliteAssemblyMarker (method)) Annotations.ProcessSatelliteAssemblies = true; } else if (method.TryGetProperty (out PropertyDefinition? property)) - MarkProperty (property, new DependencyInfo (DependencyKind.PropertyOfPropertyMethod, method)); - else if (method.TryGetEvent (out EventDefinition? @event)) - MarkEvent (@event, new DependencyInfo (DependencyKind.EventOfEventMethod, method)); + MarkProperty (property, new DependencyInfo (PropagateDependencyKindToAccessors (reason.Kind, DependencyKind.PropertyOfPropertyMethod), method)); + else if (method.TryGetEvent (out EventDefinition? @event)) { + MarkEvent (@event, new DependencyInfo (PropagateDependencyKindToAccessors(reason.Kind, DependencyKind.EventOfEventMethod), method)); + } if (method.HasMetadataParameters ()) { #pragma warning disable RS0030 // MethodReference.Parameters is banned. It's easiest to leave the code as is for now @@ -3270,6 +3279,20 @@ protected virtual void DoAdditionalMethodProcessing (MethodDefinition method) { } + static DependencyKind PropagateDependencyKindToAccessors(DependencyKind parentDependencyKind, DependencyKind kind) + { + switch (parentDependencyKind) { + // If the member is marked due to descriptor or similar, propagate the original reason to suppress some warnings correctly + case DependencyKind.AlreadyMarked: + case DependencyKind.TypePreserve: + case DependencyKind.PreservedMethod: + return parentDependencyKind; + + default: + return kind; + } + } + void MarkImplicitlyUsedFields (TypeDefinition type) { if (type?.HasFields != true) @@ -3506,9 +3529,12 @@ protected internal virtual void MarkEvent (EventDefinition evt, in DependencyInf using var eventScope = ScopeStack.PushScope (new MessageOrigin (evt)); MarkCustomAttributes (evt, new DependencyInfo (DependencyKind.CustomAttribute, evt)); - MarkMethodIfNotNull (evt.AddMethod, new DependencyInfo (DependencyKind.EventMethod, evt), ScopeStack.CurrentScope.Origin); - MarkMethodIfNotNull (evt.InvokeMethod, new DependencyInfo (DependencyKind.EventMethod, evt), ScopeStack.CurrentScope.Origin); - MarkMethodIfNotNull (evt.RemoveMethod, new DependencyInfo (DependencyKind.EventMethod, evt), ScopeStack.CurrentScope.Origin); + + DependencyKind dependencyKind = PropagateDependencyKindToAccessors(reason.Kind, DependencyKind.EventMethod); + MarkMethodIfNotNull (evt.AddMethod, new DependencyInfo (dependencyKind, evt), ScopeStack.CurrentScope.Origin); + MarkMethodIfNotNull (evt.InvokeMethod, new DependencyInfo (dependencyKind, evt), ScopeStack.CurrentScope.Origin); + MarkMethodIfNotNull (evt.RemoveMethod, new DependencyInfo (dependencyKind, evt), ScopeStack.CurrentScope.Origin); + DoAdditionalEventProcessing (evt); } diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs index 8b229be00feb3d..1722aaea3de4bf 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs @@ -2,18 +2,10 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Linq.Expressions; -using System.Runtime.CompilerServices; -using System.Text; -using System.Threading.Tasks; using Mono.Linker.Tests.Cases.Expectations.Assertions; using Mono.Linker.Tests.Cases.Expectations.Metadata; -[assembly: InternalsVisibleTo ("Microsoft.AspNetCore.Http.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] - namespace Mono.Linker.Tests.Cases.RequiresCapability { [SetupLinkerArgument ("-a", "test.exe", "library")] @@ -28,15 +20,39 @@ public static void Main () [RequiresDynamicCode ("--MethodWhichRequires--")] public static void MethodWhichRequires () { } + + [RequiresDynamicCode ("--InstanceMethodWhichRequires--")] + public void InstanceMethodWhichRequires () { } + } + + [ExpectedNoWarnings] + public sealed class ClassWithDAMAnnotatedMembers + { + public static void Method ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type type) { } + + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] + public static Type Field; + } + + [ExpectedNoWarnings] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] + public sealed class ClassWithDAMAnnotation + { + public void Method () { } } + [ExpectedNoWarnings] [RequiresUnreferencedCode ("--ClassWithRequires--")] - internal sealed class ClassWithRequires + public sealed class ClassWithRequires { public static int Field; - internal static readonly ParameterExpression HttpContextExpr = Expression.Parameter (typeof (ParameterExpression), "httpContext"); - public static void Method () { } + + public void InstanceMethod () { } + + public static int Property { get; set; } + + public static event EventHandler PropertyChanged; } } From fef344d4b80a5d4e7a6c6e59b6d9a5e670213703 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 11 Apr 2023 03:55:20 -0700 Subject: [PATCH 3/5] Fix tests --- .../RequiresCapabilityTests.cs | 8 +++++++- .../RequiresCapabilityTests.g.cs | 17 ----------------- 2 files changed, 7 insertions(+), 18 deletions(-) delete mode 100644 src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresCapabilityTests.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresCapabilityTests.cs index 12e0844b559389..b711d35078c660 100644 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresCapabilityTests.cs +++ b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/RequiresCapabilityTests.cs @@ -58,6 +58,12 @@ public Task RequiresInCompilerGeneratedCodeRelease () return RunTest (); } + [Fact] + public Task RequiresInLibraryAssembly () + { + return RunTest (); + } + [Fact] public Task RequiresOnAttribute () { @@ -112,4 +118,4 @@ public Task SuppressRequires () return RunTest (nameof (SuppressRequires)); } } -} \ No newline at end of file +} diff --git a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs b/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs deleted file mode 100644 index 3644357c1cfe47..00000000000000 --- a/src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/RequiresCapabilityTests.g.cs +++ /dev/null @@ -1,17 +0,0 @@ -using System; -using System.Threading.Tasks; -using Xunit; - -namespace ILLink.RoslynAnalyzer.Tests -{ - public sealed partial class RequiresCapabilityTests : LinkerTestBase - { - - [Fact] - public Task RequiresInLibraryAssembly () - { - return RunTest (allowMissingWarnings: true); - } - - } -} \ No newline at end of file From 6634d33181a732dcb1822378b5e60147eca9971f Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Tue, 11 Apr 2023 11:44:32 -0700 Subject: [PATCH 4/5] PR feedback --- src/tools/illink/src/linker/Linker.Steps/MarkStep.cs | 2 ++ .../RequiresCapability/RequiresInLibraryAssembly.cs | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs index 8e41afef8d6ffd..515a3018fc486e 100644 --- a/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs +++ b/src/tools/illink/src/linker/Linker.Steps/MarkStep.cs @@ -1777,7 +1777,9 @@ void ProcessAnalysisAnnotationsForField (FieldDefinition field, DependencyKind d switch (dependencyKind) { // Marked through things like descriptor - don't want to warn as it's intentional choice + case DependencyKind.AlreadyMarked: case DependencyKind.TypePreserve: + case DependencyKind.PreservedMethod: return; case DependencyKind.DynamicallyAccessedMemberOnType: diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs index 1722aaea3de4bf..d5b55e0b4970d0 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresInLibraryAssembly.cs @@ -47,6 +47,10 @@ public sealed class ClassWithRequires { public static int Field; + internal static int InternalField; + + private static int PrivateField; + public static void Method () { } public void InstanceMethod () { } From 92d106c224169ce56119c231447332611170ecf7 Mon Sep 17 00:00:00 2001 From: vitek-karas <10670590+vitek-karas@users.noreply.github.com> Date: Wed, 12 Apr 2023 02:33:53 -0700 Subject: [PATCH 5/5] Update test --- .../RequiresCapability/RequiresViaXml.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaXml.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaXml.cs index 46e199fc84bd75..5ad0c16f5503fc 100644 --- a/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaXml.cs +++ b/src/tools/illink/test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresViaXml.cs @@ -17,8 +17,6 @@ namespace Mono.Linker.Tests.Cases.RequiresCapability [SetupLinkerDescriptorFile ("RequiresViaXml.descriptor.xml")] [SkipKeptItemsValidation] [ExpectedNoWarnings] - // [LogContains ("--RequiresOnlyViaDescriptor--")] // https://github.com/dotnet/linker/issues/2103 - [ExpectedWarning ("IL2026", "RequiresOnFieldOnlyViaDescriptor.Field", FileName = "RequiresViaXml.descriptor.xml", ProducedBy = Tool.Trimmer)] class RequiresViaXml {