[ComInterfaceGenerator] Warn if StringMarshalling doesn't match base and warn if base interface cannot be generated

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceContext.cs

@@ -1,9 +1,13 @@
 // 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.Diagnostics;
 using System.Threading;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 
 namespace Microsoft.Interop
 {
@@ -12,44 +16,67 @@ internal sealed record ComInterfaceContext(ComInterfaceInfo Info, ComInterfaceCo
         /// <summary>
         /// Takes a list of ComInterfaceInfo, and creates a list of ComInterfaceContext.
         /// </summary>
-        public static ImmutableArray<ComInterfaceContext> GetContexts(ImmutableArray<ComInterfaceInfo> data, CancellationToken _)
+        public static ImmutableArray<(ComInterfaceContext? Context, Diagnostic? Diagnostic)> GetContexts(ImmutableArray<ComInterfaceInfo> data, CancellationToken _)
         {
-            Dictionary<string, ComInterfaceInfo> symbolToInterfaceInfoMap = new();
-            var accumulator = ImmutableArray.CreateBuilder<ComInterfaceContext>(data.Length);
+            Dictionary<string, ComInterfaceInfo> nameToInterfaceInfoMap = new();
+            var accumulator = ImmutableArray.CreateBuilder<(ComInterfaceContext? Context, Diagnostic? Diagnostic)>(data.Length);
             foreach (var iface in data)
             {
-                symbolToInterfaceInfoMap.Add(iface.ThisInterfaceKey, iface);
+                nameToInterfaceInfoMap.Add(iface.ThisInterfaceKey, iface);
             }
-            Dictionary<string, ComInterfaceContext> symbolToContextMap = new();
+            Dictionary<string, (ComInterfaceContext? Context, Diagnostic? Diagnostic)> nameToContextCache = new();
 
             foreach (var iface in data)
             {
                 accumulator.Add(AddContext(iface));
             }
             return accumulator.MoveToImmutable();
 
-            ComInterfaceContext AddContext(ComInterfaceInfo iface)
+            (ComInterfaceContext? Context, Diagnostic? Diagnostic) AddContext(ComInterfaceInfo iface)
             {
-                if (symbolToContextMap.TryGetValue(iface.ThisInterfaceKey, out var cachedValue))
+                if (nameToContextCache.TryGetValue(iface.ThisInterfaceKey, out var cachedValue))
                 {
                     return cachedValue;
                 }
 
                 if (iface.BaseInterfaceKey is null)
                 {
                     var baselessCtx = new ComInterfaceContext(iface, null);
-                    symbolToContextMap[iface.ThisInterfaceKey] = baselessCtx;
-                    return baselessCtx;
+                    nameToContextCache[iface.ThisInterfaceKey] = (baselessCtx, null);
+                    return (baselessCtx, null);
                 }
 
-                if (!symbolToContextMap.TryGetValue(iface.BaseInterfaceKey, out var baseContext))
+                if (
+                    // Cached base info has a diagnostic - failure
+                    (nameToContextCache.TryGetValue(iface.BaseInterfaceKey, out var basePair) && basePair.Diagnostic is not null)
+                    // Cannot find base ComInterfaceInfo - failure (failed ComInterfaceInfo creation)
+                    || !nameToInterfaceInfoMap.TryGetValue(iface.BaseInterfaceKey, out var baseInfo)
+                    // Newly calculated base context pair has a diagnostic - failure
+                    || (AddContext(baseInfo) is { } baseReturnPair && baseReturnPair.Diagnostic is not null))
                 {
-                    baseContext = AddContext(symbolToInterfaceInfoMap[iface.BaseInterfaceKey]);
+                    // The base has failed generation at some point, so this interface cannot be generated
+                    (ComInterfaceContext, Diagnostic?) diagnosticPair = (null,
+                        Diagnostic.Create(
+                            GeneratorDiagnostics.BaseInterfaceIsNotGenerated,
+                            iface.DiagnosticLocation.AsLocation(), iface.ThisInterfaceKey, iface.BaseInterfaceKey));
+                    nameToContextCache[iface.ThisInterfaceKey] = diagnosticPair;
+                    return diagnosticPair;
                 }
+                var baseContext = basePair.Context ?? baseReturnPair.Context;
+                Debug.Assert(baseContext != null);
                 var ctx = new ComInterfaceContext(iface, baseContext);
-                symbolToContextMap[iface.ThisInterfaceKey] = ctx;
-                return ctx;
+                (ComInterfaceContext, Diagnostic?) contextPair = (ctx, null);
+                nameToContextCache[iface.ThisInterfaceKey] = contextPair;
+                return contextPair;
             }
         }
+
+        internal ComInterfaceContext GetTopLevelBase()
+        {
+            var currBase = Base;
+            while (currBase is not null)
+                currBase = currBase.Base;
+            return currBase;
+        }
     }
 }

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
@@ -52,13 +53,21 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
 
             var interfaceSymbolsWithoutDiagnostics = interfaceSymbolAndDiagnostics
                 .Where(data => data.Diagnostic is null)
