Skip to content

Commit df9e3ac

Browse files
elinor-fungelinor-fung
authored
[DllImportGenerator] Stop flagging methods with well-known unsupported types for conversion (#60841)
* Make analyzer stop flagging methods with well-known unsupported types for conversion * Make fixer put attribute arguments in preferred order
1 parent e095fde commit df9e3ac

4 files changed

Lines changed: 124 additions & 12 deletions

File tree

‎src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportAnalyzer.cs‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33

44
using System.Collections.Generic;
55
using System.Collections.Immutable;
6-
using System.Diagnostics;
7-
using System.Runtime.InteropServices;
86

97
using Microsoft.CodeAnalysis;
108
using Microsoft.CodeAnalysis.Diagnostics;
@@ -18,6 +16,13 @@ public class ConvertToGeneratedDllImportAnalyzer : DiagnosticAnalyzer
1816
{
1917
private const string Category = "Interoperability";
2018

19+
private static readonly string[] s_unsupportedTypeNames = new string[]
20+
{
21+
"System.Runtime.InteropServices.CriticalHandle",
22+
"System.Runtime.InteropServices.HandleRef",
23+
"System.Text.StringBuilder"
24+
};
25+
2126
public static readonly DiagnosticDescriptor ConvertToGeneratedDllImport =
2227
new DiagnosticDescriptor(
2328
Ids.ConvertToGeneratedDllImport,
@@ -43,15 +48,21 @@ public override void Initialize(AnalysisContext context)
4348
if (generatedDllImportAttrType == null)
4449
return;
4550

46-
INamedTypeSymbol? dllImportAttrType = compilationContext.Compilation.GetTypeByMetadataName(typeof(DllImportAttribute).FullName);
47-
if (dllImportAttrType == null)
48-
return;
51+
var knownUnsupportedTypes = new List<ITypeSymbol>(s_unsupportedTypeNames.Length);
52+
foreach (string typeName in s_unsupportedTypeNames)
53+
{
54+
INamedTypeSymbol? unsupportedType = compilationContext.Compilation.GetTypeByMetadataName(typeName);
55+
if (unsupportedType != null)
56+
{
57+
knownUnsupportedTypes.Add(unsupportedType);
58+
}
59+
}
4960

50-
compilationContext.RegisterSymbolAction(symbolContext => AnalyzeSymbol(symbolContext, dllImportAttrType), SymbolKind.Method);
61+
compilationContext.RegisterSymbolAction(symbolContext => AnalyzeSymbol(symbolContext, knownUnsupportedTypes), SymbolKind.Method);
5162
});
5263
}
5364

54-
private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol dllImportAttrType)
65+
private static void AnalyzeSymbol(SymbolAnalysisContext context, List<ITypeSymbol> knownUnsupportedTypes)
5566
{
5667
var method = (IMethodSymbol)context.Symbol;
5768

@@ -64,6 +75,19 @@ private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbo
6475
if (dllImportData.ModuleName == "QCall")
6576
return;
6677

78+
// Ignore methods with unsupported parameters
79+
foreach (IParameterSymbol parameter in method.Parameters)
80+
{
81+
if (knownUnsupportedTypes.Contains(parameter.Type))
82+
{
83+
return;
84+
}
85+
}
86+
87+
// Ignore methods with unsupported returns
88+
if (method.ReturnsByRef || method.ReturnsByRefReadonly || knownUnsupportedTypes.Contains(method.ReturnType))
89+
return;
90+
6791
context.ReportDiagnostic(method.CreateDiagnostic(ConvertToGeneratedDllImport, method.Name));
6892
}
6993
}

‎src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.Collections.Generic;
55
using System.Collections.Immutable;
6+
using System.Linq;
67
using System.Runtime.InteropServices;
78
using System.Threading;
89
using System.Threading.Tasks;
@@ -28,6 +29,18 @@ public sealed class ConvertToGeneratedDllImportFixer : CodeFixProvider
2829
public const string NoPreprocessorDefinesKey = "ConvertToGeneratedDllImport";
2930
public const string WithPreprocessorDefinesKey = "ConvertToGeneratedDllImportPreprocessor";
3031

