Skip to content

Commit 81bb862

Browse files
psfinakipsfinaki
authored
Fixing code fixes, part 7 (#15804)
1 parent 8be2f03 commit 81bb862

File tree

6 files changed

+259
-118
lines changed

6 files changed

+259
-118
lines changed

‎vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,17 @@ open Microsoft.VisualStudio.FSharp.Editor.Telemetry
1616

1717
open FSharp.Compiler.Symbols
1818
open FSharp.Compiler.Syntax
19+
open FSharp.Compiler.Text
1920

2021
open CancellableTasks
2122

23+
module internal MutableCodeFixHelper =
24+
let getLineNumberAndText (sourceText: SourceText) position =
25+
let textLine = sourceText.Lines.GetLineFromPosition position
26+
let textLinePos = sourceText.Lines.GetLinePosition position
27+
let fcsTextLineNumber = Line.fromZ textLinePos.Line
28+
fcsTextLineNumber, textLine.ToString()
29+
2230
module internal UnusedCodeFixHelper =
2331
let getUnusedSymbol textSpan (document: Document) (sourceText: SourceText) codeFixName =
2432
let ident = sourceText.ToString textSpan

‎vsintegration/src/FSharp.Editor/CodeFixes/MakeDeclarationMutable.fs‎

Lines changed: 64 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3,71 +3,80 @@
33
namespace Microsoft.VisualStudio.FSharp.Editor
44

55
open System.Composition
6-
open System.Threading.Tasks
7-
open System.Threading
86
open System.Collections.Immutable
97

108
open Microsoft.CodeAnalysis.Text
119
open Microsoft.CodeAnalysis.CodeFixes
1210

1311
open FSharp.Compiler.EditorServices
14-
open FSharp.Compiler.Text
12+
1513
open CancellableTasks
1614

1715
[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.MakeDeclarationMutable); Shared>]
18-
type internal MakeDeclarationMutableFixProvider [<ImportingConstructor>] () =
16+
type internal MakeDeclarationMutableCodeFixProvider [<ImportingConstructor>] () =
1917
inherit CodeFixProvider()
2018

2119
static let title = SR.MakeDeclarationMutable()
2220

23-
override _.FixableDiagnosticIds = ImmutableArray.Create("FS0027")
24-
25-
override _.RegisterCodeFixesAsync context : Task =
26-
asyncMaybe {
27-
28-
let document = context.Document
29-
do! Option.guard (not (isSignatureFile document.FilePath))
30-
let position = context.Span.Start
31-
32-
let! lexerSymbol =
33-
document.TryFindFSharpLexerSymbolAsync(
34-
position,
35-
SymbolLookupKind.Greedy,
36-
false,
37-
false,
38-
nameof (MakeDeclarationMutableFixProvider)
39-
)
40-
41-
let! sourceText = document.GetTextAsync() |> liftTaskAsync
42-
let textLine = sourceText.Lines.GetLineFromPosition position
43-
let textLinePos = sourceText.Lines.GetLinePosition position
44-
let fcsTextLineNumber = Line.fromZ textLinePos.Line
45-
46-
let! parseFileResults, checkFileResults =
47-
document.GetFSharpParseAndCheckResultsAsync(nameof (MakeDeclarationMutableFixProvider))
48-
|> CancellableTask.start context.CancellationToken
49-
|> Async.AwaitTask
50-
|> liftAsync
51-
52-
let decl =
53-
checkFileResults.GetDeclarationLocation(
54-
fcsTextLineNumber,
55-
lexerSymbol.Ident.idRange.EndColumn,
56-
textLine.ToString(),
57-
lexerSymbol.FullIsland,
58-
false
59-
)
60-
61-
match decl with
62-
// Only do this for symbols in the same file. That covers almost all cases anyways.
63-
// We really shouldn't encourage making values mutable outside of local scopes anyways.
64-
| FindDeclResult.DeclFound declRange when declRange.FileName = document.FilePath ->
65-
let! span = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, declRange)
66-
67-
// Bail if it's a parameter, because like, that ain't allowed
68-
do! Option.guard (not (parseFileResults.IsPositionContainedInACurriedParameter declRange.Start))
69-
do context.RegisterFsharpFix(CodeFix.MakeDeclarationMutable, title, [| TextChange(TextSpan(span.Start, 0), "mutable ") |])
70-
| _ -> ()
71-
}
72-
|> Async.Ignore
73-
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
21+
override _.FixableDiagnosticIds = ImmutableArray.Create "FS0027"
22+
23+
override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this
24+
25+
interface IFSharpCodeFixProvider with
26+
member _.GetCodeFixIfAppliesAsync context =
27+
cancellableTask {
28+
let document = context.Document
29+
30+
if isSignatureFile document.FilePath then
31+
return ValueNone
32+
else
33+
let position = context.Span.Start
34+
35+
let! lexerSymbolOpt =
36+
document.TryFindFSharpLexerSymbolAsync(
37+
position,
38+
SymbolLookupKind.Greedy,
39+
false,
40+
false,
41+
nameof MakeDeclarationMutableCodeFixProvider
42+
)
43+
44+
match lexerSymbolOpt with
45+
| None -> return ValueNone
46+
| Some lexerSymbol ->
47+
let! sourceText = context.GetSourceTextAsync()
48+
49+
let fcsTextLineNumber, textLine =
50+
MutableCodeFixHelper.getLineNumberAndText sourceText position
51+
52+
let! parseFileResults, checkFileResults =
53+
document.GetFSharpParseAndCheckResultsAsync(nameof MakeDeclarationMutableCodeFixProvider)
54+
55+
let decl =
56+
checkFileResults.GetDeclarationLocation(
57+
fcsTextLineNumber,
58+
lexerSymbol.Ident.idRange.EndColumn,
59+
textLine,
60+
lexerSymbol.FullIsland,
61+
false
62+
)
63+
64+
match decl with
65+
| FindDeclResult.DeclFound declRange when
66+
// Only do this for symbols in the same file. That covers almost all cases anyways.
67+
// We really shouldn't encourage making values mutable outside of local scopes anyways.
68+
declRange.FileName = document.FilePath
69+
// Bail if it's a parameter, because like, that ain't allowed
70+
&& not <| parseFileResults.IsPositionContainedInACurriedParameter declRange.Start
71+
->
72+
let span = RoslynHelpers.FSharpRangeToTextSpan(sourceText, declRange)
73+
74+
return
75+
ValueSome
76+
{
77+
Name = CodeFix.MakeDeclarationMutable
78+
Message = title
79+
Changes = [ TextChange(TextSpan(span.Start, 0), "mutable ") ]
80+
}
81+
| _ -> return ValueNone
82+
}

