Analyze implicit indexer reference operations

eng/Versions.props

@@ -21,7 +21,7 @@
     <MicrosoftDotNetApiCompatVersion>7.0.0-beta.22313.1</MicrosoftDotNetApiCompatVersion>
     <MicrosoftDotNetCodeAnalysisVersion>6.0.0-beta.21271.1</MicrosoftDotNetCodeAnalysisVersion>
     <MicrosoftCodeAnalysisCSharpCodeStyleVersion>3.10.0-2.final</MicrosoftCodeAnalysisCSharpCodeStyleVersion>
-    <MicrosoftCodeAnalysisVersion>4.0.1</MicrosoftCodeAnalysisVersion>
+    <MicrosoftCodeAnalysisVersion>4.3.0-1.22206.2</MicrosoftCodeAnalysisVersion>
     <MicrosoftCodeAnalysisCSharpAnalyzerTestingXunitVersion>1.0.1-beta1.*</MicrosoftCodeAnalysisCSharpAnalyzerTestingXunitVersion>
     <MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
     <!-- This controls the version of the cecil package, or the version of cecil in the project graph

src/ILLink.RoslynAnalyzer/DataFlow/CapturedReferenceValue.cs

@@ -19,6 +19,7 @@ public CapturedReferenceValue (IOperation operation)
 			case OperationKind.FieldReference:
 			case OperationKind.ParameterReference:
 			case OperationKind.ArrayElementReference:
+			case OperationKind.ImplicitIndexerReference:
 				break;
 			case OperationKind.None:
 			case OperationKind.InstanceReference:

src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowVisitor.cs

@@ -136,30 +136,44 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
 					// Avoid visiting the property reference because for captured properties, we can't
 					// correctly detect whether it is used for reading or writing inside of VisitPropertyReference.
 					// https://github.com/dotnet/roslyn/issues/25057
-					IPropertySymbol? property = propertyRef.Property;
-					IMethodSymbol? setMethod;
-					while ((setMethod = property.SetMethod) == null) {
-						if ((property = property.OverriddenProperty) == null)
-							break;
-					}
+					TValue instanceValue = Visit (propertyRef.Instance, state);
+					TValue value = Visit (operation.Value, state);
+					IMethodSymbol? setMethod = propertyRef.Property.GetSetMethod ();
 					if (setMethod == null) {
 						// This can happen in a constructor - there it is possible to assign to a property
 						// without a setter. This turns into an assignment to the compiler-generated backing field.
 						// To match the linker, this should warn about the compiler-generated backing field.
 						// For now, just don't warn. https://github.com/dotnet/linker/issues/2731
 						break;
 					}
-					TValue instanceValue = Visit (propertyRef.Instance, state);
-					TValue value = Visit (operation.Value, state);
-					HandleMethodCall (
-						setMethod,
-						instanceValue,
-						ImmutableArray.Create (value),
-						operation);
+					HandleMethodCall (setMethod, instanceValue, ImmutableArray.Create (value), operation);
 					// The return value of a property set expression is the value,
 					// even though a property setter has no return value.
 					return value;
 				}