-                .Select((data, ct) =>
-                (data.InterfaceInfo, data.Symbol));
+                .Select((data, ct) => (data.InterfaceInfo, data.Symbol));
 
-            var interfaceContexts = interfaceSymbolsWithoutDiagnostics
+            var interfaceContextsAndDiagnostics = interfaceSymbolsWithoutDiagnostics
                 .Select((data, ct) => data.InterfaceInfo!)
                 .Collect()
                 .SelectMany(ComInterfaceContext.GetContexts);
+            context.RegisterDiagnostics(interfaceContextsAndDiagnostics.Select((data, ct) => data.Diagnostic));
+            var interfaceContexts = interfaceContextsAndDiagnostics
+                .Where(data => data.Context is not null)
+                .Select((data, ct) => data.Context!);
+            // Filter down interface symbols to remove those with diagnostics from GetContexts
+            interfaceSymbolsWithoutDiagnostics = interfaceSymbolsWithoutDiagnostics
+                .Zip(interfaceContextsAndDiagnostics)
+                .Where(data => data.Right.Diagnostic is null)
+                .Select((data, ct) => data.Left);
 
             var comMethodsAndSymbolsAndDiagnostics = interfaceSymbolsWithoutDiagnostics.Select(ComMethodInfo.GetMethodsFromInterface);
             context.RegisterDiagnostics(comMethodsAndSymbolsAndDiagnostics.SelectMany(static (methodList, ct) => methodList.Select(m => m.Diagnostic)));
@@ -77,6 +86,10 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
                 .Zip(methodInfosGroupedByInterface)
                 .Collect()
                 .SelectMany(static (data, ct) =>
+                {
+                    return data.GroupBy(data => data.Left.GetTopLevelBase());
+                })
+                .SelectMany(static (data, ct) =>
                 {
                     return ComMethodContext.CalculateAllMethods(data, ct);
                 });
@@ -239,16 +252,6 @@ private static IncrementalMethodStubGenerationContext CalculateStubInformation(M
                 }
             }
 
-            AttributeData? generatedComAttribute = null;
-            foreach (var attr in symbol.ContainingType.GetAttributes())
-            {
-                if (generatedComAttribute is null
-                    && attr.AttributeClass?.ToDisplayString() == TypeNames.GeneratedComInterfaceAttribute)
-                {
-                    generatedComAttribute = attr;
-                }
-            }
-
             var generatorDiagnostics = new GeneratorDiagnostics();
 
             if (lcidConversionAttr is not null)
@@ -257,12 +260,8 @@ private static IncrementalMethodStubGenerationContext CalculateStubInformation(M
                 generatorDiagnostics.ReportConfigurationNotSupported(lcidConversionAttr, nameof(TypeNames.LCIDConversionAttribute));
             }
 
