Skip to content

Commit

Permalink
[ComInterfaceGenerator] Warn on visibility of interface and StringMar…
Browse files Browse the repository at this point in the history
…shallingCustomType (#87065)

The generated code is in file scoped classes, so the interface and StringMarshallingCustomType need to be at least internal visibility. These changes warn if that condition isn't met.

Originally, I expected to report a diagnostic, but still generate code for the interface. This required some changes to DiagnosticOr<T> to allow it to hold both a diagnostic and a value if the diagnostic didn't cause failure. In the end those changes weren't necessary, but I left them because I can see them being valuable at some point in the future.

Completes #84662
  • Loading branch information
jtschuster authored Jun 14, 2023
1 parent 361a8e6 commit 1d9b36d
Show file tree
Hide file tree
Showing 35 changed files with 587 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ DiagnosticOr<ComInterfaceContext> AddContext(ComInterfaceInfo iface)
DiagnosticOr<ComInterfaceContext> baseReturnedValue;
if (
// Cached base info is a diagnostic - failure
(nameToContextCache.TryGetValue(iface.BaseInterfaceKey, out var baseCachedValue) && baseCachedValue.IsDiagnostic)
(nameToContextCache.TryGetValue(iface.BaseInterfaceKey, out var baseCachedValue) && baseCachedValue.HasDiagnostic)
// Cannot find base ComInterfaceInfo - failure (failed ComInterfaceInfo creation)
|| !nameToInterfaceInfoMap.TryGetValue(iface.BaseInterfaceKey, out var baseInfo)
// Newly calculated base context pair is a diagnostic - failure
|| (baseReturnedValue = AddContext(baseInfo)).IsDiagnostic)
|| (baseReturnedValue = AddContext(baseInfo)).HasDiagnostic)
{
// The base has failed generation at some point, so this interface cannot be generated
var diagnostic = DiagnosticOr<ComInterfaceContext>.From(
Expand All @@ -62,7 +62,7 @@ DiagnosticOr<ComInterfaceContext> AddContext(ComInterfaceInfo iface)
return diagnostic;
}
DiagnosticOr<ComInterfaceContext> baseContext = baseCachedValue ?? baseReturnedValue;
Debug.Assert(baseContext.IsValue);
Debug.Assert(baseContext.HasValue);
var ctx = DiagnosticOr<ComInterfaceContext>.From(new ComInterfaceContext(iface, baseContext.Value));
nameToContextCache[iface.ThisInterfaceKey] = ctx;
return ctx;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ public static DiagnosticOrInterfaceInfo From(INamedTypeSymbol symbol, InterfaceD
if (!IsInPartialContext(symbol, syntax, out DiagnosticInfo? partialContextDiagnostic))
return DiagnosticOrInterfaceInfo.From(partialContextDiagnostic);

if (!symbol.IsAccessibleFromFileScopedClass(out var details))
{
return DiagnosticOrInterfaceInfo.From(DiagnosticInfo.Create(
GeneratorDiagnostics.InvalidAttributedInterfaceNotAccessible,
syntax.Identifier.GetLocation(),
symbol.ToDisplayString(),
details));
}

if (!TryGetGuid(symbol, syntax, out Guid? guid, out DiagnosticInfo? guidDiagnostic))
return DiagnosticOrInterfaceInfo.From(guidDiagnostic);

Expand Down Expand Up @@ -80,41 +89,58 @@ private static bool IsInPartialContext(INamedTypeSymbol symbol, InterfaceDeclara
return true;
}

private static bool StringMarshallingIsValid(INamedTypeSymbol symbol, InterfaceDeclarationSyntax syntax, INamedTypeSymbol? baseSymbol, [NotNullWhen(false)] out DiagnosticInfo? stringMarshallingDiagnostic)
private static bool StringMarshallingIsValid(
INamedTypeSymbol interfaceSymbol,
InterfaceDeclarationSyntax syntax,
INamedTypeSymbol? baseInterfaceSymbol,
[NotNullWhen(false)] out DiagnosticInfo? stringMarshallingDiagnostic)
{
var attrInfo = GeneratedComInterfaceData.From(GeneratedComInterfaceCompilationData.GetAttributeDataFromInterfaceSymbol(symbol));
var attrSymbolInfo = GeneratedComInterfaceCompilationData.GetAttributeDataFromInterfaceSymbol(interfaceSymbol);
var attrInfo = GeneratedComInterfaceData.From(attrSymbolInfo);
if (attrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling) || attrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshallingCustomType))
{
if (attrInfo.StringMarshalling is StringMarshalling.Custom && attrInfo.StringMarshallingCustomType is null)
if (attrInfo.StringMarshalling is StringMarshalling.Custom)
{
stringMarshallingDiagnostic = DiagnosticInfo.Create(
GeneratorDiagnostics.InvalidStringMarshallingConfigurationOnInterface,
syntax.Identifier.GetLocation(),
symbol.ToDisplayString(),
SR.InvalidStringMarshallingConfigurationMissingCustomType);
return false;
if (attrInfo.StringMarshallingCustomType is null)
{
stringMarshallingDiagnostic = DiagnosticInfo.Create(
GeneratorDiagnostics.InvalidStringMarshallingConfigurationOnInterface,
syntax.Identifier.GetLocation(),
interfaceSymbol.ToDisplayString(),
SR.InvalidStringMarshallingConfigurationMissingCustomType);
return false;
}
if (!attrSymbolInfo.StringMarshallingCustomType.IsAccessibleFromFileScopedClass(out var details))
{
stringMarshallingDiagnostic = DiagnosticInfo.Create(
GeneratorDiagnostics.StringMarshallingCustomTypeNotAccessibleByGeneratedCode,
syntax.Identifier.GetLocation(),
attrInfo.StringMarshallingCustomType.FullTypeName.Replace("global::", ""),
details);
return false;
}
}
if (attrInfo.StringMarshalling is not StringMarshalling.Custom && attrInfo.StringMarshallingCustomType is not null)
else if (attrInfo.StringMarshallingCustomType is not null)
{
stringMarshallingDiagnostic = DiagnosticInfo.Create(
GeneratorDiagnostics.InvalidStringMarshallingConfigurationOnInterface,
syntax.Identifier.GetLocation(),
symbol.ToDisplayString(),
interfaceSymbol.ToDisplayString(),
SR.InvalidStringMarshallingConfigurationNotCustom);
return false;
}
}
if (baseSymbol is not null)
if (baseInterfaceSymbol is not null)
{
var baseAttrInfo = GeneratedComInterfaceData.From(GeneratedComInterfaceCompilationData.GetAttributeDataFromInterfaceSymbol(baseSymbol));
var baseAttrInfo = GeneratedComInterfaceData.From(GeneratedComInterfaceCompilationData.GetAttributeDataFromInterfaceSymbol(baseInterfaceSymbol));
// The base can be undefined string marshalling
if ((baseAttrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling) || baseAttrInfo.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshallingCustomType))
&& baseAttrInfo != attrInfo)
{
stringMarshallingDiagnostic = DiagnosticInfo.Create(
GeneratorDiagnostics.InvalidStringMarshallingMismatchBetweenBaseAndDerived,
syntax.Identifier.GetLocation(),
symbol.ToDisplayString(),
interfaceSymbol.ToDisplayString(),
SR.GeneratedComInterfaceStringMarshallingMustMatchBase);
return false;
}
Expand Down Expand Up @@ -158,7 +184,7 @@ private static bool TryGetGuid(INamedTypeSymbol interfaceSymbol, InterfaceDeclar
{
guid = null;
AttributeData? guidAttr = null;
AttributeData? _ = null; // Interface Attribute Type. We'll always assume IUnkown for now.
AttributeData? _ = null; // Interface Attribute Type. We'll always assume IUnknown for now.
foreach (var attr in interfaceSymbol.GetAttributes())
{
var attrDisplayString = attr.AttributeClass?.ToDisplayString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ public class Ids
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.InvalidStringMarshallingConfigurationDescription)));

