[ILLink analyzer] Improve handling of invalid operations

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs

@@ -24,18 +24,12 @@ public CapturedReferenceValue (IOperation operation)
 			case OperationKind.InlineArrayAccess:
 			case OperationKind.ImplicitIndexerReference:
 				break;
-			case OperationKind.None:
 			case OperationKind.InstanceReference:
 			case OperationKind.Invocation:
-			case OperationKind.Invalid:
 				// These will just be ignored when referenced later.
 				break;
 			default:
-				// Assert on anything else as it means we need to implement support for it
-				// but do not throw here as it means new Roslyn version could cause the analyzer to crash
-				// which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do
-				// so effectively ignoring constructs it doesn't understand is OK.
-				Debug.Fail ($"{operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}");
+				UnexpectedOperationHandler.Handle (operation);
 				break;
 			}
 			Reference = operation;

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs

@@ -335,9 +335,6 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
 				// Seems like this can't happen with a flow capture operation.
 				Debug.Assert (operation.Target is not IFlowCaptureReferenceOperation);
 				break;
-			case IInvalidOperation:
-				// This can happen for a field assignment in an attribute instance.
-				// TODO: validate against the field attributes.
 			case IInstanceReferenceOperation:
 				// Assignment to 'this' is not tracked currently.
 				// Not relevant for trimming dataflow.
@@ -358,16 +355,7 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe
 			// can show up in a flow capture reference (for example, where the right-hand side
 			// is a null-coalescing operator).
 			default:
-				// NoneOperation represents operations which are unimplemented by Roslyn
-				// (don't have specific I*Operation types), such as pointer dereferences.
-				if (targetOperation.Kind is OperationKind.None)
-					break;
-
-				// Assert on anything else as it means we need to implement support for it
-				// but do not throw here as it means new Roslyn version could cause the analyzer to crash
-				// which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do
-				// so effectively ignoring constructs it doesn't understand is OK.
-				Debug.Fail ($"{targetOperation.GetType ()}: {targetOperation.Syntax.GetLocation ().GetLineSpan ()}");
+				UnexpectedOperationHandler.Handle (targetOperation);
 				break;
 			}
 			return Visit (operation.Value, state);
@@ -584,9 +572,7 @@ public override TValue VisitDelegateCreation (IDelegateCreationOperation operati
 					// No method symbol.
 					break;
 				default:
-					// Unimplemented case that might need special handling.
-					// Fail in debug mode only.
-					Debug.Fail ($"{operation.Target.GetType ()}: {operation.Target.Syntax.GetLocation ().GetLineSpan ()}");
+					UnexpectedOperationHandler.Handle (operation.Target);
 					break;
 			}
 
@@ -781,7 +767,7 @@ TValue HandleMethodCallHelper (
 					argumentOperation = callOperation.Arguments[argumentIndex];
 					break;
 				default:
-					Debug.Fail ($"Unexpected operation {operation} for parameter {parameter.GetDisplayName ()}");
+					UnexpectedOperationHandler.Handle (operation);
 					continue;
 				};
 

src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/UnexpectedOperationHandler.cs

@@ -0,0 +1,49 @@
+// 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.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.CodeAnalysis
+{
+	internal static class UnexpectedOperationHandler
+	{
+		// No-op in release builds, but fails in debug builds when
+		// encountering an unexpected operation. InvalidOperation is skipped because
+		// it is expected that any part of the control-flow graph may contain an
+		// InvalidOperation for code that doesn't compile (for example, in an intermediate
+		// state while editing).
+		public static void Handle (IOperation operation)
+		{
+			// NoneOperation represents operations which are unimplemented by Roslyn
+			// (don't have specific I*Operation types), such as pointer dereferences.
+			if (operation.Kind is OperationKind.None)
+				return;
+
+			// This can happen for a field assignment in an attribute instance.
+			// TODO: validate against the field attributes.
+			if (operation.Kind is OperationKind.Invalid)
+				return;
+
+			// It's also possible to hit a case where the operation is an unexpected operation kind,
+			// but the code is in an invalid state where the unexpected operation is not IInvalidOperation, yet one
+			// of its child operations is. For example:
+			//
+			//     a + = 3;
+			//
+			// This is represented as an assignment where the target is an IBinaryOperation (a +) whose right-hand side
+			// is an IInvalidOperation. The assignment logic doesn't support assigning to a binary operation,
+			// but this should still not fail.
+			foreach (var descendant in operation.Descendants()) {
+				if (descendant.Kind is OperationKind.Invalid)
+					return;
+			}
+
+			// Assert on anything else as it means we need to implement support for it
+			// but do not throw here as it means new Roslyn version could cause the analyzer to crash
+			// which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do
+			// so effectively ignoring constructs it doesn't understand is OK.
+			Debug.Fail ($"Unexpected operation type {operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}");
+		}
+	}
+}