32+
private static readonly string[] s_preferredAttributeArgumentOrder =
33+
{
34+
nameof(DllImportAttribute.EntryPoint),
35+
nameof(DllImportAttribute.BestFitMapping),
36+
nameof(DllImportAttribute.CallingConvention),
37+
nameof(DllImportAttribute.CharSet),
38+
nameof(DllImportAttribute.ExactSpelling),
39+
nameof(DllImportAttribute.PreserveSig),
40+
nameof(DllImportAttribute.SetLastError),
41+
nameof(DllImportAttribute.ThrowOnUnmappableChar)
42+
};
43+
3144
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
3245
{
3346
// Get the syntax root and semantic model
@@ -152,8 +165,11 @@ private async Task<Document> ConvertToGeneratedDllImport(
152165
SyntaxFactory.ElasticMarker
153166
}));
154167

168+
// Sort attribute arguments so that GeneratedDllImport and DllImport match
169+
MethodDeclarationSyntax updatedDeclaration = (MethodDeclarationSyntax)generator.ReplaceNode(methodSyntax, dllImportSyntax, SortDllImportAttributeArguments(dllImportSyntax, generator));
170+
155171
// Remove existing leading trivia - it will be on the GeneratedDllImport method
156-
MethodDeclarationSyntax updatedDeclaration = methodSyntax.WithLeadingTrivia();
172+
updatedDeclaration = updatedDeclaration.WithLeadingTrivia();
157173

158174
// #endif
159175
updatedDeclaration = updatedDeclaration.WithTrailingTrivia(
@@ -225,7 +241,26 @@ private SyntaxNode GetGeneratedDllImportAttribute(
225241
}
226242
}
227243

228-
return generator.RemoveNodes(generatedDllImportSyntax, argumentsToRemove);
244+
generatedDllImportSyntax = generator.RemoveNodes(generatedDllImportSyntax, argumentsToRemove);
245+
return SortDllImportAttributeArguments((AttributeSyntax)generatedDllImportSyntax, generator);
246+
}
247+
248+
private static SyntaxNode SortDllImportAttributeArguments(AttributeSyntax attribute, SyntaxGenerator generator)
249+
{
250+
AttributeArgumentListSyntax updatedArgList = attribute.ArgumentList.WithArguments(
251+
SyntaxFactory.SeparatedList(
252+
attribute.ArgumentList.Arguments.OrderBy(arg =>
253+
{
254+
// Unnamed arguments first
255+
if (arg.NameEquals == null)
256+
return -1;
257+
258+
// Named arguments in specified order, followed by any named arguments with no preferred order
259+
string name = arg.NameEquals.Name.Identifier.Text;
260+
int index = System.Array.IndexOf(s_preferredAttributeArgumentOrder, name);
261+
return index == -1 ? int.MaxValue : index;
262+
})));
263+
return generator.ReplaceNode(attribute, attribute.ArgumentList, updatedArgList);
229264
}
230265

231266
private bool TryCreateUnmanagedCallConvAttributeToEmit(

‎src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/ConvertToGeneratedDllImportAnalyzerTests.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ public static IEnumerable<object[]> NoMarshallingRequiredTypes() => new[]
3737
new object[] { typeof(ConsoleKey) }, // enum
3838
};
3939

40+
public static IEnumerable<object[]> UnsupportedTypes() => new[]
41+
{
42+
new object[] { typeof(System.Runtime.InteropServices.CriticalHandle) },
43+
new object[] { typeof(System.Runtime.InteropServices.HandleRef) },
44+
new object[] { typeof(System.Text.StringBuilder) },
45+
};
46+
4047
[ConditionalTheory]
4148
[MemberData(nameof(MarshallingRequiredTypes))]
4249
[MemberData(nameof(NoMarshallingRequiredTypes))]
@@ -134,6 +141,14 @@ await VerifyCS.VerifyAnalyzerAsync(
134141
.WithArguments("Method2"));
135142
}
136143

