Democratizing code fixes - part one

vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs

@@ -9,17 +9,24 @@ open System.Collections.Immutable
 open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
 
+open CancellableTasks
+
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.AddInstanceMemberParameter); Shared>]
 type internal FSharpAddInstanceMemberParameterCodeFixProvider() =
     inherit CodeFixProvider()
 
     static let title = SR.AddMissingInstanceMemberParameter()
 
+    interface IFSharpCodeFix with
+        member _.GetChangesAsync _ span =
+            let changes = [ TextChange(TextSpan(span.Start, 0), "x.") ]
+            CancellableTask.singleton (title, changes)
+
     override _.FixableDiagnosticIds = ImmutableArray.Create("FS0673")
 
-    override _.RegisterCodeFixesAsync context : Task =
-        asyncMaybe {
-            do context.RegisterFsharpFix(CodeFix.AddInstanceMemberParameter, title, [| TextChange(TextSpan(context.Span.Start, 0), "x.") |])
+    override this.RegisterCodeFixesAsync context : Task =
+        cancellableTask {
+            let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span
+            context.RegisterFsharpFix(CodeFix.AddInstanceMemberParameter, title, changes)
         }
-        |> Async.Ignore
-        |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
+        |> CancellableTask.startAsTask context.CancellationToken

vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs

@@ -9,40 +9,53 @@ open System.Collections.Immutable
 open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
 
+open CancellableTasks
+
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.ChangeToUpcast); Shared>]
 type internal FSharpChangeToUpcastCodeFixProvider() =
     inherit CodeFixProvider()
 
     override _.FixableDiagnosticIds = ImmutableArray.Create("FS3198")
 
+    interface IFSharpCodeFix with
+        member _.GetChangesAsync document span =
+            cancellableTask {
+                let! cancellationToken = CancellableTask.getCurrentCancellationToken ()
+
+                let! sourceText = document.GetTextAsync(cancellationToken)
+                let text = sourceText.GetSubText(span).ToString()
+
+                // Only works if it's one or the other
+                let isDowncastOperator = text.Contains(":?>")
+                let isDowncastKeyword = text.Contains("downcast")
+
+                let changes =
+                    [
+                        if
+                            (isDowncastOperator || isDowncastKeyword)
+                            && not (isDowncastOperator && isDowncastKeyword)
+                        then
+                            let replacement =
+                                if isDowncastOperator then
+                                    text.Replace(":?>", ":>")
+                                else
+                                    text.Replace("downcast", "upcast")
+
+                            TextChange(span, replacement)
+                    ]
+
+                let title =
+                    if isDowncastOperator then
+                        SR.UseUpcastOperator()
+                    else
+                        SR.UseUpcastKeyword()
+
+                return title, changes
+            }
+
     override this.RegisterCodeFixesAsync context : Task =
-        asyncMaybe {
-            let! sourceText = context.Document.GetTextAsync(context.CancellationToken)
-            let text = sourceText.GetSubText(context.Span).ToString()
-
-            // Only works if it's one or the other
-            let isDowncastOperator = text.Contains(":?>")
-            let isDowncastKeyword = text.Contains("downcast")
-
-            do!
-                Option.guard (
-                    (isDowncastOperator || isDowncastKeyword)
-                    && not (isDowncastOperator && isDowncastKeyword)
-                )
-
-            let replacement =
-                if isDowncastOperator then
-                    text.Replace(":?>", ":>")
-                else
-                    text.Replace("downcast", "upcast")
-
-            let title =
-                if isDowncastOperator then
-                    SR.UseUpcastOperator()
-                else
-                    SR.UseUpcastKeyword()
-
-            do context.RegisterFsharpFix(CodeFix.ChangeToUpcast, title, [| TextChange(context.Span, replacement) |])
+        cancellableTask {
+            let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span
+            context.RegisterFsharpFix(CodeFix.ChangeToUpcast, title, changes)
         }
-        |> Async.Ignore
-        |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
+        |> CancellableTask.startAsTask context.CancellationToken

vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs

@@ -3,13 +3,13 @@
 namespace Microsoft.VisualStudio.FSharp.Editor
 
 open System.Composition
-open System.Threading
 open System.Threading.Tasks
 open System.Collections.Immutable
 
 open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.CodeFixes
-open Microsoft.CodeAnalysis.CodeActions
+
+open CancellableTasks
 
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.ConvertToAnonymousRecord); Shared>]
 type internal FSharpConvertToAnonymousRecordCodeFixProvider [<ImportingConstructor>] () =
