Improving ReplaceWithSuggestion code fix

vsintegration/src/FSharp.Editor/CodeFixes/ReplaceWithSuggestion.fs

@@ -13,54 +13,59 @@ open FSharp.Compiler.Diagnostics
 open FSharp.Compiler.EditorServices
 open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
+
 open CancellableTasks
 
 [<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.ReplaceWithSuggestion); Shared>]
-type internal ReplaceWithSuggestionCodeFixProvider [<ImportingConstructor>] (settings: EditorOptions) =
+type internal ReplaceWithSuggestionCodeFixProvider [<ImportingConstructor>] () =
     inherit CodeFixProvider()
 
-    override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039", "FS1129", "FS0495")
-
-    override _.RegisterCodeFixesAsync context : Task =
-        asyncMaybe {
-            do! Option.guard settings.CodeFixes.SuggestNamesForErrors
-
-            let document = context.Document
-
-            let! parseFileResults, checkFileResults =
-                document.GetFSharpParseAndCheckResultsAsync(nameof (ReplaceWithSuggestionCodeFixProvider))
-                |> CancellableTask.start context.CancellationToken
-                |> Async.AwaitTask
-                |> liftAsync
-
-            // This is all needed to get a declaration list
-            let! sourceText = document.GetTextAsync(context.CancellationToken)
-            let unresolvedIdentifierText = sourceText.GetSubText(context.Span).ToString()
-            let pos = context.Span.End
-            let caretLinePos = sourceText.Lines.GetLinePosition(pos)
-            let caretLine = sourceText.Lines.GetLineFromPosition(pos)
-            let fcsCaretLineNumber = Line.fromZ caretLinePos.Line
-            let lineText = caretLine.ToString()
-
-            let partialName =
-                QuickParse.GetPartialLongNameEx(lineText, caretLinePos.Character - 1)
-
-            let declInfo =
-                checkFileResults.GetDeclarationListInfo(Some parseFileResults, fcsCaretLineNumber, lineText, partialName)
-
-            let addNames (addToBuffer: string -> unit) =
-                for item in declInfo.Items do
-                    addToBuffer item.NameInList
-
-            for suggestion in CompilerDiagnostics.GetSuggestedNames addNames unresolvedIdentifierText do
-                let replacement = PrettyNaming.NormalizeIdentifierBackticks suggestion
-
-                do
-                    context.RegisterFsharpFix(
-                        CodeFix.ReplaceWithSuggestion,
-                        CompilerDiagnostics.GetErrorMessage(FSharpDiagnosticKind.ReplaceWithSuggestion suggestion),
-                        [| TextChange(context.Span, replacement) |]
-                    )
-        }
-        |> Async.Ignore
-        |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
+    override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039", "FS0495")
+
+    override this.RegisterCodeFixesAsync context =
+        if context.Document.Project.IsFSharpCodeFixesSuggestNamesForErrorsEnabled then
+            context.RegisterFsharpFix this
+        else
+            Task.CompletedTask
+
+    interface IFSharpCodeFixProvider with
+        member _.GetCodeFixIfAppliesAsync context =
+            cancellableTask {
+                let! parseFileResults, checkFileResults =
+                    context.Document.GetFSharpParseAndCheckResultsAsync(nameof ReplaceWithSuggestionCodeFixProvider)
+
+                let! sourceText = context.GetSourceTextAsync()
+                let! unresolvedIdentifierText = context.GetSquigglyTextAsync()
+                let pos = context.Span.End
+                let caretLinePos = sourceText.Lines.GetLinePosition(pos)
+                let caretLine = sourceText.Lines.GetLineFromPosition(pos)
+                let fcsCaretLineNumber = Line.fromZ caretLinePos.Line
+                let lineText = caretLine.ToString()
+
+                let partialName =
+                    QuickParse.GetPartialLongNameEx(lineText, caretLinePos.Character - 1)
+
+                let declInfo =
+                    checkFileResults.GetDeclarationListInfo(Some parseFileResults, fcsCaretLineNumber, lineText, partialName)
+
+                let addNames addToBuffer =
+                    for item in declInfo.Items do
+                        addToBuffer item.NameInList
+
+                let suggestionOpt =
+                    CompilerDiagnostics.GetSuggestedNames addNames unresolvedIdentifierText
+                    |> Seq.tryHead
+
+                match suggestionOpt with
+                | None -> return ValueNone
+                | Some suggestion ->
+                    let replacement = PrettyNaming.NormalizeIdentifierBackticks suggestion
+
+                    return
+                        ValueSome
+                            {
+                                Name = CodeFix.ReplaceWithSuggestion
+                                Message = CompilerDiagnostics.GetErrorMessage(FSharpDiagnosticKind.ReplaceWithSuggestion suggestion)
+                                Changes = [ TextChange(context.Span, replacement) ]
+                            }
+            }

