From ee8426cc76eafc8608677359702a9d1913c810f2 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 26 Jul 2024 14:36:51 -0700 Subject: [PATCH] Only apply StringMarshalling to GeneratedComInterface in fixer when needed even in fix-all (#105573) --- .../Analyzers/AddGeneratedComClassFixer.cs | 2 + ...rtComImportToGeneratedComInterfaceFixer.cs | 15 +++++- .../ConvertToSourceGeneratedInteropFixer.cs | 9 +++- .../Analyzers/ConvertToLibraryImportFixer.cs | 5 ++ .../ConvertToGeneratedComInterfaceTests.cs | 48 +++++++++++++++++++ 5 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/AddGeneratedComClassFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/AddGeneratedComClassFixer.cs index 716b18ca69146..f61e6c61a1279 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/AddGeneratedComClassFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/AddGeneratedComClassFixer.cs @@ -58,5 +58,7 @@ protected override ImmutableDictionary ParseOptionsFromDiagnosti { return ImmutableDictionary.Empty; } + + protected override ImmutableDictionary CombineOptions(ImmutableDictionary fixAllOptions, ImmutableDictionary diagnosticOptions) => fixAllOptions; } } diff --git a/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceFixer.cs index 8db74fa70244c..f0a06e7fad561 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceFixer.cs @@ -55,6 +55,20 @@ protected override ImmutableDictionary ParseOptionsFromDiagnosti return optionsBuilder.ToImmutable(); } + protected override ImmutableDictionary CombineOptions(ImmutableDictionary fixAllOptions, ImmutableDictionary diagnosticOptions) + { + ImmutableDictionary combinedOptions = fixAllOptions; + if (fixAllOptions.TryGetValue(AddStringMarshallingOption, out Option fixAllAddStringMarshallingOption) + && fixAllAddStringMarshallingOption is Option.Bool(true) + && (!diagnosticOptions.TryGetValue(AddStringMarshallingOption, out Option addStringMarshallingOption) + || addStringMarshallingOption is Option.Bool(false))) + { + combinedOptions = combinedOptions.Remove(AddStringMarshallingOption); + } + + return combinedOptions; + } + private static async Task ConvertComImportToGeneratedComInterfaceAsync(DocumentEditor editor, SyntaxNode node, bool mayRequireAdditionalWork, bool addStringMarshalling, CancellationToken ct) { var gen = editor.Generator; @@ -125,6 +139,5 @@ private static async Task ConvertComImportToGeneratedComInterfaceAsync(DocumentE MakeNodeParentsPartial(editor, node); } - } } diff --git a/src/libraries/System.Runtime.InteropServices/gen/Common/ConvertToSourceGeneratedInteropFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/Common/ConvertToSourceGeneratedInteropFixer.cs index ec77b75f7dc5c..dc2cef27f3b92 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Common/ConvertToSourceGeneratedInteropFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Common/ConvertToSourceGeneratedInteropFixer.cs @@ -96,6 +96,13 @@ public static string CreateEquivalenceKeyFromOptions(string baseEquivalenceKey, protected abstract ImmutableDictionary ParseOptionsFromDiagnostic(Diagnostic diagnostic); + protected abstract ImmutableDictionary CombineOptions(ImmutableDictionary fixAllOptions, ImmutableDictionary diagnosticOptions); + + private ImmutableDictionary GetOptionsForIndividualFix(ImmutableDictionary fixAllOptions, Diagnostic diagnostic) + { + return CombineOptions(fixAllOptions, ParseOptionsFromDiagnostic(diagnostic)); + } + private static async Task ApplyActionAndEnableUnsafe(Solution solution, DocumentId documentId, Func documentBasedFix, CancellationToken ct) { var editor = new SolutionEditor(solution); @@ -194,7 +201,7 @@ private sealed class CustomFixAllProvider : FixAllProvider SyntaxNode node = root.FindNode(diagnostic.Location.SourceSpan); - var documentBasedFix = codeFixProvider.CreateFixForSelectedOptions(node, options); + var documentBasedFix = codeFixProvider.CreateFixForSelectedOptions(node, codeFixProvider.GetOptionsForIndividualFix(options, diagnostic)); await documentBasedFix(editor, ct).ConfigureAwait(false); diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs index 4cfef787526fd..b83eb5803abff 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs @@ -68,6 +68,11 @@ protected override ImmutableDictionary ParseOptionsFromDiagnosti return optionsBuilder.ToImmutable(); } + protected override ImmutableDictionary CombineOptions(ImmutableDictionary fixAllOptions, ImmutableDictionary diagnosticOptions) + { + return fixAllOptions; + } + protected override IEnumerable CreateAllFixesForDiagnosticOptions(SyntaxNode node, ImmutableDictionary options) { bool warnForAdditionalWork = options.TryGetValue(Option.MayRequireAdditionalWork, out Option mayRequireAdditionalWork) && mayRequireAdditionalWork is Option.Bool(true); diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ConvertToGeneratedComInterfaceTests.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ConvertToGeneratedComInterfaceTests.cs index 7260a4d4d392c..e224f16b87d1a 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ConvertToGeneratedComInterfaceTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/ConvertToGeneratedComInterfaceTests.cs @@ -134,6 +134,54 @@ public partial interface I await VerifyCS.VerifyCodeFixAsync(source, fixedSource); } + [Fact] + public async Task StringFix_DoesNotAddStringMarshallingToNonStringScenarios() + { + string source = """ + using System.Runtime.InteropServices; + using System.Runtime.InteropServices.Marshalling; + + [ComImport] + [Guid("5DA39CDF-DCAD-447A-836E-EA80DB34D81B")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + public interface [|IUsesString|] + { + string Method(); + } + + [ComImport] + [Guid("5DA39CDF-DCAD-447A-836E-EA80DB34D81C")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + public interface [|INoString|] + { + int Method(); + } + """; + + string fixedSource = """ + using System.Runtime.InteropServices; + using System.Runtime.InteropServices.Marshalling; + + [GeneratedComInterface(StringMarshalling = StringMarshalling.Custom, StringMarshallingCustomType = typeof(BStrStringMarshaller))] + [Guid("5DA39CDF-DCAD-447A-836E-EA80DB34D81B")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + public partial interface IUsesString + { + string Method(); + } + + [GeneratedComInterface] + [Guid("5DA39CDF-DCAD-447A-836E-EA80DB34D81C")] + [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] + public partial interface INoString + { + int Method(); + } + """; + + await VerifyCS.VerifyCodeFixAsync(source, fixedSource); + } + [Fact] public async Task Array_DoesNotReportDiagnostic() {