Trim parameter names metadata when they are not needed. (#1988)

Co-authored-by: Vitek Karas <vitek.karas@microsoft.com>

14 files changed, 227 insertions, 20 deletions

diff --git a/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs b/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs
index 3b70208fa..23544a538 100644
--- a/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs
+++ b/test/ILLink.Tasks.Tests/ILLink.Tasks.Tests.cs
@@ -522,6 +522,19 @@ namespace ILLink.Tasks.Tests
 			}
 		}
 
+
+		[Fact]
+		public void TestKeepCustomMetadata ()
+		{
+			var task = new MockTask () {
+				KeepMetadata = new ITaskItem[] { new TaskItem ("parametername") }
+			};
+
+			using (var driver = task.CreateDriver ()) {
+				Assert.Equal (MetadataTrimming.None, driver.Context.MetadataTrimming);
+			}
+		}
+
 		[Theory]
 		[InlineData ("copy")]
 		[InlineData ("link")]
diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedNameValueAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedNameValueAttribute.cs
new file mode 100644
index 000000000..1e25715fe
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedNameValueAttribute.cs
@@ -0,0 +1,12 @@
+using System;
+
+namespace Mono.Linker.Tests.Cases.Expectations.Assertions
+{
+	/// <summary>
+	/// Verifies that name of the member is removed
+	/// </summary>
+	[AttributeUsage (AttributeTargets.All, AllowMultiple = false, Inherited = false)]
+	public class RemovedNameValueAttribute : BaseExpectedLinkedBehaviorAttribute
+	{
+	}
+}
diff --git a/test/Mono.Linker.Tests.Cases.Expectations/Assertions/VerifyMetadataNamesAttribute.cs b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/VerifyMetadataNamesAttribute.cs
new file mode 100644
index 000000000..8b8df3d33
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases.Expectations/Assertions/VerifyMetadataNamesAttribute.cs
@@ -0,0 +1,12 @@
+using System;
+
+namespace Mono.Linker.Tests.Cases.Expectations.Assertions
+{
+	[AttributeUsage (AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
+	public class VerifyMetadataNamesAttribute : BaseExpectedLinkedBehaviorAttribute
+	{
+		public VerifyMetadataNamesAttribute ()
+		{
+		}
+	}
+}
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs
index b56225c9b..3889225a2 100644
--- a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs
@@ -468,7 +468,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 			typeof (T).RequiresNone ();
 		}
 
-		[UnrecognizedReflectionAccessPattern (typeof (GenericParameterDataFlow), nameof (MethodRequiresPublicMethods) + "<T>()::T", messageCode: "IL2091")]
+		[UnrecognizedReflectionAccessPattern (typeof (GenericParameterDataFlow), nameof (MethodRequiresPublicMethods) + "<#1>()::T", messageCode: "IL2091")]
 		static void MethodRequiresPublicFieldsPassThrough<
 			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] T> ()
 		{
@@ -477,8 +477,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 			MethodRequiresNothing<T> ();
 		}
 
-		[UnrecognizedReflectionAccessPattern (typeof (GenericParameterDataFlow), nameof (MethodRequiresPublicFields) + "<T>()::T", messageCode: "IL2091")]
-		[UnrecognizedReflectionAccessPattern (typeof (GenericParameterDataFlow), nameof (MethodRequiresPublicMethods) + "<T>()::T", messageCode: "IL2091")]
+		[UnrecognizedReflectionAccessPattern (typeof (GenericParameterDataFlow), nameof (MethodRequiresPublicFields) + "<#1>()::T", messageCode: "IL2091")]
+		[UnrecognizedReflectionAccessPattern (typeof (GenericParameterDataFlow), nameof (MethodRequiresPublicMethods) + "<#1>()::T", messageCode: "IL2091")]
 		static void MethodRequiresNothingPassThrough<T> ()
 		{
 			MethodRequiresPublicFields<T> ();
@@ -550,7 +550,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 		class TypeWithInstantiatedGenericMethodViaGenericParameter<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] TOuter>
 			: BaseTypeWithGenericMethod, IInterfaceWithGenericMethod
 		{
-			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.StaticRequiresPublicMethods) + "<T>()::T",
+			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.StaticRequiresPublicMethods) + "<#1>()::T",
 				messageCode: "IL2091", message: new string[] {
 					"TInner",
 					"Mono.Linker.Tests.Cases.DataFlow.GenericParameterDataFlow.TypeWithInstantiatedGenericMethodViaGenericParameter<TOuter>.StaticRequiresPublicFields<TInner>()",
@@ -562,7 +562,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				StaticRequiresPublicMethods<TInner> ();
 			}
 