‎vsintegration/src/FSharp.Editor/CodeFixes/UseMutationWhenValueIsMutable.fs‎

Lines changed: 76 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -4,79 +4,92 @@ namespace Microsoft.VisualStudio.FSharp.Editor
44

55
open System
66
open System.Composition
7-
open System.Threading.Tasks
87
open System.Collections.Immutable
98

109
open Microsoft.CodeAnalysis.Text
1110
open Microsoft.CodeAnalysis.CodeFixes
1211

1312
open FSharp.Compiler.Symbols
14-
open FSharp.Compiler.Text
13+
1514
open CancellableTasks
1615

1716
[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.UseMutationWhenValueIsMutable); Shared>]
1817
type internal UseMutationWhenValueIsMutableCodeFixProvider [<ImportingConstructor>] () =
1918
inherit CodeFixProvider()
2019

2120
static let title = SR.UseMutationWhenValueIsMutable()
22-
override _.FixableDiagnosticIds = ImmutableArray.Create("FS0020")
23-
24-
override _.RegisterCodeFixesAsync context : Task =
25-
asyncMaybe {
26-
let document = context.Document
27-
do! Option.guard (not (isSignatureFile document.FilePath))
28-
29-
let! sourceText = document.GetTextAsync(context.CancellationToken)
30-
31-
let adjustedPosition =
32-
let rec loop ch pos =
33-
if Char.IsWhiteSpace(ch) then
34-
pos
35-
else
36-
loop sourceText.[pos + 1] (pos + 1)
37-
38-
loop sourceText.[context.Span.Start] context.Span.Start
39-
40-
let textLine = sourceText.Lines.GetLineFromPosition adjustedPosition
41-
let textLinePos = sourceText.Lines.GetLinePosition adjustedPosition
42-
let fcsTextLineNumber = Line.fromZ textLinePos.Line
43-
44-
let! lexerSymbol =
45-
document.TryFindFSharpLexerSymbolAsync(
46-
adjustedPosition,
47-
SymbolLookupKind.Greedy,
48-
false,
49-
false,
50-
nameof (UseMutationWhenValueIsMutableCodeFixProvider)
51-
)
52-
53-
let! _, checkFileResults =
54-
document.GetFSharpParseAndCheckResultsAsync(nameof (UseMutationWhenValueIsMutableCodeFixProvider))
55-
|> CancellableTask.start context.CancellationToken
56-
|> Async.AwaitTask
57-
|> liftAsync
58-
59-
let! symbolUse =
60-
checkFileResults.GetSymbolUseAtLocation(
61-
fcsTextLineNumber,
62-
lexerSymbol.Ident.idRange.EndColumn,
63-
textLine.ToString(),
64-
lexerSymbol.FullIsland
65-
)
66-
67-
match symbolUse.Symbol with
68-
| :? FSharpMemberOrFunctionOrValue as mfv when mfv.IsMutable || mfv.HasSetterMethod ->
69-
let! symbolSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, symbolUse.Range)
70-
let mutable pos = symbolSpan.End
71-
let mutable ch = sourceText.[pos]
72-
73-
// We're looking for the possibly erroneous '='
74-
while pos <= context.Span.Length && ch <> '=' do
75-
pos <- pos + 1
76-
ch <- sourceText.[pos]
77-
78-
do context.RegisterFsharpFix(CodeFix.UseMutationWhenValueIsMutable, title, [| TextChange(TextSpan(pos + 1, 1), "<-") |])
79-
| _ -> ()
80-
}
81-
|> Async.Ignore
82-
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
21+
22+
override _.FixableDiagnosticIds = ImmutableArray.Create "FS0020"
23+
24+
override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this
25+
26+
interface IFSharpCodeFixProvider with
27+
member _.GetCodeFixIfAppliesAsync context =
28+
cancellableTask {
29+
let document = context.Document
30+
31+
if isSignatureFile document.FilePath then
32+
return ValueNone
33+
else
34+
let! sourceText = context.GetSourceTextAsync()
35+
36+
let adjustedPosition =
37+
let rec loop ch pos =
38+
if Char.IsWhiteSpace(ch) then
39+
pos
40+
else
41+
loop sourceText[pos + 1] (pos + 1)
42+
43+
loop sourceText[context.Span.Start] context.Span.Start
44+
45+
let! lexerSymbolOpt =
46+
document.TryFindFSharpLexerSymbolAsync(
47+
adjustedPosition,
48+
SymbolLookupKind.Greedy,
49+
false,
50+
false,
51+
nameof UseMutationWhenValueIsMutableCodeFixProvider
52+
)
53+
54+
match lexerSymbolOpt with
55+
| None -> return ValueNone
56+
| Some lexerSymbol ->
57+
let fcsTextLineNumber, textLine =
58+
MutableCodeFixHelper.getLineNumberAndText sourceText adjustedPosition
59+
60+
let! _, checkFileResults =
61+
document.GetFSharpParseAndCheckResultsAsync(nameof UseMutationWhenValueIsMutableCodeFixProvider)
62+
63+
let symbolUseOpt =
64+
checkFileResults.GetSymbolUseAtLocation(
65+
fcsTextLineNumber,
66+
lexerSymbol.Ident.idRange.EndColumn,
67+
textLine,
68+
lexerSymbol.FullIsland
69+
)
70+
71+
let isValidCase (symbol: FSharpSymbol) =
72+
match symbol with
73+
| :? FSharpMemberOrFunctionOrValue as mfv when mfv.IsMutable || mfv.HasSetterMethod -> true
74+
| _ -> false
75+
76+
match symbolUseOpt with
77+
| Some symbolUse when isValidCase symbolUse.Symbol ->
78+
let symbolSpan = RoslynHelpers.FSharpRangeToTextSpan(sourceText, symbolUse.Range)
79+
let mutable pos = symbolSpan.End
80+
let mutable ch = sourceText[pos]
81+
82+
// We're looking for the possibly erroneous '='
83+
while pos <= context.Span.Length && ch <> '=' do
84+
pos <- pos + 1
85+
ch <- sourceText[pos]
86+
87+
return
88+
ValueSome
89+
{
90+
Name = CodeFix.UseMutationWhenValueIsMutable
91+
Message = title
92+
Changes = [ TextChange(TextSpan(pos + 1, 1), "<-") ]
93+
}
94+
| _ -> return ValueNone
95+
}