src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/DynamicallyAccessedMembersAnalyzerTests.cs

@@ -1384,6 +1384,25 @@ public static void Main()
 				DiagnosticResult.CompilerError ("CS0103").WithSpan (8, 3, 8, 7).WithArguments ("type"));
 		}
 
+		[Fact]
+		public Task AssignmentTargetHasNestedInvalidOperation ()
+		{
+			// The assignment target is an IBinaryOperation whose right-hand side is an IInvalidOperation.
+			var Source = $$"""
+				int a, b = 0;
+				a + = 3;
+			""";
+
+			return VerifyDynamicallyAccessedMembersAnalyzer (Source, consoleApplication: true,
+				// (2,5): error CS1525: Invalid expression term '='
+				DiagnosticResult.CompilerError("CS1525").WithSpan(2, 5, 2, 6).WithArguments("="),
+				// (2,1): error CS0165: Use of unassigned local variable 'a'
+				DiagnosticResult.CompilerError("CS0165").WithSpan(2, 1, 2, 2).WithArguments("a"),
+				// (1,8): warning CS0219: The variable 'b' is assigned but its value is never used
+				DiagnosticResult.CompilerWarning("CS0219").WithSpan(1, 8, 1, 9).WithArguments("b")
+			);
+		}
+
 		[Fact]
 		public Task CRefGenericParameterAnalysis ()
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributeFieldDataflow.cs

