Refactor most code fixes to construct the CodeFix consistently (#10691)

dotnet · Dec 11, 2020 · b286ef4 · b286ef4
1 parent ab0a0b7
commit b286ef4
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 96 deletions.
diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddNewKeywordToDisposableConstructorInvocation.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddNewKeywordToDisposableConstructorInvocation.fs
@@ -3,32 +3,33 @@
 namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System.Composition
-open System.Collections.Immutable
-open System.Threading
 open System.Threading.Tasks
 
 open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
-open Microsoft.CodeAnalysis.CodeActions
 
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = "AddNewKeyword"); Shared>]
 type internal FSharpAddNewKeywordCodeFixProvider() =
     inherit CodeFixProvider()
 
-    override __.FixableDiagnosticIds = ImmutableArray.Create "FS0760"
+    static let fixableDiagnosticIds = set ["FS0760"]
 
-    override this.RegisterCodeFixesAsync context : Task =
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+
+    override _.RegisterCodeFixesAsync context : Task =
         async {
             let title = SR.AddNewKeyword()
-            context.RegisterCodeFix(
-                CodeAction.Create(
+            let diagnostics =
+                context.Diagnostics
+                |> Seq.filter (fun x -> fixableDiagnosticIds |> Set.contains x.Id)
+                |> Seq.toImmutableArray
+
+            let codeFix =
+                CodeFixHelpers.createTextChangeCodeFix(
                     title,
-                    (fun (cancellationToken: CancellationToken) ->
-                        async {
-                            let! cancellationToken = Async.CancellationToken
-                            let! sourceText = context.Document.GetTextAsync(cancellationToken) |> Async.AwaitTask
-                            return context.Document.WithText(sourceText.WithChanges(TextChange(TextSpan(context.Span.Start, 0), "new ")))
-                        } |> RoslynHelpers.StartAsyncAsTask(cancellationToken)),
-                    title), context.Diagnostics |> Seq.filter (fun x -> this.FixableDiagnosticIds.Contains x.Id) |> Seq.toImmutableArray)
+                    context,
+                    (fun () -> asyncMaybe.Return [| TextChange(TextSpan(context.Span.Start, 0), "new ") |]))
+
+            context.RegisterCodeFix(codeFix, diagnostics)
         } |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
 
diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddOpenCodeFixProvider.fs
@@ -33,33 +33,25 @@ type internal FSharpAddOpenCodeFixProvider
     let checker = checkerProvider.Checker
     let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__")
 
-    let qualifySymbolFix (context: CodeFixContext) (fullName, qualifier) = 
-        CodeAction.Create(
+    let qualifySymbolFix (context: CodeFixContext) (fullName, qualifier) =
+        CodeFixHelpers.createTextChangeCodeFix(
             fixUnderscoresInMenuText fullName,
-            fun (cancellationToken: CancellationToken) -> 
-                async {
-                    let! cancellationToken = Async.CancellationToken
-                    let! sourceText = context.Document.GetTextAsync(cancellationToken) |> Async.AwaitTask
-                    return context.Document.WithText(sourceText.Replace(context.Span, qualifier))
-                } |> RoslynHelpers.StartAsyncAsTask(cancellationToken))
+            context,
+            (fun () -> asyncMaybe.Return [| TextChange(context.Span, qualifier) |]))
 
     let openNamespaceFix (context: CodeFixContext) ctx name ns multipleNames = 
         let displayText = "open " + ns + if multipleNames then " (" + name + ")" else ""
-        // TODO when fresh Roslyn NuGet packages are published, assign "Namespace" Tag to this CodeAction to show proper glyph.
         CodeAction.Create(
             fixUnderscoresInMenuText displayText,
             (fun (cancellationToken: CancellationToken) -> 
                 async {
-                    let! cancellationToken = Async.CancellationToken
                     let! sourceText = context.Document.GetTextAsync(cancellationToken) |> Async.AwaitTask
                     let changedText, _ = OpenDeclarationHelper.insertOpenDeclaration sourceText ctx ns
                     return context.Document.WithText(changedText)
                 } |> RoslynHelpers.StartAsyncAsTask(cancellationToken)),
             displayText)
 
-    let getSuggestions (context: CodeFixContext) (candidates: (Entity * InsertContext) list) : unit =
-        //Logging.Logging.logInfof "Candidates: %+A" candidates
-
+    let addSuggestionsAsCodeFixes (context: CodeFixContext) (candidates: (Entity * InsertContext) list) =
         let openNamespaceFixes =
             candidates
             |> Seq.choose (fun (entity, ctx) -> entity.Namespace |> Option.map (fun ns -> ns, entity.Name, ctx))
