Add diagnostic suppressor for the MarkMethodsAsStatic diagnostics on custom marshallers

docs/project/list-of-diagnostics.md

@@ -173,3 +173,9 @@ The diagnostic id values reserved for .NET Libraries analyzer warnings are `SYSL
 |  __`SYSLIB1062`__ | *_`SYSLIB1062`-`SYSLIB1064` reserved for Microsoft.Interop.LibraryImportGenerator._* |
 |  __`SYSLIB1063`__ | *_`SYSLIB1062`-`SYSLIB1064` reserved for Microsoft.Interop.LibraryImportGenerator._* |
 |  __`SYSLIB1064`__ | *_`SYSLIB1062`-`SYSLIB1064` reserved for Microsoft.Interop.LibraryImportGenerator._* |
+
+### Diagnostic Suppressions (`SYSLIBSUPPRESS****`)
+
+| Suppression ID     | Suppressed Diagnostic ID | Description |
+| :----------------- | :----------------------- | :---------- |
+| __`SYSLIBSUPPRESS0001`__ | CA1822 | Do not offer to make methods static when the methods need to be instance methods for a custom marshaller shape. |

src/libraries/Common/src/System/Runtime/InteropServices/HandleRefMarshaller.cs

@@ -23,7 +23,6 @@ public void FromManaged(HandleRef handle)
 
             public void OnInvoked() => GC.KeepAlive(_handle.Wrapper);
 
-            [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "This method is part of the marshaller shape and is required to be an instance method.")]
             public void Free() { }
         }
     }

src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.cs

@@ -89,7 +89,6 @@ public void OnInvoked()
                     GC.KeepAlive(_managed);
                 }
 
-                [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "This method is part of the marshaller shape and is required to be an instance method.")]
                 public void Free() { }
             }
         }

src/libraries/System.Drawing.Common/src/System/Drawing/Image.cs

@@ -77,7 +77,6 @@ public void OnInvoked()
                     GC.KeepAlive(_managed);
                 }
 
-                [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "This method is part of the marshaller shape and is required to be an instance method.")]
                 public void Free() { }
             }
         }

src/libraries/System.Drawing.Common/src/System/Drawing/Imaging/MetafileHeaderWmf.cs

@@ -157,7 +157,6 @@ public MetafileHeaderWmf ToManaged()
                     return _managed;
                 }
 
-                [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "This method is part of the marshaller shape and is required to be an instance method.")]
                 public void Free() { }
             }
     }

src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/ShapeBreakingDiagnosticSuppressor.cs

@@ -0,0 +1,75 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using System.Text;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+namespace Microsoft.Interop.Analyzers
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+    public class ShapeBreakingDiagnosticSuppressor : DiagnosticSuppressor
+    {
+        public static readonly SuppressionDescriptor MarkMethodsAsStaticSuppression = new SuppressionDescriptor("SYSLIBSUPPRESS0001", "CA1822", "Do not offer to make methods static when the methods need to be instance methods for a custom marshaller shape.");
+
+        public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions =>
+            ImmutableArray.Create(MarkMethodsAsStaticSuppression);
+
+        public override void ReportSuppressions(SuppressionAnalysisContext context)
+        {
+            foreach (var diagnostic in context.ReportedDiagnostics)
+            {
+                if (diagnostic.Id == MarkMethodsAsStaticSuppression.SuppressedDiagnosticId)
+                {
+                    SuppressMarkMethodsAsStaticDiagnosticIfNeeded(context, diagnostic);
+                }
+            }
+        }
+
+        private static void SuppressMarkMethodsAsStaticDiagnosticIfNeeded(SuppressionAnalysisContext context, Diagnostic diagnostic)
+        {
+            SemanticModel model = context.GetSemanticModel(diagnostic.Location.SourceTree);
+            ISymbol diagnosedSymbol = model.GetDeclaredSymbol(diagnostic.Location.SourceTree.GetRoot(context.CancellationToken).FindNode(diagnostic.Location.SourceSpan), context.CancellationToken);
+
+            if (diagnosedSymbol.Kind != SymbolKind.Method)
+            {
+                return;
+            }
+
+            if (FindContainingEntryPointTypeAndManagedType(diagnosedSymbol.ContainingType) is (INamedTypeSymbol entryPointMarshallerType, INamedTypeSymbol managedType))
+            {
+                bool isLinearCollectionMarshaller = ManualTypeMarshallingHelper.IsLinearCollectionEntryPoint(entryPointMarshallerType);
+                (MarshallerShape _, StatefulMarshallerShapeHelper.MarshallerMethods methods) = StatefulMarshallerShapeHelper.GetShapeForType(diagnosedSymbol.ContainingType, managedType, isLinearCollectionMarshaller, context.Compilation);
+                if (methods.IsShapeMethod((IMethodSymbol)diagnosedSymbol))
+                {
+                    // If we are a method of the shape on the stateful marshaller shape, then we need to be our current shape.
+                    // So, suppress the diagnostic to make this method static, as that would break the shape.
+                    context.ReportSuppression(Suppression.Create(MarkMethodsAsStaticSuppression, diagnostic));
+                }
+            }
+        }
+
+        private static (INamedTypeSymbol EntryPointType, INamedTypeSymbol ManagedType)? FindContainingEntryPointTypeAndManagedType(INamedTypeSymbol marshallerType)
+        {
+            for (INamedTypeSymbol containingType = marshallerType; containingType is not null; containingType = containingType.ContainingType)
+            {
+                AttributeData? attrData = containingType.GetAttributes().FirstOrDefault(
+                        attr => attr.AttributeClass?.ToDisplayString() == TypeNames.CustomMarshallerAttribute
+                            && attr.AttributeConstructor is not null
+                            && !attr.ConstructorArguments[0].IsNull
+                            && attr.ConstructorArguments[2].Value is INamedTypeSymbol marshallerTypeInAttribute
+                            && ManualTypeMarshallingHelper.TryResolveMarshallerType(containingType, marshallerTypeInAttribute, _ => { }, out ITypeSymbol? constructedMarshallerType)
+                            && SymbolEqualityComparer.Default.Equals(constructedMarshallerType, marshallerType));
+                if (attrData is not null)
+                {
+                    return (containingType, (INamedTypeSymbol)attrData.ConstructorArguments[0].Value);
+                }
+            }
+            return null;
+        }
+    }
+}

src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManualTypeMarshallingHelper.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Linq.Expressions;
 using System.Runtime.CompilerServices;
@@ -62,6 +63,8 @@ public static class MarshalUsingProperties
             public const string ConstantElementCount = nameof(ConstantElementCount);
         }
 
+        private static void IgnoreDiagnostic(Diagnostic diagnostic) { }
+
         public static bool IsLinearCollectionEntryPoint(INamedTypeSymbol entryPointType)
         {
             return entryPointType.IsGenericType
@@ -149,28 +152,9 @@ private static bool TryGetMarshallersFromEntryType(
                     continue;
 
                 ITypeSymbol marshallerType = marshallerTypeOnAttr;
-                if (isLinearCollectionMarshalling && marshallerTypeOnAttr is INamedTypeSymbol namedMarshallerType)
+                if (!TryResolveMarshallerType(entryPointType, marshallerType, IgnoreDiagnostic, out marshallerType))
                 {
-                    // Update the marshaller type with resolved type arguments based on the entry point type
-                    // We expect the entry point to already have its type arguments updated based on the managed type
-                    Stack<string> nestedTypeNames = new Stack<string>();
-                    INamedTypeSymbol currentType = namedMarshallerType;
-                    while (currentType is not null)
-                    {
-                        if (currentType.IsConstructedFromEqualTypes(entryPointType))
-                            break;
-
-                        nestedTypeNames.Push(currentType.Name);
-                        currentType = currentType.ContainingType;
-                    }
-
-                    currentType = entryPointType;
-                    foreach (string name in nestedTypeNames)
-                    {
-                        currentType = currentType.GetTypeMembers(name).First();
-                    }
-
-                    marshallerType = currentType;
+                    continue;
                 }
 
                 // TODO: Report invalid shape for mode
@@ -198,6 +182,72 @@ private static bool TryGetMarshallersFromEntryType(
             return true;
         }
 
+        /// <summary>
+        /// Resolve the (possibly unbound generic) marshaller type to a fully constructed type based on the entry point type's generic parameters.
+        /// </summary>
+        /// <param name="entryPointType">The entry point type</param>
+        /// <param name="attributeMarshallerType">The marshaller type from the CustomMarshallerAttribute</param>
+        /// <returns>A fully constructed marshaller type</returns>
+        public static bool TryResolveMarshallerType(INamedTypeSymbol entryPointType, ITypeSymbol? attributeMarshallerType, Action<Diagnostic> reportDiagnostic, [NotNullWhen(true)] out ITypeSymbol? marshallerType)
+        {
+            if (attributeMarshallerType is null)
+            {
+                marshallerType = null;
+                return false;
+            }
+
+            if (attributeMarshallerType is not INamedTypeSymbol namedMarshallerType)
+            {
+                marshallerType = attributeMarshallerType;
+                return true;
+            }
+
+            // Update the marshaller type with resolved type arguments based on the entry point type
+            // We expect the entry point to already have its type arguments updated based on the managed type
+            Stack<INamedTypeSymbol> nestedTypes = new();
+            INamedTypeSymbol currentType = namedMarshallerType;
+            int totalArity = 0;
+            while (currentType is not null)
+            {
+                nestedTypes.Push(currentType);
+                totalArity += currentType.Arity;
+                currentType = currentType.ContainingType;
+            }
+
+            if (totalArity != entryPointType.Arity)
+            {
+                //TODO: Report diagnostic
+                marshallerType = null;
+                return false;
+            }
+
+            int currentArityOffset = 0;
+            currentType = null;
+            while (nestedTypes.Count > 0)
+            {
+                if (currentType is null)
+                {
+                    currentType = nestedTypes.Pop();
+                }
+                else
+                {
+                    INamedTypeSymbol originalType = nestedTypes.Pop();
+                    currentType = currentType.GetTypeMembers(originalType.Name, originalType.Arity).First();
+                }
+
+                if (currentType.TypeParameters.Length > 0)
+                {
+                    currentType = currentType.ConstructedFrom.Construct(
+                        ImmutableArray.CreateRange(entryPointType.TypeArguments, currentArityOffset, currentType.TypeParameters.Length, x => x),
+                        ImmutableArray.CreateRange(entryPointType.TypeArgumentNullableAnnotations, currentArityOffset, currentType.TypeParameters.Length, x => x));
+                    currentArityOffset += currentType.TypeParameters.Length;
+                }
+            }
+
+            marshallerType = currentType;
+            return true;
+        }
+
         /// <summary>
         /// Resolve a non-<see cref="INamedTypeSymbol"/> <paramref name="managedType"/> to the correct
         /// managed type if <paramref name="entryType"/> is generic and <paramref name="managedType"/>

src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/MarshallerShape.cs

@@ -411,12 +411,30 @@ public record MarshallerMethods
             public IMethodSymbol? ToUnmanaged { get; init; }
             public IMethodSymbol? Free { get; init; }
             public IMethodSymbol? OnInvoked { get; init; }
+            public IMethodSymbol? StatefulGetPinnableReference { get; init; }
 
             // Linear collection
             public IMethodSymbol? ManagedValuesSource { get; init; }
             public IMethodSymbol? UnmanagedValuesDestination { get; init; }
             public IMethodSymbol? ManagedValuesDestination { get; init; }
             public IMethodSymbol? UnmanagedValuesSource { get; init; }
+
+            public bool IsShapeMethod(IMethodSymbol method)
+            {
+                return SymbolEqualityComparer.Default.Equals(method, FromManaged)
+                    || SymbolEqualityComparer.Default.Equals(method, FromManagedWithBuffer)
+                    || SymbolEqualityComparer.Default.Equals(method, ToManaged)
+                    || SymbolEqualityComparer.Default.Equals(method, ToManagedGuranteed)
+                    || SymbolEqualityComparer.Default.Equals(method, FromUnmanaged)
+                    || SymbolEqualityComparer.Default.Equals(method, ToUnmanaged)
+                    || SymbolEqualityComparer.Default.Equals(method, Free)
+                    || SymbolEqualityComparer.Default.Equals(method, OnInvoked)
+                    || SymbolEqualityComparer.Default.Equals(method, StatefulGetPinnableReference)
+                    || SymbolEqualityComparer.Default.Equals(method, ManagedValuesSource)
+                    || SymbolEqualityComparer.Default.Equals(method, UnmanagedValuesDestination)
+                    || SymbolEqualityComparer.Default.Equals(method, ManagedValuesDestination)
+                    || SymbolEqualityComparer.Default.Equals(method, UnmanagedValuesSource);
+            }
         }
 
         public static (MarshallerShape shape, MarshallerMethods methods) GetShapeForType(ITypeSymbol marshallerType, ITypeSymbol managedType, bool isLinearCollectionMarshaller, Compilation compilation)
@@ -518,9 +536,12 @@ public static (MarshallerShape shape, MarshallerMethods methods) GetShapeForType
             {
                 shape |= MarshallerShape.StatelessPinnableReference;
             }
-            if (GetStatefulGetPinnableReference(marshallerType) is not null)
+
+            IMethodSymbol statefulGetPinnableReference = GetStatefulGetPinnableReference(marshallerType);
+            if (statefulGetPinnableReference is not null)
             {
                 shape |= MarshallerShape.StatefulPinnableReference;
+                methods = methods with { StatefulGetPinnableReference = statefulGetPinnableReference };
             }
 
             return (shape, methods);

src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/LibraryImportGenerator.Unit.Tests.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
@@ -25,4 +25,18 @@
   <ItemGroup>
     <None Include="$(RepoRoot)/NuGet.config" Link="NuGet.config" CopyToOutputDirectory="PreserveNewest" />
   </ItemGroup>
+
+  <!-- Reference the actual NetAnalyzers project to ensure we're testing our diagnostic suppressors against the actual analyzers they'll run with, not fake shims we have to maintain -->
+  <ItemGroup Condition="'$(RunAnalyzers)' != 'false'">
+    <PackageReference Update="Microsoft.CodeAnalysis.NetAnalyzers" Version="$(MicrosoftCodeAnalysisNetAnalyzersVersion)" GeneratePathProperty="true" />
+  </ItemGroup>
+  <ItemGroup Condition="'$(RunAnalyzers)' == 'false'">
+    <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="$(MicrosoftCodeAnalysisNetAnalyzersVersion)" ExcludeAssets="all" GeneratePathProperty="true" />
+  </ItemGroup>
+
+  <Target Name="AddNetAnalyzersAssemblyAsReference" BeforeTargets="ResolveAssemblyReferences" DependsOnTargets="ResolveProjectReferences">
+    <ItemGroup>
+      <Reference Include="$(PkgMicrosoft_CodeAnalysis_NetAnalyzers)/analyzers/dotnet/cs/Microsoft.CodeAnalysis.NetAnalyzers.dll" />
+    </ItemGroup>
+  </Target>
 </Project>