ILLink: Sweep .override for interface method if the .interfaceImpl is removed

src/tools/illink/src/linker/Linker.Steps/SweepStep.cs

@@ -456,21 +456,38 @@ void SweepOverrides (MethodDefinition method)
 				//    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 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
+				//  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.
+				// OR
+				//  ov is an interface method and the interface is not implemented by the type
 #pragma warning disable RS0030 // Cecil's Resolve is banned - it's necessary when the metadata graph isn't stable
-				if (method.Overrides[i].Resolve () is not MethodDefinition ov || ov.DeclaringType is null || (IsLinkScope (ov.DeclaringType.Scope) && ShouldRemove (ov)))
+				if (method.Overrides[i].Resolve () is not MethodDefinition ov
+					|| ov.DeclaringType is null
+					|| (IsLinkScope (ov.DeclaringType.Scope) && ShouldRemove (ov))
+					|| (ov.DeclaringType.IsInterface && !IsInterfaceImplemented (method.DeclaringType, ov.DeclaringType)))
 					method.Overrides.RemoveAt (i);
 				else
 					i++;
 #pragma warning restore RS0030
 			}
 		}
 
+		private bool IsInterfaceImplemented (TypeDefinition type, TypeDefinition iface)
+		{
+			var allInterfaces = type.GetAllInterfaceImplementations (Context, false);
+			foreach(var impl in allInterfaces) {
+				// Using the resolve cache here is okay, if we resolve to a removed type than the impl will not be marked and we'll continue
+				if (Context.Resolve(impl.InterfaceType) == iface && Context.Annotations.IsMarked (impl)) {
+					return true;
+				}
+			}
+			return false;
+		}
+
 		/// <summary>
 		/// Returns true if the assembly of the <paramref name="scope"></paramref> is set to link
 		/// </summary>

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

@@ -16,6 +16,12 @@ public Task BaseProvidesInterfaceMethod ()
 			return RunTest (allowMissingWarnings: false);
 		}
 
+		[Fact]
+		public Task RemovedInterfaceImplementationRemovedOverride()
+		{
+			return RunTest (allowMissingWarnings: false);
+		}
+
 		[Fact]
 		public Task StaticAbstractInterfaceMethods ()
 		{

src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/KeptOverrideAttribute.cs

@@ -15,12 +15,12 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
 	[AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)]
 	public class KeptOverrideAttribute : KeptAttribute
 	{
-		public Type TypeWithOverriddenMethodDeclaration;
-
 		public KeptOverrideAttribute (Type typeWithOverriddenMethod)
 		{
 			ArgumentNullException.ThrowIfNull (typeWithOverriddenMethod);
-			TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod;
+		}
+		public KeptOverrideAttribute (string typeWithOverriddenMethod)
+		{
 		}
 	}
-}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/RemovedOverrideAttribute.cs

@@ -14,12 +14,14 @@ namespace Mono.Linker.Tests.Cases.Expectations.Assertions
 	[AttributeUsage (AttributeTargets.Method, AllowMultiple = true, Inherited = false)]
 	public class RemovedOverrideAttribute : BaseInAssemblyAttribute
 	{
-		public Type TypeWithOverriddenMethodDeclaration;
 		public RemovedOverrideAttribute (Type typeWithOverriddenMethod)
 		{
 			if (typeWithOverriddenMethod == null)
 				throw new ArgumentException ("Value cannot be null or empty.", nameof (typeWithOverriddenMethod));
-			TypeWithOverriddenMethodDeclaration = typeWithOverriddenMethod;
+		}
+
+		public RemovedOverrideAttribute (string nameOfOverriddenMethod)
+		{
 		}
 	}
-}
+}

src/tools/illink/test/Mono.Linker.Tests.Cases/Inheritance.Interfaces/StaticInterfaceMethods/RemovedInterfaceImplementationRemovedOverride.cs