vsintegration/src/FSharp.Editor/Options/EditorOptions.fs

@@ -250,6 +250,9 @@ module EditorOptionsExtensions =
         member this.IsFSharpCodeFixesUnusedOpensEnabled =
             this.EditorOptions.CodeFixes.UnusedOpens
 
+        member this.IsFSharpCodeFixesSuggestNamesForErrorsEnabled =
+            this.EditorOptions.CodeFixes.SuggestNamesForErrors
+
         member this.IsFSharpBlockStructureEnabled =
             this.EditorOptions.Advanced.IsBlockStructureEnabled
 

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

@@ -0,0 +1,117 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+module FSharp.Editor.Tests.CodeFixes.ReplaceWithSuggestionTests
+
+open Microsoft.VisualStudio.FSharp.Editor
+open Xunit
+
+open CodeFixTestFramework
+
+let private codeFix = ReplaceWithSuggestionCodeFixProvider()
+
+[<Fact>]
+let ``Fixes FS0039 for mistyped record field names`` () =
+    let code =
+        """
+type Song = { Title : string }
+
+let song = { Titel = "Jigsaw Falling Into Place" }
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Replace with 'Title'"
+                FixedCode =
+                    """
+type Song = { Title : string }
+
+let song = { Title = "Jigsaw Falling Into Place" }
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0039 for mistyped type names`` () =
+    let code =
+        """
+type Song = { Title : string }
+
+let someSong : Wrong = { Title = "The Narcissist" }
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Replace with 'Song'"
+                FixedCode =
+                    """
+type Song = { Title : string }
+
+let someSong : Song = { Title = "The Narcissist" }
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Doesn't fix FS0039 for out of scope stuff`` () =
+    let code =
+        """
+module Module1 =
+    type Song = { Title : string }
+
+module Module2 = 
+    let song = { Titel = "Jigsaw Falling Into Place" }
+"""
+
+    let expected = None
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Doesn't fix FS0039 for random undefined stuff`` () =
+    let code =
+        """
+let f = g
+"""
+
+    let expected = None
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)
+
+[<Fact>]
+let ``Fixes FS0495`` () =
+    let code =
+        """
+type Song(title: string) =
+    member _.Title = title
+
+let song = Song(titel = "Under The Milky Way")
+"""
+
+    let expected =
+        Some
+            {
+                Message = "Replace with 'title'"
+                FixedCode =
+                    """
+type Song(title: string) =
+    member _.Title = title
+
+let song = Song(title = "Under The Milky Way")
+"""
+            }
+
+    let actual = codeFix |> tryFix code Auto
+
+    Assert.Equal(expected, actual)

vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj

@@ -63,6 +63,7 @@
     <Compile Include="CodeFixes\SimplifyNameTests.fs" />
     <Compile Include="CodeFixes\RenameParamToMatchSignatureTests.fs" />
     <Compile Include="CodeFixes\ConvertCSharpUsingToFSharpOpenTests.fs" />
+    <Compile Include="CodeFixes\ReplaceWithSuggestionTests.fs" />
     <Compile Include="Hints\HintTestFramework.fs" />
     <Compile Include="Hints\OptionParserTests.fs" />
     <Compile Include="Hints\InlineParameterNameHintTests.fs" />