-			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.StaticRequiresPublicMethods) + "<T>()::T",
+			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.StaticRequiresPublicMethods) + "<#1>()::T",
 				messageCode: "IL2091", message: new string[] {
 					"TOuter",
 					"Mono.Linker.Tests.Cases.DataFlow.GenericParameterDataFlow.TypeWithInstantiatedGenericMethodViaGenericParameter<TOuter>",
@@ -580,7 +580,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				StaticRequiresMultipleGenericParams<TOuter, TestType> ();
 			}
 
-			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.StaticRequiresMultipleGenericParams) + "<TFields,TMethods>()::TMethods",
+			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.StaticRequiresMultipleGenericParams) + "<#2>()::TMethods",
 				messageCode: "IL2091", message: new string[] {
 					"TOuter",
 					"Mono.Linker.Tests.Cases.DataFlow.GenericParameterDataFlow.TypeWithInstantiatedGenericMethodViaGenericParameter<TOuter>",
@@ -591,7 +591,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				StaticRequiresMultipleGenericParams<TestType, TOuter> ();
 			}
 
-			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.InstanceRequiresPublicMethods) + "<T>()::T",
+			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.InstanceRequiresPublicMethods) + "<#1>()::T",
 				messageCode: "IL2091", message: new string[] {
 					"TInner",
 					"Mono.Linker.Tests.Cases.DataFlow.GenericParameterDataFlow.TypeWithInstantiatedGenericMethodViaGenericParameter<TOuter>.InstanceRequiresPublicFields<TInner>()",
@@ -603,7 +603,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				InstanceRequiresPublicMethods<TInner> ();
 			}
 
