Introduce IReflectionPatternRecorder to allow recording of patte… (#907)

* Introduce IReflectionPatternRecorder to allow external recording of patterns

The Mark step performs pattern matching for some reflection methods to be able to figure out depenedncies even across reflection calls. Currently it only writes to log when it finds a pattern it can't recognize.

This change introduces a new interface IReflectionPatternRecorder which is called every time the pattern matching either sucessfuly recognizes a pattern or fails to do so. By default this is implemented to log on failure (just like before the change).

This allows additional tools to record the failures and or the successes in a different way.

Added support to test this functionality and modified a couple of tests to validate the basics.

The biggest change is in the MarkStep where the pattern matching has been refactored a little bit to fullfill a promise of always reporting success/failure for a given callsite. This is important for some consumers to be able to handle multiple call sites to the same reflection method in calling method body (without this we would have to somehow report the actual call site location which complicates things quite a bit on the consumption side).

* Fix build break on Mono

C# 7.3 doesn't support static local functions.

* Modify test attribute for pattern recognition to be strongly typed

* Fixes Activator.CreateInstance<T>() pattern matching

This pattern is not supported (recognized), so it should be reported as such. The existing code had two bugs:
- It didn't report it through the new interface
- It didn't match the CreateInstance<T> correctly and would never actually trigger

As such this effectively adds a new unrecognized pattern to the linker, but given that before it only reported these as low importance output messages it's a change I think we should take.

Adds a test for this as well.

* Refactor the reflection pattern matching per PR feedback

5 files changed, 202 insertions, 1 deletions

diff --git a/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs b/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
index 747abf570..60781468d 100644
--- a/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
+++ b/test/Mono.Linker.Tests/Extensions/CecilExtensions.cs
@@ -56,6 +56,36 @@ namespace Mono.Linker.Tests.Extensions {
 				yield return @event;
 		}
 
+		public static IEnumerable<MethodDefinition> AllMethods (this TypeDefinition type)
+		{
+			foreach (var m in type.AllMembers()) {
+				switch (m) {
+					case MethodDefinition method: 
+						yield return method;
+						break;
+					case PropertyDefinition @property:
+						if (@property.GetMethod != null)
+							yield return @property.GetMethod;
+
+						if (@property.SetMethod != null)
+							yield return @property.SetMethod;
+
+						break;
+					case EventDefinition @event:
+						if (@event.AddMethod != null)
+							yield return @event.AddMethod;
+
+						if (@event.RemoveMethod != null)
+							yield return @event.RemoveMethod;
+
+						break;
+
+					default:
+						break;
+				}
+			}
+		}
+
 		public static bool HasAttribute (this ICustomAttributeProvider provider, string name)
 		{
 			return provider.CustomAttributes.Any (ca => ca.AttributeType.Name == name);
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/LinkerCustomizations.cs b/test/Mono.Linker.Tests/TestCasesRunner/LinkerCustomizations.cs
index d0b65edec..bb599df7f 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/LinkerCustomizations.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/LinkerCustomizations.cs
@@ -10,6 +10,8 @@ namespace Mono.Linker.Tests.TestCasesRunner
 	{
 		public TestDependencyRecorder DependencyRecorder { get; set; }
 
+		public TestReflectionPatternRecorder ReflectionPatternRecorder { get; set; }
+
 		public event Action<LinkContext> CustomizeContext;
 
 		public void CustomizeLinkContext(LinkContext context)
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
index f2cf4efd1..bb1de9289 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
@@ -168,6 +168,7 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 		{
 			VerifyLoggedMessages(original, linkResult.Logger);
 			VerifyRecordedDependencies (original, linkResult.Customizations.DependencyRecorder);
+			VerifyRecordedReflectionPatterns (original, linkResult.Customizations.ReflectionPatternRecorder);
 		}
 
 		protected virtual void InitialChecking (LinkedTestCaseResult linkResult, AssemblyDefinition original, AssemblyDefinition linked)
@@ -644,7 +645,7 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 								$"Expected to find recorded dependency '{expectedSource} -> {expectedTarget} {expectedMarked ?? string.Empty}'{Environment.NewLine}" +
 								$"Potential dependencies matching the target: {Environment.NewLine}{targetCandidates}{Environment.NewLine}" +
 								$"Potential dependencies matching the source: {Environment.NewLine}{sourceCandidates}{Environment.NewLine}" +
-								$"If there's no matches, try to specify just a part of the source/target name and rerun the test.");
+								$"If there's no matches, try to specify just a part of the source/target name and rerun the test to get potential matches.");
 						}
 					}
 				}
@@ -656,6 +657,128 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 			}
 		}
 
