From 1dcaaac73c925ee573c96288c3dee243800ae5e6 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 14 Jan 2021 20:03:22 -0800 Subject: [PATCH 1/2] Add fixer for converting to GeneratedDllImport --- .../ConvertToGeneratedDllImportFixerTests.cs | 280 ++++++++++++++++++ .../DllImportGenerator.UnitTests.csproj | 1 + .../Verifiers/CSharpAnalyzerVerifier.cs | 58 +--- .../Verifiers/CSharpCodeFixVerifier.cs | 119 ++++++++ .../DllImportGenerator.Vsix.csproj | 25 ++ .../source.extension.vsixmanifest | 22 ++ DllImportGenerator/DllImportGenerator.sln | 6 + .../ConvertToGeneratedDllImportFixer.cs | 224 ++++++++++++++ .../DllImportGenerator.csproj | 2 +- .../DllImportGenerator/Resources.Designer.cs | 33 ++- .../DllImportGenerator/Resources.resx | 6 + eng/Versions.props | 2 + 12 files changed, 717 insertions(+), 61 deletions(-) create mode 100644 DllImportGenerator/DllImportGenerator.UnitTests/ConvertToGeneratedDllImportFixerTests.cs create mode 100644 DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpCodeFixVerifier.cs create mode 100644 DllImportGenerator/DllImportGenerator.Vsix/DllImportGenerator.Vsix.csproj create mode 100644 DllImportGenerator/DllImportGenerator.Vsix/source.extension.vsixmanifest create mode 100644 DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/ConvertToGeneratedDllImportFixerTests.cs b/DllImportGenerator/DllImportGenerator.UnitTests/ConvertToGeneratedDllImportFixerTests.cs new file mode 100644 index 00000000000..ddad1cc5c67 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator.UnitTests/ConvertToGeneratedDllImportFixerTests.cs @@ -0,0 +1,280 @@ +using System.Threading.Tasks; +using Xunit; +using static Microsoft.Interop.Analyzers.ConvertToGeneratedDllImportFixer; + +using VerifyCS = DllImportGenerator.UnitTests.Verifiers.CSharpCodeFixVerifier< + Microsoft.Interop.Analyzers.ConvertToGeneratedDllImportAnalyzer, + Microsoft.Interop.Analyzers.ConvertToGeneratedDllImportFixer>; + +namespace DllImportGenerator.UnitTests +{ + public class ConvertToGeneratedDllImportFixerTests + { + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Basic(bool usePreprocessorDefines) + { + string source = @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [DllImport(""DoesNotExist"")] + public static extern int [|Method|](out int ret); +}}"; + // Fixed source will have CS8795 (Partial method must have an implementation) without generator run + string fixedSource = usePreprocessorDefines + ? @$" +using System.Runtime.InteropServices; +partial class Test +{{ +#if NET + [GeneratedDllImport(""DoesNotExist"")] + public static partial int {{|CS8795:Method|}}(out int ret); +#else + [DllImport(""DoesNotExist"")] + public static extern int Method(out int ret); +#endif +}}" + : @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [GeneratedDllImport(""DoesNotExist"")] + public static partial int {{|CS8795:Method|}}(out int ret); +}}"; + await VerifyCS.VerifyCodeFixAsync( + source, + fixedSource, + usePreprocessorDefines ? WithPreprocessorDefinesKey : NoPreprocessorDefinesKey); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task Comments(bool usePreprocessorDefines) + { + string source = @$" +using System.Runtime.InteropServices; +partial class Test +{{ + // P/Invoke + [DllImport(/*name*/""DoesNotExist"")] // comment + public static extern int [|Method1|](out int ret); + + /** P/Invoke **/ + [DllImport(""DoesNotExist"") /*name*/] + // < ... > + public static extern int [|Method2|](out int ret); +}}"; + // Fixed source will have CS8795 (Partial method must have an implementation) without generator run + string fixedSource = usePreprocessorDefines + ? @$" +using System.Runtime.InteropServices; +partial class Test +{{ + // P/Invoke +#if NET + [GeneratedDllImport(/*name*/""DoesNotExist"")] // comment + public static partial int {{|CS8795:Method1|}}(out int ret); +#else + [DllImport(/*name*/""DoesNotExist"")] // comment + public static extern int Method1(out int ret); +#endif + + /** P/Invoke **/ +#if NET + [GeneratedDllImport(""DoesNotExist"") /*name*/] + // < ... > + public static partial int {{|CS8795:Method2|}}(out int ret); +#else + [DllImport(""DoesNotExist"") /*name*/] + // < ... > + public static extern int Method2(out int ret); +#endif +}}" + : @$" +using System.Runtime.InteropServices; +partial class Test +{{ + // P/Invoke + [GeneratedDllImport(/*name*/""DoesNotExist"")] // comment + public static partial int {{|CS8795:Method1|}}(out int ret); + + /** P/Invoke **/ + [GeneratedDllImport(""DoesNotExist"") /*name*/] + // < ... > + public static partial int {{|CS8795:Method2|}}(out int ret); +}}"; + await VerifyCS.VerifyCodeFixAsync( + source, + fixedSource, + usePreprocessorDefines ? WithPreprocessorDefinesKey : NoPreprocessorDefinesKey); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task MultipleAttributes(bool usePreprocessorDefines) + { + string source = @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [System.ComponentModel.Description(""Test""), DllImport(""DoesNotExist"")] + public static extern int [|Method1|](out int ret); + + [System.ComponentModel.Description(""Test"")] + [DllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.I4)] + public static extern int [|Method2|](out int ret); +}}"; + // Fixed source will have CS8795 (Partial method must have an implementation) without generator run + string fixedSource = usePreprocessorDefines + ? @$" +using System.Runtime.InteropServices; +partial class Test +{{ +#if NET + [System.ComponentModel.Description(""Test""), GeneratedDllImport(""DoesNotExist"")] + public static partial int {{|CS8795:Method1|}}(out int ret); +#else + [System.ComponentModel.Description(""Test""), DllImport(""DoesNotExist"")] + public static extern int Method1(out int ret); +#endif + +#if NET + [System.ComponentModel.Description(""Test"")] + [GeneratedDllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.I4)] + public static partial int {{|CS8795:Method2|}}(out int ret); +#else + [System.ComponentModel.Description(""Test"")] + [DllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.I4)] + public static extern int Method2(out int ret); +#endif +}}" + : @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [System.ComponentModel.Description(""Test""), GeneratedDllImport(""DoesNotExist"")] + public static partial int {{|CS8795:Method1|}}(out int ret); + + [System.ComponentModel.Description(""Test"")] + [GeneratedDllImport(""DoesNotExist"")] + [return: MarshalAs(UnmanagedType.I4)] + public static partial int {{|CS8795:Method2|}}(out int ret); +}}"; + await VerifyCS.VerifyCodeFixAsync( + source, + fixedSource, + usePreprocessorDefines ? WithPreprocessorDefinesKey : NoPreprocessorDefinesKey); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task NamedArguments(bool usePreprocessorDefines) + { + string source = @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [DllImport(""DoesNotExist"", EntryPoint = ""Entry"")] + public static extern int [|Method1|](out int ret); + + [DllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode)] + public static extern int [|Method2|](out int ret); +}}"; + // Fixed source will have CS8795 (Partial method must have an implementation) without generator run + string fixedSource = usePreprocessorDefines + ? @$" +using System.Runtime.InteropServices; +partial class Test +{{ +#if NET + [GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")] + public static partial int {{|CS8795:Method1|}}(out int ret); +#else + [DllImport(""DoesNotExist"", EntryPoint = ""Entry"")] + public static extern int Method1(out int ret); +#endif + +#if NET + [GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode)] + public static partial int {{|CS8795:Method2|}}(out int ret); +#else + [DllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode)] + public static extern int Method2(out int ret); +#endif +}}" : @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")] + public static partial int {{|CS8795:Method1|}}(out int ret); + + [GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"", CharSet = CharSet.Unicode)] + public static partial int {{|CS8795:Method2|}}(out int ret); +}}"; + await VerifyCS.VerifyCodeFixAsync( + source, + fixedSource, + usePreprocessorDefines ? WithPreprocessorDefinesKey : NoPreprocessorDefinesKey); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task RemoveableNamedArguments(bool usePreprocessorDefines) + { + string source = @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [DllImport(""DoesNotExist"", BestFitMapping = false, EntryPoint = ""Entry"")] + public static extern int [|Method1|](out int ret); + + [DllImport(""DoesNotExist"", ThrowOnUnmappableChar = false)] + public static extern int [|Method2|](out int ret); +}}"; + // Fixed source will have CS8795 (Partial method must have an implementation) without generator run + string fixedSource = usePreprocessorDefines + ? @$" +using System.Runtime.InteropServices; +partial class Test +{{ +#if NET + [GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")] + public static partial int {{|CS8795:Method1|}}(out int ret); +#else + [DllImport(""DoesNotExist"", BestFitMapping = false, EntryPoint = ""Entry"")] + public static extern int Method1(out int ret); +#endif + +#if NET + [GeneratedDllImport(""DoesNotExist"")] + public static partial int {{|CS8795:Method2|}}(out int ret); +#else + [DllImport(""DoesNotExist"", ThrowOnUnmappableChar = false)] + public static extern int Method2(out int ret); +#endif +}}" : @$" +using System.Runtime.InteropServices; +partial class Test +{{ + [GeneratedDllImport(""DoesNotExist"", EntryPoint = ""Entry"")] + public static partial int {{|CS8795:Method1|}}(out int ret); + + [GeneratedDllImport(""DoesNotExist"")] + public static partial int {{|CS8795:Method2|}}(out int ret); +}}"; + await VerifyCS.VerifyCodeFixAsync( + source, + fixedSource, + usePreprocessorDefines ? WithPreprocessorDefinesKey : NoPreprocessorDefinesKey); + } + } +} \ No newline at end of file diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/DllImportGenerator.UnitTests.csproj b/DllImportGenerator/DllImportGenerator.UnitTests/DllImportGenerator.UnitTests.csproj index 2caf2103354..c1ed94a205f 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/DllImportGenerator.UnitTests.csproj +++ b/DllImportGenerator/DllImportGenerator.UnitTests/DllImportGenerator.UnitTests.csproj @@ -11,6 +11,7 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpAnalyzerVerifier.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpAnalyzerVerifier.cs index 33cbfb88053..bb0a07677ca 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpAnalyzerVerifier.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpAnalyzerVerifier.cs @@ -1,15 +1,10 @@ -using System.Collections.Immutable; -using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Testing; using Microsoft.CodeAnalysis.CSharp.Testing.XUnit; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; -using Microsoft.CodeAnalysis.Testing.Verifiers; namespace DllImportGenerator.UnitTests.Verifiers { @@ -40,54 +35,9 @@ public static async Task VerifyAnalyzerAsync(string source, params DiagnosticRes await test.RunAsync(CancellationToken.None); } - internal class Test : CSharpAnalyzerTest - { - public Test() - { - var (refAssem, ancillary) = TestUtils.GetReferenceAssemblies(); - ReferenceAssemblies = refAssem; - SolutionTransforms.Add((solution, projectId) => - { - var project = solution.GetProject(projectId)!; - var compilationOptions = project.CompilationOptions!; - - var diagnosticOptions = compilationOptions.SpecificDiagnosticOptions.SetItems(CSharpVerifierHelper.NullableWarnings); - - // Explicitly enable diagnostics that are not enabled by default - var enableAnalyzersOptions = new System.Collections.Generic.Dictionary(); - foreach (var analyzer in GetDiagnosticAnalyzers().ToImmutableArray()) - { - foreach (var diagnostic in analyzer.SupportedDiagnostics) - { - if (diagnostic.IsEnabledByDefault) - continue; - - // Map the default severity to the reporting behaviour. - // We cannot simply use ReportDiagnostic.Default here, as diagnostics that are not enabled by default - // are treated as suppressed (regardless of their default severity). - var report = diagnostic.DefaultSeverity switch - { - DiagnosticSeverity.Error => ReportDiagnostic.Error, - DiagnosticSeverity.Warning => ReportDiagnostic.Warn, - DiagnosticSeverity.Info => ReportDiagnostic.Info, - DiagnosticSeverity.Hidden => ReportDiagnostic.Hidden, - _ => ReportDiagnostic.Default - }; - enableAnalyzersOptions.Add(diagnostic.Id, report); - } - } - - compilationOptions = compilationOptions.WithSpecificDiagnosticOptions( - compilationOptions.SpecificDiagnosticOptions - .SetItems(CSharpVerifierHelper.NullableWarnings) - .AddRange(enableAnalyzersOptions)); - solution = solution.WithProjectCompilationOptions(projectId, compilationOptions); - solution = solution.WithProjectMetadataReferences(projectId, project.MetadataReferences.Concat(ImmutableArray.Create(ancillary))); - solution = solution.WithProjectParseOptions(projectId, ((CSharpParseOptions)project.ParseOptions!).WithLanguageVersion(LanguageVersion.Preview)); - - return solution; - }); - } - } + // Code fix tests support both analyzer and code fix testing. This test class is derived from the code fix test + // to avoid the need to maintain duplicate copies of the customization work. + internal class Test : CSharpCodeFixVerifier.Test + { } } } \ No newline at end of file diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpCodeFixVerifier.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpCodeFixVerifier.cs new file mode 100644 index 00000000000..2d04e5497a1 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Verifiers/CSharpCodeFixVerifier.cs @@ -0,0 +1,119 @@ + +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Testing; +using Microsoft.CodeAnalysis.CSharp.Testing.XUnit; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Testing.Verifiers; + +namespace DllImportGenerator.UnitTests.Verifiers +{ + public static class CSharpCodeFixVerifier + where TAnalyzer : DiagnosticAnalyzer, new() + where TCodeFix : CodeFixProvider, new() + { + /// + public static DiagnosticResult Diagnostic() + => CodeFixVerifier.Diagnostic(); + + /// + public static DiagnosticResult Diagnostic(string diagnosticId) + => CodeFixVerifier.Diagnostic(diagnosticId); + + /// + public static DiagnosticResult Diagnostic(DiagnosticDescriptor descriptor) + => CodeFixVerifier.Diagnostic(descriptor); + + /// + public static async Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + { + var test = new Test + { + TestCode = source, + }; + + test.ExpectedDiagnostics.AddRange(expected); + await test.RunAsync(CancellationToken.None); + } + + /// + public static async Task VerifyCodeFixAsync(string source, string fixedSource, string? codeActionEquivalenceKey = null) + => await VerifyCodeFixAsync(source, DiagnosticResult.EmptyDiagnosticResults, fixedSource, codeActionEquivalenceKey); + + /// + public static async Task VerifyCodeFixAsync(string source, DiagnosticResult expected, string fixedSource, string? codeActionEquivalenceKey = null) + => await VerifyCodeFixAsync(source, new[] { expected }, fixedSource, codeActionEquivalenceKey); + + /// + public static async Task VerifyCodeFixAsync(string source, DiagnosticResult[] expected, string fixedSource, string? codeActionEquivalenceKey = null) + { + var test = new Test + { + TestCode = source, + FixedCode = fixedSource, + CodeActionEquivalenceKey = codeActionEquivalenceKey, + CodeActionValidationMode = CodeActionValidationMode.None, + }; + + test.ExpectedDiagnostics.AddRange(expected); + await test.RunAsync(CancellationToken.None); + } + + internal class Test : CSharpCodeFixTest + { + public Test() + { + var (refAssem, ancillary) = TestUtils.GetReferenceAssemblies(); + ReferenceAssemblies = refAssem; + SolutionTransforms.Add((solution, projectId) => + { + var project = solution.GetProject(projectId)!; + var compilationOptions = project.CompilationOptions!; + var diagnosticOptions = compilationOptions.SpecificDiagnosticOptions.SetItems(CSharpVerifierHelper.NullableWarnings); + + // Explicitly enable diagnostics that are not enabled by default + var enableAnalyzersOptions = new System.Collections.Generic.Dictionary(); + foreach (var analyzer in GetDiagnosticAnalyzers().ToImmutableArray()) + { + foreach (var diagnostic in analyzer.SupportedDiagnostics) + { + if (diagnostic.IsEnabledByDefault) + continue; + + // Map the default severity to the reporting behaviour. + // We cannot simply use ReportDiagnostic.Default here, as diagnostics that are not enabled by default + // are treated as suppressed (regardless of their default severity). + var report = diagnostic.DefaultSeverity switch + { + DiagnosticSeverity.Error => ReportDiagnostic.Error, + DiagnosticSeverity.Warning => ReportDiagnostic.Warn, + DiagnosticSeverity.Info => ReportDiagnostic.Info, + DiagnosticSeverity.Hidden => ReportDiagnostic.Hidden, + _ => ReportDiagnostic.Default + }; + enableAnalyzersOptions.Add(diagnostic.Id, report); + } + } + + compilationOptions = compilationOptions.WithSpecificDiagnosticOptions( + compilationOptions.SpecificDiagnosticOptions + .SetItems(CSharpVerifierHelper.NullableWarnings) + .AddRange(enableAnalyzersOptions)); + solution = solution.WithProjectCompilationOptions(projectId, compilationOptions); + solution = solution.WithProjectMetadataReferences(projectId, project.MetadataReferences.Concat(ImmutableArray.Create(ancillary))); + solution = solution.WithProjectParseOptions(projectId, ((CSharpParseOptions)project.ParseOptions!).WithLanguageVersion(LanguageVersion.Preview)); + return solution; + }); + } + + protected override ParseOptions CreateParseOptions() + => ((CSharpParseOptions)base.CreateParseOptions()).WithPreprocessorSymbols("NET"); + } + } +} \ No newline at end of file diff --git a/DllImportGenerator/DllImportGenerator.Vsix/DllImportGenerator.Vsix.csproj b/DllImportGenerator/DllImportGenerator.Vsix/DllImportGenerator.Vsix.csproj new file mode 100644 index 00000000000..4df7831e957 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator.Vsix/DllImportGenerator.Vsix.csproj @@ -0,0 +1,25 @@ + + + net472 + Microsoft.Interop + Microsoft.Interop.DllImportGenerator.Vsix + false + + + DllImportGenerator + + + + false + false + false + false + false + false + + + + + + + diff --git a/DllImportGenerator/DllImportGenerator.Vsix/source.extension.vsixmanifest b/DllImportGenerator/DllImportGenerator.Vsix/source.extension.vsixmanifest new file mode 100644 index 00000000000..351d8c831b6 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator.Vsix/source.extension.vsixmanifest @@ -0,0 +1,22 @@ + + + + + Microsoft.Interop.DllImportGenerator + Microsoft.Interop.DllImportGenerator + + + + + + + + + + + + + + + + diff --git a/DllImportGenerator/DllImportGenerator.sln b/DllImportGenerator/DllImportGenerator.sln index b4f91aa6897..d1ab231d1f0 100644 --- a/DllImportGenerator/DllImportGenerator.sln +++ b/DllImportGenerator/DllImportGenerator.sln @@ -27,6 +27,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "TestAssets", "TestAssets", EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Benchmarks", "Benchmarks\Benchmarks.csproj", "{0914590B-C47A-4754-A7BE-E4CD843A92E4}" EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "DllImportGenerator.Vsix", "DllImportGenerator.Vsix\DllImportGenerator.Vsix.csproj", "{F9215CDD-7B47-4C17-9166-0BE5066CA6BB}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -73,6 +75,10 @@ Global {0914590B-C47A-4754-A7BE-E4CD843A92E4}.Debug|Any CPU.Build.0 = Debug|Any CPU {0914590B-C47A-4754-A7BE-E4CD843A92E4}.Release|Any CPU.ActiveCfg = Release|Any CPU {0914590B-C47A-4754-A7BE-E4CD843A92E4}.Release|Any CPU.Build.0 = Release|Any CPU + {F9215CDD-7B47-4C17-9166-0BE5066CA6BB}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {F9215CDD-7B47-4C17-9166-0BE5066CA6BB}.Debug|Any CPU.Build.0 = Debug|Any CPU + {F9215CDD-7B47-4C17-9166-0BE5066CA6BB}.Release|Any CPU.ActiveCfg = Release|Any CPU + {F9215CDD-7B47-4C17-9166-0BE5066CA6BB}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs b/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs new file mode 100644 index 00000000000..2c4adf76f9a --- /dev/null +++ b/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs @@ -0,0 +1,224 @@ +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; + +using static Microsoft.Interop.Analyzers.AnalyzerDiagnostics; + +namespace Microsoft.Interop.Analyzers +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + public sealed class ConvertToGeneratedDllImportFixer : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(Ids.ConvertToGeneratedDllImport); + + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + public const string NoPreprocessorDefinesKey = "ConvertToGeneratedDllImport"; + public const string WithPreprocessorDefinesKey = "ConvertToGeneratedDllImportPreprocessor"; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + // Get the syntax root and semantic model + Document doc = context.Document; + SyntaxNode? root = await doc.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + SemanticModel? model = await doc.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + if (root == null || model == null) + return; + + // Nothing to do if the GeneratedDllImportAttribute is not in the compilation + INamedTypeSymbol? generatedDllImportAttrType = model.Compilation.GetTypeByMetadataName(TypeNames.GeneratedDllImportAttribute); + if (generatedDllImportAttrType == null) + return; + + INamedTypeSymbol? dllImportAttrType = model.Compilation.GetTypeByMetadataName(typeof(DllImportAttribute).FullName); + if (dllImportAttrType == null) + return; + + // Get the syntax node tied to the diagnostic and check that it is a method declaration + if (root.FindNode(context.Span) is not MethodDeclarationSyntax methodSyntax) + return; + + if (model.GetDeclaredSymbol(methodSyntax, context.CancellationToken) is not IMethodSymbol methodSymbol) + return; + + // Make sure the method has the DllImportAttribute + AttributeData? dllImportAttr; + if (!TryGetAttribute(methodSymbol, dllImportAttrType, out dllImportAttr)) + return; + + // Register code fixes with two options for the fix - using preprocessor or not. + context.RegisterCodeFix( + CodeAction.Create( + Resources.ConvertToGeneratedDllImportNoPreprocessor, + cancelToken => ConvertToGeneratedDllImport( + context.Document, + methodSyntax, + methodSymbol, + dllImportAttr!, + generatedDllImportAttrType, + usePreprocessorDefines: false, + cancelToken), + equivalenceKey: NoPreprocessorDefinesKey), + context.Diagnostics); + + context.RegisterCodeFix( + CodeAction.Create( + Resources.ConvertToGeneratedDllImportWithPreprocessor, + cancelToken => ConvertToGeneratedDllImport( + context.Document, + methodSyntax, + methodSymbol, + dllImportAttr!, + generatedDllImportAttrType, + usePreprocessorDefines: true, + cancelToken), + equivalenceKey: WithPreprocessorDefinesKey), + context.Diagnostics); + } + + private async Task ConvertToGeneratedDllImport( + Document doc, + MethodDeclarationSyntax methodSyntax, + IMethodSymbol methodSymbol, + AttributeData dllImportAttr, + INamedTypeSymbol generatedDllImportAttrType, + bool usePreprocessorDefines, + CancellationToken cancellationToken) + { + DocumentEditor editor = await DocumentEditor.CreateAsync(doc, cancellationToken).ConfigureAwait(false); + SyntaxGenerator generator = editor.Generator; + + var dllImportSyntax = (AttributeSyntax)dllImportAttr!.ApplicationSyntaxReference!.GetSyntax(cancellationToken); + + // Create GeneratedDllImport attribute based on the DllImport attribute + var generatedDllImportSyntax = GetGeneratedDllImportAttribute( + generator, + dllImportSyntax, + methodSymbol.GetDllImportData()!, + generatedDllImportAttrType); + + // Replace DllImport with GeneratedDllImport + SyntaxNode generatedDeclaration = generator.ReplaceNode(methodSyntax, dllImportSyntax, generatedDllImportSyntax); + + // Replace extern keyword with partial keyword + generatedDeclaration = generator.WithModifiers( + generatedDeclaration, + generator.GetModifiers(methodSyntax) + .WithIsExtern(false) + .WithPartial(true)); + + if (!usePreprocessorDefines) + { + // Replace the original methad with the updated one + editor.ReplaceNode(methodSyntax, generatedDeclaration); + } + else + { + // #if NET + generatedDeclaration = generatedDeclaration.WithLeadingTrivia( + generatedDeclaration.GetLeadingTrivia() + .AddRange(new[] { + SyntaxFactory.Trivia(SyntaxFactory.IfDirectiveTrivia(SyntaxFactory.IdentifierName("NET"), isActive: true, branchTaken: true, conditionValue: true)), + SyntaxFactory.ElasticMarker + })); + + // #else + generatedDeclaration = generatedDeclaration.WithTrailingTrivia( + generatedDeclaration.GetTrailingTrivia() + .AddRange(new[] { + SyntaxFactory.Trivia(SyntaxFactory.ElseDirectiveTrivia(isActive: false, branchTaken: false)), + SyntaxFactory.ElasticMarker + })); + + // Remove existing leading trivia - it will be on the GeneratedDllImport method + var updatedDeclaration = methodSyntax.WithLeadingTrivia(); + + // #endif + updatedDeclaration = updatedDeclaration.WithTrailingTrivia( + methodSyntax.GetTrailingTrivia() + .AddRange(new[] { + SyntaxFactory.Trivia(SyntaxFactory.EndIfDirectiveTrivia(isActive: true)), + SyntaxFactory.ElasticMarker + })); + + // Add the GeneratedDllImport method + editor.InsertBefore(methodSyntax, generatedDeclaration); + + // Replace the original method with the updated DllImport method + editor.ReplaceNode(methodSyntax, updatedDeclaration); + } + + return editor.GetChangedDocument(); + } + + private SyntaxNode GetGeneratedDllImportAttribute( + SyntaxGenerator generator, + AttributeSyntax dllImportSyntax, + DllImportData dllImportData, + INamedTypeSymbol generatedDllImportAttrType) + { + // Create GeneratedDllImport based on the DllImport attribute + var generatedDllImportSyntax = generator.ReplaceNode(dllImportSyntax, + dllImportSyntax.Name, + generator.TypeExpression(generatedDllImportAttrType)); + + // Update attribute arguments for GeneratedDllImport + List argumentsToRemove = new List(); + foreach (SyntaxNode argument in generator.GetAttributeArguments(generatedDllImportSyntax)) + { + if (argument is not AttributeArgumentSyntax attrArg) + continue; + + if (dllImportData.BestFitMapping != null + && !dllImportData.BestFitMapping.Value + && IsMatchingNamedArg(attrArg, nameof(DllImportAttribute.BestFitMapping))) + { + // BestFitMapping=false is explicitly set + // GeneratedDllImport does not support setting BestFitMapping. The generated code + // has the equivalent behaviour of BestFitMapping=false, so we can remove the argument. + argumentsToRemove.Add(argument); + } + else if (dllImportData.ThrowOnUnmappableCharacter != null + && !dllImportData.ThrowOnUnmappableCharacter.Value + && IsMatchingNamedArg(attrArg, nameof(DllImportAttribute.ThrowOnUnmappableChar))) + { + // ThrowOnUnmappableChar=false is explicitly set + // GeneratedDllImport does not support setting ThrowOnUnmappableChar. The generated code + // has the equivalent behaviour of ThrowOnUnmappableChar=false, so we can remove the argument. + argumentsToRemove.Add(argument); + } + } + + return generator.RemoveNodes(generatedDllImportSyntax, argumentsToRemove); + } + + private static bool TryGetAttribute(IMethodSymbol method, INamedTypeSymbol attributeType, out AttributeData? attr) + { + attr = default; + foreach (var attrLocal in method.GetAttributes()) + { + if (SymbolEqualityComparer.Default.Equals(attrLocal.AttributeClass, attributeType)) + { + attr = attrLocal; + return true; + } + } + + return false; + } + + private static bool IsMatchingNamedArg(AttributeArgumentSyntax arg, string nameToMatch) + { + return arg.NameEquals != null && arg.NameEquals.Name.Identifier.Text == nameToMatch; + } + } +} diff --git a/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj b/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj index 108713c4edd..9d92212ea0b 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj +++ b/DllImportGenerator/DllImportGenerator/DllImportGenerator.csproj @@ -29,7 +29,7 @@ - + diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index 5a033df756e..865113a77db 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -176,7 +176,8 @@ internal static string ConfigurationNotSupportedTitle { return ResourceManager.GetString("ConfigurationNotSupportedTitle", resourceCulture); } } - + + /// /// Looks up a localized string similar to Use 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time. /// internal static string ConvertToGeneratedDllImportDescription { @@ -185,6 +186,7 @@ internal static string ConvertToGeneratedDllImportDescription { } } + /// /// Looks up a localized string similar to Mark the method '{0}' with 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time. /// internal static string ConvertToGeneratedDllImportMessage { @@ -192,7 +194,16 @@ internal static string ConvertToGeneratedDllImportMessage { return ResourceManager.GetString("ConvertToGeneratedDllImportMessage", resourceCulture); } } - + + /// + /// Looks up a localized string similar to Convert to 'GeneratedDllImport'. + /// + internal static string ConvertToGeneratedDllImportNoPreprocessor { + get { + return ResourceManager.GetString("ConvertToGeneratedDllImportNoPreprocessor", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time. /// @@ -201,7 +212,16 @@ internal static string ConvertToGeneratedDllImportTitle { return ResourceManager.GetString("ConvertToGeneratedDllImportTitle", resourceCulture); } } - + + /// + /// Looks up a localized string similar to Convert to 'GeneratedDllImport' under a preprocessor define. + /// + internal static string ConvertToGeneratedDllImportWithPreprocessor { + get { + return ResourceManager.GetString("ConvertToGeneratedDllImportWithPreprocessor", resourceCulture); + } + } + /// /// Looks up a localized string similar to The specified parameter needs to be marshalled from managed to native, but the native type '{0}' does not support it.. /// @@ -210,7 +230,7 @@ internal static string CustomTypeMarshallingManagedToNativeUnsupported { return ResourceManager.GetString("CustomTypeMarshallingManagedToNativeUnsupported", resourceCulture); } } - + /// /// Looks up a localized string similar to The specified parameter needs to be marshalled from native to managed, but the native type '{0}' does not support it.. /// @@ -219,7 +239,7 @@ internal static string CustomTypeMarshallingNativeToManagedUnsupported { return ResourceManager.GetString("CustomTypeMarshallingNativeToManagedUnsupported", resourceCulture); } } - + /// /// Looks up a localized string similar to Methods marked with 'GeneratedDllImportAttribute' should be 'static' and 'partial'. P/Invoke source generation will ignore methods that are not 'static' and 'partial'.. /// @@ -300,8 +320,9 @@ internal static string InOutAttributeByRefNotSupported { return ResourceManager.GetString("InOutAttributeByRefNotSupported", resourceCulture); } } + /// - /// Looks up a localized string similar to The '[In]' and '[Out]' attributes on this parameter are unsupported on this parameter.. + /// Looks up a localized string similar to The provided '[In]' and '[Out]' attributes on this parameter are unsupported on this parameter.. /// internal static string InOutAttributeMarshalerNotSupported { get { diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 4fe08e2f600..228b5cad5c9 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -162,9 +162,15 @@ Mark the method '{0}' with 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time + + Convert to 'GeneratedDllImport' + Use 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time + + Convert to 'GeneratedDllImport' under a preprocessor define + The specified parameter needs to be marshalled from managed to native, but the native type '{0}' does not support it. diff --git a/eng/Versions.props b/eng/Versions.props index 76c8b806783..40230cac420 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -8,6 +8,8 @@ 0 0 alpha + + true true false From b6b1468530e56a20823d292c7bfaa174d26174c2 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Tue, 19 Jan 2021 16:24:34 -0800 Subject: [PATCH 2/2] Add warning annotation --- .../Analyzers/ConvertToGeneratedDllImportFixer.cs | 8 ++++++-- .../DllImportGenerator/Resources.Designer.cs | 9 +++++++++ DllImportGenerator/DllImportGenerator/Resources.resx | 4 ++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs b/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs index 2c4adf76f9a..c08c90c3216 100644 --- a/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs +++ b/DllImportGenerator/DllImportGenerator/Analyzers/ConvertToGeneratedDllImportFixer.cs @@ -105,7 +105,11 @@ private async Task ConvertToGeneratedDllImport( dllImportSyntax, methodSymbol.GetDllImportData()!, generatedDllImportAttrType); - + + // Add annotation about potential behavioural and compatibility changes + generatedDllImportSyntax = generatedDllImportSyntax.WithAdditionalAnnotations( + WarningAnnotation.Create(string.Format(Resources.ConvertToGeneratedDllImportWarning, "[TODO] Documentation link"))); + // Replace DllImport with GeneratedDllImport SyntaxNode generatedDeclaration = generator.ReplaceNode(methodSyntax, dllImportSyntax, generatedDllImportSyntax); @@ -118,7 +122,7 @@ private async Task ConvertToGeneratedDllImport( if (!usePreprocessorDefines) { - // Replace the original methad with the updated one + // Replace the original method with the updated one editor.ReplaceNode(methodSyntax, generatedDeclaration); } else diff --git a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs index 865113a77db..071ce9c6328 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.Designer.cs +++ b/DllImportGenerator/DllImportGenerator/Resources.Designer.cs @@ -213,6 +213,15 @@ internal static string ConvertToGeneratedDllImportTitle { } } + /// + /// Looks up a localized string similar to Conversion to 'GeneratedDllImport' may change behavior and compatibility. See {0} for more information.. + /// + internal static string ConvertToGeneratedDllImportWarning { + get { + return ResourceManager.GetString("ConvertToGeneratedDllImportWarning", resourceCulture); + } + } + /// /// Looks up a localized string similar to Convert to 'GeneratedDllImport' under a preprocessor define. /// diff --git a/DllImportGenerator/DllImportGenerator/Resources.resx b/DllImportGenerator/DllImportGenerator/Resources.resx index 228b5cad5c9..7f41f03a9b9 100644 --- a/DllImportGenerator/DllImportGenerator/Resources.resx +++ b/DllImportGenerator/DllImportGenerator/Resources.resx @@ -168,6 +168,10 @@ Use 'GeneratedDllImportAttribute' instead of 'DllImportAttribute' to generate P/Invoke marshalling code at compile time + + Conversion to 'GeneratedDllImport' may change behavior and compatibility. See {0} for more information. + {0} is a documentation link + Convert to 'GeneratedDllImport' under a preprocessor define