Avoid analyzing cref for generic parameters (#2732)

Add check for cref in generic parameter dataflow to avoid failing with a nullreference exception
Refactor code to reuse the while loop for both INamedType and Method symbols
Add tests
Fixes #2724

3 files changed, 32 insertions, 12 deletions

diff --git a/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs b/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
index d5225769c..821b84678 100644
--- a/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
+++ b/src/ILLink.RoslynAnalyzer/DynamicallyAccessedMembersAnalyzer.cs
@@ -131,22 +131,31 @@ namespace ILLink.RoslynAnalyzer
 				&& context.ContainingSymbol.IsInRequiresUnreferencedCodeAttributeScope ())
 				return;
 
+			var symbol = context.SemanticModel.GetSymbolInfo (context.Node).Symbol;
+
+			// Avoid unnecesary execution if not NamedType or Method
+			if (symbol is not INamedTypeSymbol && symbol is not IMethodSymbol)
+				return;
+
+			// Members inside nameof or cref comments, commonly used to access the string value of a variable, type, or a memeber,
+			// can generate diagnostics warnings, which can be noisy and unhelpful. 
+			// Walking the node heirarchy to check if the member is inside a nameof/cref to not generate diagnostics
+			var parentNode = context.Node;
+			while (parentNode != null) {
+				if (parentNode is InvocationExpressionSyntax invocationExpression &&
+					invocationExpression.Expression is IdentifierNameSyntax ident1 &&
+					ident1.Identifier.ValueText.Equals ("nameof"))
+					return;
+				else if (parentNode is NameMemberCrefSyntax)
+					return;
+
+				parentNode = parentNode.Parent;
+			}
+
 			ImmutableArray<ITypeParameterSymbol> typeParams = default;
 			ImmutableArray<ITypeSymbol> typeArgs = default;
-			var symbol = context.SemanticModel.GetSymbolInfo (context.Node).Symbol;
 			switch (symbol) {
 			case INamedTypeSymbol type:
-				// INamedTypeSymbol inside nameof, commonly used to access the string value of a variable, type, or a memeber,
-				// can generate diagnostics warnings, which can be noisy and unhelpful. 
-				// Walking the node heirarchy to check if INamedTypeSymbol is inside a nameof to not generate diagnostics
-				var parentNode = context.Node;
-				while (parentNode != null) {
-					if (parentNode is InvocationExpressionSyntax invocationExpression && invocationExpression.Expression is IdentifierNameSyntax ident1) {
-						if (ident1.Identifier.ValueText.Equals ("nameof"))
-							return;
-					}
-					parentNode = parentNode.Parent;
-				}
 				typeParams = type.TypeParameters;
 				typeArgs = type.TypeArguments;
 				break;
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs
index fefa4584f..e30b6f1d2 100644
--- a/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/GenericParameterDataFlow.cs
@@ -165,6 +165,11 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 			new DerivedTypeWithOpenGenericOnBaseWithRequirements<TestType> ();
 		}
 
+		/// <summary>
+		/// Adding a comment to verify that analyzer doesn't null ref when trying to analyze
+		/// generic parameter in cref comments on a class
+		/// <see cref="GenericBaseTypeWithRequirements{T}"/>
+		/// </summary>
 		class GenericBaseTypeWithRequirements<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] T>
 		{
 			public GenericBaseTypeWithRequirements ()
@@ -452,6 +457,11 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 			MethodRequiresNothingPassThrough<TestType> ();
 		}
 
+		/// <summary>
+		/// Adding a comment to verify that analyzer doesn't null ref when trying to analyze
+		/// generic parameter comments on a method
+		/// <see cref="MethodRequiresPublicFields{T}"/>
+		/// </summary>
 		[ExpectedWarning ("IL2087", nameof (DataFlowTypeExtensions.RequiresPublicMethods))]
 		static void MethodRequiresPublicFields<
 			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicFields)] T> ()
diff --git a/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj b/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
index 29cde9ab6..7d1c6de28 100644
--- a/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
+++ b/test/Mono.Linker.Tests.Cases/Mono.Linker.Tests.Cases.csproj
@@ -6,6 +6,7 @@
     <AnalysisLevel>0</AnalysisLevel>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+    <GenerateDocumentationFile>true</GenerateDocumentationFile>
   </PropertyGroup>
 
   <ItemGroup>