Re-add static interface trimming with more testing

docs/removal-behavior.md

@@ -42,7 +42,7 @@ public class Program
 
 ### Method call on a constrained type parameter
 
-On a call to a static abstract interface method that is accessed through a constrained type parameter, the interface method is rooted, as well as every implementation method on every type.
+On a call to a static abstract interface method that is accessed through a constrained type parameter, the interface method is rooted, as well as every implementation method on every type that is rooted.
 
 Example:
 

src/linker/Linker.Steps/MarkStep.cs

@@ -575,6 +575,10 @@ void ProcessVirtualMethods ()
 			}
 		}
 
+		/// <summary>
+		/// Does extra handling of marked types that have interfaces when it's necessary to know what types are marked or instantiated.
+		/// e.g. Marks the "implements interface" annotations and removes override annotations for static interface methods.
+		/// </summary>
 		void ProcessMarkedTypesWithInterfaces ()
 		{
 			// We may mark an interface type later on.  Which means we need to reprocess any time with one or more interface implementations that have not been marked
@@ -3017,8 +3021,18 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 				}
 			}
 
+			// Mark overridden methods except for static interface methods
 			if (method.HasOverrides) {
 				foreach (MethodReference ov in method.Overrides) {
+					// Method implementing a static interface method will have an override to it - note nonstatic methods usually don't unless they're explicit.
+					// Calling the implementation method directly has no impact on the interface, and as such it should not mark the interface or its method.
+					// Only if the interface method is referenced, then all the methods which implemented must be kept, but not the other way round.
+					if (Context.Resolve (ov)?.IsStatic == true
+						&& Context.Resolve (ov.DeclaringType)?.IsInterface == true
+						// Public methods can be called directly on the type. If it non-public it is an explicit implementation and if the method is marked we need to mark the override.
+						&& method.IsPublic) {
+						continue;
+					}
 					MarkMethod (ov, new DependencyInfo (DependencyKind.MethodImplOverride, method), ScopeStack.CurrentScope.Origin);
 					MarkExplicitInterfaceImplementation (method, ov);
 				}

src/linker/Linker.Steps/SweepStep.cs

@@ -453,6 +453,8 @@ protected virtual void SweepMethods (Collection<MethodDefinition> methods)
 
 				SweepCustomAttributes (method.MethodReturnType);
 
+				SweepOverrides (method);
+
 				if (!method.HasParameters)
 					continue;
 
@@ -467,6 +469,38 @@ protected virtual void SweepMethods (Collection<MethodDefinition> methods)
 			}
 		}
 
+		void SweepOverrides (MethodDefinition method)
+		{
+			for (int i = 0; i < method.Overrides.Count;) {
+				// We can't rely on the context resolution cache anymore, since it may remember methods which are already removed
+				// So call the direct Resolve here and avoid the cache.
+				// We want to remove a method from the list of Overrides if:
+				//	Resolve() is null
+				//		This can happen for a couple of reasons, but it indicates the method isn't in the final assembly.
+				//		Resolve also may return a removed value if method.Overrides[i] is a MethodDefinition. In this case, Resolve short circuits and returns `this`.
+				//	OR
+				//	ov.DeclaringType is null
+				//		ov.DeclaringType may be null if Resolve short circuited and returned a removed method. In this case, we want to remove the override.
+				//	OR
+				//	ov is in a `link` scope and is unmarked
+				//		ShouldRemove returns true if the method is unmarked, but we also We need to make sure the override is in a link scope.
+				//		Only things in a link scope are marked, so ShouldRemove is only valid for items in a `link` scope.
+				if (method.Overrides[i].Resolve () is not MethodDefinition ov || ov.DeclaringType is null || (IsLinkScope (ov.DeclaringType.Scope) && ShouldRemove (ov)))
+					method.Overrides.RemoveAt (i);
+				else
+					i++;
+			}
+		}
+
+		/// <summary>
+		/// Returns true if the assembly of the <paramref name="scope"></paramref> is set to link
+		/// </summary>
+		private bool IsLinkScope (IMetadataScope scope)
+		{
+			AssemblyDefinition? assembly = Context.Resolve (scope);
+			return assembly != null && Annotations.GetAction (assembly) == AssemblyAction.Link;
+		}
+
 		void SweepDebugInfo (Collection<MethodDefinition> methods)
 		{
 			List<ScopeDebugInformation>? sweptScopes = null;

src/linker/Linker/Annotations.cs

@@ -436,6 +436,9 @@ public bool IsPublic (IMetadataTokenProvider provider)
 			return public_api.Contains (provider);
 		}
 
+		/// <summary>
+		/// Returns an IEnumerable of the methods that override this method. Note this is different than <see cref="MethodDefinition.Overrides"/>, which returns the MethodImpl's
+		/// </summary>
 		public IEnumerable<OverrideInformation>? GetOverrides (MethodDefinition method)
 		{
 			return TypeMapInfo.GetOverrides (method);

test/ILLink.RoslynAnalyzer.Tests/generated/ILLink.RoslynAnalyzer.Tests.Generator/ILLink.RoslynAnalyzer.Tests.TestCaseGenerator/Inheritance.Interfaces.StaticInterfaceMethodsTests.g.cs

@@ -0,0 +1,25 @@
+using System;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace ILLink.RoslynAnalyzer.Tests.Inheritance.Interfaces
+{
+	public sealed partial class StaticInterfaceMethodsTests : LinkerTestBase
+	{
+
+		protected override string TestSuiteName => "Inheritance.Interfaces.StaticInterfaceMethods";
+
+		[Fact]
+		public Task StaticAbstractInterfaceMethods ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
+		[Fact]
+		public Task StaticAbstractInterfaceMethodsLibrary ()
+		{
+			return RunTest (allowMissingWarnings: true);
+		}
+
+	}
+}

test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/InterfaceVariants.cs

@@ -71,41 +71,32 @@ internal class ImplementsUnusedStaticInterface
 			// The interface methods themselves are not used, but the implementation of these methods is
 			internal interface IStaticInterfaceMethodUnused
 			{
-				// Can be removed with Static Interface trimming optimization
-				[Kept]
 				static abstract void InterfaceUsedMethodNot ();
 			}
 
-			// Can be removed with Static Interface Trimming
-			[Kept]
 			internal interface IStaticInterfaceUnused
 			{
-				// Can be removed with Static Interface Trimming
-				[Kept]
 				static abstract void InterfaceAndMethodNoUsed ();
 			}
 
 			[Kept]
-			[KeptInterface (typeof (IStaticInterfaceUnused))]
-			[KeptInterface (typeof (IStaticInterfaceMethodUnused))]
 			internal class InterfaceMethodUsedThroughImplementation : IStaticInterfaceMethodUnused, IStaticInterfaceUnused
 			{
 				[Kept]
+				[RemovedOverride (typeof (IStaticInterfaceMethodUnused))]
 				public static void InterfaceUsedMethodNot () { }
 
 				[Kept]
+				[RemovedOverride (typeof (IStaticInterfaceUnused))]
 				public static void InterfaceAndMethodNoUsed () { }
 			}
 
 			[Kept]
-			[KeptInterface (typeof (IStaticInterfaceMethodUnused))]
-			[KeptInterface (typeof (IStaticInterfaceUnused))]
+			[KeptInterface (typeof (IStaticInterfaceMethodUnused))] // Bug
 			internal class InterfaceMethodUnused : IStaticInterfaceMethodUnused, IStaticInterfaceUnused
 			{
-				[Kept]
 				public static void InterfaceUsedMethodNot () { }
 
-				[Kept]
 				public static void InterfaceAndMethodNoUsed () { }
 			}