Add MakeOuterBindingRecursive code fix (dotnet#10666)

* Add MakeOuterBindingRecursive code fix * Allow other bindings to come before a nested should-be-recursive binding * more fixy * formatting and comment * Add to service layer * Area and add name to message
xamarin · Feb 22, 2021 · 51ae2bc · 51ae2bc
1 parent 9575b4f
commit 51ae2bc
Show file tree

Hide file tree

Showing 20 changed files with 256 additions and 0 deletions.
diff --git a/src/fsharp/service/ServiceUntypedParse.fs b/src/fsharp/service/ServiceUntypedParse.fs
@@ -99,6 +99,59 @@ type FSharpParseFileResults(errors: FSharpErrorInfo[], input: ParsedInput option
     member scope.ParseHadErrors = parseHadErrors
 
     member scope.ParseTree = input
+
+    member scope.TryRangeOfNameOfNearestOuterBindingContainingPos pos =
+        let tryGetIdentRangeFromBinding binding =
+            match binding with
+            | SynBinding.Binding (_, _, _, _, _, _, _, headPat, _, _, _, _) ->
+                match headPat with
+                | SynPat.LongIdent (longIdentWithDots, _, _, _, _, _) ->
+                    Some longIdentWithDots.Range
+                | SynPat.Named(_, ident, false, _, _) ->
+                    Some ident.idRange
+                | _ ->
+                    None
+
+        let rec walkBinding expr workingRange =
+            match expr with
+
+            // This lets us dive into subexpressions that may contain the binding we're after
+            | SynExpr.Sequential (_, _, expr1, expr2, _) ->
+                if rangeContainsPos expr1.Range pos then
+                    walkBinding expr1 workingRange
+                else
+                    walkBinding expr2 workingRange
+
+
+            | SynExpr.LetOrUse(_, _, bindings, bodyExpr, _) ->
+                let potentialNestedRange =
+                    bindings
+                    |> List.tryFind (fun binding -> rangeContainsPos binding.RangeOfBindingAndRhs pos)
+                    |> Option.bind tryGetIdentRangeFromBinding
+                match potentialNestedRange with
+                | Some range ->
+                    walkBinding bodyExpr range
+                | None ->
+                    walkBinding bodyExpr workingRange
+
+
+            | _ ->
+                Some workingRange
+
+        match scope.ParseTree with
+        | Some input ->
+            AstTraversal.Traverse(pos, input, { new AstTraversal.AstVisitorBase<_>() with
+                member _.VisitExpr(_, _, defaultTraverse, expr) =                        
+                    defaultTraverse expr
+
+                override _.VisitBinding(defaultTraverse, binding) =
+                    match binding with
+                    | SynBinding.Binding (_, _, _, _, _, _, _, _, _, expr, _range, _) as b when rangeContainsPos b.RangeOfBindingAndRhs pos ->
+                        match tryGetIdentRangeFromBinding b with
+                        | Some range -> walkBinding expr range
+                        | None -> None
+                    | _ -> defaultTraverse binding })
+        | None -> None
 
     member scope.TryIdentOfPipelineContainingPosAndNumArgsApplied pos =
         match scope.ParseTree with

diff --git a/src/fsharp/service/ServiceUntypedParse.fsi b/src/fsharp/service/ServiceUntypedParse.fsi
@@ -19,6 +19,9 @@ type public FSharpParseFileResults =
     /// The syntax tree resulting from the parse
     member ParseTree : ParsedInput option
 
+    /// Attempts to find the range of the name of the nearest outer binding that contains a given position.
+    member TryRangeOfNameOfNearestOuterBindingContainingPos: pos: pos -> Option<range>
+
     /// Attempts to find the range of an attempted lambda expression or pattern, the argument range, and the expr range when writing a C#-style "lambda" (which is actually an operator application)
     member TryRangeOfParenEnclosingOpEqualsGreaterUsage: opGreaterEqualPos: pos -> Option<range * range * range>
 

diff --git a/tests/FSharp.Compiler.Service.Tests/SurfaceArea.netstandard.fs b/tests/FSharp.Compiler.Service.Tests/SurfaceArea.netstandard.fs
@@ -22729,6 +22729,7 @@ FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Boolean get_ParseHadE
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpErrorInfo[] Errors
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpErrorInfo[] get_Errors()
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpNavigationItems GetNavigationItems()
+FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] TryRangeOfNameOfNearestOuterBindingContainingPos(pos)
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] TryRangeOfRefCellDereferenceContainingPos(pos)
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] TryRangeOfRecordExpressionContainingPos(pos)
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] TryRangeOfExprInYieldOrReturn(pos)