+			case IImplicitIndexerReferenceOperation indexerRef: {
+					// An implicit reference to an indexer where the argument is a System.Index
+					TValue instanceValue = Visit (indexerRef.Instance, state);
+					TValue indexArgumentValue = Visit (indexerRef.Argument, state);
+					TValue value = Visit (operation.Value, state);
+
+					var property = (IPropertySymbol) indexerRef.IndexerSymbol;
+
+					var argumentsBuilder = ImmutableArray.CreateBuilder<TValue> ();
+					argumentsBuilder.Add (indexArgumentValue);
+					argumentsBuilder.Add (value);
+
+					IMethodSymbol? setMethod = property.GetSetMethod ();
+					if (setMethod == null) {
+						// It might actually be a call to a ref-returning get method,
+						// like Span<T>.this[int].get. We don't handle ref returns yet.
+						break;
+					}
+
+					HandleMethodCall (setMethod, instanceValue, argumentsBuilder.ToImmutableArray (), operation);
+					return value;
+				}
+
 			// TODO: when setting a property in an attribute, target is an IPropertyReference.
 			case ILocalReferenceOperation localRef: {
 					TValue value = Visit (operation.Value, state);
@@ -208,7 +222,7 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
 				// (don't have specific I*Operation types), such as pointer dereferences.
 				if (targetOperation.Kind is OperationKind.None)
 					break;
-				throw new NotImplementedException (targetOperation.GetType ().ToString ());
+				throw new NotImplementedException ($"{targetOperation.GetType ().ToString ()}: {targetOperation.Syntax.GetLocation ().GetLineSpan ()}");
 			}
 			return Visit (operation.Value, state);
 		}
@@ -272,17 +286,26 @@ public override TValue VisitPropertyReference (IPropertyReferenceOperation opera
 			// Accessing property for reading is really a call to the getter
 			// The setter case is handled in assignment operation since here we don't have access to the value to pass to the setter
 			TValue instanceValue = Visit (operation.Instance, state);
-			IPropertySymbol? property = operation.Property;
-			IMethodSymbol? getMethod;
-			while ((getMethod = property.GetMethod) == null) {
-				if ((property = property.OverriddenProperty) == null)
-					break;
+			IMethodSymbol? getMethod = operation.Property.GetGetMethod ();
+			return HandleMethodCall (getMethod!, instanceValue, ImmutableArray<TValue>.Empty, operation);
+		}
+
+		public override TValue VisitImplicitIndexerReference (IImplicitIndexerReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)
+		{
+			if (!operation.GetValueUsageInfo (Context.OwningSymbol).HasFlag (ValueUsageInfo.Read))
+				return TopValue;
+
+			TValue instanceValue = Visit (operation.Instance, state);
+			TValue indexArgumentValue = Visit (operation.Argument, state);
+
+			if (operation.IndexerSymbol is not IPropertySymbol indexerProperty) {
+				// For example, System.Span<T>.Slice(int, int).
+				// Don't try to handle it for now.
+				return TopValue;
 			}
-			return HandleMethodCall (
-				getMethod!,
-				instanceValue,
-				ImmutableArray<TValue>.Empty,
-				operation);
+
+			IMethodSymbol getMethod = indexerProperty.GetGetMethod ()!;
+			return HandleMethodCall (getMethod, instanceValue, ImmutableArray.Create (indexArgumentValue), operation);
 		}
 
 		public override TValue VisitArrayElementReference (IArrayElementReferenceOperation operation, LocalDataFlowState<TValue, TValueLattice> state)

src/ILLink.RoslynAnalyzer/DataFlow/OperationWalker.cs

@@ -13,8 +13,7 @@ public abstract class OperationWalker<TArgument, TResult> : OperationVisitor<TAr
 
 		private void VisitChildOperations (IOperation operation, TArgument argument)
 		{
-			// https://github.com/dotnet/roslyn/issues/49475 would let us use ChildOperations, a struct enumerable.
-			foreach (var child in operation.Children)
+			foreach (var child in operation.ChildOperations)
 				Visit (child, argument);
 		}
 

src/ILLink.RoslynAnalyzer/IPropertySymbolExtensions.cs

@@ -0,0 +1,32 @@
+// 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 Microsoft.CodeAnalysis;
+
+namespace ILLink.RoslynAnalyzer
+{
+	public static class IPropertySymbolExtensions
+	{
+		public static IMethodSymbol? GetGetMethod (this IPropertySymbol property)
+		{
+			IPropertySymbol? declaringProperty = property;
+			IMethodSymbol? getMethod;
+			while ((getMethod = declaringProperty.GetMethod) == null) {
+				if ((declaringProperty = declaringProperty.OverriddenProperty) == null)
+					break;
+			}
+			return getMethod;
+		}
+
+		public static IMethodSymbol? GetSetMethod (this IPropertySymbol property)
+		{
+			IPropertySymbol? declaringProperty = property;
+			IMethodSymbol? setMethod;
+			while ((setMethod = declaringProperty.SetMethod) == null) {
+				if ((declaringProperty = declaringProperty.OverriddenProperty) == null)
+					break;
+			}
+			return setMethod;
+		}
+	}
+}

test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptDelegateCacheFieldAttribute.cs

@@ -8,10 +8,12 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
 	[AttributeUsage (AttributeTargets.Class | AttributeTargets.Struct, Inherited = false, AllowMultiple = true)]
 	public class KeptDelegateCacheFieldAttribute : KeptAttribute
 	{
-		public KeptDelegateCacheFieldAttribute (string uniquePartOfName)
+		public KeptDelegateCacheFieldAttribute (string classIndex, string fieldName)
 		{
-			if (string.IsNullOrEmpty (uniquePartOfName))
-				throw new ArgumentNullException (nameof (uniquePartOfName));
+			if (string.IsNullOrEmpty (classIndex))
+				throw new ArgumentNullException (nameof (classIndex));
+			if (string.IsNullOrEmpty (fieldName))
+				throw new ArgumentNullException (nameof (fieldName));
 		}
 	}
 }