@@ -0,0 +1,230 @@
+// 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;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+namespace Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods
+{
+	[ExpectedNoWarnings]
+	public class RemovedInterfaceImplementationRemovedOverride
+	{
+		[Kept]
+		public static void Main ()
+		{
+			Basic.Test ();
+			GenericInterface.Test ();
+			GenericInterfaceGenericType.Test ();
+			PrivateExplicitImplementationReflectedOver.Test ();
+		}
+
+		[Kept]
+		public class Basic
+		{
+			[Kept]
+			public static void Test ()
+			{
+				// Get message via generic method
+				var message = MessageFactory.GetMessage<TypeWithMethodAccessedViaInterface> ();
+				Console.WriteLine (message);
+
+				// Get message directly from type
+				var message2 = TypeWithMethodAccessedDirectly.GetMessage ();
+				Console.WriteLine (message2);
+			}
+
+			[Kept]
+			public static class MessageFactory
+			{
+				[Kept]
+				public static string GetMessage<T> () where T : IStaticMethod
+				{
+					return T.GetMessage ();
+				}
+			}
+
+			[Kept]
+			[KeptInterface (typeof (IStaticMethod))]
+			public class TypeWithMethodAccessedViaInterface : IStaticMethod
+			{
+				// Force the string to have dynamic element so that it doesn't get compiled in as a literal
+				[Kept]
+				[KeptOverride (typeof (IStaticMethod))]
+				public static string GetMessage () => $"Hello from {nameof (TypeWithMethodAccessedViaInterface)}, the time is {TimeOnly.FromDateTime (DateTime.Now)}";
+			}
+
+			[Kept]
+			public class TypeWithMethodAccessedDirectly : IStaticMethod
+			{
+				// Force the string to have dynamic element so that it doesn't get compiled in as a literal
+				[Kept]
+				[RemovedOverride (typeof (IStaticMethod))]
+				public static string GetMessage () => $"Hello from {nameof (TypeWithMethodAccessedDirectly)}, the time is {TimeOnly.FromDateTime (DateTime.Now)}";
+			}
+
+			[Kept]
+			public interface IStaticMethod
+			{
+				[Kept]
+				abstract static string GetMessage ();
+			}
+		}
+
+		[Kept]
+		public class GenericInterface
+		{
+			[Kept]
+			public static void Test ()
+			{
+				// Get message via generic method
+				var message = MessageFactory.GetMessage<TypeWithMethodAccessedViaInterface, int> ();
+				Console.WriteLine (message);
+
+				// Get message directly from type
+				var message2 = TypeWithMethodAccessedDirectly.GetMessage ();
+				Console.WriteLine (message2);
+			}
+
+			[Kept]
+			public static class MessageFactory
+			{
+				[Kept]
+				public static U GetMessage<T, U> () where T : IStaticAbstractMethodGeneric<U>
+				{
+					return T.GetMessage ();
+				}
+			}
+
+			[Kept]
+			[KeptInterface (typeof (IStaticAbstractMethodGeneric<int>))]
+			public class TypeWithMethodAccessedViaInterface : IStaticAbstractMethodGeneric<int>
+			{
+				// Force the string to have dynamic element so that it doesn't get compiled in as a literal
+				[Kept]
+				[KeptOverride (typeof (IStaticAbstractMethodGeneric<int>))]
+				public static int GetMessage () => 0;
+			}
+
+			[Kept]
+			public class TypeWithMethodAccessedDirectly : IStaticAbstractMethodGeneric<int>
+			{
+				// Force the string to have dynamic element so that it doesn't get compiled in as a literal
+				[Kept]
+				[RemovedOverride (typeof (IStaticAbstractMethodGeneric<int>))]
+				public static int GetMessage () => 0;
+			}
+
+			[Kept]
+			public interface IStaticAbstractMethodGeneric<T>
+			{
+				[Kept]
+				abstract static T GetMessage ();
+			}
+		}
+
+		[Kept]
+		public class GenericInterfaceGenericType
+		{
+			[Kept]
+			public static void Test ()
+			{
+				// Get message via generic method
+				var message = MessageFactory.GetMessage<TypeWithMethodAccessedViaInterface<int>, int> ();
+				Console.WriteLine (message);
+
+				// Get message directly from type
+				var message2 = TypeWithMethodAccessedDirectly<int>.GetMessage ();
+				Console.WriteLine (message2);
+			}
+
+			[Kept]
+			public static class MessageFactory
+			{
+				[Kept]
+				public static U GetMessage<T, U> () where T : IStaticAbstractMethodGeneric<U>
+				{
+					return T.GetMessage ();
+				}
+			}
+
+			[Kept]
+			[KeptInterface (typeof (IStaticAbstractMethodGeneric<>), "T")]
+			public class TypeWithMethodAccessedViaInterface<T> : IStaticAbstractMethodGeneric<T>
+			{
+				// Force the string to have dynamic element so that it doesn't get compiled in as a literal
+				[Kept]
+				[KeptOverride ("Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods.RemovedInterfaceImplementationRemovedOverride/" +
+					"GenericInterfaceGenericType/IStaticAbstractMethodGeneric`1<T>")]
+				public static T GetMessage () => default;
+			}
+
+			[Kept]
+			public class TypeWithMethodAccessedDirectly<T> : IStaticAbstractMethodGeneric<T>
+			{
+				// Force the string to have dynamic element so that it doesn't get compiled in as a literal
+				[Kept]
+				[RemovedOverride ("Mono.Linker.Tests.Cases.Inheritance.Interfaces.StaticInterfaceMethods.RemovedInterfaceImplementationRemovedOverride/" +
+					"GenericInterfaceGenericType/IStaticAbstractMethodGeneric`1<T>")]
+				public static T GetMessage () => default;
+			}
+
+			[Kept]
+			public interface IStaticAbstractMethodGeneric<T>
+			{
+				[Kept]
+				abstract static T GetMessage ();
+			}
+		}
+
+		[Kept]
+		public class PrivateExplicitImplementationReflectedOver
+		{
+			[Kept]
+			public static void Test ()
+			{
+				IInstanceMethod i = new MyType ();
+				Console.WriteLine (i.GetMessage ());
+				var type = typeof (MyTypeReflected);
+				var method = type.GetMethod ("GetMessage");
+				method.Invoke (null, null);
+			}
+
+			[Kept]
+			public interface IInstanceMethod
+			{
+				[Kept]
+				public string GetMessage ();
+			}
+
+			[Kept]
+			[KeptInterface (typeof (IInstanceMethod))]
+			public class MyType : IInstanceMethod
+			{
+				[Kept]
+				public MyType () { }
+
+				[Kept]
+				[KeptOverride (typeof (IInstanceMethod))]
+				string IInstanceMethod.GetMessage () => "hello";
+			}
+
+			[Kept]
+			// Reflecting over MyTypeReflected to get the GetMessage method makes MyTypeReflected 'RelevantToVariantCasting' which marks the interface
+			// It's hard to think of a scenario where the method would be kept but not the interface implementation
+			[KeptInterface (typeof (IInstanceMethod))]
+			public class MyTypeReflected : IInstanceMethod
+			{
+				public MyTypeReflected () { }
+
+				[Kept]
+				[KeptOverride (typeof (IInstanceMethod))]
+				[ExpectBodyModified]
+				string IInstanceMethod.GetMessage () => "hello";
+			}
+		}
+	}
+}