-			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.InstanceRequiresPublicMethods) + "<T>()::T",
+			[UnrecognizedReflectionAccessPattern (typeof (BaseTypeWithGenericMethod), nameof (BaseTypeWithGenericMethod.InstanceRequiresPublicMethods) + "<#1>()::T",
 				messageCode: "IL2091", message: new string[] {
 					"TOuter",
 					"Mono.Linker.Tests.Cases.DataFlow.GenericParameterDataFlow.TypeWithInstantiatedGenericMethodViaGenericParameter<TOuter>",
@@ -639,7 +639,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				typeof (T).RequiresPublicMethods ();
 			}
 
-			[UnrecognizedReflectionAccessPattern (typeof (IInterfaceWithGenericMethod), nameof (IInterfaceWithGenericMethod.InterfaceRequiresPublicMethods) + "<T>()::T",
+			[UnrecognizedReflectionAccessPattern (typeof (IInterfaceWithGenericMethod), nameof (IInterfaceWithGenericMethod.InterfaceRequiresPublicMethods) + "<#1>()::T",
 				messageCode: "IL2091", message: new string[] {
 					"TOuter",
 					"Mono.Linker.Tests.Cases.DataFlow.GenericParameterDataFlow.TypeWithInstantiatedGenericMethodViaGenericParameter<TOuter>",
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/MethodParametersDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/MethodParametersDataFlow.cs
index fe11b6b03..01047bc7a 100644
--- a/test/Mono.Linker.Tests.Cases/DataFlow/MethodParametersDataFlow.cs
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/MethodParametersDataFlow.cs
@@ -14,6 +14,7 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 	// Note: this test's goal is to validate that the product correctly reports unrecognized patterns
 	//   - so the main validation is done by the UnrecognizedReflectionAccessPattern attributes.
 	[SkipKeptItemsValidation]
+	[SetupLinkerArgument ("--keep-metadata", "parametername")]
 	public class MethodParametersDataFlow
 	{
 		public static void Main ()
diff --git a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs
index af105d086..e9e7ecf29 100644
--- a/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs
+++ b/test/Mono.Linker.Tests.Cases/Libraries/RootLibrary.cs
@@ -8,6 +8,7 @@ namespace Mono.Linker.Tests.Cases.Libraries
 {
 	[SetupLinkerArgument ("-a", "test.exe", "library")]
 	[SetupLinkerArgument ("--enable-opt", "ipconstprop")]
+	[VerifyMetadataNames]
 	public class RootLibrary
 	{
 		private int field;
diff --git a/test/Mono.Linker.Tests.Cases/Metadata/NamesAreKept.cs b/test/Mono.Linker.Tests.Cases/Metadata/NamesAreKept.cs
new file mode 100644
index 000000000..5dabdf1f9
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/Metadata/NamesAreKept.cs
@@ -0,0 +1,29 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.Metadata
+{
+	[SetupLinkerArgument ("--keep-metadata", "parametername")]
+	[VerifyMetadataNames]
+	public class NamesAreKept
+	{
+		public static void Main ()
+		{
+			var n = new N (5);
+			N.Foo ("aa");
+		}
+
+		class N
+		{
+			[Kept]
+			public N (int arg)
+			{
+			}
+
+			[Kept]
+			public static void Foo (string str)
+			{
+			}
+		}
+	}
+}
diff --git a/test/Mono.Linker.Tests.Cases/Metadata/NamesAreRemoved.cs b/test/Mono.Linker.Tests.Cases/Metadata/NamesAreRemoved.cs
new file mode 100644
index 000000000..af793ee6c
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/Metadata/NamesAreRemoved.cs
@@ -0,0 +1,27 @@
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.Metadata
+{
+	public class NamesAreRemoved
+	{
+		public static void Main ()
+		{
+			var n = new N (5);
+			N.Foo (null, 1);
+		}
+
+		class N
+		{
+			[Kept]
+			public N ([RemovedNameValueAttribute] int arg)
+			{
+			}
+
+			[Kept]
+			public static void Foo ([RemovedNameValueAttribute] string str, [RemovedNameValueAttribute] long _)
+			{
+			}
+		}
+	}
+}
diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs b/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs
index 6da378265..7b5f97803 100644
--- a/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs
+++ b/test/Mono.Linker.Tests.Cases/Reflection/ActivatorCreateInstance.cs
@@ -454,10 +454,10 @@ namespace Mono.Linker.Tests.Cases.Reflection
 		}
 
 		[Kept]
-		[UnrecognizedReflectionAccessPattern (typeof (Activator), nameof (Activator.CreateInstance) + "<T>", new Type[0], messageCode: "IL2091")]
+		[UnrecognizedReflectionAccessPattern (typeof (Activator), nameof (Activator.CreateInstance) + "<#1>", new Type[0], messageCode: "IL2091")]
 #if NETCOREAPP
 		// Warnings are currently duplicated in NETCORE (annotation and intrinsics) - but they're not identical in this case, so we have to list both cases
-		[UnrecognizedReflectionAccessPattern (typeof (Activator), nameof (Activator.CreateInstance) + "<T>()::T", messageCode: "IL2091")]
+		[UnrecognizedReflectionAccessPattern (typeof (Activator), nameof (Activator.CreateInstance) + "<#1>()::T", messageCode: "IL2091")]
 #endif
 		private static void TestCreateInstanceOfTWithNoConstraint<T> ()
 		{
diff --git a/test/Mono.Linker.Tests.Cases/Reflection/ParametersUsedViaReflection.cs b/test/Mono.Linker.Tests.Cases/Reflection/ParametersUsedViaReflection.cs
new file mode 100644
index 000000000..57b16569a
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/Reflection/ParametersUsedViaReflection.cs
@@ -0,0 +1,75 @@
+using System;
+using System.Reflection;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+namespace Mono.Linker.Tests.Cases.Reflection
+{
+	public class ParametersUsedViaReflection
+	{
+		public static void Main ()
+		{
+			TestMethodParameters ();
+			TestClassParameters ();
+		}
+
+		[Kept]
+		static void TestMethodParameters ()
+		{
+			var method = typeof (GetMethod_Name).GetMethod ("OnlyCalledViaReflection");
+			var name = method.GetParameters ()[0].Name;
+
+			GetMethod_Name.CalledDirectly (11);
+			GetMethod_Name.CalledDirectly2<string> (1);
+		}
+
+		[Kept]
+		static void TestClassParameters ()
+		{
+			var type = Type.GetType ("Mono.Linker.Tests.Cases.Reflection.ParametersUsedViaReflection/GenericClass1`1");
+
+			var type2 = new GenericClass2<int> ();
+		}
+
+		[Kept]
+		class GetMethod_Name
+		{
+			[Kept]
+			public static int OnlyCalledViaReflection (int firstName)
+			{
+				return 2;
+			}
+
+			[Kept]
+			public static int OnlyCalledViaReflection (int arg1, int arg2)
+			{
+				return 3;
+			}
+
+			[Kept]
+			public static void CalledDirectly ([RemovedNameValue] int firstArg)
+			{
+			}
+
+			[Kept]
+			public static void CalledDirectly2</*[RemovedNameValue]*/LongGenericName> ([RemovedNameValue] int firstArg)
+			{
+			}
+		}
+
+		[Kept]
+		public class GenericClass1<TKey>
+		{
+		}
+
+		[Kept]
+		public class GenericClass2</*[RemovedNameValue]*/TRKey>
+		{
+			[Kept]
+			public GenericClass2 ()
+			{
+			}
+		}
+	}
+
+}
diff --git a/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs b/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
index f18803377..c6ed39935 100644
--- a/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
+++ b/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
@@ -126,12 +126,7 @@ namespace Mono.Linker.Tests.Extensions
 			var builder = new StringBuilder ();
 			builder.Append (method.Name);
 			if (method.HasGenericParameters) {
-				builder.Append ('<');
-
-				for (int i = 0; i < method.GenericParameters.Count - 1; i++)
-					builder.Append ($"{method.GenericParameters[i]},");
-
-				builder.Append ($"{method.GenericParameters[method.GenericParameters.Count - 1]}>");
+				builder.Append ($"<#{method.GenericParameters.Count}>");
 			}
 
 			builder.Append ("(");
diff --git a/test/Mono.Linker.Tests/TestCases/TestDatabase.cs b/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
index 73e5ed671..fcab71eb7 100644
--- a/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
+++ b/test/Mono.Linker.Tests/TestCases/TestDatabase.cs
@@ -212,6 +212,11 @@ namespace Mono.Linker.Tests.TestCases
 			return NUnitCasesBySuiteName ("LinkAttributes");
 		}
 
+		public static IEnumerable<TestCaseData> MetadataTests ()
+		{
+			return NUnitCasesBySuiteName ("Metadata");
+		}
+
 		public static TestCaseCollector CreateCollector ()
 		{
 			GetDirectoryPaths (out string rootSourceDirectory, out string testCaseAssemblyPath);
diff --git a/test/Mono.Linker.Tests/TestCases/TestSuites.cs b/test/Mono.Linker.Tests/TestCases/TestSuites.cs
index ae2d45090..3860cccbc 100644
--- a/test/Mono.Linker.Tests/TestCases/TestSuites.cs
+++ b/test/Mono.Linker.Tests/TestCases/TestSuites.cs
@@ -254,6 +254,12 @@ namespace Mono.Linker.Tests.TestCases
 			Run (testCase);
 		}
 
+		[TestCaseSource (typeof (TestDatabase), nameof (TestDatabase.MetadataTests))]
+		public void MetadataTests (TestCase testCase)
+		{
+			Run (testCase);
+		}
+
 		protected virtual void Run (TestCase testCase)
 		{
 			var runner = new TestRunner (new ObjectFactory ());
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
index 421e4ac1a..a4a4b1f64 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
@@ -18,11 +18,15 @@ namespace Mono.Linker.Tests.TestCasesRunner
 		readonly HashSet<string> verifiedGeneratedFields = new HashSet<string> ();
 		readonly HashSet<string> verifiedEventMethods = new HashSet<string> ();
 		readonly HashSet<string> verifiedGeneratedTypes = new HashSet<string> ();
+		bool checkNames;
 
 		public AssemblyChecker (AssemblyDefinition original, AssemblyDefinition linked)
 		{
 			this.originalAssembly = original;
 			this.linkedAssembly = linked;
+
+			checkNames = original.MainModule.GetTypeReferences ().Any (attr =>
+				attr.Name == nameof (RemovedNameValueAttribute));
 		}
 
 		public void Verify ()
@@ -107,8 +111,13 @@ namespace Mono.Linker.Tests.TestCasesRunner
 				return;
 			}
 
+			bool prev = checkNames;
+			checkNames |= original.HasAttribute (nameof (VerifyMetadataNamesAttribute));
+
 			VerifyTypeDefinitionKept (original, linked);
 
+			checkNames = prev;
+
 			if (original.HasAttribute (nameof (CreatedMemberAttribute))) {
 				foreach (var attr in original.CustomAttributes.Where (l => l.AttributeType.Name == nameof (CreatedMemberAttribute))) {
 					var newName = original.FullName + "::" + attr.ConstructorArguments[0].Value.ToString ();
@@ -163,7 +172,8 @@ namespace Mono.Linker.Tests.TestCasesRunner
 			foreach (var m in original.Methods) {
 				if (verifiedEventMethods.Contains (m.FullName))
 					continue;
-				VerifyMethod (m, linked?.Methods.FirstOrDefault (l => m.GetSignature () == l.GetSignature ()));
+				var msign = m.GetSignature ();
+				VerifyMethod (m, linked?.Methods.FirstOrDefault (l => msign == l.GetSignature ()));
 				linkedMembers.Remove (m.FullName);
 			}
 		}
@@ -765,7 +775,18 @@ namespace Mono.Linker.Tests.TestCasesRunner
 			if (src.HasGenericParameters) {
 				for (int i = 0; i < src.GenericParameters.Count; ++i) {
 					// TODO: Verify constraints
-					VerifyCustomAttributes (src.GenericParameters[i], linked.GenericParameters[i]);
+					var srcp = src.GenericParameters[i];
+					var lnkp = linked.GenericParameters[i];
+					VerifyCustomAttributes (srcp, lnkp);
+
+					if (checkNames) {
+						if (srcp.CustomAttributes.Any (attr => attr.AttributeType.Name == nameof (RemovedNameValueAttribute))) {
+							string name = (src.GenericParameterType == GenericParameterType.Method ? "!!" : "!") + srcp.Position;
+							Assert.AreEqual (name, lnkp.Name, "Expected empty generic parameter name");
+						} else {
+							Assert.AreEqual (srcp.Name, lnkp.Name, "Mismatch in generic parameter name");
+						}
+					}
 				}
 			}
 		}
@@ -775,7 +796,17 @@ namespace Mono.Linker.Tests.TestCasesRunner
 			Assert.AreEqual (src.HasParameters, linked.HasParameters);
 			if (src.HasParameters) {
 				for (int i = 0; i < src.Parameters.Count; ++i) {
-					VerifyCustomAttributes (src.Parameters[i], linked.Parameters[i]);
+					var srcp = src.Parameters[i];
+					var lnkp = linked.Parameters[i];
+
+					VerifyCustomAttributes (srcp, lnkp);
+
+					if (checkNames) {
+						if (srcp.CustomAttributes.Any (attr => attr.AttributeType.Name == nameof (RemovedNameValueAttribute)))
+							Assert.IsEmpty (lnkp.Name, "Expected empty parameter name");
+						else
+							Assert.AreEqual (srcp.Name, lnkp.Name, "Mismatch in parameter name");
+					}
 				}
 			}
 		}