‎vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeDeclarationMutableTests.fs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.
2+
3+
module FSharp.Editor.Tests.CodeFixes.MakeDeclarationMutableTests
4+
5+
open Microsoft.VisualStudio.FSharp.Editor
6+
open Xunit
7+
8+
open CodeFixTestFramework
9+
10+
let private codeFix = MakeDeclarationMutableCodeFixProvider()
11+
12+
[<Fact>]
13+
let ``Fixes FS0027 for let bindings`` () =
14+
let code =
15+
"""
16+
let x = 42
17+
x <- 43
18+
"""
19+
20+
let expected =
21+
Some
22+
{
23+
Message = "Make declaration 'mutable'"
24+
FixedCode =
25+
"""
26+
let mutable x = 42
27+
x <- 43
28+
"""
29+
}
30+
31+
let actual = codeFix |> tryFix code Auto
32+
33+
Assert.Equal(expected, actual)
34+
35+
[<Fact>]
36+
let ``Doesn't fix FS0027 for parameters`` () =
37+
let code =
38+
"""
39+
let f x =
40+
x <- 42
41+
"""
42+
43+
let expected = None
44+
45+
let actual = codeFix |> tryFix code Auto
46+
47+
Assert.Equal(expected, actual)

0 commit comments

Comments
 (0)