@@ -19,29 +19,34 @@ type internal FSharpConvertToAnonymousRecordCodeFixProvider [<ImportingConstruct
 
     override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039")
 
-    override _.RegisterCodeFixesAsync context : Task =
-        asyncMaybe {
-            let document = context.Document
+    interface IFSharpCodeFix with
+        member _.GetChangesAsync document span =
+            cancellableTask {
+                let! cancellationToken = CancellableTask.getCurrentCancellationToken ()
 
-            let! parseResults =
-                document.GetFSharpParseResultsAsync(nameof (FSharpConvertToAnonymousRecordCodeFixProvider))
-                |> liftAsync
+                let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpConvertToAnonymousRecordCodeFixProvider))
 
-            let! sourceText = context.Document.GetTextAsync(context.CancellationToken)
+                let! sourceText = document.GetTextAsync(cancellationToken)
 
-            let errorRange =
-                RoslynHelpers.TextSpanToFSharpRange(document.FilePath, context.Span, sourceText)
+                let errorRange =
+                    RoslynHelpers.TextSpanToFSharpRange(document.FilePath, span, sourceText)
 
-            let! recordRange = parseResults.TryRangeOfRecordExpressionContainingPos errorRange.Start
-            let! recordSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, recordRange)
+                let changes =
+                    parseResults.TryRangeOfRecordExpressionContainingPos errorRange.Start
+                    |> Option.bind (fun range -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range))
+                    |> Option.map (fun span ->
+                        [
+                            TextChange(TextSpan(span.Start + 1, 0), "|")
+                            TextChange(TextSpan(span.End - 1, 0), "|")
+                        ])
+                    |> Option.defaultValue []
 
-            let changes =
-                [
-                    TextChange(TextSpan(recordSpan.Start + 1, 0), "|")
-                    TextChange(TextSpan(recordSpan.End, 0), "|")
-                ]
+                return title, changes
+            }
 
-            context.RegisterFsharpFix(CodeFix.ConvertToAnonymousRecord, title, changes)
+    override this.RegisterCodeFixesAsync context : Task =
+        cancellableTask {
+            let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span
+            return context.RegisterFsharpFix(CodeFix.ConvertToAnonymousRecord, title, changes)
         }
-        |> Async.Ignore
-        |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
+        |> CancellableTask.startAsTask context.CancellationToken

vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs

@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace Microsoft.VisualStudio.FSharp.Editor
+
+open Microsoft.CodeAnalysis
+open Microsoft.CodeAnalysis.Text
+
+open CancellableTasks
+
+type IFSharpCodeFix =
+    abstract member GetChangesAsync: document: Document -> span: TextSpan -> CancellableTask<string * TextChange list>

vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj

@@ -101,6 +101,7 @@
     <Compile Include="Refactor\ChangeTypeofWithNameToNameofExpression.fs" />
     <Compile Include="Refactor\AddExplicitTypeToParameter.fs" />
     <Compile Include="Refactor\ChangeDerefToValueRefactoring.fs" />
+    <Compile Include="CodeFix\IFSharpCodeFix.fs" />
     <Compile Include="CodeFix\CodeFixHelpers.fs" />
     <Compile Include="CodeFix\AddInstanceMemberParameter.fs" />
     <Compile Include="CodeFix\AddTypeAnnotationToObjectOfIndeterminateType.fs" />

vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+module FSharp.Editor.Tests.CodeFixes.AddInstanceMemberParameterTests
+
+open Microsoft.VisualStudio.FSharp.Editor
+open Xunit
+
+open CodeFixTestFramework
+
+let private codeFix = FSharpAddInstanceMemberParameterCodeFixProvider()
+let private diagnostic = 0673 // This instance member needs a parameter to represent the object being invoked...
+
+[<Fact>]
+let ``Fixes FS0673`` () =
+    let code =
+        """
+type UsefulTestHarness() =
+    member FortyTwo = 42
+"""
+
+    let expected =
+        {
+            Title = "Add missing instance member parameter"
+            FixedCode =
+                """
+type UsefulTestHarness() =
+    member x.FortyTwo = 42
+"""
+        }
+
+    let actual = codeFix |> fix code diagnostic
+
+    Assert.Equal(expected, actual)

vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs

@@ -0,0 +1,66 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+module FSharp.Editor.Tests.CodeFixes.ChangeToUpcastTests
+
+open Microsoft.VisualStudio.FSharp.Editor
+open Xunit
+
+open CodeFixTestFramework
+
+let private codeFix = FSharpChangeToUpcastCodeFixProvider()
+let private diagnostic = 3198 // The conversion is an upcast, not a downcast...
+
+// Test cases are taken from the original PR:
+// https://github.com/dotnet/fsharp/pull/10463
+
+[<Fact>]
+let ``Fixes FS3198 - operator`` () =
+    let code =
+        """
+type IFoo = abstract member Bar : unit -> unit
+type Foo() = interface IFoo with member __.Bar () = ()
+
+let Thing : IFoo = Foo() :?> IFoo
+"""
+
+    let expected =
+        {
+            Title = "Use ':>' operator"
+            FixedCode =
+                """
+type IFoo = abstract member Bar : unit -> unit
+type Foo() = interface IFoo with member __.Bar () = ()
+
+let Thing : IFoo = Foo() :> IFoo
+"""
+        }
+
+    let actual = codeFix |> fix code diagnostic
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS3198 - keyword`` () =
+    let code =
+        """
+type IFoo = abstract member Bar : unit -> unit
+type Foo() = interface IFoo with member __.Bar () = ()
+
+let Thing : IFoo = downcast Foo()
+"""
+
+    let expected =
+        {
+            Title = "Use 'upcast'"
+            FixedCode =
+                """
+type IFoo = abstract member Bar : unit -> unit
+type Foo() = interface IFoo with member __.Bar () = ()
+
+let Thing : IFoo = upcast Foo()
+"""
+        }
+
+    let actual = codeFix |> fix code diagnostic
+
+    Assert.Equal(expected, actual)

vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs

@@ -0,0 +1,46 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+module FSharp.Editor.Tests.CodeFixes.CodeFixTestFramework
+
+open System.Threading
+
+open Microsoft.CodeAnalysis
+open Microsoft.CodeAnalysis.Text
+open Microsoft.VisualStudio.FSharp.Editor
+open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks
+
+open FSharp.Editor.Tests.Helpers
+
+type TestCodeFix = { Title: string; FixedCode: string }
+
+let getRelevantDiagnostic (document: Document) errorNumber =
+    cancellableTask {
+        let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync "test"
+
+        return
+            checkFileResults.Diagnostics
+            |> Seq.where (fun d -> d.ErrorNumber = errorNumber)
+            |> Seq.exactlyOne
+    }
+
+let fix (code: string) diagnostic (fixProvider: IFSharpCodeFix) =
+    cancellableTask {
+        let sourceText = SourceText.From code
+        let document = RoslynTestHelpers.GetFsDocument code
+
+        let! diagnostic = getRelevantDiagnostic document diagnostic
+
+        let diagnosticSpan =
+            RoslynHelpers.FSharpRangeToTextSpan(sourceText, diagnostic.Range)
+
+        let! title, changes = fixProvider.GetChangesAsync document diagnosticSpan
+        let fixedSourceText = sourceText.WithChanges changes
+
+        return
+            {
+                Title = title
+                FixedCode = fixedSourceText.ToString()
+            }
+    }
+    |> CancellableTask.start CancellationToken.None
+    |> fun task -> task.Result

vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs

@@ -0,0 +1,31 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+module FSharp.Editor.Tests.CodeFixes.ConvertToAnonymousRecordTests
+
+open Microsoft.VisualStudio.FSharp.Editor
+open Xunit
+
+open CodeFixTestFramework
+
+let private codeFix = FSharpConvertToAnonymousRecordCodeFixProvider()
+let private diagnostic = 0039 // The record label is not defined...
+
+[<Fact>]
+let ``Fixes FS0039`` () =
+    let code =
+        """
+let band = { Name = "The Velvet Underground" }
+"""
+
+    let expected =
+        {
+            Title = "Convert to Anonymous Record"
+            FixedCode =
+                """
+let band = {| Name = "The Velvet Underground" |}
+"""
+        }
+
+    let actual = codeFix |> fix code diagnostic
+
+    Assert.Equal(expected, actual)