diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptByAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptByAttribute.cs new file mode 100644 index 000000000000..d7871d9ca378 --- /dev/null +++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptByAttribute.cs @@ -0,0 +1,35 @@ +// 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; + +namespace Mono.Linker.Tests.Cases.Expectations.Assertions +{ + [AttributeUsage (AttributeTargets.All, Inherited = false)] + public class KeptByAttribute : KeptAttribute + { + private KeptByAttribute () { } + + /// + /// Place on an type member to indicate that the linker should log that the member is kept as a depenendency of with reason . + /// + /// Cecil's FullName property of the item that provides the dependency that keeps the item + /// The string representation of the DependencyKind that is recorded as the reason for the dependency + public KeptByAttribute (string dependencyProvider, string reason) { } + + /// + /// Place on an type member to indicate that the linker should log that the member is kept as a depenendency of with reason . + /// + /// The type that is providing the dependency that keeps the item + /// The string representation of the DependencyKind that is recorded as the reason for the dependency + public KeptByAttribute (Type dependencyProviderType, string reason) { } + + /// + /// Place on an type member to indicate that the linker should log that the member is kept as a depenendency of . with reason . + /// + /// The declaring type of the member that is providing the dependency that keeps the item + /// Cecil's 'Name' property of the member that provides the dependency that keeps the item + /// The string representation of the DependencyKind that is recorded as the reason for the dependency + public KeptByAttribute (Type dependencyProviderType, string memberName, string reason) { } + } +} diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsPreservedIfBaseMethodGetsInvoked.cs b/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsPreservedIfBaseMethodGetsInvoked.cs index 70323315c31d..8b876f777371 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsPreservedIfBaseMethodGetsInvoked.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsPreservedIfBaseMethodGetsInvoked.cs @@ -23,7 +23,7 @@ public virtual void Foo () [KeptBaseType (typeof (B))] class A : B { - [Kept] + [KeptBy (typeof (A), "OverrideOnInstantiatedType")] public override void Foo () { } diff --git a/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsStrippedIfImplementingMethodGetsInvokedDirectly.cs b/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsStrippedIfImplementingMethodGetsInvokedDirectly.cs index 76457ce53659..eaae2a4aaa55 100644 --- a/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsStrippedIfImplementingMethodGetsInvokedDirectly.cs +++ b/test/Mono.Linker.Tests.Cases/Inheritance.VirtualMethods/VirtualMethodGetsStrippedIfImplementingMethodGetsInvokedDirectly.cs @@ -12,7 +12,9 @@ public static void Main () [KeptMember (".ctor()")] class B { - [Kept] // TODO: Would be nice to be removed + // TODO: Would be nice to be removed + // https://github.com/dotnet/linker/issues/3097 + [KeptBy (typeof (A), nameof (A.Foo), "BaseMethod")] public virtual void Foo () { } @@ -22,7 +24,10 @@ public virtual void Foo () [KeptBaseType (typeof (B))] class A : B { - [Kept] + // Bug: https://github.com/dotnet/linker/issues/3078 + // Linker should mark for DirectCall as well as OverrideOnInstantiatedType, not just OverrideOnInstantiatedType + //[KeptBy (typeof(A), nameof(Foo), DependencyKind.DirectCall)] + [KeptBy (typeof (A), "OverrideOnInstantiatedType")] public override void Foo () { } diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs index 41210172f6e4..26ad1309b8b1 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs @@ -16,6 +16,7 @@ namespace Mono.Linker.Tests.TestCasesRunner public class AssemblyChecker { readonly AssemblyDefinition originalAssembly, linkedAssembly; + readonly LinkedTestCaseResult linkedTestCase; HashSet linkedMembers; readonly HashSet verifiedGeneratedFields = new HashSet (); @@ -23,10 +24,11 @@ public class AssemblyChecker readonly HashSet verifiedGeneratedTypes = new HashSet (); bool checkNames; - public AssemblyChecker (AssemblyDefinition original, AssemblyDefinition linked) + public AssemblyChecker (AssemblyDefinition original, AssemblyDefinition linked, LinkedTestCaseResult linkedTestCase) { this.originalAssembly = original; this.linkedAssembly = linked; + this.linkedTestCase = linkedTestCase; checkNames = original.MainModule.GetTypeReferences ().Any (attr => attr.Name == nameof (RemovedNameValueAttribute)); @@ -44,6 +46,7 @@ public void Verify () VerifyResources (originalAssembly, linkedAssembly); VerifyReferences (originalAssembly, linkedAssembly); + VerifyKeptByAttributes (originalAssembly, originalAssembly.FullName); linkedMembers = new HashSet (linkedAssembly.MainModule.AllMembers ().Select (s => { return s.FullName; @@ -139,11 +142,76 @@ protected virtual void VerifyTypeDefinition (TypeDefinition original, TypeDefini } } + /// + /// Validates that all instances on a member are valid (i.e. the linker recorded a marked dependency described in the attribute) + /// + void VerifyKeptByAttributes (IMemberDefinition src, IMemberDefinition linked) + { + foreach (var keptByAttribute in src.CustomAttributes.Where (ca => ca.AttributeType.IsTypeOf ())) + VerifyKeptByAttribute (linked.FullName, keptByAttribute); + } + + /// + /// Validates that all instances on an attribute provider are valid (i.e. the linker recorded a marked dependency described in the attribute) + /// is the attribute provider that may have a , and is the 'FullName' of . + /// + void VerifyKeptByAttributes (ICustomAttributeProvider src, string attributeProviderFullName) + { + foreach (var keptByAttribute in src.CustomAttributes.Where (ca => ca.AttributeType.IsTypeOf ())) + VerifyKeptByAttribute (attributeProviderFullName, keptByAttribute); + } + + void VerifyKeptByAttribute (string keptAttributeProviderName, CustomAttribute attribute) + { + // public KeptByAttribute (string dependencyProvider, string reason) { } + // public KeptByAttribute (Type dependencyProvider, string reason) { } + // public KeptByAttribute (Type dependencyProvider, string memberName, string reason) { } + + Assert.AreEqual (nameof (KeptByAttribute), attribute.AttributeType.Name); + + // Create the expected TestDependencyRecorder.Dependency that should be in the recorded dependencies + TestDependencyRecorder.Dependency expectedDependency = new (); + expectedDependency.Target = keptAttributeProviderName; + expectedDependency.Marked = true; + if (attribute.ConstructorArguments.Count == 2) { + // public KeptByAttribute (string dependencyProvider, string reason) { } + // public KeptByAttribute (Type dependencyProvider, string reason) { } + if (attribute.ConstructorArguments[0].Type.IsTypeOf ()) + expectedDependency.Source = (string) attribute.ConstructorArguments[0].Value; + else if (attribute.ConstructorArguments[0].Type.IsTypeOf ()) + expectedDependency.Source = ((TypeDefinition) attribute.ConstructorArguments[0].Value).FullName; + else + throw new NotImplementedException ("Unexpected KeptByAttribute ctor variant"); + + expectedDependency.DependencyKind = (string) attribute.ConstructorArguments[1].Value; + } else if (attribute.ConstructorArguments.Count == 3) { + // public KeptByAttribute (Type dependencyProvider, string memberName, string reason) { } + if (!attribute.ConstructorArguments[0].Type.IsTypeOf ()) + throw new NotImplementedException ("Unexpected KeptByAttribute ctor variant"); + var type = (TypeDefinition) attribute.ConstructorArguments[0].Value; + string memberName = (string) attribute.ConstructorArguments[1].Value; + var memberDefinition = type.AllMembers ().Where (m => m.Name == memberName).Single (); + expectedDependency.Source = memberDefinition.FullName; + expectedDependency.DependencyKind = (string) attribute.ConstructorArguments[2].Value; + } else { + throw new NotImplementedException ("Unexpected KeptByAttribute ctor variant"); + } + + foreach (var dep in this.linkedTestCase.Customizations.DependencyRecorder.Dependencies) { + if (dep == expectedDependency) { + return; + } + } + string errorMessage = $"{keptAttributeProviderName} was expected to be kept by {expectedDependency.Source} with reason {expectedDependency.DependencyKind.ToString ()}."; + Assert.Fail (errorMessage); + } + protected virtual void VerifyTypeDefinitionKept (TypeDefinition original, TypeDefinition linked) { if (linked == null) Assert.Fail ($"Type `{original}' should have been kept"); + VerifyKeptByAttributes (original, linked); if (!original.IsInterface) VerifyBaseType (original, linked); @@ -313,6 +381,7 @@ void VerifyFieldKept (FieldDefinition src, FieldDefinition linked) Assert.AreEqual (src?.Constant, linked?.Constant, $"Field `{src}' value"); + VerifyKeptByAttributes (src, linked); VerifyPseudoAttributes (src, linked); VerifyCustomAttributes (src, linked); } @@ -335,6 +404,7 @@ void VerifyProperty (PropertyDefinition src, PropertyDefinition linked, TypeDefi Assert.AreEqual (src?.Constant, linked?.Constant, $"Property `{src}' value"); + VerifyKeptByAttributes (src, linked); VerifyPseudoAttributes (src, linked); VerifyCustomAttributes (src, linked); } @@ -367,6 +437,7 @@ void VerifyEvent (EventDefinition src, EventDefinition linked, TypeDefinition li linkedMembers.Remove (src.RemoveMethod.FullName); } + VerifyKeptByAttributes (src, linked); VerifyPseudoAttributes (src, linked); VerifyCustomAttributes (src, linked); } @@ -430,6 +501,7 @@ protected virtual void VerifyMethodKept (MethodDefinition src, MethodDefinition VerifySecurityAttributes (src, linked); VerifyArrayInitializers (src, linked); VerifyMethodBody (src, linked); + VerifyKeptByAttributes (src, linked); } protected virtual void VerifyMethodBody (MethodDefinition src, MethodDefinition linked) @@ -989,7 +1061,7 @@ protected virtual bool ShouldMethodBeKept (MethodDefinition method) protected virtual bool ShouldBeKept (T member, string signature = null) where T : MemberReference, ICustomAttributeProvider { - if (member.HasAttribute (nameof (KeptAttribute))) + if (member.HasAttribute (nameof (KeptAttribute)) || member.HasAttribute (nameof (KeptByAttribute))) return true; ICustomAttributeProvider cap = (ICustomAttributeProvider) member.DeclaringType; diff --git a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs index 4a7de6de60bd..af4c6e22c72a 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs @@ -94,7 +94,7 @@ public virtual void Check (LinkedTestCaseResult linkResult) PerformOutputSymbolChecks (original, linkResult.OutputAssemblyPath.Parent); if (!HasAttribute (original.MainModule.GetType (linkResult.TestCase.ReconstructedFullTypeName), nameof (SkipKeptItemsValidationAttribute))) { - CreateAssemblyChecker (original, linked).Verify (); + CreateAssemblyChecker (original, linked, linkResult).Verify (); } } @@ -106,9 +106,9 @@ public virtual void Check (LinkedTestCaseResult linkResult) } } - protected virtual AssemblyChecker CreateAssemblyChecker (AssemblyDefinition original, AssemblyDefinition linked) + protected virtual AssemblyChecker CreateAssemblyChecker (AssemblyDefinition original, AssemblyDefinition linked, LinkedTestCaseResult linkedTestCase) { - return new AssemblyChecker (original, linked); + return new AssemblyChecker (original, linked, linkedTestCase); } void InitializeResolvers (LinkedTestCaseResult linkedResult) diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadataProvider.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadataProvider.cs index a3290c4f662b..6f0ba6d0c1f9 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadataProvider.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadataProvider.cs @@ -89,8 +89,10 @@ public virtual TestCaseLinkerOptions GetLinkerOptions (NPath inputPath) public virtual void CustomizeLinker (LinkerDriver linker, LinkerCustomizations customizations) { - if (_testCaseTypeDefinition.CustomAttributes.Any (attr => - attr.AttributeType.Name == nameof (DependencyRecordedAttribute))) { + if (!_testCaseTypeDefinition.CustomAttributes.Any (a => a.AttributeType.IsTypeOf ()) + || _testCaseTypeDefinition.CustomAttributes.Any (attr => + attr.AttributeType.Name == nameof (DependencyRecordedAttribute) + || attr.AttributeType.Name == nameof (KeptByAttribute))) { customizations.DependencyRecorder = new TestDependencyRecorder (); customizations.CustomizeContext += context => { context.Tracer.AddRecorder (customizations.DependencyRecorder); diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestDependencyRecorder.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestDependencyRecorder.cs index cbf41bbe9684..643832414e3e 100644 --- a/test/Mono.Linker.Tests/TestCasesRunner/TestDependencyRecorder.cs +++ b/test/Mono.Linker.Tests/TestCasesRunner/TestDependencyRecorder.cs @@ -7,11 +7,12 @@ namespace Mono.Linker.Tests.TestCasesRunner { public class TestDependencyRecorder : IDependencyRecorder { - public struct Dependency + public record struct Dependency { public string Source; public string Target; public bool Marked; + public string DependencyKind; } public List Dependencies = new List (); @@ -30,7 +31,8 @@ public void RecordDependency (object target, in DependencyInfo reason, bool mark Dependencies.Add (new Dependency () { Source = reason.Source?.ToString (), Target = target.ToString (), - Marked = marked + Marked = marked, + DependencyKind = reason.Kind.ToString () }); }