Skip to content

Fixing code fixes, part 7 #15804

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Fixing code fixes, part 7 #15804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,17 @@ open Microsoft.VisualStudio.FSharp.Editor.Telemetry

open FSharp.Compiler.Symbols
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text

open CancellableTasks

module internal MutableCodeFixHelper =
let getLineNumberAndText (sourceText: SourceText) position =
let textLine = sourceText.Lines.GetLineFromPosition position
let textLinePos = sourceText.Lines.GetLinePosition position
let fcsTextLineNumber = Line.fromZ textLinePos.Line
fcsTextLineNumber, textLine.ToString()

module internal UnusedCodeFixHelper =
let getUnusedSymbol textSpan (document: Document) (sourceText: SourceText) codeFixName =
let ident = sourceText.ToString textSpan
Expand Down
119 changes: 64 additions & 55 deletions vsintegration/src/FSharp.Editor/CodeFixes/MakeDeclarationMutable.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,71 +3,80 @@
namespace Microsoft.VisualStudio.FSharp.Editor

open System.Composition
open System.Threading.Tasks
open System.Threading
open System.Collections.Immutable

open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes

open FSharp.Compiler.EditorServices
open FSharp.Compiler.Text

open CancellableTasks

[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.MakeDeclarationMutable); Shared>]
type internal MakeDeclarationMutableFixProvider [<ImportingConstructor>] () =
type internal MakeDeclarationMutableCodeFixProvider [<ImportingConstructor>] () =
inherit CodeFixProvider()

static let title = SR.MakeDeclarationMutable()

override _.FixableDiagnosticIds = ImmutableArray.Create("FS0027")

override _.RegisterCodeFixesAsync context : Task =
asyncMaybe {

let document = context.Document
do! Option.guard (not (isSignatureFile document.FilePath))
let position = context.Span.Start

let! lexerSymbol =
document.TryFindFSharpLexerSymbolAsync(
position,
SymbolLookupKind.Greedy,
false,
false,
nameof (MakeDeclarationMutableFixProvider)
)

let! sourceText = document.GetTextAsync() |> liftTaskAsync
let textLine = sourceText.Lines.GetLineFromPosition position
let textLinePos = sourceText.Lines.GetLinePosition position
let fcsTextLineNumber = Line.fromZ textLinePos.Line

let! parseFileResults, checkFileResults =
document.GetFSharpParseAndCheckResultsAsync(nameof (MakeDeclarationMutableFixProvider))
|> CancellableTask.start context.CancellationToken
|> Async.AwaitTask
|> liftAsync

let decl =
checkFileResults.GetDeclarationLocation(
fcsTextLineNumber,
lexerSymbol.Ident.idRange.EndColumn,
textLine.ToString(),
lexerSymbol.FullIsland,
false
)

match decl with
// Only do this for symbols in the same file. That covers almost all cases anyways.
// We really shouldn't encourage making values mutable outside of local scopes anyways.
| FindDeclResult.DeclFound declRange when declRange.FileName = document.FilePath ->
let! span = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, declRange)

// Bail if it's a parameter, because like, that ain't allowed
do! Option.guard (not (parseFileResults.IsPositionContainedInACurriedParameter declRange.Start))
do context.RegisterFsharpFix(CodeFix.MakeDeclarationMutable, title, [| TextChange(TextSpan(span.Start, 0), "mutable ") |])
| _ -> ()
}
|> Async.Ignore
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)
override _.FixableDiagnosticIds = ImmutableArray.Create "FS0027"

override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this

interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
cancellableTask {
let document = context.Document

if isSignatureFile document.FilePath then
return ValueNone
else
let position = context.Span.Start

let! lexerSymbolOpt =
document.TryFindFSharpLexerSymbolAsync(
position,
SymbolLookupKind.Greedy,
false,
false,
nameof MakeDeclarationMutableCodeFixProvider
)

match lexerSymbolOpt with
| None -> return ValueNone
| Some lexerSymbol ->
let! sourceText = context.GetSourceTextAsync()

let fcsTextLineNumber, textLine =
MutableCodeFixHelper.getLineNumberAndText sourceText position

let! parseFileResults, checkFileResults =
document.GetFSharpParseAndCheckResultsAsync(nameof MakeDeclarationMutableCodeFixProvider)

let decl =
checkFileResults.GetDeclarationLocation(
fcsTextLineNumber,
lexerSymbol.Ident.idRange.EndColumn,
textLine,
lexerSymbol.FullIsland,
false
)

match decl with
| FindDeclResult.DeclFound declRange when
// Only do this for symbols in the same file. That covers almost all cases anyways.
// We really shouldn't encourage making values mutable outside of local scopes anyways.
declRange.FileName = document.FilePath
// Bail if it's a parameter, because like, that ain't allowed
&& not <| parseFileResults.IsPositionContainedInACurriedParameter declRange.Start
->
let span = RoslynHelpers.FSharpRangeToTextSpan(sourceText, declRange)

return
ValueSome
{
Name = CodeFix.MakeDeclarationMutable
Message = title
Changes = [ TextChange(TextSpan(span.Start, 0), "mutable ") ]
}
| _ -> return ValueNone
}
139 changes: 76 additions & 63 deletions vsintegration/src/FSharp.Editor/CodeFixes/UseMutationWhenValueIsMutable.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,79 +4,92 @@ namespace Microsoft.VisualStudio.FSharp.Editor

open System
open System.Composition
open System.Threading.Tasks
open System.Collections.Immutable

open Microsoft.CodeAnalysis.Text
open Microsoft.CodeAnalysis.CodeFixes