-            var generatedComInterfaceAttributeData = new InteropAttributeCompilationData();
-            if (generatedComAttribute is not null)
-            {
-                var args = generatedComAttribute.NamedArguments.ToImmutableDictionary();
-                generatedComInterfaceAttributeData = generatedComInterfaceAttributeData.WithValuesFromNamedArguments(args);
-            }
+            GeneratedComInterfaceCompilationData.TryGetGeneratedComInterfaceAttributeFromInterface(symbol.ContainingType, out var generatedComAttribute);
+            var generatedComInterfaceAttributeData = GeneratedComInterfaceCompilationData.GetDataFromAttribute(generatedComAttribute);
             // Create the stub.
             var signatureContext = SignatureContext.Create(
                 symbol,

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceInfo.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Microsoft.CodeAnalysis;
@@ -20,7 +21,8 @@ internal sealed record ComInterfaceInfo(
         InterfaceDeclarationSyntax Declaration,
         ContainingSyntaxContext TypeDefinitionContext,
         ContainingSyntax ContainingSyntax,
-        Guid InterfaceId)
+        Guid InterfaceId,
+        LocationInfo DiagnosticLocation)
     {
         public static (ComInterfaceInfo? Info, Diagnostic? Diagnostic) From(INamedTypeSymbol symbol, InterfaceDeclarationSyntax syntax)
         {
@@ -58,14 +60,63 @@ public static (ComInterfaceInfo? Info, Diagnostic? Diagnostic) From(INamedTypeSy
             if (!TryGetBaseComInterface(symbol, syntax, out INamedTypeSymbol? baseSymbol, out Diagnostic? baseDiagnostic))
                 return (null, baseDiagnostic);
 
-            return (new ComInterfaceInfo(
-                ManagedTypeInfo.CreateTypeInfoForTypeSymbol(symbol),
-                symbol.ToDisplayString(),
-                baseSymbol?.ToDisplayString(),
-                syntax,
-                new ContainingSyntaxContext(syntax),
-                new ContainingSyntax(syntax.Modifiers, syntax.Kind(), syntax.Identifier, syntax.TypeParameterList),
-                guid ?? Guid.Empty), null);
+            if (!StringMarshallingIsValid(symbol, syntax, baseSymbol, out Diagnostic? stringMarshallingDiagnostic))
+                return (null, stringMarshallingDiagnostic);
+
+            return (
+                new ComInterfaceInfo(
+                    ManagedTypeInfo.CreateTypeInfoForTypeSymbol(symbol),
+                    symbol.ToDisplayString(),
+                    baseSymbol?.ToDisplayString(),
+                    syntax,
+                    new ContainingSyntaxContext(syntax),
+                    new ContainingSyntax(syntax.Modifiers, syntax.Kind(), syntax.Identifier, syntax.TypeParameterList),
+                    guid ?? Guid.Empty,
+                    LocationInfo.From(symbol)),
+                null);
+        }
+
+        private static bool StringMarshallingIsValid(INamedTypeSymbol symbol, InterfaceDeclarationSyntax syntax, INamedTypeSymbol? baseSymbol, [NotNullWhen(false)] out Diagnostic? stringMarshallingDiagnostic)
+        {
+            var attrInfo = GeneratedComInterfaceData.From(GeneratedComInterfaceCompilationData.GetAttributeDataFromInterfaceSymbol(symbol));
+            if (attrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling) || attrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshallingCustomType))
+            {
+                if (attrInfo.StringMarshalling is StringMarshalling.Custom && attrInfo.StringMarshallingCustomType is null)
+                {
+                    stringMarshallingDiagnostic = Diagnostic.Create(
+                        GeneratorDiagnostics.InvalidStringMarshallingConfigurationOnInterface,
+                        syntax.Identifier.GetLocation(),
+                        symbol.ToDisplayString(),
+                        SR.InvalidStringMarshallingConfigurationMissingCustomType);
+                    return false;
+                }
+                if (attrInfo.StringMarshalling is not StringMarshalling.Custom && attrInfo.StringMarshallingCustomType is not null)
+                {
+                    stringMarshallingDiagnostic = Diagnostic.Create(
+                        GeneratorDiagnostics.InvalidStringMarshallingConfigurationOnInterface,
+                        syntax.Identifier.GetLocation(),
+                        symbol.ToDisplayString(),
+                        SR.InvalidStringMarshallingConfigurationNotCustom);
+                    return false;
+                }
+            }
+            if (baseSymbol is not null)
+            {
+                var baseAttrInfo = GeneratedComInterfaceData.From(GeneratedComInterfaceCompilationData.GetAttributeDataFromInterfaceSymbol(baseSymbol));
+                // The base can be undefined string marshalling
+                if ((baseAttrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling) || baseAttrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshallingCustomType))
+                    && baseAttrInfo != attrInfo)
+                {
+                    stringMarshallingDiagnostic = Diagnostic.Create(
+                        GeneratorDiagnostics.InvalidStringMarshallingMismatchBetweenBaseAndDerived,
+                        syntax.Identifier.GetLocation(),
+                        symbol.ToDisplayString(),
+                        SR.GeneratedComInterfaceStringMarshallingMustMatchBase);
+                    return false;
+                }
+            }
+            stringMarshallingDiagnostic = null;
+            return true;
         }
 
         /// <summary>

