From 42ff67f310b31d8edebfcf65b3d1162731c8ec22 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 8 Oct 2020 01:51:44 -0700 Subject: [PATCH 1/8] Report diagnostics on parameters and return types where marshalling generation is not supported --- .../DllImportGenerator.UnitTests/Compiles.cs | 2 +- .../DllImportGenerator.csproj | 1 + .../DllImportGenerator/DllImportStub.cs | 8 +- .../GeneratorDiagnostics.cs | 72 ++++++++++ .../ManualTypeMarshallingAnalyzer.cs | 2 +- .../Marshalling/MarshallingGenerator.cs | 8 +- .../DllImportGenerator/Resources.Designer.cs | 67 ++++++++- .../DllImportGenerator/Resources.resx | 21 +++ .../DllImportGenerator/StubCodeGenerator.cs | 131 ++++++++++-------- .../DllImportGenerator/TypePositionInfo.cs | 12 +- 10 files changed, 256 insertions(+), 68 deletions(-) create mode 100644 DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs index 2275072099fa..0c58aa3875f5 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs @@ -101,7 +101,7 @@ public async Task ValidateSnippets(string source) TestUtils.AssertPreSourceGeneratorCompilation(comp); var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); - Assert.Empty(generatorDiags); + Assert.All(generatorDiags, d => Assert.NotEqual(DiagnosticSeverity.Error, d.Severity)); var newCompDiags = newComp.GetDiagnostics(); Assert.Empty(newCompDiags); diff --git a/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj b/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj index 5e60199190e1..20b90b8c0045 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj +++ b/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj @@ -7,6 +7,7 @@ True true Preview + Microsoft.Interop diff --git a/DllImportGenerator/DllImportGenerator/DllImportStub.cs b/DllImportGenerator/DllImportGenerator/DllImportStub.cs index 3218db8e00e1..a81067bacaff 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportStub.cs +++ b/DllImportGenerator/DllImportGenerator/DllImportStub.cs @@ -106,6 +106,8 @@ public static DllImportStub Create( // Cancel early if requested token.ThrowIfCancellationRequested(); + var diagnostics = new List(); + // Determine the namespace string stubTypeNamespace = null; if (!(method.ContainingNamespace is null) @@ -162,7 +164,9 @@ public static DllImportStub Create( } // Generate stub code - var (code, dllImport) = StubCodeGenerator.GenerateSyntax(method, paramsTypeInfo, retTypeInfo); + var stubGenerator = new StubCodeGenerator(method, paramsTypeInfo, retTypeInfo); + var (code, dllImport) = stubGenerator.GenerateSyntax(); + diagnostics.AddRange(stubGenerator.Diagnostics); return new DllImportStub() { @@ -172,7 +176,7 @@ public static DllImportStub Create( StubContainingTypes = containingTypes, StubCode = code, DllImportDeclaration = dllImport, - Diagnostics = Enumerable.Empty(), + Diagnostics = diagnostics, }; } } diff --git a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs new file mode 100644 index 000000000000..197807530777 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs @@ -0,0 +1,72 @@ +using System.Collections.Generic; +using System.Linq; + +using Microsoft.CodeAnalysis; + +#nullable enable + +namespace Microsoft.Interop +{ + public static class GeneratorDiagnostics + { + private class Ids + { + public const string Prefix = "DLLIMPORTGEN"; + public const string TypeNotSupported = Prefix + "001"; + public const string ConfigurationNotSupported = Prefix + "002"; + } + + private const string Category = "DllImportGenerator"; + + public readonly static DiagnosticDescriptor ParameterTypeNotSupported = + new DiagnosticDescriptor( + Ids.TypeNotSupported, + GetResourceString(nameof(Resources.TypeNotSupportedTitle)), + GetResourceString(nameof(Resources.TypeNotSupportedMessageParameter)), + Category, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: GetResourceString(Resources.TypeNotSupportedDescription)); + + public readonly static DiagnosticDescriptor ReturnTypeNotSupported = + new DiagnosticDescriptor( + Ids.TypeNotSupported, + GetResourceString(nameof(Resources.TypeNotSupportedTitle)), + GetResourceString(nameof(Resources.TypeNotSupportedMessageReturn)), + Category, + DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: GetResourceString(Resources.TypeNotSupportedDescription)); + + public readonly static DiagnosticDescriptor ConfigurationNotSupported = + new DiagnosticDescriptor( + Ids.ConfigurationNotSupported, + GetResourceString(nameof(Resources.ConfigurationNotSupportedTitle)), + GetResourceString(nameof(Resources.ConfigurationNotSupportedMessage)), + Category, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: GetResourceString(Resources.ConfigurationNotSupportedDescription)); + + public static Diagnostic CreateDiagnostic( + this ISymbol symbol, + DiagnosticDescriptor descriptor, + params object[] args) + { + IEnumerable locationsInSource = symbol.Locations.Where(l => l.IsInSource); + if (!locationsInSource.Any()) + return Diagnostic.Create(descriptor, Location.None, args); + + return Diagnostic.Create( + descriptor, + location: locationsInSource.First(), + additionalLocations: locationsInSource.Skip(1), + messageArgs: args); + } + + private static LocalizableResourceString GetResourceString(string resourceName) + { + return new LocalizableResourceString(resourceName, Resources.ResourceManager, typeof(Resources)); + } + } +} diff --git a/DllImportGenerator/DllImportGenerator/ManualTypeMarshallingAnalyzer.cs b/DllImportGenerator/DllImportGenerator/ManualTypeMarshallingAnalyzer.cs index a58c81291b7c..8366ca698ed9 100644 --- a/DllImportGenerator/DllImportGenerator/ManualTypeMarshallingAnalyzer.cs +++ b/DllImportGenerator/DllImportGenerator/ManualTypeMarshallingAnalyzer.cs @@ -1,7 +1,7 @@ using System; using System.Collections.Immutable; using System.Linq; -using DllImportGenerator; + using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Diagnostics; diff --git a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs index cebc19932604..b05ac6af0ed5 100644 --- a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs @@ -117,7 +117,7 @@ public static bool TryCreate(TypePositionInfo info, StubCodeContext context, out generator = Blittable; return true; - // Marshalling in new model + // Marshalling in new model case { MarshallingAttributeInfo: NativeMarshallingAttributeInfo marshalInfo }: generator = Forwarder; return false; @@ -127,10 +127,14 @@ public static bool TryCreate(TypePositionInfo info, StubCodeContext context, out generator = Forwarder; return false; - case { MarshallingAttributeInfo: SafeHandleMarshallingInfo _}: + case { MarshallingAttributeInfo: SafeHandleMarshallingInfo _}: generator = SafeHandle; return true; + case { ManagedType: { SpecialType: SpecialType.System_Void } }: + generator = Forwarder; + return true; + default: generator = Forwarder; return false; diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index c7ccc3443b27..c1a47f0dfac4 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -8,7 +8,7 @@ // //------------------------------------------------------------------------------ -namespace DllImportGenerator { +namespace Microsoft.Interop { using System; @@ -39,7 +39,7 @@ internal Resources() { internal static global::System.Resources.ResourceManager ResourceManager { get { if (object.ReferenceEquals(resourceMan, null)) { - global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("DllImportGenerator.Resources", typeof(Resources).Assembly); + global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Interop.Resources", typeof(Resources).Assembly); resourceMan = temp; } return resourceMan; @@ -96,6 +96,33 @@ internal static string CannotHaveMultipleMarshallingAttributesMessage { } } + /// + /// Looks up a localized string similar to Source-generated P/Invokes will ignore any configuration that is not supported. If the specified configuration is required, use a regular `DllImport` instead. + /// + internal static string ConfigurationNotSupportedDescription { + get { + return ResourceManager.GetString("ConfigurationNotSupportedDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The '{0}' configuration is not supported by source-generated P/Invokes. The generated source will ignore the specified configuration.. + /// + internal static string ConfigurationNotSupportedMessage { + get { + return ResourceManager.GetString("ConfigurationNotSupportedMessage", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Specified configuration is not supported by source-generated P/Invokes.. + /// + internal static string ConfigurationNotSupportedTitle { + get { + return ResourceManager.GetString("ConfigurationNotSupportedTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to The return type of 'GetPinnableReference' (after accounting for 'ref') must be blittable.. /// @@ -240,6 +267,42 @@ internal static string StackallocMarshallingShouldSupportAllocatingMarshallingFa } } + /// + /// Looks up a localized string similar to For types that are not supported by source-generated P/Invokes, the resulting P/Invoke will rely on the underlying runtime to marshal the specified type.. + /// + internal static string TypeNotSupportedDescription { + get { + return ResourceManager.GetString("TypeNotSupportedDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of parameter '{1}'.. + /// + internal static string TypeNotSupportedMessageParameter { + get { + return ResourceManager.GetString("TypeNotSupportedMessageParameter", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of the return value for method '{1}'.. + /// + internal static string TypeNotSupportedMessageReturn { + get { + return ResourceManager.GetString("TypeNotSupportedMessageReturn", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Specified type is not supported by source-generated P/Invokes. + /// + internal static string TypeNotSupportedTitle { + get { + return ResourceManager.GetString("TypeNotSupportedTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to The native type's 'Value' property must have a getter to support marshalling from managed to native.. /// diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index e21cecab2e6f..45912f286e39 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -129,6 +129,15 @@ Type '{0}' is marked with 'BlittableTypeAttribute' and 'NativeMarshallingAttribute'. A type can only have one of these two attributes. + + Source-generated P/Invokes will ignore any configuration that is not supported. If the specified configuration is required, use a regular `DllImport` instead + + + The '{0}' configuration is not supported by source-generated P/Invokes. The generated source will ignore the specified configuration. + + + Specified configuration is not supported by source-generated P/Invokes. + The return type of 'GetPinnableReference' (after accounting for 'ref') must be blittable. @@ -177,6 +186,18 @@ Native type '{0}' has a stack-allocating constructor does not support marshalling in scenarios where stack allocation is impossible. + + For types that are not supported by source-generated P/Invokes, the resulting P/Invoke will rely on the underlying runtime to marshal the specified type. + + + The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of parameter '{1}'. + + + The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of the return value for method '{1}'. + + + Specified type is not supported by source-generated P/Invokes + The native type's 'Value' property must have a getter to support marshalling from managed to native. diff --git a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs index 043819bc068f..c82341578fb0 100644 --- a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs @@ -12,11 +12,6 @@ namespace Microsoft.Interop { internal sealed class StubCodeGenerator : StubCodeContext { - private StubCodeGenerator(Stage stage) - { - CurrentStage = stage; - } - public override bool PinningSupported => true; public override bool StackSpaceUsable => true; @@ -34,6 +29,66 @@ private StubCodeGenerator(Stage stage) private const string InvokeReturnIdentifier = "__invokeRetVal"; + private static readonly Stage[] Stages = new Stage[] + { + Stage.Setup, + Stage.Marshal, + Stage.Pin, + Stage.Invoke, + Stage.KeepAlive, + Stage.Unmarshal, + Stage.GuaranteedUnmarshal, + Stage.Cleanup + }; + + public IEnumerable Diagnostics => diagnostics; + private readonly List diagnostics = new List(); + + private readonly IMethodSymbol stubMethod; + private readonly List<(TypePositionInfo TypeInfo, IMarshallingGenerator Generator)> paramMarshallers; + private readonly (TypePositionInfo TypeInfo, IMarshallingGenerator Generator) retMarshaller; + + public StubCodeGenerator( + IMethodSymbol stubMethod, + IEnumerable paramsTypeInfo, + TypePositionInfo retTypeInfo) + { + Debug.Assert(retTypeInfo.IsNativeReturnPosition); + + this.CurrentStage = Stages[0]; + this.stubMethod = stubMethod; + + // Get marshallers for parameters + this.paramMarshallers = paramsTypeInfo.Select(p => + { + IMarshallingGenerator generator; + if (!MarshallingGenerators.TryCreate(p, this, out generator)) + { + IParameterSymbol paramSymbol = stubMethod.Parameters[p.ManagedIndex]; + this.diagnostics.Add( + paramSymbol.CreateDiagnostic( + GeneratorDiagnostics.ParameterTypeNotSupported, + paramSymbol.Type.ToDisplayString(), + paramSymbol.Name)); + } + + return (p, generator); + }).ToList(); + + // Get marshaller for return + IMarshallingGenerator retGenerator; + if (!MarshallingGenerators.TryCreate(retTypeInfo, this, out retGenerator)) + { + this.diagnostics.Add( + stubMethod.CreateDiagnostic( + GeneratorDiagnostics.ReturnTypeNotSupported, + stubMethod.ReturnType.ToDisplayString(), + stubMethod.Name)); + } + + this.retMarshaller = (retTypeInfo, retGenerator); + } + /// /// Generate an identifier for the native return value and update the context with the new value /// @@ -70,24 +125,15 @@ public override (string managed, string native) GetIdentifiers(TypePositionInfo } } - public static (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSyntax( - IMethodSymbol stubMethod, - IEnumerable paramsTypeInfo, - TypePositionInfo retTypeInfo) + public (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSyntax() { - Debug.Assert(retTypeInfo.IsNativeReturnPosition); - - var context = new StubCodeGenerator(Stage.Setup); - + this.CurrentStage = Stages[0]; string dllImportName = stubMethod.Name + "__PInvoke__"; - var paramMarshallers = paramsTypeInfo.Select(p => GetMarshalInfo(p, context)).ToList(); - var retMarshaller = GetMarshalInfo(retTypeInfo, context); - var statements = new List(); - if (retMarshaller.Generator.UsesNativeIdentifier(retTypeInfo, context)) + if (retMarshaller.Generator.UsesNativeIdentifier(retMarshaller.TypeInfo, this)) { - context.GenerateReturnNativeIdentifier(); + this.GenerateReturnNativeIdentifier(); } foreach (var marshaller in paramMarshallers) @@ -106,21 +152,21 @@ public static (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSynt Token(SyntaxKind.DefaultKeyword))))); } - bool invokeReturnsVoid = retTypeInfo.ManagedType.SpecialType == SpecialType.System_Void; + bool invokeReturnsVoid = retMarshaller.TypeInfo.ManagedType.SpecialType == SpecialType.System_Void; bool stubReturnsVoid = stubMethod.ReturnsVoid; // Stub return is not the same as invoke return - if (!stubReturnsVoid && !retTypeInfo.IsManagedReturnPosition) + if (!stubReturnsVoid && !retMarshaller.TypeInfo.IsManagedReturnPosition) { - Debug.Assert(paramsTypeInfo.Any() && paramsTypeInfo.Last().IsManagedReturnPosition); + Debug.Assert(paramMarshallers.Any() && paramMarshallers.Last().TypeInfo.IsManagedReturnPosition); // Declare variable for stub return value - TypePositionInfo info = paramsTypeInfo.Last(); + TypePositionInfo info = paramMarshallers.Last().TypeInfo; statements.Add(LocalDeclarationStatement( VariableDeclaration( info.ManagedType.AsTypeSyntax(), SingletonSeparatedList( - VariableDeclarator(context.GetIdentifiers(info).managed))))); + VariableDeclarator(this.GetIdentifiers(info).managed))))); } if (!invokeReturnsVoid) @@ -128,34 +174,22 @@ public static (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSynt // Declare variable for invoke return value statements.Add(LocalDeclarationStatement( VariableDeclaration( - retTypeInfo.ManagedType.AsTypeSyntax(), + retMarshaller.TypeInfo.ManagedType.AsTypeSyntax(), SingletonSeparatedList( - VariableDeclarator(context.GetIdentifiers(retTypeInfo).managed))))); + VariableDeclarator(this.GetIdentifiers(retMarshaller.TypeInfo).managed))))); } - var stages = new Stage[] - { - Stage.Setup, - Stage.Marshal, - Stage.Pin, - Stage.Invoke, - Stage.KeepAlive, - Stage.Unmarshal, - Stage.GuaranteedUnmarshal, - Stage.Cleanup - }; - var invoke = InvocationExpression(IdentifierName(dllImportName)); var fixedStatements = new List(); - foreach (var stage in stages) + foreach (var stage in Stages) { int initialCount = statements.Count; - context.CurrentStage = stage; + this.CurrentStage = stage; if (!invokeReturnsVoid && (stage == Stage.Setup || stage == Stage.Unmarshal || stage == Stage.GuaranteedUnmarshal)) { // Handle setup and unmarshalling for return - var retStatements = retMarshaller.Generator.Generate(retMarshaller.TypeInfo, context); + var retStatements = retMarshaller.Generator.Generate(retMarshaller.TypeInfo, this); statements.AddRange(retStatements); } @@ -165,12 +199,12 @@ public static (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSynt if (stage == Stage.Invoke) { // Get arguments for invocation - ArgumentSyntax argSyntax = marshaller.Generator.AsArgument(marshaller.TypeInfo, context); + ArgumentSyntax argSyntax = marshaller.Generator.AsArgument(marshaller.TypeInfo, this); invoke = invoke.AddArgumentListArguments(argSyntax); } else { - var generatedStatements = marshaller.Generator.Generate(marshaller.TypeInfo, context); + var generatedStatements = marshaller.Generator.Generate(marshaller.TypeInfo, this); if (stage == Stage.Pin) { // Collect all the fixed statements. These will be used in the Invoke stage. @@ -203,7 +237,7 @@ public static (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSynt invokeStatement = ExpressionStatement( AssignmentExpression( SyntaxKind.SimpleAssignmentExpression, - IdentifierName(context.GetIdentifiers(retMarshaller.TypeInfo).native), + IdentifierName(this.GetIdentifiers(retMarshaller.TypeInfo).native), invoke)); } @@ -257,16 +291,5 @@ public static (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSynt return (codeBlock, dllImport); } - - private static (TypePositionInfo TypeInfo, IMarshallingGenerator Generator) GetMarshalInfo(TypePositionInfo info, StubCodeContext context) - { - IMarshallingGenerator generator; - if (!MarshallingGenerators.TryCreate(info, context, out generator)) - { - // [TODO] Report warning - } - - return (info, generator); - } } } diff --git a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs index e816f8457d79..b99184c704cb 100644 --- a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs +++ b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs @@ -2,7 +2,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Runtime.InteropServices; -using DllImportGenerator; + using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -29,7 +29,7 @@ private TypePositionInfo() public RefKind RefKind { get; private set; } public SyntaxKind RefKindSyntax { get; private set; } - + public bool IsByRef => RefKind != RefKind.None; public bool IsManagedReturnPosition { get => this.ManagedIndex == ReturnIndex; } @@ -187,7 +187,7 @@ static MarshalAsInfo CreateMarshalAsInfo(AttributeData attrData) } } - + return new MarshalAsInfo( UnmanagedType: unmanagedType, CustomMarshallerTypeName: customMarshallerTypeName, @@ -197,7 +197,7 @@ static MarshalAsInfo CreateMarshalAsInfo(AttributeData attrData) ArraySizeParamIndex: arraySizeParamIndex ); } - + NativeMarshallingAttributeInfo CreateNativeMarshallingInfo(AttributeData attrData, bool allowGetPinnableReference) { ITypeSymbol spanOfByte = compilation.GetTypeByMetadataName(TypeNames.System_Span)!.Construct(compilation.GetSpecialType(SpecialType.System_Byte)); @@ -239,11 +239,11 @@ NativeMarshallingAttributeInfo CreateNativeMarshallingInfo(AttributeData attrDat valueProperty?.Type, methods); } - + static MarshallingInfo? CreateTypeBasedMarshallingInfo(ITypeSymbol type, Compilation compilation) { var conversion = compilation.ClassifyCommonConversion(type, compilation.GetTypeByMetadataName(TypeNames.System_Runtime_InteropServices_SafeHandle)!); - if (conversion.Exists && + if (conversion.Exists && conversion.IsImplicit && conversion.IsReference && !type.IsAbstract) From 4dff9ae75fffd7b2094895ad81f07b184a650741 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 8 Oct 2020 13:37:12 -0700 Subject: [PATCH 2/8] Add class for reporting diagnostics instead of creating/returning collections --- .../DllImportGenerator/DllImportGenerator.cs | 10 +- .../DllImportGenerator/DllImportStub.cs | 7 +- .../GeneratorDiagnostics.cs | 125 ++++++++++++++++-- .../DllImportGenerator/Resources.Designer.cs | 24 +++- .../DllImportGenerator/Resources.resx | 12 +- 5 files changed, 147 insertions(+), 31 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs b/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs index 4d9ce3989205..7d7e42eacb0c 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs @@ -34,6 +34,8 @@ public void Execute(GeneratorExecutionContext context) // this caching. var syntaxToModel = new Dictionary(); + var generatorDiagnostics = new GeneratorDiagnostics(context); + var generatedDllImports = new StringBuilder(); foreach (SyntaxReference synRef in synRec.Methods) { @@ -55,13 +57,7 @@ public void Execute(GeneratorExecutionContext context) Debug.Assert(!(dllImportAttr is null) && !(dllImportData is null)); // Create the stub. - var dllImportStub = DllImportStub.Create(methodSymbolInfo, dllImportData, context.Compilation, context.CancellationToken); - - // Report any diagnostics from the stub generation step. - foreach (var diag in dllImportStub.Diagnostics) - { - context.ReportDiagnostic(diag); - } + var dllImportStub = DllImportStub.Create(methodSymbolInfo, dllImportData, context.Compilation, generatorDiagnostics, context.CancellationToken); PrintGeneratedSource(generatedDllImports, methodSyntax, dllImportStub, dllImportAttr); } diff --git a/DllImportGenerator/DllImportGenerator/DllImportStub.cs b/DllImportGenerator/DllImportGenerator/DllImportStub.cs index a81067bacaff..f8ab681f8901 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportStub.cs +++ b/DllImportGenerator/DllImportGenerator/DllImportStub.cs @@ -101,13 +101,12 @@ public static DllImportStub Create( IMethodSymbol method, GeneratedDllImportData dllImportData, Compilation compilation, + GeneratorDiagnostics diagnostics, CancellationToken token = default) { // Cancel early if requested token.ThrowIfCancellationRequested(); - var diagnostics = new List(); - // Determine the namespace string stubTypeNamespace = null; if (!(method.ContainingNamespace is null) @@ -164,9 +163,8 @@ public static DllImportStub Create( } // Generate stub code - var stubGenerator = new StubCodeGenerator(method, paramsTypeInfo, retTypeInfo); + var stubGenerator = new StubCodeGenerator(method, paramsTypeInfo, retTypeInfo, diagnostics); var (code, dllImport) = stubGenerator.GenerateSyntax(); - diagnostics.AddRange(stubGenerator.Diagnostics); return new DllImportStub() { @@ -176,7 +174,6 @@ public static DllImportStub Create( StubContainingTypes = containingTypes, StubCode = code, DllImportDeclaration = dllImport, - Diagnostics = diagnostics, }; } } diff --git a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs index 197807530777..d36bf26c2a89 100644 --- a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs +++ b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using Microsoft.CodeAnalysis; @@ -7,7 +8,42 @@ namespace Microsoft.Interop { - public static class GeneratorDiagnostics + internal static class DiagnosticExtensions + { + public static Diagnostic CreateDiagnostic( + this ISymbol symbol, + DiagnosticDescriptor descriptor, + params object[] args) + { + IEnumerable locationsInSource = symbol.Locations.Where(l => l.IsInSource); + if (!locationsInSource.Any()) + return Diagnostic.Create(descriptor, Location.None, args); + + return Diagnostic.Create( + descriptor, + location: locationsInSource.First(), + additionalLocations: locationsInSource.Skip(1), + messageArgs: args); + } + + public static Diagnostic CreateDiagnostic( + this AttributeData attributeData, + DiagnosticDescriptor descriptor, + params object[] args) + { + SyntaxReference? syntaxReference = attributeData.ApplicationSyntaxReference; + Location location = syntaxReference is not null + ? syntaxReference.GetSyntax().GetLocation() + : Location.None; + + return Diagnostic.Create( + descriptor, + location: location.IsInSource ? location : Location.None, + messageArgs: args); + } + } + + internal class GeneratorDiagnostics { private class Ids { @@ -38,6 +74,26 @@ private class Ids isEnabledByDefault: true, description: GetResourceString(Resources.TypeNotSupportedDescription)); + public readonly static DiagnosticDescriptor ParameterConfigurationNotSupported = + new DiagnosticDescriptor( + Ids.ConfigurationNotSupported, + GetResourceString(nameof(Resources.ConfigurationNotSupportedTitle)), + GetResourceString(nameof(Resources.ConfigurationNotSupportedMessageParameter)), + Category, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: GetResourceString(Resources.ConfigurationNotSupportedDescription)); + + public readonly static DiagnosticDescriptor ReturnConfigurationNotSupported = + new DiagnosticDescriptor( + Ids.ConfigurationNotSupported, + GetResourceString(nameof(Resources.ConfigurationNotSupportedTitle)), + GetResourceString(nameof(Resources.ConfigurationNotSupportedMessageReturn)), + Category, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: GetResourceString(Resources.ConfigurationNotSupportedDescription)); + public readonly static DiagnosticDescriptor ConfigurationNotSupported = new DiagnosticDescriptor( Ids.ConfigurationNotSupported, @@ -48,20 +104,63 @@ private class Ids isEnabledByDefault: true, description: GetResourceString(Resources.ConfigurationNotSupportedDescription)); - public static Diagnostic CreateDiagnostic( - this ISymbol symbol, - DiagnosticDescriptor descriptor, - params object[] args) + private readonly GeneratorExecutionContext context; + + public GeneratorDiagnostics(GeneratorExecutionContext context) { - IEnumerable locationsInSource = symbol.Locations.Where(l => l.IsInSource); - if (!locationsInSource.Any()) - return Diagnostic.Create(descriptor, Location.None, args); + this.context = context; + } - return Diagnostic.Create( - descriptor, - location: locationsInSource.First(), - additionalLocations: locationsInSource.Skip(1), - messageArgs: args); + public void ReportMarshallingNotSupported( + IMethodSymbol method, + TypePositionInfo info) + { + if (info.MarshallingAttributeInfo != null && info.MarshallingAttributeInfo is MarshalAsInfo) + { + // Report that the specified marshalling configuration is not supported. + // We don't forward marshalling attributes, so this is reported differently + // than when there is no attribute and the type itself is not supported. + if (info.IsManagedReturnPosition) + { + this.context.ReportDiagnostic( + method.CreateDiagnostic( + GeneratorDiagnostics.ReturnConfigurationNotSupported, + nameof(System.Runtime.InteropServices.MarshalAsAttribute), + method.Name)); + } + else + { + Debug.Assert(info.ManagedIndex <= method.Parameters.Length); + IParameterSymbol paramSymbol = method.Parameters[info.ManagedIndex]; + this.context.ReportDiagnostic( + paramSymbol.CreateDiagnostic( + GeneratorDiagnostics.ParameterConfigurationNotSupported, + nameof(System.Runtime.InteropServices.MarshalAsAttribute), + paramSymbol.Name)); + } + } + else + { + // Report that the type is not supported + if (info.IsManagedReturnPosition) + { + this.context.ReportDiagnostic( + method.CreateDiagnostic( + GeneratorDiagnostics.ReturnTypeNotSupported, + method.ReturnType.ToDisplayString(), + method.Name)); + } + else + { + Debug.Assert(info.ManagedIndex <= method.Parameters.Length); + IParameterSymbol paramSymbol = method.Parameters[info.ManagedIndex]; + this.context.ReportDiagnostic( + paramSymbol.CreateDiagnostic( + GeneratorDiagnostics.ParameterTypeNotSupported, + paramSymbol.Type.ToDisplayString(), + paramSymbol.Name)); + } + } } private static LocalizableResourceString GetResourceString(string resourceName) diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index c1a47f0dfac4..42a64d4fb085 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -97,7 +97,7 @@ internal static string CannotHaveMultipleMarshallingAttributesMessage { } /// - /// Looks up a localized string similar to Source-generated P/Invokes will ignore any configuration that is not supported. If the specified configuration is required, use a regular `DllImport` instead. + /// Looks up a localized string similar to Source-generated P/Invokes will ignore any configuration that is not supported.. /// internal static string ConfigurationNotSupportedDescription { get { @@ -106,7 +106,7 @@ internal static string ConfigurationNotSupportedDescription { } /// - /// Looks up a localized string similar to The '{0}' configuration is not supported by source-generated P/Invokes. The generated source will ignore the specified configuration.. + /// Looks up a localized string similar to The '{0}' configuration is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead.. /// internal static string ConfigurationNotSupportedMessage { get { @@ -114,6 +114,24 @@ internal static string ConfigurationNotSupportedMessage { } } + /// + /// Looks up a localized string similar to The specified '{0}' configuration for parameter '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead.. + /// + internal static string ConfigurationNotSupportedMessageParameter { + get { + return ResourceManager.GetString("ConfigurationNotSupportedMessageParameter", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The specified '{0}' configuration for the return value of method '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead.. + /// + internal static string ConfigurationNotSupportedMessageReturn { + get { + return ResourceManager.GetString("ConfigurationNotSupportedMessageReturn", resourceCulture); + } + } + /// /// Looks up a localized string similar to Specified configuration is not supported by source-generated P/Invokes.. /// @@ -286,7 +304,7 @@ internal static string TypeNotSupportedMessageParameter { } /// - /// Looks up a localized string similar to The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of the return value for method '{1}'.. + /// Looks up a localized string similar to The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of the return value of method '{1}'.. /// internal static string TypeNotSupportedMessageReturn { get { diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 45912f286e39..5bb281a8de46 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -130,10 +130,16 @@ Type '{0}' is marked with 'BlittableTypeAttribute' and 'NativeMarshallingAttribute'. A type can only have one of these two attributes. - Source-generated P/Invokes will ignore any configuration that is not supported. If the specified configuration is required, use a regular `DllImport` instead + Source-generated P/Invokes will ignore any configuration that is not supported. - The '{0}' configuration is not supported by source-generated P/Invokes. The generated source will ignore the specified configuration. + The '{0}' configuration is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead. + + + The specified '{0}' configuration for parameter '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead. + + + The specified '{0}' configuration for the return value of method '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead. Specified configuration is not supported by source-generated P/Invokes. @@ -193,7 +199,7 @@ The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of parameter '{1}'. - The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of the return value for method '{1}'. + The type '{0}' is not supported by source-generated P/Invokes. The generated source will not handle marshalling of the return value of method '{1}'. Specified type is not supported by source-generated P/Invokes From 8b4e0ae138d0ecabeb0911879c42092aad4e48c9 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 8 Oct 2020 16:13:21 -0700 Subject: [PATCH 3/8] Report diagnostics for unsupported p/invoke or marshalling configuration --- .../DllImportGenerator/DllImportStub.cs | 4 +-- .../GeneratorDiagnostics.cs | 36 +++++++++++++++++-- .../DllImportGenerator/Resources.Designer.cs | 9 +++++ .../DllImportGenerator/Resources.resx | 3 ++ .../DllImportGenerator/StubCodeGenerator.cs | 20 ++++------- .../DllImportGenerator/TypePositionInfo.cs | 21 ++++++----- 6 files changed, 64 insertions(+), 29 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/DllImportStub.cs b/DllImportGenerator/DllImportGenerator/DllImportStub.cs index f8ab681f8901..c2b7ca04271c 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportStub.cs +++ b/DllImportGenerator/DllImportGenerator/DllImportStub.cs @@ -138,13 +138,13 @@ public static DllImportStub Create( for (int i = 0; i < method.Parameters.Length; i++) { var param = method.Parameters[i]; - var typeInfo = TypePositionInfo.CreateForParameter(param, compilation); + var typeInfo = TypePositionInfo.CreateForParameter(param, compilation, diagnostics); typeInfo.ManagedIndex = i; typeInfo.NativeIndex = paramsTypeInfo.Count; paramsTypeInfo.Add(typeInfo); } - TypePositionInfo retTypeInfo = TypePositionInfo.CreateForType(method.ReturnType, method.GetReturnTypeAttributes(), compilation); + TypePositionInfo retTypeInfo = TypePositionInfo.CreateForType(method.ReturnType, method.GetReturnTypeAttributes(), compilation, diagnostics); retTypeInfo.ManagedIndex = TypePositionInfo.ReturnIndex; retTypeInfo.NativeIndex = TypePositionInfo.ReturnIndex; if (!dllImportData.PreserveSig) diff --git a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs index d36bf26c2a89..5ac6973b8003 100644 --- a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs +++ b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs @@ -43,7 +43,7 @@ public static Diagnostic CreateDiagnostic( } } - internal class GeneratorDiagnostics + public class GeneratorDiagnostics { private class Ids { @@ -104,6 +104,16 @@ private class Ids isEnabledByDefault: true, description: GetResourceString(Resources.ConfigurationNotSupportedDescription)); + public readonly static DiagnosticDescriptor ConfigurationValueNotSupported = + new DiagnosticDescriptor( + Ids.ConfigurationNotSupported, + GetResourceString(nameof(Resources.ConfigurationNotSupportedTitle)), + GetResourceString(nameof(Resources.ConfigurationNotSupportedMessageValue)), + Category, + DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: GetResourceString(Resources.ConfigurationNotSupportedDescription)); + private readonly GeneratorExecutionContext context; public GeneratorDiagnostics(GeneratorExecutionContext context) @@ -111,7 +121,29 @@ public GeneratorDiagnostics(GeneratorExecutionContext context) this.context = context; } - public void ReportMarshallingNotSupported( + public void ReportConfigurationNotSupported( + AttributeData attributeData, + string configurationName, + string? unsupportedValue = null) + { + if (unsupportedValue == null) + { + this.context.ReportDiagnostic( + attributeData.CreateDiagnostic( + GeneratorDiagnostics.ConfigurationNotSupported, + configurationName)); + } + else + { + this.context.ReportDiagnostic( + attributeData.CreateDiagnostic( + GeneratorDiagnostics.ConfigurationValueNotSupported, + unsupportedValue, + configurationName)); + } + } + + internal void ReportMarshallingNotSupported( IMethodSymbol method, TypePositionInfo info) { diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index 42a64d4fb085..dd85e66f82c4 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -132,6 +132,15 @@ internal static string ConfigurationNotSupportedMessageReturn { } } + /// + /// Looks up a localized string similar to The specified value '{0}' for '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead.. + /// + internal static string ConfigurationNotSupportedMessageValue { + get { + return ResourceManager.GetString("ConfigurationNotSupportedMessageValue", resourceCulture); + } + } + /// /// Looks up a localized string similar to Specified configuration is not supported by source-generated P/Invokes.. /// diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 5bb281a8de46..0da0c43a33db 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -141,6 +141,9 @@ The specified '{0}' configuration for the return value of method '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead. + + The specified value '{0}' for '{1}' is not supported by source-generated P/Invokes. If the specified configuration is required, use a regular `DllImport` instead. + Specified configuration is not supported by source-generated P/Invokes. diff --git a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs index c82341578fb0..59cd20b8cf76 100644 --- a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs @@ -41,8 +41,7 @@ internal sealed class StubCodeGenerator : StubCodeContext Stage.Cleanup }; - public IEnumerable Diagnostics => diagnostics; - private readonly List diagnostics = new List(); + private readonly GeneratorDiagnostics diagnostics; private readonly IMethodSymbol stubMethod; private readonly List<(TypePositionInfo TypeInfo, IMarshallingGenerator Generator)> paramMarshallers; @@ -51,12 +50,14 @@ internal sealed class StubCodeGenerator : StubCodeContext public StubCodeGenerator( IMethodSymbol stubMethod, IEnumerable paramsTypeInfo, - TypePositionInfo retTypeInfo) + TypePositionInfo retTypeInfo, + GeneratorDiagnostics generatorDiagnostics) { Debug.Assert(retTypeInfo.IsNativeReturnPosition); this.CurrentStage = Stages[0]; this.stubMethod = stubMethod; + this.diagnostics = generatorDiagnostics; // Get marshallers for parameters this.paramMarshallers = paramsTypeInfo.Select(p => @@ -64,12 +65,7 @@ public StubCodeGenerator( IMarshallingGenerator generator; if (!MarshallingGenerators.TryCreate(p, this, out generator)) { - IParameterSymbol paramSymbol = stubMethod.Parameters[p.ManagedIndex]; - this.diagnostics.Add( - paramSymbol.CreateDiagnostic( - GeneratorDiagnostics.ParameterTypeNotSupported, - paramSymbol.Type.ToDisplayString(), - paramSymbol.Name)); + this.diagnostics.ReportMarshallingNotSupported(this.stubMethod, p); } return (p, generator); @@ -79,11 +75,7 @@ public StubCodeGenerator( IMarshallingGenerator retGenerator; if (!MarshallingGenerators.TryCreate(retTypeInfo, this, out retGenerator)) { - this.diagnostics.Add( - stubMethod.CreateDiagnostic( - GeneratorDiagnostics.ReturnTypeNotSupported, - stubMethod.ReturnType.ToDisplayString(), - stubMethod.Name)); + this.diagnostics.ReportMarshallingNotSupported(this.stubMethod, retTypeInfo); } this.retMarshaller = (retTypeInfo, retGenerator); diff --git a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs index b99184c704cb..c7a576b0c898 100644 --- a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs +++ b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs @@ -41,9 +41,9 @@ private TypePositionInfo() public MarshallingInfo MarshallingAttributeInfo { get; private set; } - public static TypePositionInfo CreateForParameter(IParameterSymbol paramSymbol, Compilation compilation) + public static TypePositionInfo CreateForParameter(IParameterSymbol paramSymbol, Compilation compilation, GeneratorDiagnostics diagnostics) { - var marshallingInfo = GetMarshallingInfo(paramSymbol.Type, paramSymbol.GetAttributes(), compilation); + var marshallingInfo = GetMarshallingInfo(paramSymbol.Type, paramSymbol.GetAttributes(), compilation, diagnostics); var typeInfo = new TypePositionInfo() { ManagedType = paramSymbol.Type, @@ -56,9 +56,9 @@ public static TypePositionInfo CreateForParameter(IParameterSymbol paramSymbol, return typeInfo; } - public static TypePositionInfo CreateForType(ITypeSymbol type, IEnumerable attributes, Compilation compilation) + public static TypePositionInfo CreateForType(ITypeSymbol type, IEnumerable attributes, Compilation compilation, GeneratorDiagnostics diagnostics) { - var marshallingInfo = GetMarshallingInfo(type, attributes, compilation); + var marshallingInfo = GetMarshallingInfo(type, attributes, compilation, diagnostics); var typeInfo = new TypePositionInfo() { ManagedType = type, @@ -72,7 +72,7 @@ public static TypePositionInfo CreateForType(ITypeSymbol type, IEnumerable attributes, Compilation compilation) + private static MarshallingInfo? GetMarshallingInfo(ITypeSymbol type, IEnumerable attributes, Compilation compilation, GeneratorDiagnostics diagnostics) { MarshallingInfo? marshallingInfo = null; // Look at attributes on the type. @@ -87,7 +87,7 @@ public static TypePositionInfo CreateForType(ITypeSymbol type, IEnumerable Date: Thu, 8 Oct 2020 18:56:44 -0700 Subject: [PATCH 4/8] Fix handling of UnmanagedType if MarshalAs constructor with short is used --- DllImportGenerator/DllImportGenerator/TypePositionInfo.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs index c7a576b0c898..ac77297b71bb 100644 --- a/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs +++ b/DllImportGenerator/DllImportGenerator/TypePositionInfo.cs @@ -143,7 +143,10 @@ public static TypePositionInfo CreateForType(ITypeSymbol type, IEnumerable Date: Thu, 8 Oct 2020 18:57:37 -0700 Subject: [PATCH 5/8] Add tests for generator diagnostics Split basic compile tests into with/without diagnostics. As we add support for different types / configuration, the appropriate snippets can be moved from expecting diagnostics to not expecting diagnostics. Enable emitting generated files in demo project. --- DllImportGenerator/Demo/Demo.csproj | 4 + .../DllImportGenerator.UnitTests/Compiles.cs | 117 ++++++-- .../Diagnostics.cs | 264 ++++++++++++++++++ 3 files changed, 356 insertions(+), 29 deletions(-) create mode 100644 DllImportGenerator/DllImportGenerator.UnitTests/Diagnostics.cs diff --git a/DllImportGenerator/Demo/Demo.csproj b/DllImportGenerator/Demo/Demo.csproj index 4cee92efcd88..0ca8c597da9a 100644 --- a/DllImportGenerator/Demo/Demo.csproj +++ b/DllImportGenerator/Demo/Demo.csproj @@ -6,6 +6,10 @@ true preview + + true + true diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs index 0c58aa3875f5..ab1b7aff1032 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs @@ -9,7 +9,7 @@ namespace DllImportGenerator.UnitTests { public class Compiles { - public static IEnumerable CodeSnippetsToCompile() + public static IEnumerable CodeSnippetsToCompile_NoDiagnostics() { yield return new[] { CodeSnippets.TrivialClassDeclarations }; yield return new[] { CodeSnippets.TrivialStructDeclarations }; @@ -20,7 +20,7 @@ public static IEnumerable CodeSnippetsToCompile() yield return new[] { CodeSnippets.AllDllImportNamedArguments }; yield return new[] { CodeSnippets.DefaultParameters }; yield return new[] { CodeSnippets.UseCSharpFeaturesForConstants }; - yield return new[] { CodeSnippets.MarshalAsAttributeOnTypes }; + //yield return new[] { CodeSnippets.MarshalAsAttributeOnTypes }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; @@ -32,10 +32,74 @@ public static IEnumerable CodeSnippetsToCompile() yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; - yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; - yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + //yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.Bool) }; + yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.VariantBool) }; + yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.I1) }; + //yield return new[] { CodeSnippets.EnumParameters }; + yield return new[] { CodeSnippets.PreserveSigFalseVoidReturn }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + //yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.DelegateParametersAndModifiers }; + yield return new[] { CodeSnippets.DelegateMarshalAsParametersAndModifiers }; + yield return new[] { CodeSnippets.BlittableStructParametersAndModifiers }; + yield return new[] { CodeSnippets.GenericBlittableStructParametersAndModifiers }; + yield return new[] { CodeSnippets.BasicParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle") }; + } + + public static IEnumerable CodeSnippetsToCompile_WithDiagnostics() + { + yield return new[] { CodeSnippets.MarshalAsAttributeOnTypes }; + + yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; + yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; @@ -51,26 +115,12 @@ public static IEnumerable CodeSnippetsToCompile() yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; yield return new[] { CodeSnippets.BasicParametersAndModifiers() }; - yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.Bool) }; - yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.VariantBool) }; - yield return new[] { CodeSnippets.MarshalAsParametersAndModifiers(UnmanagedType.I1) }; + yield return new[] { CodeSnippets.EnumParameters }; - yield return new[] { CodeSnippets.PreserveSigFalseVoidReturn }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.PreserveSigFalse() }; + yield return new[] { CodeSnippets.PreserveSigFalse() }; yield return new[] { CodeSnippets.PreserveSigFalse() }; yield return new[] { CodeSnippets.PreserveSigFalse() }; @@ -86,22 +136,31 @@ public static IEnumerable CodeSnippetsToCompile() yield return new[] { CodeSnippets.PreserveSigFalse() }; yield return new[] { CodeSnippets.PreserveSigFalse() }; yield return new[] { CodeSnippets.PreserveSigFalse() }; - yield return new[] { CodeSnippets.DelegateParametersAndModifiers }; - yield return new[] { CodeSnippets.DelegateMarshalAsParametersAndModifiers }; - yield return new[] { CodeSnippets.BlittableStructParametersAndModifiers }; - yield return new[] { CodeSnippets.GenericBlittableStructParametersAndModifiers }; - yield return new[] { CodeSnippets.BasicParametersAndModifiers("Microsoft.Win32.SafeHandles.SafeFileHandle") }; } [Theory] - [MemberData(nameof(CodeSnippetsToCompile))] - public async Task ValidateSnippets(string source) + [MemberData(nameof(CodeSnippetsToCompile_NoDiagnostics))] + public async Task ValidateSnippets_NoDiagnostics(string source) + { + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + Assert.Empty(generatorDiags); + + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + [Theory] + [MemberData(nameof(CodeSnippetsToCompile_WithDiagnostics))] + public async Task ValidateSnippets_WithDiagnostics(string source) { Compilation comp = await TestUtils.CreateCompilation(source); TestUtils.AssertPreSourceGeneratorCompilation(comp); var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); - Assert.All(generatorDiags, d => Assert.NotEqual(DiagnosticSeverity.Error, d.Severity)); + Assert.NotEmpty(generatorDiags); var newCompDiags = newComp.GetDiagnostics(); Assert.Empty(newCompDiags); diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Diagnostics.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Diagnostics.cs new file mode 100644 index 000000000000..0ba00530c7c1 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Diagnostics.cs @@ -0,0 +1,264 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Threading.Tasks; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Text; +using Microsoft.Interop; +using Xunit; + +namespace DllImportGenerator.UnitTests +{ + public class Diagnostics + { + [Fact] + public async Task ParameterTypeNotSupported_ReportsDiagnostic() + { + string source = @" +using System.Collections.Generic; +using System.Runtime.InteropServices; +namespace NS +{ + class MyClass { } +} +partial class Test +{ + [GeneratedDllImport(""DoesNotExist"")] + public static partial void Method1(NS.MyClass c); + + [GeneratedDllImport(""DoesNotExist"")] + public static partial void Method2(int i, List list); +} +"; + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + DiagnosticResult[] expectedDiags = new DiagnosticResult[] + { + (new DiagnosticResult(GeneratorDiagnostics.ParameterTypeNotSupported)) + .WithSpan(11, 51, 11, 52) + .WithArguments("NS.MyClass", "c"), + (new DiagnosticResult(GeneratorDiagnostics.ParameterTypeNotSupported)) + .WithSpan(14, 57, 14, 61) + .WithArguments("System.Collections.Generic.List", "list"), + }; + VerifyDiagnostics(expectedDiags, GetSortedDiagnostics(generatorDiags)); + + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + [Fact] + public async Task ReturnTypeNotSupported_ReportsDiagnostic() + { + string source = @" +using System.Collections.Generic; +using System.Runtime.InteropServices; +namespace NS +{ + class MyClass { } +} +partial class Test +{ + [GeneratedDllImport(""DoesNotExist"")] + public static partial NS.MyClass Method1(); + + [GeneratedDllImport(""DoesNotExist"")] + public static partial List Method2(); +} +"; + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + DiagnosticResult[] expectedDiags = new DiagnosticResult[] + { + (new DiagnosticResult(GeneratorDiagnostics.ReturnTypeNotSupported)) + .WithSpan(11, 38, 11, 45) + .WithArguments("NS.MyClass", "Method1"), + (new DiagnosticResult(GeneratorDiagnostics.ReturnTypeNotSupported)) + .WithSpan(14, 37, 14, 44) + .WithArguments("System.Collections.Generic.List", "Method2"), + }; + VerifyDiagnostics(expectedDiags, GetSortedDiagnostics(generatorDiags)); + + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + [Fact] + public async Task ParameterConfigurationNotSupported_ReportsDiagnostic() + { + string source = @" +using System.Runtime.InteropServices; +partial class Test +{ + [GeneratedDllImport(""DoesNotExist"")] + public static partial void Method1([MarshalAs(UnmanagedType.BStr)] int i1, int i2); + + [GeneratedDllImport(""DoesNotExist"")] + public static partial void Method2(bool b1, [MarshalAs(UnmanagedType.FunctionPtr)] bool b2); +} +"; + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + DiagnosticResult[] expectedDiags = new DiagnosticResult[] + { + (new DiagnosticResult(GeneratorDiagnostics.ParameterConfigurationNotSupported)) + .WithSpan(6, 76, 6, 78) + .WithArguments(nameof(MarshalAsAttribute), "i1"), + (new DiagnosticResult(GeneratorDiagnostics.ParameterConfigurationNotSupported)) + .WithSpan(9, 93, 9, 95) + .WithArguments(nameof(MarshalAsAttribute), "b2"), + }; + VerifyDiagnostics(expectedDiags, GetSortedDiagnostics(generatorDiags)); + + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + [Fact] + public async Task ReturnConfigurationNotSupported_ReportsDiagnostic() + { + string source = @" +using System.Runtime.InteropServices; +partial class Test +{ + [GeneratedDllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.BStr)] + public static partial int Method1(int i); + + [GeneratedDllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.FunctionPtr)] + public static partial bool Method2(bool b); +} +"; + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + DiagnosticResult[] expectedDiags = new DiagnosticResult[] + { + (new DiagnosticResult(GeneratorDiagnostics.ReturnConfigurationNotSupported)) + .WithSpan(7, 31, 7, 38) + .WithArguments(nameof(MarshalAsAttribute), "Method1"), + (new DiagnosticResult(GeneratorDiagnostics.ReturnConfigurationNotSupported)) + .WithSpan(11, 32, 11, 39) + .WithArguments(nameof(MarshalAsAttribute), "Method2"), + }; + VerifyDiagnostics(expectedDiags, GetSortedDiagnostics(generatorDiags)); + + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + [Fact] + public async Task MarshalAsUnmanagedTypeNotSupported_ReportsDiagnostic() + { + string source = @" +using System.Runtime.InteropServices; +partial class Test +{ + [GeneratedDllImport(""DoesNotExist"")] + [return: MarshalAs(1)] + public static partial int Method1(int i); + + [GeneratedDllImport(""DoesNotExist"")] + public static partial bool Method2([MarshalAs((short)0)] bool b); +} +"; + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + DiagnosticResult[] expectedDiags = new DiagnosticResult[] + { + (new DiagnosticResult(GeneratorDiagnostics.ConfigurationValueNotSupported)) + .WithSpan(6, 14, 6, 26) + .WithArguments(1, nameof(UnmanagedType)), + (new DiagnosticResult(GeneratorDiagnostics.ReturnConfigurationNotSupported)) + .WithSpan(7, 31, 7, 38) + .WithArguments(nameof(MarshalAsAttribute), "Method1"), + (new DiagnosticResult(GeneratorDiagnostics.ConfigurationValueNotSupported)) + .WithSpan(10, 41, 10, 60) + .WithArguments(0, nameof(UnmanagedType)), + (new DiagnosticResult(GeneratorDiagnostics.ParameterConfigurationNotSupported)) + .WithSpan(10, 67, 10, 68) + .WithArguments(nameof(MarshalAsAttribute), "b"), + }; + VerifyDiagnostics(expectedDiags, GetSortedDiagnostics(generatorDiags)); + + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + [Fact] + public async Task MarshalAsFieldNotSupported_ReportsDiagnostic() + { + string source = @" +using System.Runtime.InteropServices; +partial class Test +{ + [GeneratedDllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.I4, SafeArraySubType=VarEnum.VT_I4)] + public static partial int Method1(int i); + + [GeneratedDllImport(""DoesNotExist"")] + public static partial bool Method2([MarshalAs(UnmanagedType.I1, IidParameterIndex = 1)] bool b); +} +"; + Compilation comp = await TestUtils.CreateCompilation(source); + TestUtils.AssertPreSourceGeneratorCompilation(comp); + + var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); + DiagnosticResult[] expectedDiags = new DiagnosticResult[] + { + (new DiagnosticResult(GeneratorDiagnostics.ConfigurationNotSupported)) + .WithSpan(6, 14, 6, 73) + .WithArguments($"{nameof(MarshalAsAttribute)}{Type.Delimiter}{nameof(MarshalAsAttribute.SafeArraySubType)}"), + (new DiagnosticResult(GeneratorDiagnostics.ConfigurationNotSupported)) + .WithSpan(10, 41, 10, 91) + .WithArguments($"{nameof(MarshalAsAttribute)}{Type.Delimiter}{nameof(MarshalAsAttribute.IidParameterIndex)}"), + }; + VerifyDiagnostics(expectedDiags, GetSortedDiagnostics(generatorDiags)); + var newCompDiags = newComp.GetDiagnostics(); + Assert.Empty(newCompDiags); + } + + private static void VerifyDiagnostics(DiagnosticResult[] expectedDiagnostics, Diagnostic[] actualDiagnostics) + { + Assert.Equal(expectedDiagnostics.Length, actualDiagnostics.Length); + for (var i = 0; i < expectedDiagnostics.Length; i++) + { + DiagnosticResult expected = expectedDiagnostics[i]; + Diagnostic actual = actualDiagnostics[i]; + + Assert.Equal(expected.Id, actual.Id); + Assert.Equal(expected.Message, actual.GetMessage()); + Assert.Equal(expected.Severity, actual.Severity); + if (expected.HasLocation) + { + FileLinePositionSpan expectedSpan = expected.Spans[0].Span; + FileLinePositionSpan actualSpan = actual.Location.GetLineSpan(); + Assert.Equal(expectedSpan, actualSpan); + } + } + } + + private static Diagnostic[] GetSortedDiagnostics(IEnumerable diagnostics) + { + return diagnostics + .OrderBy(d => d.Location.GetLineSpan().Path, StringComparer.Ordinal) + .ThenBy(d => d.Location.SourceSpan.Start) + .ThenBy(d => d.Location.SourceSpan.End) + .ThenBy(d => d.Id) + .ToArray(); + } + } +} From 216e71705f518a6e1db86e8271a313b195d438ec Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 8 Oct 2020 19:39:59 -0700 Subject: [PATCH 6/8] Fix violations of RS1032: Define diagnostic message correctly Update README to point out emitting generated source to disk --- .../GeneratorDiagnostics.cs | 14 ++++++++++++ .../DllImportGenerator/Resources.Designer.cs | 22 +++++++++---------- .../DllImportGenerator/Resources.resx | 22 +++++++++---------- DllImportGenerator/readme.md | 2 +- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs index 5ac6973b8003..e3f7a39a9543 100644 --- a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs +++ b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs @@ -43,6 +43,9 @@ public static Diagnostic CreateDiagnostic( } } + /// + /// Class for reporting diagnostics in the DLL import generator + /// public class GeneratorDiagnostics { private class Ids @@ -121,6 +124,12 @@ public GeneratorDiagnostics(GeneratorExecutionContext context) this.context = context; } + /// + /// Report diagnostic for configuration that is not supported by the DLL import source generator + /// + /// Attribute specifying the unsupported configuration + /// Name of the configuration + /// [Optiona] Unsupported configuration value public void ReportConfigurationNotSupported( AttributeData attributeData, string configurationName, @@ -143,6 +152,11 @@ public void ReportConfigurationNotSupported( } } + /// + /// Report diagnostic for marshalling of a parameter/return that is not supported + /// + /// Method with the parameter/return + /// Type info for the parameter/return internal void ReportMarshallingNotSupported( IMethodSymbol method, TypePositionInfo info) diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index dd85e66f82c4..67f0b5f785e3 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -70,7 +70,7 @@ internal static string BlittableTypeMustBeBlittableDescription { } /// - /// Looks up a localized string similar to Type '{0}' is marked with 'BlittableTypeAttribute' but is not blittable.. + /// Looks up a localized string similar to Type '{0}' is marked with 'BlittableTypeAttribute' but is not blittable. /// internal static string BlittableTypeMustBeBlittableMessage { get { @@ -160,7 +160,7 @@ internal static string GetPinnableReferenceReturnTypeBlittableDescription { } /// - /// Looks up a localized string similar to The dereferenced type of the return type of the 'GetPinnableReference' method must be blittable.. + /// Looks up a localized string similar to The dereferenced type of the return type of the 'GetPinnableReference' method must be blittable. /// internal static string GetPinnableReferenceReturnTypeBlittableMessage { get { @@ -178,7 +178,7 @@ internal static string GetPinnableReferenceShouldSupportAllocatingMarshallingFal } /// - /// Looks up a localized string similar to Type '{0}' has a 'GetPinnableReference' method but its native type does not support marshalling in scenarios where pinning is impossible.. + /// Looks up a localized string similar to Type '{0}' has a 'GetPinnableReference' method but its native type does not support marshalling in scenarios where pinning is impossible. /// internal static string GetPinnableReferenceShouldSupportAllocatingMarshallingFallbackMessage { get { @@ -196,7 +196,7 @@ internal static string NativeTypeMustBeBlittableDescription { } /// - /// Looks up a localized string similar to The native type '{0}' for the type '{1}' is not blittable.. + /// Looks up a localized string similar to The native type '{0}' for the type '{1}' is not blittable. /// internal static string NativeTypeMustBeBlittableMessage { get { @@ -214,7 +214,7 @@ internal static string NativeTypeMustBeNonNullDescription { } /// - /// Looks up a localized string similar to The native type for the type '{0}' is null.. + /// Looks up a localized string similar to The native type for the type '{0}' is null. /// internal static string NativeTypeMustBeNonNullMessage { get { @@ -232,7 +232,7 @@ internal static string NativeTypeMustBePointerSizedDescription { } /// - /// Looks up a localized string similar to The native type '{0}' must be pointer sized because the managed type '{1}' has a 'GetPinnableReference' method.. + /// Looks up a localized string similar to The native type '{0}' must be pointer sized because the managed type '{1}' has a 'GetPinnableReference' method. /// internal static string NativeTypeMustBePointerSizedMessage { get { @@ -250,7 +250,7 @@ internal static string NativeTypeMustHaveRequiredShapeDescription { } /// - /// Looks up a localized string similar to The native type '{0}' must be a value type and have a constructor that takes one parameter of type '{1}' or a parameterless instance method named 'ToManaged' that returns '{1}'.. + /// Looks up a localized string similar to The native type '{0}' must be a value type and have a constructor that takes one parameter of type '{1}' or a parameterless instance method named 'ToManaged' that returns '{1}'. /// internal static string NativeTypeMustHaveRequiredShapeMessage { get { @@ -268,7 +268,7 @@ internal static string StackallocConstructorMustHaveStackBufferSizeConstantDescr } /// - /// Looks up a localized string similar to The native type '{0}' must have a 'public const int StackBufferSize' field that specifies the size of the stack buffer because it has a constructor that takes a stack-allocated Span<byte>.. + /// Looks up a localized string similar to The native type '{0}' must have a 'public const int StackBufferSize' field that specifies the size of the stack buffer because it has a constructor that takes a stack-allocated Span<byte>. /// internal static string StackallocConstructorMustHaveStackBufferSizeConstantMessage { get { @@ -286,7 +286,7 @@ internal static string StackallocMarshallingShouldSupportAllocatingMarshallingFa } /// - /// Looks up a localized string similar to Native type '{0}' has a stack-allocating constructor does not support marshalling in scenarios where stack allocation is impossible.. + /// Looks up a localized string similar to Native type '{0}' has a stack-allocating constructor does not support marshalling in scenarios where stack allocation is impossible. /// internal static string StackallocMarshallingShouldSupportAllocatingMarshallingFallbackMessage { get { @@ -340,7 +340,7 @@ internal static string ValuePropertyMustHaveGetterDescription { } /// - /// Looks up a localized string similar to The 'Value' property on the native type '{0}' must have a getter.. + /// Looks up a localized string similar to The 'Value' property on the native type '{0}' must have a getter. /// internal static string ValuePropertyMustHaveGetterMessage { get { @@ -358,7 +358,7 @@ internal static string ValuePropertyMustHaveSetterDescription { } /// - /// Looks up a localized string similar to The 'Value' property on the native type '{0}' must have a setter.. + /// Looks up a localized string similar to The 'Value' property on the native type '{0}' must have a setter. /// internal static string ValuePropertyMustHaveSetterMessage { get { diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 0da0c43a33db..9568b0305d55 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -121,7 +121,7 @@ A type marked with 'BlittableTypeAttribute' must be blittable. - Type '{0}' is marked with 'BlittableTypeAttribute' but is not blittable. + Type '{0}' is marked with 'BlittableTypeAttribute' but is not blittable The 'BlittableTypeAttribute' and 'NativeMarshallingAttribute' attributes are mutually exclusive. @@ -151,49 +151,49 @@ The return type of 'GetPinnableReference' (after accounting for 'ref') must be blittable. - The dereferenced type of the return type of the 'GetPinnableReference' method must be blittable. + The dereferenced type of the return type of the 'GetPinnableReference' method must be blittable A type that supports marshalling from managed to native by pinning should also support marshalling from managed to native where pinning is impossible. - Type '{0}' has a 'GetPinnableReference' method but its native type does not support marshalling in scenarios where pinning is impossible. + Type '{0}' has a 'GetPinnableReference' method but its native type does not support marshalling in scenarios where pinning is impossible A native type for a given type must be blittable. - The native type '{0}' for the type '{1}' is not blittable. + The native type '{0}' for the type '{1}' is not blittable A native type for a given type must be non-null. - The native type for the type '{0}' is null. + The native type for the type '{0}' is null The native type must be pointer sized so the pinned result of 'GetPinnableReference' can be cast to the native type. - The native type '{0}' must be pointer sized because the managed type '{1}' has a 'GetPinnableReference' method. + The native type '{0}' must be pointer sized because the managed type '{1}' has a 'GetPinnableReference' method The native type must have at least one of the two marshalling methods to enable marshalling the managed type. - The native type '{0}' must be a value type and have a constructor that takes one parameter of type '{1}' or a parameterless instance method named 'ToManaged' that returns '{1}'. + The native type '{0}' must be a value type and have a constructor that takes one parameter of type '{1}' or a parameterless instance method named 'ToManaged' that returns '{1}' When constructor taking a Span<byte> is specified on the native type, the type must also have a public integer constant named StackBufferSize to provide the size of the stack-allocated buffer. - The native type '{0}' must have a 'public const int StackBufferSize' field that specifies the size of the stack buffer because it has a constructor that takes a stack-allocated Span<byte>. + The native type '{0}' must have a 'public const int StackBufferSize' field that specifies the size of the stack buffer because it has a constructor that takes a stack-allocated Span<byte> A type that supports marshalling from managed to native by stack allocation should also support marshalling from managed to native where stack allocation is impossible. - Native type '{0}' has a stack-allocating constructor does not support marshalling in scenarios where stack allocation is impossible. + Native type '{0}' has a stack-allocating constructor does not support marshalling in scenarios where stack allocation is impossible For types that are not supported by source-generated P/Invokes, the resulting P/Invoke will rely on the underlying runtime to marshal the specified type. @@ -211,12 +211,12 @@ The native type's 'Value' property must have a getter to support marshalling from managed to native. - The 'Value' property on the native type '{0}' must have a getter. + The 'Value' property on the native type '{0}' must have a getter The native type's 'Value' property must have a setter to support marshalling from native to managed. - The 'Value' property on the native type '{0}' must have a setter. + The 'Value' property on the native type '{0}' must have a setter \ No newline at end of file diff --git a/DllImportGenerator/readme.md b/DllImportGenerator/readme.md index 30e05076eb5c..e4b19e8da00f 100644 --- a/DllImportGenerator/readme.md +++ b/DllImportGenerator/readme.md @@ -4,7 +4,7 @@ Work on this project can be tracked and discussed via the [official issue](https ## Example -The [Demo project](./DllImportGenerator/Demo) is designed to be immediately consumable by everyone. It demonstrates a simple use case where the marshalling code is generated and a native function call with only [blittable types](https://docs.microsoft.com/dotnet/framework/interop/blittable-and-non-blittable-types) is made. A managed assembly with [native exports](./DllImportGenerator/TestAssets/NativeExports) is used in the P/Invoke scenario. +The [Demo project](./DllImportGenerator/Demo) is designed to be immediately consumable by everyone. It demonstrates a simple use case where the marshalling code is generated and a native function call with only [blittable types](https://docs.microsoft.com/dotnet/framework/interop/blittable-and-non-blittable-types) is made. The project is configured to output the generated source to disk; the files can be found in the project's intermediate directory (`obj///generated`). A managed assembly with [native exports](./DllImportGenerator/TestAssets/NativeExports) is used in the P/Invoke scenario. ### Recommended scenarios: From 9d04790370e3274367833104f8aa7d4e07d7ff7d Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 9 Oct 2020 14:10:19 -0700 Subject: [PATCH 7/8] Switch all diagnostics to errors Make Compiles test check that all reported diagnostics are DLLIMPORTGEN Add doc listing diagnostics reported by the P/Invoke source generator --- .../DllImportGenerator.UnitTests/Compiles.cs | 1 + .../GeneratorDiagnostics.cs | 6 +-- DllImportGenerator/designs/Diagnostics.md | 39 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 DllImportGenerator/designs/Diagnostics.md diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs index ab1b7aff1032..815db781816b 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs @@ -161,6 +161,7 @@ public async Task ValidateSnippets_WithDiagnostics(string source) var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); Assert.NotEmpty(generatorDiags); + Assert.All(generatorDiags, d => Assert.StartsWith(Microsoft.Interop.GeneratorDiagnostics.Ids.Prefix, d.Id)); var newCompDiags = newComp.GetDiagnostics(); Assert.Empty(newCompDiags); diff --git a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs index e3f7a39a9543..17657fe4eaf5 100644 --- a/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs +++ b/DllImportGenerator/DllImportGenerator/GeneratorDiagnostics.cs @@ -48,7 +48,7 @@ public static Diagnostic CreateDiagnostic( /// public class GeneratorDiagnostics { - private class Ids + public class Ids { public const string Prefix = "DLLIMPORTGEN"; public const string TypeNotSupported = Prefix + "001"; @@ -63,7 +63,7 @@ private class Ids GetResourceString(nameof(Resources.TypeNotSupportedTitle)), GetResourceString(nameof(Resources.TypeNotSupportedMessageParameter)), Category, - DiagnosticSeverity.Warning, + DiagnosticSeverity.Error, isEnabledByDefault: true, description: GetResourceString(Resources.TypeNotSupportedDescription)); @@ -73,7 +73,7 @@ private class Ids GetResourceString(nameof(Resources.TypeNotSupportedTitle)), GetResourceString(nameof(Resources.TypeNotSupportedMessageReturn)), Category, - DiagnosticSeverity.Warning, + DiagnosticSeverity.Error, isEnabledByDefault: true, description: GetResourceString(Resources.TypeNotSupportedDescription)); diff --git a/DllImportGenerator/designs/Diagnostics.md b/DllImportGenerator/designs/Diagnostics.md new file mode 100644 index 000000000000..d69d633022b0 --- /dev/null +++ b/DllImportGenerator/designs/Diagnostics.md @@ -0,0 +1,39 @@ +# Generator Diagnostics + +For all [Roslyn diagnostics](https://docs.microsoft.com/dotnet/api/microsoft.codeanalysis.diagnostic) reported by the P/Invoke source generator: + +| Setting | Value | +| -------- | ------------------ | +| Category | DllImportGenerator | +| Severity | Error | +| Enabled | True | + +The P/Invoke source generator emits the following diagnostics. + +## `DLLIMPORTGEN001`: Specified type is not supported by source-generated P/Invokes + +A method marked `GeneratedDllImport` has a parameter or return type that is not supported by source-generated P/Invokes. + +```C# +// 'object' without any specific marshalling configuration +[GeneratedDllImport("NativeLib")] +public static partial void Method(object o); +``` + +## `DLLIMPORTGEN002`: Specified configuration is not supported by source-generated P/Invokes + +A method marked `GeneratedDllImport` has configuration that is not supported by source-generated P/Invokes. This may be configuration of the method itself or its parameter or return types. + +```C# +// MarshalAs value that does not map to an UnmanagedType +[GeneratedDllImport("NativeLib")] +public static partial void Method([MarshalAs(1)] int i); + +// Unsupported field on MarshalAs (SafeArraySubType, SafeArrayUserDefinedSubType, IidParameterIndex) +[GeneratedDllImport("NativeLib")] +public static partial void Method([MarshalAs(UnmanagedType.SafeArray, SafeArraySubType = VarEnum.VT_BOOL)] bool[] bArr); + +// Unsupported combination of MarshalAs and type being marshalled +[GeneratedDllImport("NativeLib")] +public static partial void Method([MarshalAs(UnmanagedType.LPStr)] bool b); +``` \ No newline at end of file From 2d5dc6892466d9505faeb0072902c82cfa5639d8 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Fri, 9 Oct 2020 14:12:38 -0700 Subject: [PATCH 8/8] PR feedback --- .../DllImportGenerator/DllImportStub.cs | 2 -- .../Marshalling/MarshallingGenerator.cs | 3 +++ .../DllImportGenerator/StubCodeContext.cs | 7 ++++++- .../DllImportGenerator/StubCodeGenerator.cs | 18 ++---------------- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/DllImportStub.cs b/DllImportGenerator/DllImportGenerator/DllImportStub.cs index c2b7ca04271c..f062532c7409 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportStub.cs +++ b/DllImportGenerator/DllImportGenerator/DllImportStub.cs @@ -49,8 +49,6 @@ public IEnumerable StubParameters public MethodDeclarationSyntax DllImportDeclaration { get; private set; } - public IEnumerable Diagnostics { get; private set; } - /// /// Flags used to indicate members on GeneratedDllImport attribute. /// diff --git a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs index b05ac6af0ed5..e2572ad6cf7b 100644 --- a/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs @@ -53,6 +53,9 @@ internal interface IMarshallingGenerator /// Object to marshal /// Code generation context /// If the marshaller uses an identifier for the native value, true; otherwise, false. + /// + /// of may not be valid. + /// bool UsesNativeIdentifier(TypePositionInfo info, StubCodeContext context); } diff --git a/DllImportGenerator/DllImportGenerator/StubCodeContext.cs b/DllImportGenerator/DllImportGenerator/StubCodeContext.cs index 0b5f9fe0a862..b3618c898bfc 100644 --- a/DllImportGenerator/DllImportGenerator/StubCodeContext.cs +++ b/DllImportGenerator/DllImportGenerator/StubCodeContext.cs @@ -9,6 +9,11 @@ internal abstract class StubCodeContext /// public enum Stage { + /// + /// Invalid stage + /// + Invalid, + /// /// Perform any setup required /// @@ -55,7 +60,7 @@ public enum Stage GuaranteedUnmarshal } - public Stage CurrentStage { get; protected set; } + public Stage CurrentStage { get; protected set; } = Stage.Invalid; public abstract bool PinningSupported { get; } diff --git a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs index 59cd20b8cf76..a28c856026ac 100644 --- a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs @@ -55,7 +55,6 @@ public StubCodeGenerator( { Debug.Assert(retTypeInfo.IsNativeReturnPosition); - this.CurrentStage = Stages[0]; this.stubMethod = stubMethod; this.diagnostics = generatorDiagnostics; @@ -81,19 +80,6 @@ public StubCodeGenerator( this.retMarshaller = (retTypeInfo, retGenerator); } - /// - /// Generate an identifier for the native return value and update the context with the new value - /// - /// Identifier for the native return value - public void GenerateReturnNativeIdentifier() - { - if (CurrentStage != Stage.Setup) - throw new InvalidOperationException(); - - // Update the native identifier for the return value - ReturnNativeIdentifier = $"{ReturnIdentifier}{GeneratedNativeIdentifierSuffix}"; - } - public override (string managed, string native) GetIdentifiers(TypePositionInfo info) { if (info.IsManagedReturnPosition && !info.IsNativeReturnPosition) @@ -119,13 +105,13 @@ public override (string managed, string native) GetIdentifiers(TypePositionInfo public (BlockSyntax Code, MethodDeclarationSyntax DllImport) GenerateSyntax() { - this.CurrentStage = Stages[0]; string dllImportName = stubMethod.Name + "__PInvoke__"; var statements = new List(); if (retMarshaller.Generator.UsesNativeIdentifier(retMarshaller.TypeInfo, this)) { - this.GenerateReturnNativeIdentifier(); + // Update the native identifier for the return value + ReturnNativeIdentifier = $"{ReturnIdentifier}{GeneratedNativeIdentifierSuffix}"; } foreach (var marshaller in paramMarshallers)