@@ -12,20 +12,56 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 	[ExpectedNoWarnings]
 	class AttributeFieldDataflow
 	{
+		public static void Main ()
+		{
+			TestKeepsPublicConstructors ();
+			TestKeepsPublicMethods ();
+			TestKeepsPublicMethodsString ();
+			TestKeepsPublicFields ();
+			TestTypeArray ();
+		}
+
+		[Kept]
 		[KeptAttributeAttribute (typeof (KeepsPublicConstructorsAttribute))]
+		[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
+		public static void TestKeepsPublicConstructors ()
+		{
+			typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicConstructors)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
+		}
+
+		[Kept]
 		[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
+		[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--")]
+		[KeepsPublicMethods (Type = typeof (ClassWithKeptPublicMethods))]
+		public static void TestKeepsPublicMethods ()
+		{
+			typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicMethods)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+		}
+
+		[Kept]
+		[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
+		// Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273
+		[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
+		[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributeFieldDataflow+ClassWithKeptPublicMethods")]
+		public static void TestKeepsPublicMethodsString ()
+		{
+			typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicMethodsString)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+		}
+
+		[Kept]
 		[KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))]
-		[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
-		[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
-		[KeepsPublicMethods (Type = "Mono.Linker.Tests.Cases.DataFlow.AttributeFieldDataflow+ClassWithKeptPublicMethods")]
 		[KeepsPublicFields (Type = null, TypeName = null)]
+		public static void TestKeepsPublicFields ()
+		{
+			typeof (AttributeFieldDataflow).GetMethod (nameof (TestKeepsPublicFields)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute));
+		}
+
+		[Kept]
+		[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
 		[TypeArray (Types = new Type[] { typeof (AttributeFieldDataflow) })]
-		// Trimmer only for now - https://github.com/dotnet/linker/issues/2273
-		[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
-		public static void Main ()
+		public static void TestTypeArray ()
 		{
-			typeof (AttributeFieldDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
-			typeof (AttributeFieldDataflow).GetMethod ("Main").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+			typeof (AttributeFieldDataflow).GetMethod (nameof (TestTypeArray)).GetCustomAttribute (typeof (TypeArrayAttribute));
 		}
 
 		[Kept]
@@ -55,7 +91,12 @@ public KeepsPublicMethodsAttribute ()
 			[Kept]
 			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
 			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
-			public string Type;
+			public string TypeName;
+
+			[Kept]
+			[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			public Type Type;
 		}
 
 		// Use to test null values

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/AttributePropertyDataflow.cs

@@ -27,20 +27,56 @@ public static void Main ()
 
 		class AttributesOnMethod
 		{
+			[Kept]
+			public static void Test () {
+				TestKeepsPublicConstructors ();
+				TestKeepsPublicMethods ();
+				TestKeepsPublicMethodsByName ();
+				TestKeepsPublicFields ();
+				TestTypeArray ();
+			}
+
 			[Kept]
 			[KeptAttributeAttribute (typeof (KeepsPublicConstructorsAttribute))]
+			[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
+			public static void TestKeepsPublicConstructors ()
+			{
+				typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicConstructors)).GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
+			}
+
+			[Kept]
+			[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
+			[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--")]
+			[KeepsPublicMethods (Type = typeof (ClassWithKeptPublicMethods))]
+			public static void TestKeepsPublicMethods ()
+			{
+				typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicMethods)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+			}
+
+			[Kept]
 			[KeptAttributeAttribute (typeof (KeepsPublicMethodsAttribute))]
+			// Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273
+			[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethodsKeptByName--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
+			[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+AttributesOnMethod+ClassWithKeptPublicMethodsKeptByName")]
+			public static void TestKeepsPublicMethodsByName ()
+			{
+				typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicMethodsByName)).GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+			}
+
+			[Kept]
 			[KeptAttributeAttribute (typeof (KeepsPublicFieldsAttribute))]
-			[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
-			[KeepsPublicConstructors (Type = typeof (ClassWithKeptPublicConstructor))]
-			[KeepsPublicMethods (TypeName = "Mono.Linker.Tests.Cases.DataFlow.AttributePropertyDataflow+AttributesOnMethod+ClassWithKeptPublicMethods")]
 			[KeepsPublicFields (Type = null, TypeName = null)]
+			public static void TestKeepsPublicFields ()
+			{
+				typeof (AttributesOnMethod).GetMethod (nameof (TestKeepsPublicFields)).GetCustomAttribute (typeof (KeepsPublicFieldsAttribute));
+			}
+
+			[Kept]
+			[KeptAttributeAttribute (typeof (TypeArrayAttribute))]
 			[TypeArray (Types = new Type[] { typeof (AttributePropertyDataflow) })]
-			// Trimmer/NativeAot only for now - https://github.com/dotnet/linker/issues/2273
-			[ExpectedWarning ("IL2026", "--ClassWithKeptPublicMethods--", ProducedBy = Tool.Trimmer | Tool.NativeAot)]
-			public static void Test () {
-				typeof (AttributesOnMethod).GetMethod ("Test").GetCustomAttribute (typeof (KeepsPublicConstructorsAttribute));
-				typeof (AttributePropertyDataflow).GetMethod ("Test").GetCustomAttribute (typeof (KeepsPublicMethodsAttribute));
+			public static void TestTypeArray ()
+			{
+				typeof (AttributesOnMethod).GetMethod (nameof (TestTypeArray)).GetCustomAttribute (typeof (TypeArrayAttribute));
 			}
 
 			[Kept]
@@ -63,6 +99,16 @@ class ClassWithKeptPublicMethods
 				public static void KeptMethod () { }
 				static void Method () { }
 			}
+
+			[Kept]
+			class ClassWithKeptPublicMethodsKeptByName
+			{
+				[Kept]
+				[KeptAttributeAttribute (typeof (RequiresUnreferencedCodeAttribute))]
+				[RequiresUnreferencedCode ("--ClassWithKeptPublicMethodsKeptByName--")]
+				public static void KeptMethod () { }
+				static void Method () { }
+			}
 		}
 
 		class AttributesOnField