src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/GeneratedComInterfaceAttributeData.cs

@@ -0,0 +1,68 @@
+// 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.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+using System.Text;
+using Microsoft.CodeAnalysis;
+
+namespace Microsoft.Interop
+{
+    /// <summary>
+    /// Contains the data related to a GeneratedComInterfaceAttribute, without references to Roslyn symbols.
+    /// See <seealso cref="GeneratedComInterfaceCompilationData"/> for a type with a reference to the StringMarshallingCustomType
+    /// </summary>
+    internal sealed record GeneratedComInterfaceData : InteropAttributeData
+    {
+        public static GeneratedComInterfaceData From(GeneratedComInterfaceCompilationData generatedComInterfaceAttr)
+            => new GeneratedComInterfaceData() with
+            {
+                IsUserDefined = generatedComInterfaceAttr.IsUserDefined,
+                SetLastError = generatedComInterfaceAttr.SetLastError,
+                StringMarshalling = generatedComInterfaceAttr.StringMarshalling,
+                StringMarshallingCustomType = generatedComInterfaceAttr.StringMarshallingCustomType is not null
+                    ? ManagedTypeInfo.CreateTypeInfoForTypeSymbol(generatedComInterfaceAttr.StringMarshallingCustomType)
+                    : null
+            };
+    }
+
+    /// <summary>
+    /// Contains the data related to a GeneratedComInterfaceAttribute, with references to Roslyn symbols.
+    /// Use <seealso cref="GeneratedComInterfaceData"/> instead when using for incremental compilation state to avoid keeping a compilation alive
+    /// </summary>
+    internal sealed record GeneratedComInterfaceCompilationData : InteropAttributeCompilationData
+    {
+        public static bool TryGetGeneratedComInterfaceAttributeFromInterface(INamedTypeSymbol interfaceSymbol, [NotNullWhen(true)] out AttributeData? generatedComInterfaceAttribute)
+        {
+            generatedComInterfaceAttribute = null;
+            foreach (var attr in interfaceSymbol.GetAttributes())
+            {
+                if (generatedComInterfaceAttribute is null
+                    && attr.AttributeClass?.ToDisplayString() == TypeNames.GeneratedComInterfaceAttribute)
+                {
+                    generatedComInterfaceAttribute = attr;
+                }
+            }
+            return generatedComInterfaceAttribute is not null;
+        }
+
+        public static GeneratedComInterfaceCompilationData GetAttributeDataFromInterfaceSymbol(INamedTypeSymbol interfaceSymbol)
+        {
+            bool found = TryGetGeneratedComInterfaceAttributeFromInterface(interfaceSymbol, out var attr);
+            Debug.Assert(found);
+            return GetDataFromAttribute(attr);
+        }
+
+        public static GeneratedComInterfaceCompilationData GetDataFromAttribute(AttributeData attr)
+        {
+            Debug.Assert(attr.AttributeClass.ToDisplayString() == TypeNames.GeneratedComInterfaceAttribute);
+            var generatedComInterfaceAttributeData = new GeneratedComInterfaceCompilationData();
+            var args = attr.NamedArguments.ToImmutableDictionary();
+            generatedComInterfaceAttributeData = generatedComInterfaceAttributeData.WithValuesFromNamedArguments(args);
+            return generatedComInterfaceAttributeData;
+        }
+    }
+}