@@ -91,9 +83,9 @@ type internal FSharpAddOpenCodeFixProvider
         for codeFix in openNamespaceFixes @ qualifiedSymbolFixes do
             context.RegisterCodeFix(codeFix, context.Diagnostics |> Seq.filter (fun x -> fixableDiagnosticIds |> List.contains x.Id) |> Seq.toImmutableArray)
 
-    override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
 
-    override __.RegisterCodeFixesAsync context : Task =
+    override _.RegisterCodeFixesAsync context : Task =
         asyncMaybe {
             let document = context.Document
             let! parsingOptions, projectOptions = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document, context.CancellationToken, userOpName)
@@ -150,7 +142,7 @@ type internal FSharpAddOpenCodeFixProvider
                 else OpenStatementInsertionPoint.Nearest
 
             let createEntity = ParsedInput.tryFindInsertionContext unresolvedIdentRange.StartLine parsedInput maybeUnresolvedIdents insertionPoint
-            return entities |> Seq.map createEntity |> Seq.concat |> Seq.toList |> getSuggestions context
+            return entities |> Seq.map createEntity |> Seq.concat |> Seq.toList |> addSuggestionsAsCodeFixes context
         } 
         |> Async.Ignore 
         |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs
@@ -15,7 +15,6 @@ module internal CodeFixHelpers =
             title,
             (fun (cancellationToken: CancellationToken) ->
                 async {
-                    let! cancellationToken = Async.CancellationToken
                     let! sourceText = context.Document.GetTextAsync(cancellationToken) |> Async.AwaitTask
                     let! changesOpt = computeTextChanges()
                     match changesOpt with

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/FixIndexerAccess.fs b/vsintegration/src/FSharp.Editor/CodeFix/FixIndexerAccess.fs
@@ -15,9 +15,9 @@ type internal FSharpFixIndexerAccessCodeFixProvider() =
     inherit CodeFixProvider()
     let fixableDiagnosticIds = set ["FS3217"]
 
-    override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
 
-    override __.RegisterCodeFixesAsync context : Task =
+    override _.RegisterCodeFixesAsync context : Task =
         async {
             let diagnostics = 
                 context.Diagnostics 

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ImplementInterfaceCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFix/ImplementInterfaceCodeFixProvider.fs
@@ -12,7 +12,6 @@ open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
 open Microsoft.CodeAnalysis.CodeActions
 
-open FSharp.Compiler
 open FSharp.Compiler.Range
 open FSharp.Compiler.SourceCodeServices
 open FSharp.Compiler.SyntaxTree

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/MissingReferenceCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFix/MissingReferenceCodeFixProvider.fs
@@ -14,8 +14,8 @@ open Microsoft.CodeAnalysis.CodeFixes
 open Microsoft.CodeAnalysis.CodeActions
 
 type private ReferenceType =
-| AddProjectRef of ProjectReference
-| AddMetadataRef of MetadataReference
+    | AddProjectRef of ProjectReference
+    | AddMetadataRef of MetadataReference
 
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = "MissingReference"); Shared>]
 type internal MissingReferenceCodeFixProvider() =

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ProposeUppercaseLabel.fs b/vsintegration/src/FSharp.Editor/CodeFix/ProposeUppercaseLabel.fs
@@ -19,9 +19,9 @@ type internal FSharpProposeUpperCaseLabelCodeFixProvider
     let fixableDiagnosticIds = ["FS0053"]
     static let userOpName = "ProposeUpperCaseLabel"
 
-    override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
 
-    override __.RegisterCodeFixesAsync context : Task =
+    override _.RegisterCodeFixesAsync context : Task =
         asyncMaybe {
             let textChanger (originalText: string) = originalText.[0].ToString().ToUpper() + originalText.Substring(1)
             let! solutionChanger, originalText = SymbolHelpers.changeAllSymbolReferences(context.Document, context.Span, textChanger, projectInfoManager, checkerProvider.Checker, userOpName)

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/RemoveUnusedOpens.fs b/vsintegration/src/FSharp.Editor/CodeFix/RemoveUnusedOpens.fs
@@ -3,13 +3,10 @@
 namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System.Composition
-open System.Threading
 open System.Threading.Tasks
 
-open Microsoft.CodeAnalysis.Diagnostics
 open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
-open Microsoft.CodeAnalysis.CodeActions
 open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics
 
 open FSharp.Compiler.Range
@@ -24,37 +21,40 @@ type internal FSharpRemoveUnusedOpensCodeFixProvider
     inherit CodeFixProvider()
     let userOpName = "FSharpRemoveUnusedOpensCodeFixProvider"
     let fixableDiagnosticIds = [FSharpIDEDiagnosticIds.RemoveUnnecessaryImportsDiagnosticId]
-
-    let createCodeFix (title: string, context: CodeFixContext) =
-        CodeAction.Create(
-            title,
-            (fun (cancellationToken: CancellationToken) ->
-                asyncMaybe {
-                    let document = context.Document
-                    let! sourceText = document.GetTextAsync()
-                    let checker = checkerProvider.Checker
-                    let! _parsingOptions, projectOptions = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document, context.CancellationToken, userOpName)
-                    let! unusedOpens = UnusedOpensDiagnosticAnalyzer.GetUnusedOpenRanges(document, projectOptions, checker)
-                    let changes =
-                        unusedOpens
-                        |> List.map (fun m ->
-                            let span = sourceText.Lines.[Line.toZ m.StartLine].SpanIncludingLineBreak
-                            TextChange(span, ""))
-                        |> List.toArray
-
-                    return document.WithText(sourceText.WithChanges(changes))
-                }
-                |> Async.map (Option.defaultValue context.Document)
-                |> RoslynHelpers.StartAsyncAsTask(cancellationToken)),
-            title)
-
-    override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
-
-    override __.RegisterCodeFixesAsync context : Task =
-        async {
-            let diagnostics = context.Diagnostics |> Seq.filter (fun x -> fixableDiagnosticIds |> List.contains x.Id) |> Seq.toImmutableArray
-            context.RegisterCodeFix(createCodeFix(SR.RemoveUnusedOpens(), context), diagnostics)
-        } |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
+
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+
+    override _.RegisterCodeFixesAsync context : Task =
+        asyncMaybe {
+            let document = context.Document
+            let! sourceText = document.GetTextAsync()
+            let checker = checkerProvider.Checker
+            let! _, projectOptions = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document, context.CancellationToken, userOpName)
+            let! unusedOpens = UnusedOpensDiagnosticAnalyzer.GetUnusedOpenRanges(document, projectOptions, checker)
+            let changes =
+                unusedOpens
+                |> List.map (fun m ->
+                    let span = sourceText.Lines.[Line.toZ m.StartLine].SpanIncludingLineBreak
+                    TextChange(span, ""))
+                |> List.toArray
+
+            let diagnostics =
+                context.Diagnostics
+                |> Seq.filter (fun x -> fixableDiagnosticIds |> List.contains x.Id)
+                |> Seq.toImmutableArray
+
+            let title = SR.RemoveUnusedOpens()
+
+            let codefix =
+                CodeFixHelpers.createTextChangeCodeFix(
+                    title,
+                    context,
+                    (fun () -> asyncMaybe.Return changes))
+
+            context.RegisterCodeFix(codefix, diagnostics)
+        }
+        |> Async.Ignore
+        |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
 
     override __.GetFixAllProvider() = WellKnownFixAllProviders.BatchFixer
 
