Skip interfaces not publicly accessible in authoring scenarios (#1394)

* Add internal COM interfaces to authoring test

* Add MixedWinRTClassicCOM authoring tests

* Skip interface types not publicly accessible

* Minor code refactoring

* Suppress diagnostics for not publicly accessible types

* Use fully qualified name for [Guid] to avoid conflicts

* Remmove collection expressions in projection attributes

* Skip processing explicit members of internal interfaces

* Skip processing symbols nested in internal types

* Add ABI types for AOT generator

* Restore original order/filtering to gather interfaces

* Fix build errors in AuthoringConsumptionTest

* Add TestMixedWinRTCOMWrapper to activation manifest

* Fix unit test

* Fix typos in ABI method names

src/Authoring/WinRT.SourceGenerator/DiagnosticUtils.cs

@@ -87,10 +87,12 @@ private void CheckDeclarations()
             foreach (var declaration in syntaxReceiver.Declarations)
             {
                 var model = _context.Compilation.GetSemanticModel(declaration.SyntaxTree);
+                var symbol = model.GetDeclaredSymbol(declaration);
 
                 // Check symbol information for whether it is public to properly detect partial types
-                // which can leave out modifier.
-                if (model.GetDeclaredSymbol(declaration).DeclaredAccessibility != Accessibility.Public)
+                // which can leave out modifier. Also ignore nested types not effectively public
+                if (symbol.DeclaredAccessibility != Accessibility.Public ||
+                    (symbol is ITypeSymbol typeSymbol && !typeSymbol.IsPubliclyAccessible()))
                 {
                     continue;
                 }

src/Authoring/WinRT.SourceGenerator/Extensions/SymbolExtensions.cs

@@ -0,0 +1,54 @@
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+
+#nullable enable
+
+namespace Generator;
+
+/// <summary>
+/// Extensions for symbol types.
+/// </summary>
+internal static class SymbolExtensions
+{
+    /// <summary>
+    /// Checks whether a given type symbol is publicly accessible (ie. it's public and not nested in any non public type).
+    /// </summary>
+    /// <param name="type">The type symbol to check for public accessibility.</param>
+    /// <returns>Whether <paramref name="type"/> is publicly accessible.</returns>
+    public static bool IsPubliclyAccessible(this ITypeSymbol type)
+    {
+        for (ITypeSymbol? currentType = type; currentType is not null; currentType = currentType.ContainingType)
+        {
+            // If any type in the type hierarchy is not public, the type is not public.
+            // This makes sure to detect public types nested into eg. a private type.
+            if (currentType.DeclaredAccessibility is not Accessibility.Public)
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    /// <summary>
+    /// Checks whether a given symbol is an explicit interface implementation of a member of an internal interface (or more than one).
+    /// </summary>
+    /// <param name="symbol">The input member symbol to check.</param>
+    /// <returns>Whether <paramref name="symbol"/> is an explicit interface implementation of internal interfaces.</returns>
+    public static bool IsExplicitInterfaceImplementationOfInternalInterfaces(this ISymbol symbol)
+    {
+        static bool IsAnyContainingTypePublic(IEnumerable<ISymbol> symbols)
+        {
+            return symbols.Any(static symbol => symbol.ContainingType!.IsPubliclyAccessible());
+        }
+
+        return symbol switch
+        {
+            IMethodSymbol { ExplicitInterfaceImplementations: { Length: > 0 } methods } => !IsAnyContainingTypePublic(methods),
+            IPropertySymbol { ExplicitInterfaceImplementations: { Length: > 0 } properties } => !IsAnyContainingTypePublic(properties),
+            IEventSymbol { ExplicitInterfaceImplementations: { Length: > 0 } events } => !IsAnyContainingTypePublic(events),
+            _ => false
+        };
+    }
+}

src/Authoring/WinRT.SourceGenerator/WinRTTypeWriter.cs

@@ -1303,21 +1303,38 @@ Symbol GetType(string type, bool isGeneric = false, int genericIndex = -1, bool
 
         private IEnumerable<INamedTypeSymbol> GetInterfaces(INamedTypeSymbol symbol, bool includeInterfacesWithoutMappings = false)
         {
-            HashSet<INamedTypeSymbol> interfaces = new HashSet<INamedTypeSymbol>();
-            foreach (var @interface in symbol.Interfaces)
+            HashSet<INamedTypeSymbol> interfaces = new();
+
+            // Gather all interfaces that are publicly accessible. We specifically need to exclude interfaces
+            // that are not public, as eg. those might be used for additional cloaked WinRT/COM interfaces.
+            // Ignoring them here makes sure that they're not processed to be part of the .winmd file.
+            void GatherPubliclyAccessibleInterfaces(ITypeSymbol symbol)
             {
-                interfaces.Add(@interface);
-                interfaces.UnionWith(@interface.AllInterfaces);
+                foreach (var @interface in symbol.Interfaces)
+                {
+                    if (@interface.IsPubliclyAccessible())
+                    {
+                        _ = interfaces.Add(@interface);
+                    }
+
+                    // We're not using AllInterfaces on purpose: we only want to gather all interfaces but not
+                    // from the base type. That's handled below to skip types that are already WinRT projections.
+                    foreach (var @interface2 in @interface.AllInterfaces)
+                    {
+                        if (@interface2.IsPubliclyAccessible())
+                        {
+                            _ = interfaces.Add(@interface2);
+                        }
+                    }
+                }
             }
 
+            GatherPubliclyAccessibleInterfaces(symbol);
+
             var baseType = symbol.BaseType;
             while (baseType != null && !IsWinRTType(baseType))
             {
-                interfaces.UnionWith(baseType.Interfaces);
-                foreach (var @interface in baseType.Interfaces)
-                {
-                    interfaces.UnionWith(@interface.AllInterfaces);
-                }
+                GatherPubliclyAccessibleInterfaces(baseType);
 
                 baseType = baseType.BaseType;
             }
@@ -2010,6 +2027,13 @@ void AddComponentType(INamedTypeSymbol type, Action visitTypeDeclaration = null)
                 }
                 else
                 {
+                    // Special case: skip members that are explicitly implementing internal interfaces.
+                    // This allows implementing classic COM internal interfaces with non-WinRT signatures.
+                    if (member.IsExplicitInterfaceImplementationOfInternalInterfaces())
+                    {
+                        continue;
+                    }
+
                     if (member is IMethodSymbol method &&
                         (method.MethodKind == MethodKind.Ordinary ||
                          method.MethodKind == MethodKind.ExplicitInterfaceImplementation ||
@@ -2736,12 +2760,19 @@ public void FinalizeGeneration()
             }
         }
 
-        public bool IsPublic(ISymbol type)
+        public bool IsPublic(ISymbol symbol)
         {
-            return type.DeclaredAccessibility == Accessibility.Public ||
-                type is IMethodSymbol method && !method.ExplicitInterfaceImplementations.IsDefaultOrEmpty ||
-                type is IPropertySymbol property && !property.ExplicitInterfaceImplementations.IsDefaultOrEmpty ||
-                type is IEventSymbol @event && !@event.ExplicitInterfaceImplementations.IsDefaultOrEmpty;
+            // Check that the type has either public accessibility, or is an explicit interface implementation
+            if (symbol.DeclaredAccessibility == Accessibility.Public ||
+                symbol is IMethodSymbol method && !method.ExplicitInterfaceImplementations.IsDefaultOrEmpty ||
+                symbol is IPropertySymbol property && !property.ExplicitInterfaceImplementations.IsDefaultOrEmpty ||
+                symbol is IEventSymbol @event && !@event.ExplicitInterfaceImplementations.IsDefaultOrEmpty)
+            {
+                // If we have a containing type, we also check that it's publicly accessible
+                return symbol.ContainingType is not { } containingType || containingType.IsPubliclyAccessible();
+            }
+
+            return false;
         }
 
         public void GetNamespaceAndTypename(string qualifiedName, out string @namespace, out string typename)

src/Tests/AuthoringConsumptionTest/AuthoringConsumptionTest.exe.manifest

@@ -74,5 +74,9 @@
         name="AuthoringTest.TestClass"
         threadingModel="both"
         xmlns="urn:schemas-microsoft-com:winrt.v1" />
+    <activatableClass
+       name="AuthoringTest.TestMixedWinRTCOMWrapper"
+       threadingModel="both"
+       xmlns="urn:schemas-microsoft-com:winrt.v1" />
   </file>
 </assembly>

src/Tests/AuthoringConsumptionTest/pch.h

@@ -4,6 +4,7 @@
 // conflict with Storyboard::GetCurrentTime
 #undef GetCurrentTime
 
+#include <Windows.h>
 #include <winrt/Windows.Foundation.h>
 #include <winrt/Windows.Foundation.Collections.h>
 

src/Tests/AuthoringConsumptionTest/test.cpp

@@ -639,4 +639,38 @@ TEST(AuthoringTest, PartialClass)
     EXPECT_EQ(partialStruct.X, 3);
     EXPECT_EQ(partialStruct.Y, 4);
     EXPECT_EQ(partialStruct.Z, 5);
+}
+
+TEST(AuthoringTest, MixedWinRTClassicCOM)
+{
+    TestMixedWinRTCOMWrapper wrapper;
+
+    // Normal WinRT methods work as you'd expect
+    EXPECT_EQ(wrapper.HelloWorld(), L"Hello from mixed WinRT/COM");
+
+    // Verify we can grab the internal interface
+    IID internalInterface1Iid;
+    check_hresult(IIDFromString(L"{C7850559-8FF2-4E54-A237-6ED813F20CDC}", &internalInterface1Iid));
+    winrt::com_ptr<::IUnknown> unknown1 = wrapper.as<::IUnknown>();
+    winrt::com_ptr<::IUnknown> internalInterface1;
+    EXPECT_EQ(unknown1->QueryInterface(internalInterface1Iid, internalInterface1.put_void()), S_OK);
+
+    // Verify we can grab the nested public interface (in an internal type)
+    IID internalInterface2Iid;
+    check_hresult(IIDFromString(L"{8A08E18A-8D20-4E7C-9242-857BFE1E3159}", &internalInterface2Iid));
+    winrt::com_ptr<::IUnknown> unknown2 = wrapper.as<::IUnknown>();
+    winrt::com_ptr<::IUnknown> internalInterface2;
+    EXPECT_EQ(unknown2->QueryInterface(internalInterface2Iid, internalInterface2.put_void()), S_OK);
+
+    typedef int (__stdcall* GetNumber)(void*, int*);
+
+    int number;
+
+    // Validate the first call on IInternalInterface1
+    EXPECT_EQ(reinterpret_cast<GetNumber>((*reinterpret_cast<void***>(internalInterface1.get()))[3])(internalInterface1.get(), &number), S_OK);
+    EXPECT_EQ(number, 42);
+
+    // Validate the second call on IInternalInterface2
+    EXPECT_EQ(reinterpret_cast<GetNumber>((*reinterpret_cast<void***>(internalInterface2.get()))[3])(internalInterface2.get(), &number), S_OK);
+    EXPECT_EQ(number, 123);
 }

src/Tests/AuthoringTest/Program.cs

@@ -9,12 +9,17 @@
 using System.ComponentModel.DataAnnotations;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
 using System.Windows.Input;
 using Windows.Foundation;
 using Windows.Foundation.Collections;
 using Windows.Foundation.Metadata;
+using Windows.Graphics.Effects;
+using WinRT;
+using WinRT.Interop;
 
 #pragma warning disable CA1416
 
@@ -1569,6 +1574,142 @@ public partial struct PartialStruct
     {
         public double Z;
     }
+
+    public sealed class TestMixedWinRTCOMWrapper : IGraphicsEffectSource, IPublicInterface, IInternalInterface1, SomeInternalType.IInternalInterface2
+    {
+        public string HelloWorld()
+        {
+            return "Hello from mixed WinRT/COM";
+        }
+
+        unsafe int IInternalInterface1.GetNumber(int* value)
+        {
+            *value = 42;
+
+            return 0;
+        }
+
+        unsafe int SomeInternalType.IInternalInterface2.GetNumber(int* value)
+        {
+            *value = 123;
+
+            return 0;
+        }
+    }
+
+    public interface IPublicInterface
+    {
+        string HelloWorld();
+    }
+
+    // Internal, classic COM interface
+    [global::System.Runtime.InteropServices.Guid("C7850559-8FF2-4E54-A237-6ED813F20CDC")]
+    [WindowsRuntimeType]
+    [WindowsRuntimeHelperType(typeof(IInternalInterface1))]
+    internal unsafe interface IInternalInterface1
+    {
+        int GetNumber(int* value);
+
+        [global::System.Runtime.InteropServices.Guid("C7850559-8FF2-4E54-A237-6ED813F20CDC")]
+        public struct Vftbl
+        {
+            public static readonly IntPtr AbiToProjectionVftablePtr = InitVtbl();
+
+            private static IntPtr InitVtbl()
+            {
+                Vftbl* lpVtbl = (Vftbl*)ComWrappersSupport.AllocateVtableMemory(typeof(Vftbl), sizeof(Vftbl));
+
+                lpVtbl->IUnknownVftbl = IUnknownVftbl.AbiToProjectionVftbl;
+                lpVtbl->GetNumber = &GetNumberFromAbi;
+
+                return (IntPtr)lpVtbl;
+            }
+
+            private IUnknownVftbl IUnknownVftbl;
+            private delegate* unmanaged[Stdcall]<void*, int*, int> GetNumber;
+
+            [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]
+            private static int GetNumberFromAbi(void* thisPtr, int* value)
+            {
+                try
+                {
+                    return ComWrappersSupport.FindObject<IInternalInterface1>((IntPtr)thisPtr).GetNumber(value);
+                }
+                catch (Exception e)
+                {
+                    ExceptionHelpers.SetErrorInfo(e);
+
+                    return Marshal.GetHRForException(e);
+                }
+            }
+        }
+    }
+
+    internal struct SomeInternalType
+    {
+        // Nested, classic COM interface
+        [global::System.Runtime.InteropServices.Guid("8A08E18A-8D20-4E7C-9242-857BFE1E3159")]
+        [WindowsRuntimeType]
+        [WindowsRuntimeHelperType(typeof(IInternalInterface2))]
+        public unsafe interface IInternalInterface2
+        {
+            int GetNumber(int* value);
+
+            [global::System.Runtime.InteropServices.Guid("8A08E18A-8D20-4E7C-9242-857BFE1E3159")]
+            public struct Vftbl
+            {
+                public static readonly IntPtr AbiToProjectionVftablePtr = InitVtbl();
+
+                private static IntPtr InitVtbl()
+                {
+                    Vftbl* lpVtbl = (Vftbl*)ComWrappersSupport.AllocateVtableMemory(typeof(Vftbl), sizeof(Vftbl));
+
+                    lpVtbl->IUnknownVftbl = IUnknownVftbl.AbiToProjectionVftbl;
+                    lpVtbl->GetNumber = &GetNumberFromAbi;
+
+                    return (IntPtr)lpVtbl;
+                }
+
+                private IUnknownVftbl IUnknownVftbl;
+                private delegate* unmanaged[Stdcall]<void*, int*, int> GetNumber;
+
+                [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]
+                private static int GetNumberFromAbi(void* thisPtr, int* value)
+                {
+                    try
+                    {
+                        return ComWrappersSupport.FindObject<IInternalInterface2>((IntPtr)thisPtr).GetNumber(value);
+                    }
+                    catch (Exception e)
+                    {
+                        ExceptionHelpers.SetErrorInfo(e);
+
+                        return Marshal.GetHRForException(e);
+                    }
+                }
+            }
+        }
+    }
+}
+
+namespace ABI.AuthoringTest
+{
+    internal static class IInternalInterface1Methods
+    {
+        public static Guid IID => typeof(global::AuthoringTest.IInternalInterface1).GUID;
+
+        public static IntPtr AbiToProjectionVftablePtr => global::AuthoringTest.IInternalInterface1.Vftbl.AbiToProjectionVftablePtr;
+    }
+
+    internal struct SomeInternalType
+    {
+        internal static class IInternalInterface2Methods
+        {
+            public static Guid IID => typeof(global::AuthoringTest.SomeInternalType.IInternalInterface2).GUID;
+
+            public static IntPtr AbiToProjectionVftablePtr => global::AuthoringTest.SomeInternalType.IInternalInterface2.Vftbl.AbiToProjectionVftablePtr;
+        }
+    }
 }
 
 namespace AnotherNamespace