open FSharp.Compiler.Symbols
open FSharp.Compiler.Text

open CancellableTasks

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

static let title = SR.UseMutationWhenValueIsMutable()
override _.FixableDiagnosticIds = ImmutableArray.Create("FS0020")

override _.RegisterCodeFixesAsync context : Task =
asyncMaybe {
let document = context.Document
do! Option.guard (not (isSignatureFile document.FilePath))

let! sourceText = document.GetTextAsync(context.CancellationToken)

let adjustedPosition =
let rec loop ch pos =
if Char.IsWhiteSpace(ch) then
pos
else
loop sourceText.[pos + 1] (pos + 1)

loop sourceText.[context.Span.Start] context.Span.Start

let textLine = sourceText.Lines.GetLineFromPosition adjustedPosition
let textLinePos = sourceText.Lines.GetLinePosition adjustedPosition
let fcsTextLineNumber = Line.fromZ textLinePos.Line

let! lexerSymbol =
document.TryFindFSharpLexerSymbolAsync(
adjustedPosition,
SymbolLookupKind.Greedy,
false,
false,
nameof (UseMutationWhenValueIsMutableCodeFixProvider)
)

let! _, checkFileResults =
document.GetFSharpParseAndCheckResultsAsync(nameof (UseMutationWhenValueIsMutableCodeFixProvider))
|> CancellableTask.start context.CancellationToken
|> Async.AwaitTask
|> liftAsync

let! symbolUse =
checkFileResults.GetSymbolUseAtLocation(
fcsTextLineNumber,
lexerSymbol.Ident.idRange.EndColumn,
textLine.ToString(),
lexerSymbol.FullIsland
)

match symbolUse.Symbol with
| :? FSharpMemberOrFunctionOrValue as mfv when mfv.IsMutable || mfv.HasSetterMethod ->
let! symbolSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, symbolUse.Range)
let mutable pos = symbolSpan.End
let mutable ch = sourceText.[pos]

// We're looking for the possibly erroneous '='
while pos <= context.Span.Length && ch <> '=' do
pos <- pos + 1
ch <- sourceText.[pos]

do context.RegisterFsharpFix(CodeFix.UseMutationWhenValueIsMutable, title, [| TextChange(TextSpan(pos + 1, 1), "<-") |])
| _ -> ()
}
|> Async.Ignore
|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken)

override _.FixableDiagnosticIds = ImmutableArray.Create "FS0020"

override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this

interface IFSharpCodeFixProvider with
member _.GetCodeFixIfAppliesAsync context =
cancellableTask {
let document = context.Document

if isSignatureFile document.FilePath then
return ValueNone
else
let! sourceText = context.GetSourceTextAsync()

let adjustedPosition =
let rec loop ch pos =
if Char.IsWhiteSpace(ch) then
pos
else
loop sourceText[pos + 1] (pos + 1)

loop sourceText[context.Span.Start] context.Span.Start

let! lexerSymbolOpt =
document.TryFindFSharpLexerSymbolAsync(
adjustedPosition,
SymbolLookupKind.Greedy,
false,
false,
nameof UseMutationWhenValueIsMutableCodeFixProvider
)

match lexerSymbolOpt with
| None -> return ValueNone
| Some lexerSymbol ->
let fcsTextLineNumber, textLine =
MutableCodeFixHelper.getLineNumberAndText sourceText adjustedPosition

let! _, checkFileResults =
document.GetFSharpParseAndCheckResultsAsync(nameof UseMutationWhenValueIsMutableCodeFixProvider)

let symbolUseOpt =
checkFileResults.GetSymbolUseAtLocation(
fcsTextLineNumber,
lexerSymbol.Ident.idRange.EndColumn,
textLine,
lexerSymbol.FullIsland
)

let isValidCase (symbol: FSharpSymbol) =
match symbol with
| :? FSharpMemberOrFunctionOrValue as mfv when mfv.IsMutable || mfv.HasSetterMethod -> true
| _ -> false

match symbolUseOpt with
| Some symbolUse when isValidCase symbolUse.Symbol ->
let symbolSpan = RoslynHelpers.FSharpRangeToTextSpan(sourceText, symbolUse.Range)
let mutable pos = symbolSpan.End
let mutable ch = sourceText[pos]

// We're looking for the possibly erroneous '='
while pos <= context.Span.Length && ch <> '=' do
pos <- pos + 1
ch <- sourceText[pos]

return
ValueSome
{
Name = CodeFix.UseMutationWhenValueIsMutable
Message = title
Changes = [ TextChange(TextSpan(pos + 1, 1), "<-") ]
}
| _ -> return ValueNone
}
47 changes: 47 additions & 0 deletions vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeDeclarationMutableTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

module FSharp.Editor.Tests.CodeFixes.MakeDeclarationMutableTests

open Microsoft.VisualStudio.FSharp.Editor
open Xunit

open CodeFixTestFramework

let private codeFix = MakeDeclarationMutableCodeFixProvider()

[<Fact>]
let ``Fixes FS0027 for let bindings`` () =
let code =
"""
let x = 42
x <- 43
"""

let expected =
Some
{
Message = "Make declaration 'mutable'"
FixedCode =
"""
let mutable x = 42
x <- 43
"""
}

let actual = codeFix |> tryFix code Auto

Assert.Equal(expected, actual)

[<Fact>]
let ``Doesn't fix FS0027 for parameters`` () =
let code =
"""
let f x =
x <- 42
"""

let expected = None

let actual = codeFix |> tryFix code Auto

Assert.Equal(expected, actual)
Loading