Don't emit default ctor for non activatable types (#1817)

* Don't emit default ctor for non activatable types * Add test for non activatable type * Add more unit tests * Remove 'NonActivatableType' test * Add one more test case
microsoft · Oct 11, 2024 · 8482124 · 8482124
1 parent 7578691
commit 8482124
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 3 deletions.
diff --git a/src/Authoring/WinRT.SourceGenerator/WinRTTypeWriter.cs b/src/Authoring/WinRT.SourceGenerator/WinRTTypeWriter.cs
@@ -1932,12 +1932,25 @@ void AddComponentType(INamedTypeSymbol type, Action visitTypeDeclaration = null)
 
             bool isInterface = type.TypeKind == TypeKind.Interface;
             bool hasConstructor = false;
+            bool hasAtLeastOneNonPublicConstructor = false;
             bool hasDefaultConstructor = false;
             foreach (var member in type.GetMembers())
             {
                 if (!IsPublic(member) || typeDeclaration.CustomMappedSymbols.Contains(member))
                 {
                     Logger.Log(member.Kind + " member skipped " + member.Name);
+
+                    // We want to track whether a given public class has at least one non-public constructor.
+                    // In this case, the class is still exposed to WinRT, but it's not activatable. This is
+                    // different than a class with no explicit constructor, where we do want to generate that
+                    // in the .winmd, and make the class activatable. But we want to avoid always emitting a
+                    // default constructor for classes that only have non-public ones.
+                    if (type.TypeKind == TypeKind.Class &&
+                        member is IMethodSymbol { MethodKind: MethodKind.Constructor })
+                    {
+                        hasAtLeastOneNonPublicConstructor = true;
+                    }
+
                     continue;
                 }
 
@@ -1988,8 +2001,8 @@ void AddComponentType(INamedTypeSymbol type, Action visitTypeDeclaration = null)
                 CheckAndMarkSymbolForAttributes(member);
             }
 
-            // implicit constructor if none defined
-            if (!hasConstructor && type.TypeKind == TypeKind.Class && !type.IsStatic)
+            // Implicit constructor if none defined, but only if the type doesn't already have some non-public constructor
+            if (!hasConstructor && !hasAtLeastOneNonPublicConstructor && type.TypeKind == TypeKind.Class && !type.IsStatic)
             {
                 string constructorMethodName = ".ctor";
                 var methodDefinitionHandle = AddMethodDefinition(

diff --git a/src/Tests/AuthoringConsumptionTest/AuthoringConsumptionTest.exe.manifest b/src/Tests/AuthoringConsumptionTest/AuthoringConsumptionTest.exe.manifest
@@ -82,9 +82,21 @@
        name="AuthoringTest.CustomXamlMetadataProvider"
        threadingModel="both"
        xmlns="urn:schemas-microsoft-com:winrt.v1" />
-     <activatableClass
+    <activatableClass
        name="AuthoringTest.CustomInterfaceGuidClass"
        threadingModel="both"
        xmlns="urn:schemas-microsoft-com:winrt.v1" />
+    <activatableClass
+       name="AuthoringTest.NonActivatableType"
+       threadingModel="both"
+       xmlns="urn:schemas-microsoft-com:winrt.v1" />
+    <activatableClass
+       name="AuthoringTest.NonActivatableFactory"
+       threadingModel="both"
+       xmlns="urn:schemas-microsoft-com:winrt.v1" />
+    <activatableClass
+       name="AuthoringTest.TypeOnlyActivatableViaItsOwnFactory"
+       threadingModel="both"
+       xmlns="urn:schemas-microsoft-com:winrt.v1" />
   </file>
 </assembly>
diff --git a/src/Tests/AuthoringConsumptionTest/test.cpp b/src/Tests/AuthoringConsumptionTest/test.cpp
@@ -735,4 +735,14 @@ TEST(AuthoringTest, CustomInterfaceGuid)
     check_hresult(customInterfaceClassUnknown->QueryInterface(customInterfaceIid, reinterpret_cast<void**>(winrt::put_abi(customInterface))));
 
     EXPECT_EQ(customInterface.HelloWorld(), L"Hello World!");
+}
+
+TEST(AuthoringTest, NonActivatableFactory)
+{
+    EXPECT_EQ(NonActivatableFactory::Create().GetText(), L"Test123");
+}
+
+TEST(AuthoringTest, TypeOnlyActivatableViaItsOwnFactory)
+{
+    EXPECT_EQ(TypeOnlyActivatableViaItsOwnFactory::Create().GetText(), L"Hello!");
 }
diff --git a/src/Tests/AuthoringTest/Program.cs b/src/Tests/AuthoringTest/Program.cs
@@ -1865,6 +1865,50 @@ public sealed class CustomInterfaceGuidClass : ICustomInterfaceGuid
     {
         public string HelloWorld() => "Hello World!";
     }
+
+    public sealed class NonActivatableType
+    {
+        private readonly string _text;
+
+        // This should not be referenced by the generated activation factory
+        internal NonActivatableType(string text)
+        {
+            _text = text;
+        }
+
+        public string GetText()
+        {
+            return _text;
+        }
+    }
+
+    public static class NonActivatableFactory
+    {
+        public static NonActivatableType Create()
+        {
+            return new("Test123");
+        }
+    }
+
+    public sealed class TypeOnlyActivatableViaItsOwnFactory
+    {
+        private readonly string _text;
+
+        private TypeOnlyActivatableViaItsOwnFactory(string text)
+        {
+            _text = text;
+        }
+
+        public static TypeOnlyActivatableViaItsOwnFactory Create()
+        {
+            return new("Hello!");
+        }
+
+        public string GetText()
+        {
+            return _text;
+        }
+    }
 }
 
 namespace ABI.AuthoringTest