Fix a bug in the reflection pattern recorder usage (#921)

* Fix a bug in the reflection pattern recorder usage

The code was written such that it would report almost all calls it didn't consider (or try to analyze) as unrecognized patterns.

This change adds the test infra to validate that no extra reflection patterns are recorded on a given test. It then uses this on two tests to validate the functionality.

The fix adds a boolean which tracks if the code is trying to analyze a given reflection pattern or not. It only reports unrecognized pattern if it did try to analyze a pattern, but didn't report any result explicitly.

* PR feedback and adding more tests

As per PR feedback added explicit reporting of unrecognized patterns in all branches. The "using" is now asserting if no result was reported (instead of reporting a generic unrecognized message).

Added many tests to improve the coverage of this code. Not all interesting code paths yet.

2 files changed, 32 insertions, 3 deletions

diff --git a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
index bb1de9289..df47260d7 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
@@ -676,6 +676,7 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 							if (GetFullMemberNameFromDefinition (pattern.AccessedItem) != expectedAccessedItem)
 								return false;
 
+							reflectionPatternRecorder.RecognizedPatterns.Remove (pattern);
 							return true;
 						})) {
 							string sourceMethodCandidates = string.Join (Environment.NewLine, reflectionPatternRecorder.RecognizedPatterns
@@ -706,6 +707,7 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 							if (expectedMessage != null && pattern.Message != expectedMessage)
 								return false;
 
+							reflectionPatternRecorder.UnrecognizedPatterns.Remove (pattern);
 							return true;
 						})) {
 							string sourceMethodCandidates = string.Join (Environment.NewLine, reflectionPatternRecorder.UnrecognizedPatterns
@@ -724,6 +726,31 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 					}
 				}
 			}
+
+			foreach (var typeToVerify in original.MainModule.AllDefinedTypes ()) {
+				foreach (var attr in typeToVerify.CustomAttributes) {
+					if (attr.AttributeType.Resolve ().Name == nameof (VerifyAllReflectionAccessPatternsAreValidatedAttribute)) {
+						// By now all verified recorded patterns were removed from the test recorder lists, so validate
+						// that there are no remaining patterns for this type.
+						var recognizedPatternsForType = reflectionPatternRecorder.RecognizedPatterns
+							.Where (pattern => pattern.SourceMethod.DeclaringType.FullName == typeToVerify.FullName);
+						var unrecognizedPatternsForType = reflectionPatternRecorder.UnrecognizedPatterns
+							.Where (pattern => pattern.SourceMethod.DeclaringType.FullName == typeToVerify.FullName);
+
+						if (recognizedPatternsForType.Any () || unrecognizedPatternsForType.Any ()) {
+							string recognizedPatterns = string.Join (Environment.NewLine, recognizedPatternsForType
+								.Select (p => "\t" + RecognizedReflectionAccessPatternToString (p)));
+							string unrecognizedPatterns = string.Join (Environment.NewLine, unrecognizedPatternsForType
+								.Select (p => "\t" + UnrecognizedReflectionAccessPatternToString (p)));
+
+							Assert.Fail (
+								$"All reflection patterns should be verified by test attributes for type {typeToVerify.FullName}, but some were not: {Environment.NewLine}" +
+								$"Recognized patterns which were not verified: {Environment.NewLine}{recognizedPatterns}{Environment.NewLine}" +
+								$"Unrecognized patterns which were not verified: {Environment.NewLine}{unrecognizedPatterns}{Environment.NewLine}");
+						}
+					}
+				}
+			}
 		}
 
 		static string GetFullMemberNameFromReflectionAccessPatternAttribute (CustomAttribute attr, int constructorArgumentsOffset) 
diff --git a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs
index 7d0f22da1..589ed2ce1 100644
--- a/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs
+++ b/test/Mono.Linker.Tests/TestCasesRunner/TestCaseMetadaProvider.cs
@@ -76,9 +76,11 @@ namespace Mono.Linker.Tests.TestCasesRunner {
 				};
 			}
 
-			if (_testCaseTypeDefinition.AllMethods().Any(method => method.CustomAttributes.Any (attr =>
-				attr.AttributeType.Name == nameof (RecognizedReflectionAccessPatternAttribute) ||
-				attr.AttributeType.Name == nameof (UnrecognizedReflectionAccessPatternAttribute)))) {
+			if (_testCaseTypeDefinition.CustomAttributes.Any (attr =>
+					attr.AttributeType.Name == nameof (VerifyAllReflectionAccessPatternsAreValidatedAttribute))
+				|| _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;