test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptPrivateImplementationDetails.cs

@@ -0,0 +1,17 @@
+// 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;
+
+namespace Mono.Linker.Tests.Cases.Expectations.Assertions
+{
+	[AttributeUsage (AttributeTargets.Class)]
+	public sealed class KeptPrivateImplementationDetailsAttribute : KeptAttribute
+	{
+		public KeptPrivateImplementationDetailsAttribute (string methodName)
+		{
+			if (string.IsNullOrEmpty (methodName))
+				throw new ArgumentException ("Value cannot be null or empty.", nameof (methodName));
+		}
+	}
+}

test/Mono.Linker.Tests.Cases/Attributes/Csc/OnlyTypeUsedInAssemblyIsTypeOnAttributeCtorOnEvent.cs

@@ -14,7 +14,7 @@ namespace Mono.Linker.Tests.Cases.Attributes.Csc
 	[KeptTypeInAssembly ("LibraryWithType.dll", typeof (TypeDefinedInReference))]
 	[RemovedMemberInAssembly ("LibraryWithType.dll", typeof (TypeDefinedInReference), "Unused()")]
 	[KeptMemberInAssembly ("LibraryWithAttribute.dll", typeof (AttributeDefinedInReference), ".ctor(System.Type)")]
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (FooOnMyEvent))]
 	public class OnlyTypeUsedInAssemblyIsTypeOnAttributeCtorOnEvent
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/Attributes/Csc/OnlyTypeUsedInAssemblyIsTypeOnAttributeFieldOnEvent.cs

@@ -15,7 +15,7 @@ namespace Mono.Linker.Tests.Cases.Attributes.Csc
 	[RemovedMemberInAssembly ("LibraryWithType.dll", typeof (TypeDefinedInReference), "Unused()")]
 	[KeptMemberInAssembly ("LibraryWithAttribute.dll", typeof (AttributeDefinedInReference), ".ctor()")]
 	[KeptMemberInAssembly ("LibraryWithAttribute.dll", typeof (AttributeDefinedInReference), "FieldType")]
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (FooOnMyEvent))]
 	public class OnlyTypeUsedInAssemblyIsTypeOnAttributeFieldOnEvent
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/Attributes/Csc/OnlyTypeUsedInAssemblyIsTypeOnAttributePropertyOnEvent.cs