+		void VerifyRecordedReflectionPatterns (AssemblyDefinition original, TestReflectionPatternRecorder reflectionPatternRecorder)
+		{
+			foreach (var expectedSourceMethodDefinition in original.MainModule.AllDefinedTypes ().SelectMany (t => t.AllMethods ())) {
+				foreach (var attr in expectedSourceMethodDefinition.CustomAttributes) {
+					if (attr.AttributeType.Resolve ().Name == nameof (RecognizedReflectionAccessPatternAttribute)) {
+						string expectedSourceMethod = GetFullMemberNameFromDefinition (expectedSourceMethodDefinition);
+						string expectedReflectionMethod = GetFullMemberNameFromReflectionAccessPatternAttribute (attr, constructorArgumentsOffset: 0);
+						string expectedAccessedItem = GetFullMemberNameFromReflectionAccessPatternAttribute (attr, constructorArgumentsOffset: 3);
+
+						if (!reflectionPatternRecorder.RecognizedPatterns.Any (pattern => {
+							if (GetFullMemberNameFromDefinition (pattern.SourceMethod) != expectedSourceMethod)
+								return false;
+
+							if (GetFullMemberNameFromDefinition (pattern.ReflectionMethod) != expectedReflectionMethod)
+								return false;
+
+							if (GetFullMemberNameFromDefinition (pattern.AccessedItem) != expectedAccessedItem)
+								return false;
+
+							return true;
+						})) {
+							string sourceMethodCandidates = string.Join (Environment.NewLine, reflectionPatternRecorder.RecognizedPatterns
+								.Where (p => GetFullMemberNameFromDefinition (p.SourceMethod).ToLowerInvariant ().Contains (expectedSourceMethod.ToLowerInvariant ()))
+								.Select (p => "\t" + RecognizedReflectionAccessPatternToString (p)));
+							string reflectionMethodCandidates = string.Join (Environment.NewLine, reflectionPatternRecorder.RecognizedPatterns
+								.Where (p => GetFullMemberNameFromDefinition (p.ReflectionMethod).ToLowerInvariant ().Contains (expectedReflectionMethod.ToLowerInvariant ()))
+								.Select (p => "\t" + RecognizedReflectionAccessPatternToString (p)));
+
+							Assert.Fail (
+								$"Expected to find recognized reflection access pattern '{expectedSourceMethod.ToString()}: Call to {expectedReflectionMethod} accessed {expectedAccessedItem}'{Environment.NewLine}" +
+								$"Potential patterns matching the source method: {Environment.NewLine}{sourceMethodCandidates}{Environment.NewLine}" +
+								$"Potential patterns matching the reflection method: {Environment.NewLine}{reflectionMethodCandidates}{Environment.NewLine}" +
+								$"If there's no matches, try to specify just a part of the source method or reflection method name and rerun the test to get potential matches.");
+						}
+					} else if (attr.AttributeType.Resolve ().Name == nameof (UnrecognizedReflectionAccessPatternAttribute)) {
+						string expectedSourceMethod = GetFullMemberNameFromDefinition (expectedSourceMethodDefinition);
+						string expectedReflectionMethod = GetFullMemberNameFromReflectionAccessPatternAttribute (attr, constructorArgumentsOffset: 0);
+						string expectedMessage = (string)attr.ConstructorArguments [3].Value;
+
+						if (!reflectionPatternRecorder.UnrecognizedPatterns.Any (pattern => {
+							if (GetFullMemberNameFromDefinition (pattern.SourceMethod) != expectedSourceMethod)
+								return false;
+
+							if (GetFullMemberNameFromDefinition (pattern.ReflectionMethod) != expectedReflectionMethod)
+								return false;
+
+							if (expectedMessage != null && pattern.Message != expectedMessage)
+								return false;
+
+							return true;
+						})) {
+							string sourceMethodCandidates = string.Join (Environment.NewLine, reflectionPatternRecorder.UnrecognizedPatterns
+								.Where (p => GetFullMemberNameFromDefinition (p.SourceMethod).ToLowerInvariant ().Contains (expectedSourceMethod.ToLowerInvariant ()))
+								.Select (p => "\t" + UnrecognizedReflectionAccessPatternToString (p)));
+							string reflectionMethodCandidates = string.Join (Environment.NewLine, reflectionPatternRecorder.UnrecognizedPatterns
+								.Where (p => GetFullMemberNameFromDefinition (p.ReflectionMethod).ToLowerInvariant ().Contains (expectedReflectionMethod.ToLowerInvariant ()))
+								.Select (p => "\t" + UnrecognizedReflectionAccessPatternToString (p)));
+
+							Assert.Fail (
+								$"Expected to find unrecognized reflection access pattern '{expectedSourceMethod}: Call to {expectedReflectionMethod} unrecognized {expectedMessage ?? string.Empty}'{Environment.NewLine}" +
+								$"Potential patterns matching the source method: {Environment.NewLine}{sourceMethodCandidates}{Environment.NewLine}" +
+								$"Potential patterns matching the reflection method: {Environment.NewLine}{reflectionMethodCandidates}{Environment.NewLine}" +
+								$"If there's no matches, try to specify just a part of the source method or reflection method name and rerun the test to get potential matches.");
+						}
+					}
+				}
+			}
+		}
+
+		static string GetFullMemberNameFromReflectionAccessPatternAttribute (CustomAttribute attr, int constructorArgumentsOffset) 
+		{
+			var type = attr.ConstructorArguments [constructorArgumentsOffset].Value;
+			var memberName = (string)attr.ConstructorArguments [constructorArgumentsOffset + 1].Value;
+			var parameterTypes = (CustomAttributeArgument[])attr.ConstructorArguments [constructorArgumentsOffset + 2].Value;
+
+			string fullName = type.ToString ();
+			if (memberName == null) {
+				return fullName;
+			}
+
+			fullName += "::" + memberName;
+			if (parameterTypes != null) {
+				fullName += "(" + string.Join (",", parameterTypes.Select (t => t.Value.ToString ())) + ")";
+			}
+
+			return fullName;
+		}
+
+		static string GetFullMemberNameFromDefinition (IMemberDefinition member)
+		{
+			// Method which basically returns the same as member.ToString() but without the return type
+			// of a method (if it's a method).
+			// We need this as the GetFullMemberNameFromReflectionAccessPatternAttribute can't guess the return type
+			// as it would have to actually resolve the referenced method, which is very expensive and no necessary
+			// for the tests to work (the return types are redundant piece of information anyway).
+
+			if (member is TypeDefinition) {
+				return member.FullName;
+			}
+
+			string fullName = member.DeclaringType.FullName + "::";
+			if (member is MethodDefinition method) {
+				fullName += method.GetSignature ();
+			}
+			else {
+				fullName += member.Name;
+			}
+
+			return fullName;
+		}
+
+		static string RecognizedReflectionAccessPatternToString (TestReflectionPatternRecorder.ReflectionAccessPattern pattern)
+		{
+			return $"{GetFullMemberNameFromDefinition (pattern.SourceMethod)}: Call to {GetFullMemberNameFromDefinition (pattern.ReflectionMethod)} accessed {GetFullMemberNameFromDefinition (pattern.AccessedItem)}";
+		}
+
+		static string UnrecognizedReflectionAccessPatternToString (TestReflectionPatternRecorder.ReflectionAccessPattern pattern)
+		{
+			return $"{GetFullMemberNameFromDefinition (pattern.SourceMethod)}: Call to {GetFullMemberNameFromDefinition (pattern.ReflectionMethod)} unrecognized '{pattern.Message}'";
+		}
+
+
 		protected TypeDefinition GetOriginalTypeFromInAssemblyAttribute (CustomAttribute inAssemblyAttribute)
 		{
 			return GetOriginalTypeFromInAssemblyAttribute (inAssemblyAttribute.ConstructorArguments [0].Value.ToString (), inAssemblyAttribute.ConstructorArguments [1].Value);
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs
index d7a1d983d..7d0f22da1 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs
@@ -75,6 +75,15 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 					context.Tracer.AddRecorder (customizations.DependencyRecorder);
 				};
 			}
