Add RemoveReturnOrYield code fixer (dotnet#10530)

xamarin · Feb 22, 2021 · 69ce2d7 · 69ce2d7
1 parent 3cb1914
commit 69ce2d7
Show file tree

Hide file tree

Showing 20 changed files with 374 additions and 0 deletions.
diff --git a/src/fsharp/service/ServiceUntypedParse.fs b/src/fsharp/service/ServiceUntypedParse.fs
@@ -99,6 +99,18 @@ type FSharpParseFileResults(errors: FSharpErrorInfo[], input: ParsedInput option
     member scope.ParseHadErrors = parseHadErrors
 
     member scope.ParseTree = input
+
+    member scope.TryRangeOfExprInYieldOrReturn pos =
+        match scope.ParseTree with
+        | Some parseTree ->
+            AstTraversal.Traverse(pos, parseTree, { new AstTraversal.AstVisitorBase<_>() with 
+                member __.VisitExpr(_path, _, defaultTraverse, expr) =
+                    match expr with
+                    | SynExpr.YieldOrReturn(_, expr, range)
+                    | SynExpr.YieldOrReturnFrom(_, expr, range) when rangeContainsPos range pos ->
+                        Some expr.Range
+                    | _ -> defaultTraverse expr })
+        | None -> None
 
     member scope.TryRangeOfRecordExpressionContainingPos pos =
         match input 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 an expression `expr` contained in a `yield expr`  or `return expr` expression (and bang-variants).
+    member TryRangeOfExprInYieldOrReturn: pos: pos -> Option<range>
+
     /// Attempts to find the range of a record expression containing the given position.
     member TryRangeOfRecordExpressionContainingPos: pos: pos -> Option<range>
 

diff --git a/tests/FSharp.Compiler.Service.Tests/SurfaceArea.netstandard.fs b/tests/FSharp.Compiler.Service.Tests/SurfaceArea.netstandard.fs
@@ -22754,6 +22754,7 @@ FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.Sourc
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: FSharp.Compiler.SourceCodeServices.FSharpNavigationItems GetNavigationItems()
 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)
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Range+range] ValidateBreakpointLocation(pos)
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.SourceCodeServices.FSharpNoteworthyParamInfoLocations] FindNoteworthyParamInfoLocations(pos)
 FSharp.Compiler.SourceCodeServices.FSharpParseFileResults: Boolean IsPositionContainedInACurriedParameter(pos)

diff --git a/tests/service/ServiceUntypedParseTests.fs b/tests/service/ServiceUntypedParseTests.fs
@@ -382,3 +382,29 @@ let x = { Name = "Hello" }
     let parseFileResults, _ = getParseAndCheckResults source
     let res = parseFileResults.TryRangeOfRecordExpressionContainingPos (mkPos 2 7)
     Assert.True(res.IsNone, "Expected not to find a range.")
+
+[<Test>]
+let ``TryRangeOfExprInYieldOrReturn - not contained``() =
+    let source = """
+let f x =
+    x
+"""
+    let parseFileResults, _ = getParseAndCheckResults source
+    let res = parseFileResults.TryRangeOfExprInYieldOrReturn (mkPos 3 4)
+    Assert.True(res.IsNone, "Expected not to find a range.")
+
+[<Test>]
+let ``TryRangeOfExprInYieldOrReturn - contained``() =
+    let source = """
+let f x =
+    return x
+"""
+    let parseFileResults, _ = getParseAndCheckResults source
+    let res = parseFileResults.TryRangeOfExprInYieldOrReturn (mkPos 3 4)
+    match res with
+    | Some range ->
+        range
+        |> tups
+        |> shouldEqual ((3, 11), (3, 12))
+    | None ->
+        Assert.Fail("Expected to get a range back, but got none.")
diff --git a/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs b/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs
@@ -0,0 +1,59 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace Microsoft.VisualStudio.FSharp.Editor
+
+open System.Composition
+
+open Microsoft.CodeAnalysis.Text
+open Microsoft.CodeAnalysis.CodeFixes
+
+[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = "RemoveReturnOrYield"); Shared>]
+type internal FSharpRemoveReturnOrYieldCodeFixProvider
+    [<ImportingConstructor>]
+    (
+        checkerProvider: FSharpCheckerProvider, 
+        projectInfoManager: FSharpProjectOptionsManager
+    ) =
+    inherit CodeFixProvider()
+
+    static let userOpName = "RemoveReturnOrYield"
+    let fixableDiagnosticIds = set ["FS0748"; "FS0747"]
+
+    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 errorRange = RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText)
+            let! exprRange = parseResults.TryRangeOfExprInYieldOrReturn errorRange.Start
+            let! exprSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, exprRange)
+
+            let diagnostics =
+                context.Diagnostics
+                |> Seq.filter (fun x -> fixableDiagnosticIds |> Set.contains x.Id)
+                |> Seq.toImmutableArray
+
+            let title =
+                let text = sourceText.GetSubText(context.Span).ToString()
+                if text.StartsWith("return!") then
+                    SR.RemoveReturnBang()
+                elif text.StartsWith("return") then
+                    SR.RemoveReturn()
+                elif text.StartsWith("yield!") then
+                    SR.RemoveYieldBang()
+                else
+                    SR.RemoveYield()
+
+            let codeFix =
+                CodeFixHelpers.createTextChangeCodeFix(
+                    title,
+                    context,
+                    (fun () -> asyncMaybe.Return [| TextChange(context.Span, sourceText.GetSubText(exprSpan).ToString()) |]))
+
+            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
@@ -184,6 +184,7 @@
     <Compile Include="Commands\FsiCommandService.fs" />
     <Compile Include="Commands\XmlDocCommandService.fs" />
     <Compile Include="CodeFix\CodeFixHelpers.fs" />