@@ -15,7 +15,7 @@ namespace Mono.Linker.Tests.Cases.Attributes.Csc
 	[RemovedMemberInAssembly ("LibraryWithType.dll", typeof (TypeDefinedInReference), "Unused()")]
 	[KeptMemberInAssembly ("LibraryWithAttribute.dll", typeof (AttributeDefinedInReference), ".ctor()")]
 	[KeptMemberInAssembly ("LibraryWithAttribute.dll", typeof (AttributeDefinedInReference), "set_PropertyType(System.Type)")]
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (FooOnMyEvent))]
 	public class OnlyTypeUsedInAssemblyIsTypeOnAttributePropertyOnEvent
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/Attributes/OnlyKeepUsed/UnusedAttributeTypeOnEventIsRemoved.cs

@@ -5,7 +5,7 @@
 namespace Mono.Linker.Tests.Cases.Attributes.OnlyKeepUsed
 {
 	[SetupLinkerArgument ("--used-attrs-only", "true")]
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (Tmp_Something))]
 	class UnusedAttributeTypeOnEventIsRemoved
 	{
 		static void Main ()

test/Mono.Linker.Tests.Cases/Attributes/OnlyKeepUsed/UsedAttributeTypeOnEventIsKept.cs

@@ -5,7 +5,7 @@
 namespace Mono.Linker.Tests.Cases.Attributes.OnlyKeepUsed
 {
 	[SetupLinkerArgument ("--used-attrs-only", "true")]
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (Tmp_Something))]
 	class UsedAttributeTypeOnEventIsKept
 	{
 		static void Main ()

test/Mono.Linker.Tests.Cases/Basic/DelegateBeginInvokeEndInvokePair.cs

@@ -5,6 +5,9 @@
 
 namespace Mono.Linker.Tests.Cases.Basic
 {
+	[KeptDelegateCacheField ("0", nameof (Method))]
+	[KeptDelegateCacheField ("1", nameof (Method))]
+	[KeptDelegateCacheField ("2", nameof (Method))]
 	class DelegateBeginInvokeEndInvokePair
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/Basic/UsedEventIsKept.cs

@@ -3,8 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Basic
 {
-	[KeptDelegateCacheField ("0")]
-	[KeptDelegateCacheField ("1")]
+	[KeptDelegateCacheField ("0", nameof (Tmp_Bar))]
 	class UsedEventIsKept
 	{
 		public static void Main ()

test/Mono.Linker.Tests.Cases/Basic/UsedEventOnInterfaceIsKept.cs

@@ -3,7 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Basic
 {
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (Bar_Ping))]
 	class UsedEventOnInterfaceIsKept
 	{
 		static void Main ()

test/Mono.Linker.Tests.Cases/Basic/UsedEventOnInterfaceIsRemovedWhenUsedFromClass.cs

@@ -3,7 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Basic
 {
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (Bar_Ping))]
 	class UsedEventOnInterfaceIsRemovedWhenUsedFromClass
 	{
 		static void Main ()

test/Mono.Linker.Tests.Cases/DataFlow/NullableAnnotations.cs

@@ -11,6 +11,7 @@
 namespace Mono.Linker.Tests.Cases.DataFlow
 {
 	[ExpectedNoWarnings]
+	[KeptPrivateImplementationDetails ("ThrowSwitchExpressionException")]
 	class NullableAnnotations
 	{
 		[Kept]

test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs

@@ -45,6 +45,9 @@ public static void Main ()
 
 			BasePropertyAccess.Test ();
 			AccessReturnedInstanceProperty.Test ();
+
+			ExplicitIndexerAccess.Test ();
+			ImplicitIndexerAccess.Test ();
 		}
 
 		[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors)]
@@ -653,6 +656,85 @@ public static void Test ()
 			}
 		}
 
+		class ExplicitIndexerAccess
+		{
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			Type this[Index idx] {
+				get => throw new NotImplementedException ();
+				set => throw new NotImplementedException ();
+			}
+
+			[ExpectedWarning ("IL2072", "this[Index].get", nameof (DataFlowTypeExtensions.RequiresAll), ProducedBy = ProducedBy.Analyzer)]
+			[ExpectedWarning ("IL2072", "Item.get", nameof (DataFlowTypeExtensions.RequiresAll), ProducedBy = ProducedBy.Trimmer)]
+			static void TestRead (ExplicitIndexerAccess instance = null)
+			{
+				instance[new Index (1)].RequiresAll ();
+			}
+
+			[ExpectedWarning ("IL2072", nameof (GetTypeWithPublicConstructors), "this[Index].set", ProducedBy = ProducedBy.Analyzer)]
+			[ExpectedWarning ("IL2072", nameof (GetTypeWithPublicConstructors), "Item.set", ProducedBy = ProducedBy.Trimmer)]
+			static void TestWrite (ExplicitIndexerAccess instance = null)
+			{
+				instance[^1] = GetTypeWithPublicConstructors ();
+			}
+
+			public static void Test ()
+			{
+				TestRead ();
+				TestWrite ();
+			}
+		}
+
+		class ImplicitIndexerAccess
+		{
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			Type this[int idx] {
+				get => throw new NotImplementedException ();
+				set => throw new NotImplementedException ();
+			}
+
+			int Length => throw new NotImplementedException ();
+
+			[ExpectedWarning ("IL2072", "this[Int32].get", nameof (DataFlowTypeExtensions.RequiresAll), ProducedBy = ProducedBy.Analyzer)]
+			[ExpectedWarning ("IL2072", "Item.get", nameof (DataFlowTypeExtensions.RequiresAll), ProducedBy = ProducedBy.Trimmer)]
+			static void TestRead (ImplicitIndexerAccess instance = null)
+			{
+				instance[new Index (1)].RequiresAll ();
+			}
+
+			[ExpectedWarning ("IL2072", nameof (GetTypeWithPublicConstructors), "this[Int32].set", ProducedBy = ProducedBy.Analyzer)]
+			[ExpectedWarning ("IL2072", nameof (GetTypeWithPublicConstructors), "Item.set", ProducedBy = ProducedBy.Trimmer)]
+			static void TestWrite (ImplicitIndexerAccess instance = null)
+			{
+				instance[^1] = GetTypeWithPublicConstructors ();
+			}
+
+			[ExpectedWarning ("IL2072", nameof (GetUnknownType), "this[Int32].set", ProducedBy = ProducedBy.Analyzer)]
+			[ExpectedWarning ("IL2072", nameof (GetUnknownType), "Item.set", ProducedBy = ProducedBy.Trimmer)]
+			static void TestNullCoalescingAssignment (ImplicitIndexerAccess instance = null)
+			{
+				instance[new Index (1)] ??= GetUnknownType ();
+			}
+
+			[ExpectedWarning ("IL2062", nameof (DataFlowTypeExtensions.RequiresAll))]
+			static void TestSpanIndexerAccess (int start = 0, int end = 3)
+			{
+				Span<byte> bytes = stackalloc byte[4] { 1, 2, 3, 4 };
+				bytes[^4] = 0; // This calls the get indexer which has a ref return.
+				int index = bytes[0];
+				Type[] types = new Type[] { GetUnknownType () };
+				types[index].RequiresAll ();
+			}
+
+			public static void Test ()
+			{
+				TestRead ();
+				TestWrite ();
+				TestNullCoalescingAssignment ();
+				TestSpanIndexerAccess ();
+			}
+		}
+
 		[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
 		private static Type GetTypeWithPublicParameterlessConstructor ()
 		{

test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/OnReferenceType/BaseProvidesInterfaceMember/GenericInterfaceWithEvent.cs

@@ -2,7 +2,7 @@
 
 namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.OnReferenceType.BaseProvidesInterfaceMember
 {
-	[KeptDelegateCacheField ("0")]
+	[KeptDelegateCacheField ("0", nameof (EventMethod))]
 	public class GenericInterfaceWithEvent
 	{
 		public static void Main ()