src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs

@@ -356,45 +356,27 @@ void VerifyOverrides (MethodDefinition original, MethodDefinition linked)
 				return;
 			var expectedBaseTypesOverridden = new HashSet<string> (original.CustomAttributes
 				.Where (ca => ca.AttributeType.Name == nameof (KeptOverrideAttribute))
-				.Select (ca => (ca.ConstructorArguments[0].Value as TypeReference).FullName));
+				.Select (ca => (ca.ConstructorArguments[0].Value as TypeReference)?.FullName ?? (string) ca.ConstructorArguments[0].Value));
 			var originalBaseTypesOverridden = new HashSet<string> (original.Overrides.Select (ov => ov.DeclaringType.FullName));
 			var linkedBaseTypesOverridden = new HashSet<string> (linked.Overrides.Select (ov => ov.DeclaringType.FullName));
 			foreach (var expectedBaseType in expectedBaseTypesOverridden) {
 				Assert.IsTrue (originalBaseTypesOverridden.Contains (expectedBaseType),
 					$"Method {linked.FullName} was expected to keep override {expectedBaseType}::{linked.Name}, " +
-					 "but it wasn't in the unlinked assembly");
+					 "but it wasn't in the unlinked assembly" + string.Join(Environment.NewLine, originalBaseTypesOverridden));
 				Assert.IsTrue (linkedBaseTypesOverridden.Contains (expectedBaseType),
 					$"Method {linked.FullName} was expected to override {expectedBaseType}::{linked.Name}");
 			}
 
 			var expectedBaseTypesNotOverridden = new HashSet<string> (original.CustomAttributes
 				.Where (ca => ca.AttributeType.Name == nameof (RemovedOverrideAttribute))
-				.Select (ca => (ca.ConstructorArguments[0].Value as TypeReference).FullName));
+				.Select (ca => (ca.ConstructorArguments[0].Value as TypeReference)?.FullName ?? (string) ca.ConstructorArguments[0].Value));
 			foreach (var expectedRemovedBaseType in expectedBaseTypesNotOverridden) {
 				Assert.IsTrue (originalBaseTypesOverridden.Contains (expectedRemovedBaseType),
 					$"Method {linked.FullName} was expected to remove override {expectedRemovedBaseType}::{linked.Name}, " +
 					$"but it wasn't in the unlinked assembly");
 				Assert.IsFalse (linkedBaseTypesOverridden.Contains (expectedRemovedBaseType),
 					$"Method {linked.FullName} was expected to not override {expectedRemovedBaseType}::{linked.Name}");
 			}
