Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MakeOuterBindingRecursive code fix #10666

Merged
merged 7 commits into from
Dec 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/fsharp/service/ServiceUntypedParse.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/service/ServiceUntypedParse.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22752,6 +22752,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)
Expand Down
74 changes: 74 additions & 0 deletions tests/service/ServiceUntypedParseTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Original file line number Diff line number Diff line change
@@ -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)
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,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" />
Expand Down
3 changes: 3 additions & 0 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.resx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down