/// <inheritdoc cref="SR.StringMarshallingCustomTypeNotAccessibleByGeneratedCode"/>
public static readonly DiagnosticDescriptor StringMarshallingCustomTypeNotAccessibleByGeneratedCode =
new DiagnosticDescriptor(
Ids.InvalidGeneratedComInterfaceAttributeUsage,
GetResourceString(nameof(SR.InvalidGeneratedComInterfaceAttributeUsageTitle)),
GetResourceString(nameof(SR.StringMarshallingCustomTypeNotAccessibleByGeneratedCode)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true);

/// <inheritdoc cref="SR.InvalidExceptionMarshallingConfigurationMessage"/>
public static readonly DiagnosticDescriptor InvalidExceptionMarshallingConfiguration =
new DiagnosticDescriptor(
Expand Down Expand Up @@ -240,6 +250,17 @@ public class Ids
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.MethodNotDeclaredInAttributedInterfaceDescription)));

/// <inheritdoc cref="SR.InvalidGeneratedComInterfaceAttributeUsageInterfaceNotAccessible"/>
public static readonly DiagnosticDescriptor InvalidAttributedInterfaceNotAccessible =
new DiagnosticDescriptor(
Ids.InvalidGeneratedComInterfaceAttributeUsage,
GetResourceString(nameof(SR.InvalidGeneratedComInterfaceAttributeUsageTitle)),
GetResourceString(nameof(SR.InvalidGeneratedComInterfaceAttributeUsageInterfaceNotAccessible)),
Category,
DiagnosticSeverity.Error,
isEnabledByDefault: true,
description: GetResourceString(nameof(SR.InvalidGeneratedComInterfaceAttributeUsageDescription)));