-
-			foreach (var overriddenMethod in linked.Overrides) {
-				if (overriddenMethod.Resolve () is not MethodDefinition overriddenDefinition) {
-					Assert.Fail ($"Method {linked.GetDisplayName ()} overrides method {overriddenMethod} which does not exist");
-				} else if (overriddenDefinition.DeclaringType.IsInterface) {
-					Assert.True (linked.DeclaringType.Interfaces.Select (i => i.InterfaceType).Contains (overriddenMethod.DeclaringType),
-						$"Method {linked} overrides method {overriddenMethod}, but {linked.DeclaringType} does not implement interface {overriddenMethod.DeclaringType}");
-				} else {
-					TypeDefinition baseType = linked.DeclaringType;
-					TypeReference overriddenType = overriddenMethod.DeclaringType;
-					while (baseType is not null) {
-						if (baseType.Equals (overriddenType))
-							break;
-						if (baseType.Resolve ()?.BaseType is null)
-							Assert.Fail ($"Method {linked} overrides method {overriddenMethod} from, but {linked.DeclaringType} does not inherit from type {overriddenMethod.DeclaringType}");
-					}
-				}
-			}
 		}
 
 		static string FormatBaseOrInterfaceAttributeValue (CustomAttribute attr)
@@ -551,6 +533,8 @@ IEnumerable<string> VerifyMethodInternal (MethodDefinition src, MethodDefinition
 					yield return $"Method `{src.FullName}' should have been removed";
 			}
 
+			VerifyOverrides(src, linked);
+
 			foreach (var err in VerifyMethodKept (src, linked, compilerGenerated))
 				yield return err;
 		}