Fix scenario where source generator isn't detecting parameters passed to WinRT mapped interfaces (#1867)

manodasanW · web-flow · commit 8649ee3eeb24 · 2024-11-10T16:54:30.000-08:00
* Fix scenario where source generator isn't detecting parameters passed to WinRT mapped interfaces

* Add another test case
diff --git a/src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs b/src/Authoring/WinRT.SourceGenerator/AotOptimizer.cs
@@ -239,9 +239,7 @@ private static (VtableAttribute, EquatableArray<VtableAttribute>) GetVtableAttri
             bool isCsWinRTCcwLookupTableGeneratorEnabled)
         {
             var isManagedOnlyType = GeneratorHelper.IsManagedOnlyType(compilation);
-            var isWinRTTypeFunc = checkForComponentTypes ?
-                GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(compilation) :
-                GeneratorHelper.IsWinRTType;
+            var isWinRTTypeFunc = GeneratorHelper.IsWinRTType(compilation, checkForComponentTypes);
             var vtableAttribute = GetVtableAttributeToAdd(symbol, isManagedOnlyType, isWinRTTypeFunc, typeMapper, compilation, false);
             if (vtableAttribute != default)
             {
@@ -317,7 +315,7 @@ private static VtableAttribute GetVtableAttributesForTaskAdapters(GeneratorSynta
                         return GetVtableAttributeToAdd(
                             constructedAdapterType,
                             GeneratorHelper.IsManagedOnlyType(context.SemanticModel.Compilation),
-                            !isCsWinRTComponent ? GeneratorHelper.IsWinRTType : GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(context.SemanticModel.Compilation),
+                            GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isCsWinRTComponent),
                             typeMapper,
                             context.SemanticModel.Compilation,
                             false);
@@ -1176,20 +1174,21 @@ private static EquatableArray<VtableAttribute> GetVtableAttributesToAddOnLookupT
             TypeMapper typeMapper,
             bool isCsWinRTComponent)
         {
+            var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isCsWinRTComponent);
             return GetVtableAttributesToAddOnLookupTable(
                     context,
                     typeMapper,
                     GeneratorHelper.IsManagedOnlyType(context.SemanticModel.Compilation),
-                    !isCsWinRTComponent ? GeneratorHelper.IsWinRTType : GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(context.SemanticModel.Compilation),
-                    GeneratorHelper.IsWinRTClass(context.SemanticModel.Compilation));
+                    isWinRTType,
+                    GeneratorHelper.IsWinRTClassOrInterface(context.SemanticModel.Compilation, isWinRTType, typeMapper));
         }
 
         private static EquatableArray<VtableAttribute> GetVtableAttributesToAddOnLookupTable(
             GeneratorSyntaxContext context,
             TypeMapper typeMapper,
             Func<ISymbol, bool> isManagedOnlyType,
             Func<ISymbol, TypeMapper, bool> isWinRTType,
-            Func<ISymbol, bool> isWinRTClass)
+            Func<ISymbol, bool, bool> isWinRTClassOrInterface)
         {
             HashSet<ITypeSymbol> visitedTypes = new(SymbolEqualityComparer.Default);
             HashSet<VtableAttribute> vtableAttributes = new();
@@ -1203,7 +1202,7 @@ private static EquatableArray<VtableAttribute> GetVtableAttributesToAddOnLookupT
                     // and end up calling a projection function (i.e. ones generated by XAML compiler)
                     // In theory, another library can also be called which can call a projected function
                     // but not handling those scenarios for now.
-                    (isWinRTClass(methodSymbol.ContainingSymbol) ||
+                    (isWinRTClassOrInterface(methodSymbol.ContainingSymbol, true) ||
                      SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))
                 {
                     // Get the concrete types directly from the argument rather than
@@ -1232,13 +1231,14 @@ private static EquatableArray<VtableAttribute> GetVtableAttributesToAddOnLookupT
             {
                 var leftSymbol = context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol;
                 if (leftSymbol is IPropertySymbol propertySymbol &&
-                    (isWinRTClass(propertySymbol.ContainingSymbol) ||
+                    (isWinRTClassOrInterface(propertySymbol.ContainingSymbol, true) ||
                      SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))
                 {
                     AddVtableAttributesForType(context.SemanticModel.GetTypeInfo(assignment.Right), propertySymbol.Type);
                 }
                 else if (leftSymbol is IFieldSymbol fieldSymbol &&
-                    (isWinRTClass(fieldSymbol.ContainingSymbol) || 
+                    // WinRT interfaces don't have fields, so we don't need to check for them.
+                    (isWinRTClassOrInterface(fieldSymbol.ContainingSymbol, false) || 
                      SymbolEqualityComparer.Default.Equals(fieldSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))
                 {
                     AddVtableAttributesForType(context.SemanticModel.GetTypeInfo(assignment.Right), fieldSymbol.Type);
@@ -1423,7 +1423,7 @@ private static EquatableArray<VtableAttribute> GetVtableAttributesToAddOnLookupT
             bool isCsWinRTComponent)
         {
             var isManagedOnlyType = GeneratorHelper.IsManagedOnlyType(context.SemanticModel.Compilation);
-            var isWinRTType = !isCsWinRTComponent ? GeneratorHelper.IsWinRTType : GeneratorHelper.IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(context.SemanticModel.Compilation);
+            var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isCsWinRTComponent);
             HashSet<VtableAttribute> vtableAttributes = new();
 
             foreach (var attributeData in context.Attributes)
diff --git a/src/Authoring/WinRT.SourceGenerator/Helper.cs b/src/Authoring/WinRT.SourceGenerator/Helper.cs
@@ -368,16 +368,22 @@ public static bool AllowUnsafe(Compilation compilation)
             return compilation is CSharpCompilation csharpCompilation && csharpCompilation.Options.AllowUnsafe;
         }
 
-        // Whether the class itself is a WinRT projected class.
-        // This is similar to whether it is a WinRT type, but custom type mappings
-        // are excluded given those are C# implemented classes.
-        public static Func<ISymbol, bool> IsWinRTClass(Compilation compilation)
+        // Returns whether it is a WinRT class or interface.
+        // If the bool parameter is true, then custom mapped interfaces are also considered.
+        // This function is similar to whether it is a WinRT type, but custom type mapped
+        // classes are excluded given those are C# implemented classes such as string.
+        public static Func<ISymbol, bool, bool> IsWinRTClassOrInterface(Compilation compilation, Func<ISymbol, TypeMapper, bool> isWinRTType, TypeMapper mapper)
         {
             var winrtRuntimeTypeAttribute = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute");
-            return IsWinRTClassHelper;
+            return IsWinRTClassOrInterfaceHelper;
 
-            bool IsWinRTClassHelper(ISymbol type)
+            bool IsWinRTClassOrInterfaceHelper(ISymbol type, bool includeMappedInterfaces)
             {
+                if (type is ITypeSymbol typeSymbol && typeSymbol.TypeKind == TypeKind.Interface)
+                {
+                    return isWinRTType(type, mapper);
+                }
+
                 return HasAttributeWithType(type, winrtRuntimeTypeAttribute);
             }
         }
@@ -668,14 +674,14 @@ public static bool IsManagedOnlyType(ISymbol symbol, ITypeSymbol winrtExposedTyp
             return false;
         }
 
-        public static Func<ISymbol, TypeMapper, bool> IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(Compilation compilation)
+        public static Func<ISymbol, TypeMapper, bool> IsWinRTType(Compilation compilation, bool isComponentProject)
         {
             var winrtTypeAttribute = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute");
             return IsWinRTTypeHelper;
 
             bool IsWinRTTypeHelper(ISymbol type, TypeMapper typeMapper)
             {
-                return IsWinRTType(type, winrtTypeAttribute, typeMapper, true, compilation.Assembly);
+                return IsWinRTType(type, winrtTypeAttribute, typeMapper, isComponentProject, compilation.Assembly);
             }
         }
 
diff --git a/src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs b/src/Authoring/WinRT.SourceGenerator/WinRTAotCodeFixer.cs
@@ -146,7 +146,8 @@ public override void Initialize(AnalysisContext context)
                 {
                     context.RegisterSyntaxNodeAction(context =>
                     {
-                        var isWinRTClass = GeneratorHelper.IsWinRTClass(context.SemanticModel.Compilation);
+                        var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isComponentProject);
+                        var isWinRTClassOrInterface = GeneratorHelper.IsWinRTClassOrInterface(context.SemanticModel.Compilation, isWinRTType, typeMapper);
 
                         if (context.Node is InvocationExpressionSyntax invocation)
                         {
@@ -164,7 +165,7 @@ public override void Initialize(AnalysisContext context)
                                 // and end up calling a projection function (i.e. ones generated by XAML compiler)
                                 // In theory, another library can also be called which can call a projected function
                                 // but not handling those scenarios for now.
-                                else if(isWinRTClass(methodSymbol.ContainingSymbol) ||
+                                else if(isWinRTClassOrInterface(methodSymbol.ContainingSymbol, true) ||
                                  SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))
                                 {
                                     // Get the concrete types directly from the argument rather than
@@ -198,7 +199,7 @@ public override void Initialize(AnalysisContext context)
                         {
                             var leftSymbol = context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol;
                             if (leftSymbol is IPropertySymbol propertySymbol &&
-                                (isWinRTClass(propertySymbol.ContainingSymbol) ||
+                                (isWinRTClassOrInterface(propertySymbol.ContainingSymbol, true) ||
                                  SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))
                             {
                                 var assignmentType = context.SemanticModel.GetTypeInfo(assignment.Right);
@@ -209,7 +210,8 @@ public override void Initialize(AnalysisContext context)
                                 }
                             }
                             else if (leftSymbol is IFieldSymbol fieldSymbol &&
-                                (isWinRTClass(fieldSymbol.ContainingSymbol) ||
+                                // WinRT interfaces don't have fields, so we don't need to check for them.
+                                (isWinRTClassOrInterface(fieldSymbol.ContainingSymbol, false) ||
                                  SymbolEqualityComparer.Default.Equals(fieldSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))
                             {
                                 var assignmentType = context.SemanticModel.GetTypeInfo(assignment.Right);
diff --git a/src/Tests/FunctionalTests/Collections/Program.cs b/src/Tests/FunctionalTests/Collections/Program.cs
@@ -1,5 +1,6 @@
 ﻿using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.ComponentModel;
 using System.Linq;
 using System.Threading;
@@ -268,6 +269,25 @@
 Action<int, int> s = (a, b) => { _ = a + b; };
 ActionToFunction(s)(2, 3);
 
+var intToListDict = instance.GetIntToListDictionary();
+intToListDict.Add(2, new List<EnumValue> { EnumValue.One, EnumValue.Two });
+intToListDict[4] = new Collection<EnumValue> { EnumValue.Two };
+if (intToListDict.Count != 3 ||
+    intToListDict[1].First() != EnumValue.One)
+{
+    return 101;
+}
+
+if (intToListDict[2].Count != 2 || intToListDict[2].First() != EnumValue.One)
+{
+    return 101;
+}
+
+if (intToListDict[4].Count != 1 || intToListDict[4].First() != EnumValue.Two)
+{
+    return 101;
+}
+
 return 100;
 
 static bool SequencesEqual<T>(IEnumerable<T> x, params IEnumerable<T>[] list) => list.All((y) => x.SequenceEqual(y));
diff --git a/src/Tests/TestComponentCSharp/Class.cpp b/src/Tests/TestComponentCSharp/Class.cpp
@@ -1097,6 +1097,12 @@ namespace winrt::TestComponentCSharp::implementation
             { L"String2", ComposedNonBlittableStruct{ { 2 }, { L"String2" }, { true, false, true, false }, { 2 } } }
         });
     }
+
+    IMap<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>> Class::GetIntToListDictionary()
+    {
+        auto vector = winrt::single_threaded_vector(std::vector { TestComponentCSharp::EnumValue::One });
+        return single_threaded_map<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>>(std::map<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>>{ {1, vector}});
+    }
     
     struct ComposedBlittableStructComparer
     {
diff --git a/src/Tests/TestComponentCSharp/Class.h b/src/Tests/TestComponentCSharp/Class.h
@@ -291,7 +291,8 @@ namespace winrt::TestComponentCSharp::implementation
         Windows::Foundation::Collections::IMap<hstring, TestComponentCSharp::ComposedBlittableStruct> GetStringToBlittableDictionary();
         Windows::Foundation::Collections::IMap<hstring, TestComponentCSharp::ComposedNonBlittableStruct> GetStringToNonBlittableDictionary();
         Windows::Foundation::Collections::IMap<TestComponentCSharp::ComposedBlittableStruct, Windows::Foundation::IInspectable> GetBlittableToObjectDictionary();
-       
+        Windows::Foundation::Collections::IMap<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>> GetIntToListDictionary();
+
         // Test IIDOptimizer -- testing the windows projection covers most code paths, and these two types exercise the rest.
         Windows::Foundation::Collections::IVectorView<Microsoft::UI::Xaml::Data::DataErrorsChangedEventArgs> GetEventArgsVector();
         Windows::Foundation::Collections::IVectorView<TestComponentCSharp::ProvideUri> GetNonGenericDelegateVector();
diff --git a/src/Tests/TestComponentCSharp/TestComponentCSharp.idl b/src/Tests/TestComponentCSharp/TestComponentCSharp.idl
@@ -338,6 +338,7 @@ namespace TestComponentCSharp
         Windows.Foundation.Collections.IMap<String, ComposedBlittableStruct> GetStringToBlittableDictionary();
         Windows.Foundation.Collections.IMap<String, ComposedNonBlittableStruct> GetStringToNonBlittableDictionary();
         Windows.Foundation.Collections.IMap<ComposedBlittableStruct, Object> GetBlittableToObjectDictionary();
+        Windows.Foundation.Collections.IMap<Int32, Windows.Foundation.Collections.IVector<EnumValue> > GetIntToListDictionary();
 
         // Test IIDOptimizer
         Windows.Foundation.Collections.IVectorView<Microsoft.UI.Xaml.Data.DataErrorsChangedEventArgs> GetEventArgsVector();

Original file line number	Diff line number	Diff line change
`@@ -368,16 +368,22 @@ public static bool AllowUnsafe(Compilation compilation)`
`368`	`368`	`return compilation is CSharpCompilation csharpCompilation && csharpCompilation.Options.AllowUnsafe;`
`369`	`369`	`}`
`370`	`370`
`371`		`- // Whether the class itself is a WinRT projected class.`
`372`		`- // This is similar to whether it is a WinRT type, but custom type mappings`
`373`		`- // are excluded given those are C# implemented classes.`
`374`		`- public static Func<ISymbol, bool> IsWinRTClass(Compilation compilation)`
	`371`	`+ // Returns whether it is a WinRT class or interface.`
	`372`	`+ // If the bool parameter is true, then custom mapped interfaces are also considered.`
	`373`	`+ // This function is similar to whether it is a WinRT type, but custom type mapped`
	`374`	`+ // classes are excluded given those are C# implemented classes such as string.`
	`375`	`+ public static Func<ISymbol, bool, bool> IsWinRTClassOrInterface(Compilation compilation, Func<ISymbol, TypeMapper, bool> isWinRTType, TypeMapper mapper)`
`375`	`376`	`{`
`376`	`377`	`var winrtRuntimeTypeAttribute = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute");`
`377`		`- return IsWinRTClassHelper;`
	`378`	`+ return IsWinRTClassOrInterfaceHelper;`
`378`	`379`
`379`		`- bool IsWinRTClassHelper(ISymbol type)`
	`380`	`+ bool IsWinRTClassOrInterfaceHelper(ISymbol type, bool includeMappedInterfaces)`
`380`	`381`	`{`
	`382`	`+ if (type is ITypeSymbol typeSymbol && typeSymbol.TypeKind == TypeKind.Interface)`
	`383`	`+ {`
	`384`	`+ return isWinRTType(type, mapper);`
	`385`	`+ }`
	`386`	`+`
`381`	`387`	`return HasAttributeWithType(type, winrtRuntimeTypeAttribute);`
`382`	`388`	`}`
`383`	`389`	`}`
`@@ -668,14 +674,14 @@ public static bool IsManagedOnlyType(ISymbol symbol, ITypeSymbol winrtExposedTyp`
`668`	`674`	`return false;`
`669`	`675`	`}`
`670`	`676`
`671`		`- public static Func<ISymbol, TypeMapper, bool> IsWinRTTypeWithPotentialAuthoringComponentTypesFunc(Compilation compilation)`
	`677`	`+ public static Func<ISymbol, TypeMapper, bool> IsWinRTType(Compilation compilation, bool isComponentProject)`
`672`	`678`	`{`
`673`	`679`	`var winrtTypeAttribute = compilation.GetTypeByMetadataName("WinRT.WindowsRuntimeTypeAttribute");`
`674`	`680`	`return IsWinRTTypeHelper;`
`675`	`681`
`676`	`682`	`bool IsWinRTTypeHelper(ISymbol type, TypeMapper typeMapper)`
`677`	`683`	`{`
`678`		`- return IsWinRTType(type, winrtTypeAttribute, typeMapper, true, compilation.Assembly);`
	`684`	`+ return IsWinRTType(type, winrtTypeAttribute, typeMapper, isComponentProject, compilation.Assembly);`
`679`	`685`	`}`
`680`	`686`	`}`
`681`	`687`
Original file line number	Diff line number	Diff line change
`@@ -146,7 +146,8 @@ public override void Initialize(AnalysisContext context)`
`146`	`146`	`{`
`147`	`147`	`context.RegisterSyntaxNodeAction(context =>`
`148`	`148`	`{`
`149`		`- var isWinRTClass = GeneratorHelper.IsWinRTClass(context.SemanticModel.Compilation);`
	`149`	`+ var isWinRTType = GeneratorHelper.IsWinRTType(context.SemanticModel.Compilation, isComponentProject);`
	`150`	`+ var isWinRTClassOrInterface = GeneratorHelper.IsWinRTClassOrInterface(context.SemanticModel.Compilation, isWinRTType, typeMapper);`
`150`	`151`
`151`	`152`	`if (context.Node is InvocationExpressionSyntax invocation)`
`152`	`153`	`{`
`@@ -164,7 +165,7 @@ public override void Initialize(AnalysisContext context)`
`164`	`165`	`// and end up calling a projection function (i.e. ones generated by XAML compiler)`
`165`	`166`	`// In theory, another library can also be called which can call a projected function`
`166`	`167`	`// but not handling those scenarios for now.`
`167`		`- else if(isWinRTClass(methodSymbol.ContainingSymbol) \|\|`
	`168`	`+ else if(isWinRTClassOrInterface(methodSymbol.ContainingSymbol, true) \|\|`
`168`	`169`	`SymbolEqualityComparer.Default.Equals(methodSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly))`
`169`	`170`	`{`
`170`	`171`	`// Get the concrete types directly from the argument rather than`
`@@ -198,7 +199,7 @@ public override void Initialize(AnalysisContext context)`
`198`	`199`	`{`
`199`	`200`	`var leftSymbol = context.SemanticModel.GetSymbolInfo(assignment.Left).Symbol;`
`200`	`201`	`if (leftSymbol is IPropertySymbol propertySymbol &&`
`201`		`- (isWinRTClass(propertySymbol.ContainingSymbol) \|\|`
	`202`	`+ (isWinRTClassOrInterface(propertySymbol.ContainingSymbol, true) \|\|`
`202`	`203`	`SymbolEqualityComparer.Default.Equals(propertySymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))`
`203`	`204`	`{`
`204`	`205`	`var assignmentType = context.SemanticModel.GetTypeInfo(assignment.Right);`
`@@ -209,7 +210,8 @@ public override void Initialize(AnalysisContext context)`
`209`	`210`	`}`
`210`	`211`	`}`
`211`	`212`	`else if (leftSymbol is IFieldSymbol fieldSymbol &&`
`212`		`- (isWinRTClass(fieldSymbol.ContainingSymbol) \|\|`
	`213`	`+ // WinRT interfaces don't have fields, so we don't need to check for them.`
	`214`	`+ (isWinRTClassOrInterface(fieldSymbol.ContainingSymbol, false) \|\|`
`213`	`215`	`SymbolEqualityComparer.Default.Equals(fieldSymbol.ContainingAssembly, context.SemanticModel.Compilation.Assembly)))`
`214`	`216`	`{`
`215`	`217`	`var assignmentType = context.SemanticModel.GetTypeInfo(assignment.Right);`
Original file line number	Diff line number	Diff line change
`@@ -1097,6 +1097,12 @@ namespace winrt::TestComponentCSharp::implementation`
`1097`	`1097`	`{ L"String2", ComposedNonBlittableStruct{ { 2 }, { L"String2" }, { true, false, true, false }, { 2 } } }`
`1098`	`1098`	`});`
`1099`	`1099`	`}`
	`1100`	`+`
	`1101`	`+ IMap<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>> Class::GetIntToListDictionary()`
	`1102`	`+ {`
	`1103`	`+ auto vector = winrt::single_threaded_vector(std::vector { TestComponentCSharp::EnumValue::One });`
	`1104`	`+ return single_threaded_map<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>>(std::map<int32_t, Windows::Foundation::Collections::IVector<TestComponentCSharp::EnumValue>>{ {1, vector}});`
	`1105`	`+ }`
`1100`	`1106`
`1101`	`1107`	`struct ComposedBlittableStructComparer`
`1102`	`1108`	`{`