Only apply StringMarshalling to GeneratedComInterface in fixer when n…

…eeded even in fix-all (dotnet#105573)
BrzVlad · Jul 26, 2024 · ee8426c · ee8426c
1 parent 007b9ff
commit ee8426c
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 2 deletions.
diff --git a/....Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/AddGeneratedComClassFixer.cs b/....Runtime.InteropServices/gen/ComInterfaceGenerator/Analyzers/AddGeneratedComClassFixer.cs
@@ -58,5 +58,7 @@ protected override ImmutableDictionary<string, Option> ParseOptionsFromDiagnosti
         {
             return ImmutableDictionary<string, Option>.Empty;
         }
+
+        protected override ImmutableDictionary<string, Option> CombineOptions(ImmutableDictionary<string, Option> fixAllOptions, ImmutableDictionary<string, Option> diagnosticOptions) => fixAllOptions;
     }
 }
diff --git a/...vices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceFixer.cs b/...vices/gen/ComInterfaceGenerator/Analyzers/ConvertComImportToGeneratedComInterfaceFixer.cs
@@ -55,6 +55,20 @@ protected override ImmutableDictionary<string, Option> ParseOptionsFromDiagnosti
             return optionsBuilder.ToImmutable();
         }
 
+        protected override ImmutableDictionary<string, Option> CombineOptions(ImmutableDictionary<string, Option> fixAllOptions, ImmutableDictionary<string, Option> diagnosticOptions)
+        {
+            ImmutableDictionary<string, Option> 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/...braries/System.Runtime.InteropServices/gen/Common/ConvertToSourceGeneratedInteropFixer.cs b/...braries/System.Runtime.InteropServices/gen/Common/ConvertToSourceGeneratedInteropFixer.cs
@@ -96,6 +96,13 @@ public static string CreateEquivalenceKeyFromOptions(string baseEquivalenceKey,
 
         protected abstract ImmutableDictionary<string, Option> ParseOptionsFromDiagnostic(Diagnostic diagnostic);
 
+        protected abstract ImmutableDictionary<string, Option> CombineOptions(ImmutableDictionary<string, Option> fixAllOptions, ImmutableDictionary<string, Option> diagnosticOptions);
+
+        private ImmutableDictionary<string, Option> GetOptionsForIndividualFix(ImmutableDictionary<string, Option> fixAllOptions, Diagnostic diagnostic)
+        {
+            return CombineOptions(fixAllOptions, ParseOptionsFromDiagnostic(diagnostic));
+        }
+
         private static async Task<Solution> ApplyActionAndEnableUnsafe(Solution solution, DocumentId documentId, Func<DocumentEditor, CancellationToken, Task> 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/...ntime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs b/...ntime.InteropServices/gen/LibraryImportGenerator/Analyzers/ConvertToLibraryImportFixer.cs
@@ -68,6 +68,11 @@ protected override ImmutableDictionary<string, Option> ParseOptionsFromDiagnosti
             return optionsBuilder.ToImmutable();
         }
 
+        protected override ImmutableDictionary<string, Option> CombineOptions(ImmutableDictionary<string, Option> fixAllOptions, ImmutableDictionary<string, Option> diagnosticOptions)
+        {
+            return fixAllOptions;
+        }
+
         protected override IEnumerable<ConvertToSourceGeneratedInteropFix> CreateAllFixesForDiagnosticOptions(SyntaxNode node, ImmutableDictionary<string, Option> options)
         {
             bool warnForAdditionalWork = options.TryGetValue(Option.MayRequireAdditionalWork, out Option mayRequireAdditionalWork) && mayRequireAdditionalWork is Option.Bool(true);

diff --git a/...ropServices/tests/ComInterfaceGenerator.Unit.Tests/ConvertToGeneratedComInterfaceTests.cs b/...ropServices/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()
         {