Process static interface methods as virtual methods (#2926)

Static methods implementing static interface methods are not marked as virtual and were not checked for matching RUC and DAM annotations. This change adds all base methods to the _virtual_methods list to be checked for matching DAM and RUC annotations. This may cause extra/unnecessary warnings for mismatched annotations even a method is removed by the linker.

Author: jtschuster

src/linker/Linker.Steps/MarkStep.cs

@@ -3166,6 +3166,7 @@ protected virtual void ProcessMethod (MethodDefinition method, in DependencyInfo
 			}
 
 			MarkMethodSpecialCustomAttributes (method);
+
 			if (method.IsVirtual)
 				_virtual_methods.Add ((method, ScopeStack.CurrentScope));
 
@@ -3345,8 +3346,15 @@ void MarkBaseMethods (MethodDefinition method)
 				return;
 
 			foreach (MethodDefinition base_method in base_methods) {
-				if (base_method.DeclaringType.IsInterface && !method.DeclaringType.IsInterface)
+				// We should add all interface base methods to _virtual_methods for virtual override annotation validation
+				// Interfaces from preserved scope will be missed if we don't add them here
+				// This will produce warnings for all interface methods and virtual methods regardless of whether the interface, interface implementation, or interface method is kept or not.
+				if (base_method.DeclaringType.IsInterface && !method.DeclaringType.IsInterface) {
+					// These are all virtual, no need to check IsVirtual before adding to list
+					_virtual_methods.Add ((base_method, ScopeStack.CurrentScope));
+					// _virtual_methods is a list and might have duplicates, but it's mostly just used for override validation, so it shouldn't matter
 					continue;
+				}
 
 				MarkMethod (base_method, new DependencyInfo (DependencyKind.BaseMethod, method), ScopeStack.CurrentScope.Origin);
 				MarkBaseMethods (base_method);

src/linker/Linker/Annotations.cs

@@ -608,7 +608,7 @@ internal bool IsInRequiresUnreferencedCodeScope (MethodDefinition method)
 		/// <summary>
 		/// Determines if a member requires unreferenced code (and thus any usage of such method should be warned about).
 		/// </summary>
-		/// <remarks>Unlike <see cref="IsInRequiresUnreferencedCodeScope(MethodDefinition)"/> only static methods 
+		/// <remarks>Unlike <see cref="IsInRequiresUnreferencedCodeScope(MethodDefinition)"/> only static methods
 		/// and .ctors are reported as requiring unreferenced code when the declaring type has RUC on it.</remarks>
 		internal bool DoesMemberRequireUnreferencedCode (IMemberDefinition member, [NotNullWhen (returnValue: true)] out RequiresUnreferencedCodeAttribute? attribute)
 		{
@@ -673,11 +673,27 @@ internal bool DoesFieldRequireUnreferencedCode (FieldDefinition field, [NotNullW
 			return TryGetLinkerAttribute (field.DeclaringType, out attribute);
 		}
 
+		/// <Summary>
+		/// Adds a virtual method to the queue if it is annotated and must have matching annotations on its bases and overrides. It does not check if the method is marked before producing a warning about mismatched annotations.
+		/// </summary>
 		public void EnqueueVirtualMethod (MethodDefinition method)
 		{
 			if (!method.IsVirtual)
 				return;
 
+			// Implementations of static interface methods are not virtual and won't reach here
+			// We'll search through the implementations of static interface methods to find if any need to be enqueued
+			if (method.IsStatic) {
+				Debug.Assert (method.DeclaringType.IsInterface);
+				var overrides = GetOverrides (method);
+				if (overrides is not null) {
+					foreach (var @override in overrides) {
+						if (FlowAnnotations.RequiresVirtualMethodDataFlowAnalysis (@override.Override) || HasLinkerAttribute<RequiresUnreferencedCodeAttribute> (@override.Override))
+							VirtualMethodsWithAnnotationsToValidate.Add (@override.Override);
+					}
+				}
+			}
+
 			if (FlowAnnotations.RequiresVirtualMethodDataFlowAnalysis (method) || HasLinkerAttribute<RequiresUnreferencedCodeAttribute> (method))
 				VirtualMethodsWithAnnotationsToValidate.Add (method);
 		}

test/ILLink.RoslynAnalyzer.Tests/StaticInterfaceMethods.cs

@@ -10,6 +10,12 @@ public sealed partial class StaticInterfaceMethodsTests : LinkerTestBase
 	{
 		protected override string TestSuiteName => "Inheritance.Interfaces.StaticInterfaceMethods";
 
+		[Fact]
+		public Task BaseProvidesInterfaceMethod ()
+		{
+			return RunTest (allowMissingWarnings: false);
+		}
+
 		[Fact]
 		public Task StaticAbstractInterfaceMethods ()
 		{

test/Mono.Linker.Tests.Cases/DataFlow/Dependencies/Library.cs

@@ -0,0 +1,66 @@
+// 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.Diagnostics.CodeAnalysis;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Mono.Linker.Tests.Cases.DataFlow.Dependencies
+{
+	public class Library
+	{
+		public interface IAnnotatedMethods
+		{
+			static abstract void GenericWithMethodsStatic<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ();
+
+			static abstract void ParamWithMethodsStatic ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type t);
+
+			[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			static abstract Type ReturnWithMethodsStatic ();
+
+			void GenericWithMethods<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ();
+
+			void ParamWithMethods ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type t);
+
+			[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			Type ReturnWithMethods ();
+		}
+
+		public interface IUnannotatedMethods
+		{
+			static abstract void GenericStatic<T> ();
+
+			static abstract void ParamStatic (Type t);
+
+			static abstract Type ReturnStatic ();
+
+			void Generic<T> ();
+
+			void Param (Type t);
+
+			Type Return ();
+		}
+
+		public abstract class AnnotatedMethods
+		{
+			public abstract void GenericWithMethods<[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] T> ();
+
+			public abstract void ParamWithMethods ([DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type t);
+
+			[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			public abstract Type ReturnWithMethods ();
+		}
+
+		public abstract class UnannotatedMethods
+		{
+			public abstract void Generic<T> ();
+
+			public abstract void Param (Type t);
+
+			public abstract Type Return ();
+		}
+	}
+}