diff --git a/vsintegration/src/FSharp.Editor/CodeFix/RenameParamToMatchSignature.fs b/vsintegration/src/FSharp.Editor/CodeFix/RenameParamToMatchSignature.fs
@@ -26,9 +26,9 @@ type internal FSharpRenameParamToMatchSignature
     let fixableDiagnosticIds = ["FS3218"]
 
 
-    override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
 
-    override __.RegisterCodeFixesAsync context : Task =
+    override _.RegisterCodeFixesAsync context : Task =
         asyncMaybe {
             match context.Diagnostics |> Seq.filter (fun x -> fixableDiagnosticIds |> List.contains x.Id) |> Seq.toList with 
             | [diagnostic] -> 

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/RenameUnusedValue.fs b/vsintegration/src/FSharp.Editor/CodeFix/RenameUnusedValue.fs
@@ -25,23 +25,8 @@ type internal FSharpRenameUnusedValueCodeFixProvider
 
     inherit CodeFixProvider()
     static let userOpName = "RenameUnusedValueCodeFix"
-    let fixableDiagnosticIds = ["FS1182"]
+    let fixableDiagnosticIds = set ["FS1182"]
     let checker = checkerProvider.Checker
-
-    let createCodeFix (context: CodeFixContext, symbolName: string, titleFormat: string, textChange: TextChange) =
-        let title = String.Format(titleFormat, symbolName)
-        let codeAction =
-            CodeAction.Create(
-                title,
-                (fun (cancellationToken: CancellationToken) ->
-                    async {
-                        let! cancellationToken = Async.CancellationToken
-                        let! sourceText = context.Document.GetTextAsync(cancellationToken) |> Async.AwaitTask
-                        return context.Document.WithText(sourceText.WithChanges(textChange))
-                    } |> RoslynHelpers.StartAsyncAsTask(cancellationToken)),
-                title)
-        let diagnostics = context.Diagnostics |> Seq.filter (fun x -> fixableDiagnosticIds |> List.contains x.Id) |> Seq.toImmutableArray
-        context.RegisterCodeFix(codeAction, diagnostics)
 
     override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
 
@@ -66,10 +51,29 @@ type internal FSharpRenameUnusedValueCodeFixProvider
                 let! symbolUse = checkResults.GetSymbolUseAtLocation(m.StartLine, m.EndColumn, lineText, lexerSymbol.FullIsland)
                 let symbolName = symbolUse.Symbol.DisplayName
 
+                let diagnostics =
+                    context.Diagnostics
+                    |> Seq.filter (fun x -> fixableDiagnosticIds |> Set.contains x.Id)
+                    |> Seq.toImmutableArray
+
                 match symbolUse.Symbol with
                 | :? FSharpMemberOrFunctionOrValue as func ->
-                    createCodeFix(context, symbolName, SR.PrefixValueNameWithUnderscore(), TextChange(TextSpan(context.Span.Start, 0), "_"))
-                    if func.IsValue then createCodeFix(context, symbolName, SR.RenameValueToUnderscore(), TextChange(context.Span, "_"))
+                    let prefixTitle = String.Format(SR.PrefixValueNameWithUnderscore(), symbolName)
+                    let prefixCodeFix =
+                        CodeFixHelpers.createTextChangeCodeFix(
+                            prefixTitle,
+                            context,
+                            (fun () -> asyncMaybe.Return [| TextChange(TextSpan(context.Span.Start, 0), "_") |]))
+                    context.RegisterCodeFix(prefixCodeFix, diagnostics)
+
+                    if func.IsValue then
+                        let replaceTitle = String.Format(SR.RenameValueToUnderscore(), symbolName)
+                        let replaceCodeFix =
+                            CodeFixHelpers.createTextChangeCodeFix(
+                                replaceTitle,
+                                context,
+                                (fun () -> asyncMaybe.Return [| TextChange(context.Span, "_") |]))
+                        context.RegisterCodeFix(replaceCodeFix, diagnostics)
                 | _ -> ()
         } 
         |> Async.Ignore

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ReplaceWithSuggestion.fs b/vsintegration/src/FSharp.Editor/CodeFix/ReplaceWithSuggestion.fs
@@ -26,9 +26,9 @@ type internal FSharpReplaceWithSuggestionCodeFixProvider
     let fixableDiagnosticIds = set ["FS0039"; "FS1129"; "FS0495"]
     let checker = checkerProvider.Checker
 
-    override __.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
 
-    override __.RegisterCodeFixesAsync context : Task =
+    override _.RegisterCodeFixesAsync context : Task =
         asyncMaybe {
             do! Option.guard settings.CodeFixes.SuggestNamesForErrors
 

diff --git a/vsintegration/src/FSharp.Editor/CodeFix/SimplifyName.fs b/vsintegration/src/FSharp.Editor/CodeFix/SimplifyName.fs
@@ -15,9 +15,9 @@ type internal FSharpSimplifyNameCodeFixProvider() =
     inherit CodeFixProvider()
     let fixableDiagnosticId = FSharpIDEDiagnosticIds.SimplifyNamesDiagnosticId
 
-    override __.FixableDiagnosticIds = ImmutableArray.Create(fixableDiagnosticId)
+    override _.FixableDiagnosticIds = ImmutableArray.Create(fixableDiagnosticId)
 
-    override __.RegisterCodeFixesAsync(context: CodeFixContext) : Task =
+    override _.RegisterCodeFixesAsync(context: CodeFixContext) : Task =
        async {
            for diagnostic in context.Diagnostics |> Seq.filter (fun x -> x.Id = fixableDiagnosticId) do
                let title =