/// <inheritdoc cref="SR.InvalidGeneratedComInterfaceAttributeUsageMissingGuidAttribute"/>
public static readonly DiagnosticDescriptor InvalidAttributedInterfaceMissingGuidAttribute =
new DiagnosticDescriptor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@
<value>Method is declared in different partial declaration than the 'GeneratedComInterface' attribute.</value>
</data>
<data name="InvalidGeneratedComInterfaceAttributeUsageDescription" xml:space="preserve">
<value>Interfaces attributed with 'GeneratedComInterfaceAttribute' must be partial, non-generic, and must specify a GUID with 'System.Runtime.InteropServices.GuidAttribute'.</value>
<value>Interfaces attributed with 'GeneratedComInterfaceAttribute' must have 'public' or 'internal' accessibility and be partial, non-generic, and must specify a GUID with 'System.Runtime.InteropServices.GuidAttribute'.</value>
</data>
<data name="InvalidGeneratedComInterfaceAttributeUsageMissingGuidAttribute" xml:space="preserve">
<value>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is missing 'System.Runtime.InteropServices.GuidAttribute'.</value>
Expand Down Expand Up @@ -267,6 +267,14 @@
<data name="InvalidStringMarshallingConfigurationOnInterfaceMessage" xml:space="preserve">
<value>The configuration of 'StringMarshalling' and 'StringMarshallingCustomType' on interface '{0}' is invalid. {1}</value>
</data>
<data name="InvalidGeneratedComInterfaceAttributeUsageInterfaceNotAccessible" xml:space="preserve">
<value>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is not accessible by generated code. The interface and all containing types must have accessibility 'internal' or 'public' for generated code to access it. {1}</value>
<comment>{1} is details about which type/containing type is not accessible</comment>
</data>
<data name="StringMarshallingCustomTypeNotAccessibleByGeneratedCode" xml:space="preserve">
<value>The type '{0}' specified as 'GeneratedComInterfaceAttribute.StringMarshallingCustomType' is not accessible by generated code. The type must have at least 'internal' accessibility. {1}</value>
<comment>{1} is details about which type/containing type is not accessible</comment>
</data>
<data name="RequiresAllowUnsafeBlocksDescription" xml:space="preserve">
<value>'GeneratedComInterfaceAttribute' and 'GeneratedComClassAttribute' require unsafe code. Project must be updated with '&lt;AllowUnsafeBlocks&gt;true&lt;/AllowUnsafeBlocks&gt;'.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,20 @@
<note />
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageDescription">
<source>Interfaces attributed with 'GeneratedComInterfaceAttribute' must be partial, non-generic, and must specify a GUID with 'System.Runtime.InteropServices.GuidAttribute'.</source>
<target state="translated">Rozhraní s atributem GeneratedComInterfaceAttribute musí být částečná, neobecná a musí určovat identifikátor GUID s atributem System.Runtime.InteropServices.GuidAttribute.</target>
<source>Interfaces attributed with 'GeneratedComInterfaceAttribute' must have 'public' or 'internal' accessibility and be partial, non-generic, and must specify a GUID with 'System.Runtime.InteropServices.GuidAttribute'.</source>
<target state="needs-review-translation">Rozhraní s atributem GeneratedComInterfaceAttribute musí být částečná, neobecná a musí určovat identifikátor GUID s atributem System.Runtime.InteropServices.GuidAttribute.</target>
<note />
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageInterfaceIsGeneric">
<source>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is generic.</source>
<target state="translated">Rozhraní „{0}“ má atribut GeneratedComInterfaceAttribute, ale je obecné.</target>
<note />
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageInterfaceNotAccessible">
<source>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is not accessible by generated code. The interface and all containing types must have accessibility 'internal' or 'public' for generated code to access it. {1}</source>
<target state="new">Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is not accessible by generated code. The interface and all containing types must have accessibility 'internal' or 'public' for generated code to access it. {1}</target>
<note>{1} is details about which type/containing type is not accessible</note>
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageMissingGuidAttribute">
<source>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is missing 'System.Runtime.InteropServices.GuidAttribute'.</source>
<target state="translated">Rozhraní {0} má atribut GeneratedComInterfaceAttribute, ale chybí atribut System.Runtime.InteropServices.GuidAttribute.</target>
Expand Down Expand Up @@ -352,6 +357,11 @@
<target state="new">COM Interop APIs on 'System.Runtime.InteropServices.Marshal' do not support source-generated COM</target>
<note />
</trans-unit>
<trans-unit id="StringMarshallingCustomTypeNotAccessibleByGeneratedCode">
<source>The type '{0}' specified as 'GeneratedComInterfaceAttribute.StringMarshallingCustomType' is not accessible by generated code. The type must have at least 'internal' accessibility. {1}</source>
<target state="new">The type '{0}' specified as 'GeneratedComInterfaceAttribute.StringMarshallingCustomType' is not accessible by generated code. The type must have at least 'internal' accessibility. {1}</target>
<note>{1} is details about which type/containing type is not accessible</note>
</trans-unit>
<trans-unit id="TypeNotSupportedDescription">
<source>For types that are not supported by source-generated COM, the resulting function pointer will rely on the underlying runtime to marshal the specified type.</source>
<target state="translated">U typů, které nejsou podporovány modelem COM generovaným zdrojem, bude výsledný ukazatel funkce při zařazování zadaného typu spoléhat na základní modul runtime.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,15 +238,20 @@
<note />
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageDescription">
<source>Interfaces attributed with 'GeneratedComInterfaceAttribute' must be partial, non-generic, and must specify a GUID with 'System.Runtime.InteropServices.GuidAttribute'.</source>
<target state="translated">Schnittstellen, die mit "GeneratedComInterfaceAttribute" attributiert sind, müssen partielle, nicht generische Schnittstellen sein und eine GUID mit "System.Runtime.InteropServices.GuidAttribute" angeben.</target>
<source>Interfaces attributed with 'GeneratedComInterfaceAttribute' must have 'public' or 'internal' accessibility and be partial, non-generic, and must specify a GUID with 'System.Runtime.InteropServices.GuidAttribute'.</source>
<target state="needs-review-translation">Schnittstellen, die mit "GeneratedComInterfaceAttribute" attributiert sind, müssen partielle, nicht generische Schnittstellen sein und eine GUID mit "System.Runtime.InteropServices.GuidAttribute" angeben.</target>
<note />
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageInterfaceIsGeneric">
<source>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is generic.</source>
<target state="translated">Die Schnittstelle "{0}" wird mit "GeneratedComInterfaceAttribute" attributiert, ist aber generisch.</target>
<note />
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageInterfaceNotAccessible">
<source>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is not accessible by generated code. The interface and all containing types must have accessibility 'internal' or 'public' for generated code to access it. {1}</source>
<target state="new">Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is not accessible by generated code. The interface and all containing types must have accessibility 'internal' or 'public' for generated code to access it. {1}</target>
<note>{1} is details about which type/containing type is not accessible</note>
</trans-unit>
<trans-unit id="InvalidGeneratedComInterfaceAttributeUsageMissingGuidAttribute">
<source>Interface '{0}' is attributed with 'GeneratedComInterfaceAttribute' but is missing 'System.Runtime.InteropServices.GuidAttribute'.</source>
<target state="translated">Die Schnittstelle „{0}“ ist mit „GeneratedComInterfaceAttribute“ attribuiert, aber „System.Runtime.InteropServices.GuidAttribute“ fehlt.</target>
Expand Down Expand Up @@ -352,6 +357,11 @@
<target state="new">COM Interop APIs on 'System.Runtime.InteropServices.Marshal' do not support source-generated COM</target>
<note />
</trans-unit>
<trans-unit id="StringMarshallingCustomTypeNotAccessibleByGeneratedCode">
<source>The type '{0}' specified as 'GeneratedComInterfaceAttribute.StringMarshallingCustomType' is not accessible by generated code. The type must have at least 'internal' accessibility. {1}</source>
<target state="new">The type '{0}' specified as 'GeneratedComInterfaceAttribute.StringMarshallingCustomType' is not accessible by generated code. The type must have at least 'internal' accessibility. {1}</target>
<note>{1} is details about which type/containing type is not accessible</note>
</trans-unit>
<trans-unit id="TypeNotSupportedDescription">
<source>For types that are not supported by source-generated COM, the resulting function pointer will rely on the underlying runtime to marshal the specified type.</source>
<target state="translated">Bei Typen, die vom quellgenerierten COM nicht unterstützt werden, basiert der resultierende Funktionszeiger auf der zugrunde liegenden Laufzeit, um den angegebenen Typ zu marshallen.</target>
Expand Down
Loading

0 comments on commit 1d9b36d

Please sign in to comment.