diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index dec879fd7d8..426715ee4ca 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -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 diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/MakeDeclarationMutable.fs b/vsintegration/src/FSharp.Editor/CodeFixes/MakeDeclarationMutable.fs index a4d8e276ba9..3de1e34fb72 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/MakeDeclarationMutable.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/MakeDeclarationMutable.fs @@ -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 [] -type internal MakeDeclarationMutableFixProvider [] () = +type internal MakeDeclarationMutableCodeFixProvider [] () = 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 + } diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/UseMutationWhenValueIsMutable.fs b/vsintegration/src/FSharp.Editor/CodeFixes/UseMutationWhenValueIsMutable.fs index cb0e9dda55a..27ad14755a2 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/UseMutationWhenValueIsMutable.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/UseMutationWhenValueIsMutable.fs @@ -4,14 +4,13 @@ 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 [] @@ -19,64 +18,78 @@ type internal 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 + } diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeDeclarationMutableTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeDeclarationMutableTests.fs new file mode 100644 index 00000000000..0b270f30c67 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/MakeDeclarationMutableTests.fs @@ -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() + +[] +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) + +[] +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) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseMutationWhenValueIsMutableTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseMutationWhenValueIsMutableTests.fs new file mode 100644 index 00000000000..7323774a53d --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/UseMutationWhenValueIsMutableTests.fs @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.UseMutationWhenValueIsMutableTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = UseMutationWhenValueIsMutableCodeFixProvider() + +[] +let ``Fixes FS0020 for let bindings`` () = + let code = + """ +let mutable x = 42 +x = 43 +""" + + let expected = + Some + { + Message = "Use '<-' to mutate value" + FixedCode = + """ +let mutable x = 42 +x <- 43 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS0020 for parameters`` () = + let code = + """ +let f x = + x = 42 + () +""" + + let expected = None + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix unrelated FS0020`` () = + let code = + """ +let square x = x * x +square 32 +""" + + let expected = None + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index 65851ab5b71..a07399eeec4 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -55,6 +55,8 @@ + +