+
+			if (_testCaseTypeDefinition.AllMethods().Any(method => method.CustomAttributes.Any (attr =>
+				attr.AttributeType.Name == nameof (RecognizedReflectionAccessPatternAttribute) ||
+				attr.AttributeType.Name == nameof (UnrecognizedReflectionAccessPatternAttribute)))) {
+				customizations.ReflectionPatternRecorder = new TestReflectionPatternRecorder ();
+				customizations.CustomizeContext += context => {
+					context.ReflectionPatternRecorder = customizations.ReflectionPatternRecorder;
+				};
+			};
 		}
 
 #if NETCOREAPP
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestReflectionPatternRecorder.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestReflectionPatternRecorder.cs
new file mode 100644
index 000000000..88b78ec3b
--- /dev/null
+++ b/test/Mono.Linker.Tests/TestCasesRunner/TestReflectionPatternRecorder.cs
@@ -0,0 +1,37 @@
+using Mono.Cecil;
+using System.Collections.Generic;
+
+namespace Mono.Linker.Tests.TestCasesRunner
+{
+	public class TestReflectionPatternRecorder : IReflectionPatternRecorder
+	{
+		public struct ReflectionAccessPattern
+		{
+			public MethodDefinition SourceMethod;
+			public MethodDefinition ReflectionMethod;
+			public IMemberDefinition AccessedItem;
+			public string Message;
+		}
+
+		public List<ReflectionAccessPattern> RecognizedPatterns = new List<ReflectionAccessPattern> ();
+		public List<ReflectionAccessPattern> UnrecognizedPatterns = new List<ReflectionAccessPattern> ();
+
+		public void RecognizedReflectionAccessPattern (MethodDefinition sourceMethod, MethodDefinition reflectionMethod, IMemberDefinition accessedItem)
+		{
+			RecognizedPatterns.Add (new ReflectionAccessPattern {
+				SourceMethod = sourceMethod,
+				ReflectionMethod = reflectionMethod,
+				AccessedItem = accessedItem
+			});
+		}
+
+		public void UnrecognizedReflectionAccessPattern (MethodDefinition sourceMethod, MethodDefinition reflectionMethod, string message)
+		{
+			UnrecognizedPatterns.Add (new ReflectionAccessPattern {
+				SourceMethod = sourceMethod,
+				ReflectionMethod = reflectionMethod,
+				Message = message
+			});
+		}
+	}
+}