+    <Compile Include="CodeFix\RemoveReturnOrYield.fs" />
     <Compile Include="CodeFix\ConvertToAnonymousRecord.fs" />
     <Compile Include="CodeFix\UseMutationWhenValueIsMutable.fs" />
     <Compile Include="CodeFix\MakeDeclarationMutable.fs" />

diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx
@@ -243,6 +243,18 @@
   <data name="ChangePrefixNegationToInfixSubtraction" xml:space="preserve">
     <value>Use subtraction instead of negation</value>
   </data>
+  <data name="RemoveReturn" xml:space="preserve">
+    <value>Remove 'return'</value>
+  </data>
+  <data name="RemoveReturnBang" xml:space="preserve">
+    <value>Remove 'return!'</value>
+  </data>
+  <data name="RemoveYield" xml:space="preserve">
+    <value>Remove 'yield'</value>
+  </data>
+  <data name="RemoveYieldBang" xml:space="preserve">
+    <value>Remove yield!'</value>
+  </data>
   <data name="ConvertToAnonymousRecord" xml:space="preserve">
     <value>Convert to Anonymous Record</value>
   </data>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf
@@ -67,6 +67,26 @@
         <target state="translated">Před {0} vložte podtržítko.</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">Přejmenujte {0} na _.</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf
@@ -67,6 +67,26 @@
         <target state="translated">"{0}" einen Unterstrich voranstellen</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">"{0}" in "_" umbenennen</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf
@@ -67,6 +67,26 @@
         <target state="translated">Colocar un carácter de subrayado delante de "{0}"</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">Cambiar nombre de "{0}" por "_"</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf
@@ -67,6 +67,26 @@
         <target state="translated">Faire précéder '{0}' d'un trait de soulignement</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">Renommer '{0}' en '_'</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf
@@ -67,6 +67,26 @@
         <target state="translated">Anteponi a '{0}' un carattere di sottolineatura</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">Rinomina '{0}' in '_'</target>

diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf
@@ -67,6 +67,26 @@
         <target state="translated">アンダースコアが含まれているプレフィックス '{0}'</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</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
@@ -67,6 +67,26 @@
         <target state="translated">밑줄이 있는 '{0}' 접두사</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</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
@@ -67,6 +67,26 @@
         <target state="translated">Prefiks „{0}” ze znakiem podkreślenia</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">Zmień nazwę z „{0}” na „_”</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
@@ -67,6 +67,26 @@
         <target state="translated">Prefixo '{0}' sem sublinhado</target>
         <note />
       </trans-unit>
+      <trans-unit id="RemoveReturn">
+        <source>Remove 'return'</source>
+        <target state="new">Remove 'return'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveReturnBang">
+        <source>Remove 'return!'</source>
+        <target state="new">Remove 'return!'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYield">
+        <source>Remove 'yield'</source>
+        <target state="new">Remove 'yield'</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="RemoveYieldBang">
+        <source>Remove yield!'</source>
+        <target state="new">Remove yield!'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="RenameValueToUnderscore">
         <source>Rename '{0}' to '_'</source>
         <target state="translated">Renomear '{0}' para '_'</target>