diff --git a/tests/service/ServiceUntypedParseTests.fs b/tests/service/ServiceUntypedParseTests.fs
@@ -927,3 +927,77 @@ let ``TryRangeOfParenEnclosingOpEqualsGreaterUsage - parenthesized lambda``() =
         |> shouldEqual [((2, 21), (2, 31)); ((2, 21), (2, 22)); ((2, 26), (2, 31))]
     | None ->
         Assert.Fail("Expected to get a range back, but got none.")
+
+[<Test>]
+let ``TryRangeOfNameOfNearestOuterBindingContainingPos - simple``() =
+    let source = """
+let x = nameof x
+"""
+    let parseFileResults, _ = getParseAndCheckResults source
+    let res = parseFileResults.TryRangeOfNameOfNearestOuterBindingContainingPos (mkPos 2 15)
+    match res with
+    | Some range ->
+        range
+        |> tups
+        |> shouldEqual ((2, 4), (2, 5))
+    | None ->
+        Assert.Fail("Expected to get a range back, but got none.")
+
+[<Test>]
+let ``TryRangeOfNameOfNearestOuterBindingContainingPos - inside match``() =
+    let source = """
+let mySum xs acc =
+    match xs with
+    | [] -> acc
+    | _ :: tail ->
+        mySum tail (acc + 1)
+"""
+    let parseFileResults, _ = getParseAndCheckResults source
+    let res = parseFileResults.TryRangeOfNameOfNearestOuterBindingContainingPos (mkPos 6 8)
+    match res with
+    | Some range ->
+        range
+        |> tups
+        |> shouldEqual ((2, 4), (2, 9))
+    | None ->
+        Assert.Fail("Expected to get a range back, but got none.")
+
+[<Test>]
+let ``TryRangeOfNameOfNearestOuterBindingContainingPos - nested binding``() =
+    let source = """
+let f x =
+    let z = 12
+    let h x = 
+        h x
+    g x
+"""
+    let parseFileResults, _ = getParseAndCheckResults source
+    let res = parseFileResults.TryRangeOfNameOfNearestOuterBindingContainingPos (mkPos 5 8)
+    match res with
+    | Some range ->
+        range
+        |> tups
+        |> shouldEqual ((4, 8), (4, 9))
+    | None ->
+        Assert.Fail("Expected to get a range back, but got none.")
+
+[<Test>]
+let ``TryRangeOfNameOfNearestOuterBindingContainingPos - nested and after other statements``() =
+    let source = """
+let f x =
+    printfn "doot doot"
+    printfn "toot toot"
+    let z = 12
+    let h x = 
+        h x
+    g x
+"""
+    let parseFileResults, _ = getParseAndCheckResults source
+    let res = parseFileResults.TryRangeOfNameOfNearestOuterBindingContainingPos (mkPos 7 8)
+    match res with
+    | Some range ->
+        range
+        |> tups
+        |> shouldEqual ((6, 8), (6, 9))
+    | None ->
+        Assert.Fail("Expected to get a range back, but got none.")
diff --git a/vsintegration/src/FSharp.Editor/CodeFix/MakeOuterBindingRecursive.fs b/vsintegration/src/FSharp.Editor/CodeFix/MakeOuterBindingRecursive.fs
@@ -0,0 +1,56 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace Microsoft.VisualStudio.FSharp.Editor
+
+open System
+open System.Composition
+
+open Microsoft.CodeAnalysis.Text
+open Microsoft.CodeAnalysis.CodeFixes
+
+[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = "MakeOuterBindingRecursive"); Shared>]
+type internal FSharpMakeOuterBindingRecursiveCodeFixProvider
+    [<ImportingConstructor>]
+    (
+        checkerProvider: FSharpCheckerProvider, 
+        projectInfoManager: FSharpProjectOptionsManager
+    ) =
+    inherit CodeFixProvider()
+
+    static let userOpName = "MakeOuterBindingRecursive"
+    let fixableDiagnosticIds = set ["FS0039"]
+
+    override _.FixableDiagnosticIds = Seq.toImmutableArray fixableDiagnosticIds
+
+    override _.RegisterCodeFixesAsync context =
+        asyncMaybe {
+            let! sourceText = context.Document.GetTextAsync(context.CancellationToken)
+            let! parsingOptions, _ = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(context.Document, context.CancellationToken, userOpName)
+            let! parseResults = checkerProvider.Checker.ParseFile(context.Document.FilePath, sourceText.ToFSharpSourceText(), parsingOptions, userOpName) |> liftAsync
+
+            let diagnosticRange = RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText)
+            do! Option.guard (parseResults.IsPosContainedInApplication diagnosticRange.Start)
+
+            let! outerBindingRange = parseResults.TryRangeOfNameOfNearestOuterBindingContainingPos diagnosticRange.Start
+            let! outerBindingNameSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, outerBindingRange)
+
+            // One last check to verify the names are the same
+            do! Option.guard (sourceText.GetSubText(outerBindingNameSpan).ContentEquals(sourceText.GetSubText(context.Span)))
+
+            let diagnostics =
+                context.Diagnostics
+                |> Seq.filter (fun x -> fixableDiagnosticIds |> Set.contains x.Id)
+                |> Seq.toImmutableArray
+
+            let title = String.Format(SR.MakeOuterBindingRecursive(), sourceText.GetSubText(outerBindingNameSpan).ToString())
+
+            let codeFix =
+                CodeFixHelpers.createTextChangeCodeFix(
+                    title,
+                    context,
+                    (fun () -> asyncMaybe.Return [| TextChange(TextSpan(outerBindingNameSpan.Start, 0), "rec ") |]))
+
+            context.RegisterCodeFix(codeFix, diagnostics)
+        }
+        |> Async.Ignore
+        |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) 
diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
@@ -185,6 +185,7 @@
     <Compile Include="Commands\XmlDocCommandService.fs" />
     <Compile Include="CodeFix\CodeFixHelpers.fs" />
     <Compile Include="CodeFix\ConvertCSharpLambdaToFSharpLambda.fs" />