144+
[ConditionalTheory]
145+
[MemberData(nameof(UnsupportedTypes))]
146+
public async Task UnsupportedType_NoDiagnostic(Type type)
147+
{
148+
string source = DllImportWithType(type.FullName!);
149+
await VerifyCS.VerifyAnalyzerAsync(source);
150+
}
151+
137152
[ConditionalFact]
138153
public async Task NotDllImport_NoDiagnostic()
139154
{

‎src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/ConvertToGeneratedDllImportFixerTests.cs‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ partial class Test
257257
[GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")]
258258
public static partial int {{|CS8795:Method1|}}(out int ret);
259259
#else
260-
[DllImport(""DoesNotExist"", BestFitMapping = false, EntryPoint = ""Entry"")]
260+
[DllImport(""DoesNotExist"", EntryPoint = ""Entry"", BestFitMapping = false)]
261261
public static extern int Method1(out int ret);
262262
#endif
263263
@@ -306,7 +306,7 @@ partial class Test
306306
[GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")]
307307
public static partial int {{|CS8795:Method1|}}(out int ret);
308308
#else
309-
[DllImport(""DoesNotExist"", CallingConvention = CallingConvention.Winapi, EntryPoint = ""Entry"")]
309+
[DllImport(""DoesNotExist"", EntryPoint = ""Entry"", CallingConvention = CallingConvention.Winapi)]
310310
public static extern int Method1(out int ret);
311311
#endif
312312
}}" : @$"
@@ -351,7 +351,7 @@ partial class Test
351351
[UnmanagedCallConv(CallConvs = new System.Type[] {{ typeof({callConvType.FullName}) }})]
352352
public static partial int {{|CS8795:Method1|}}(out int ret);
353353
#else
354-
[DllImport(""DoesNotExist"", CallingConvention = CallingConvention.{callConv}, EntryPoint = ""Entry"")]
354+
[DllImport(""DoesNotExist"", EntryPoint = ""Entry"", CallingConvention = CallingConvention.{callConv})]
355355
public static extern int Method1(out int ret);
356356
#endif
357357
}}" : @$"
@@ -361,6 +361,44 @@ partial class Test
361361
[GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")]
362362
[UnmanagedCallConv(CallConvs = new System.Type[] {{ typeof({callConvType.FullName}) }})]
363363
public static partial int {{|CS8795:Method1|}}(out int ret);
364+
}}";
365+
await VerifyCS.VerifyCodeFixAsync(
366+
source,
367+
fixedSource,
368+
usePreprocessorDefines ? WithPreprocessorDefinesKey : NoPreprocessorDefinesKey);
369+
}
370+
371+
[Theory]
372+
[InlineData(true)]
373+
[InlineData(false)]
374+
public async Task PreferredAttributeOrder(bool usePreprocessorDefines)
375+
{
376+
string source = @$"
377+
using System.Runtime.InteropServices;
378+
partial class Test
379+
{{
380+
[DllImport(""DoesNotExist"", SetLastError = true, EntryPoint = ""Entry"", ExactSpelling = true, CharSet = CharSet.Unicode)]
381+
public static extern int [|Method|](out int ret);
382+
}}";
383+
// Fixed source will have CS8795 (Partial method must have an implementation) without generator run
384+
string fixedSource = usePreprocessorDefines
385+
? @$"
386+
using System.Runtime.InteropServices;
387+
partial class Test
388+
{{
389+
#if DLLIMPORTGENERATOR_ENABLED
390+
[GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)]
391+
public static partial int {{|CS8795:Method|}}(out int ret);
392+
#else
393+
[DllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)]
394+
public static extern int Method(out int ret);
395+
#endif
396+
}}" : @$"
397+
using System.Runtime.InteropServices;
398+
partial class Test
399+
{{
400+
[GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode, ExactSpelling = true, SetLastError = true)]
401+
public static partial int {{|CS8795:Method|}}(out int ret);
364402
}}";
365403
await VerifyCS.VerifyCodeFixAsync(
366404
source,

0 commit comments

Comments
 (0)