+    <Compile Include="CodeFix\MakeOuterBindingRecursive.fs" />
     <Compile Include="CodeFix\RemoveReturnOrYield.fs" />
     <Compile Include="CodeFix\ConvertToAnonymousRecord.fs" />
     <Compile Include="CodeFix\UseMutationWhenValueIsMutable.fs" />

diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx
@@ -264,4 +264,7 @@
   <data name="UseFSharpLambda" xml:space="preserve">
     <value>Use F# lambda syntax</value>
   </data>
+  <data name="MakeOuterBindingRecursive" xml:space="preserve">
+    <value>Make '{0}' recursive</value>
+  </data>
 </root>
diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Nastavte deklaraci jako mutable.</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Před {0} vložte podtržítko.</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Deklaration "änderbar" machen</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">"{0}" einen Unterstrich voranstellen</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Convertir la declaración en "mutable"</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Colocar un carácter de subrayado delante de "{0}"</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Rendre la déclaration 'mutable'</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Faire précéder '{0}' d'un trait de soulignement</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Impostare la dichiarazione come 'mutable'</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Anteponi a '{0}' un carattere di sottolineatura</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf
@@ -62,6 +62,11 @@
         <target state="translated">'mutable' を宣言する</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">アンダースコアが含まれているプレフィックス '{0}'</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf
@@ -62,6 +62,11 @@
         <target state="translated">선언을 '변경 가능'으로 지정</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">밑줄이 있는 '{0}' 접두사</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Nadaj deklaracji właściwość „mutable”</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Prefiks „{0}” ze znakiem podkreślenia</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Fazer com que a declaração seja 'mutable'</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Prefixo '{0}' sem sublinhado</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Сделайте объявление "изменяемым"</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">Добавить символ подчеркивания как префикс "{0}"</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf
@@ -62,6 +62,11 @@
         <target state="translated">Bildirimi 'mutable' yapın</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">'{0}' öğesinin önüne alt çizgi ekleme</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf
@@ -62,6 +62,11 @@
         <target state="translated">将声明设为“可变”</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">带下划线的前缀“{0}”</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf
@@ -62,6 +62,11 @@
         <target state="translated">將宣告設定為「可變動」</target>
         <note />
       </trans-unit>
+      <trans-unit id="MakeOuterBindingRecursive">
+        <source>Make '{0}' recursive</source>
+        <target state="new">Make '{0}' recursive</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PrefixValueNameWithUnderscore">
         <source>Prefix '{0}' with underscore</source>
         <target state="translated">有底線的前置詞 '{0}'</target>