From 63842852a7793fe6614d4f78fba111eb8e0d3bf9 Mon Sep 17 00:00:00 2001 From: Petr Date: Thu, 8 Jun 2023 17:56:20 +0200 Subject: [PATCH 01/10] Democratizing code fixes - part one --- .../CodeFix/AddInstanceMemberParameter.fs | 20 ++++-- .../FSharp.Editor/CodeFix/ChangeToUpcast.fs | 71 +++++++++++-------- .../CodeFix/ConvertToAnonymousRecord.fs | 50 +++++++------ .../FSharp.Editor/CodeFix/IFSharpCodeFix.fs | 11 +++ .../src/FSharp.Editor/FSharp.Editor.fsproj | 1 + .../AddInstanceMemberParameterTests.fs | 33 +++++++++ .../CodeFixes/ChangeToUpcastTests.fs | 66 +++++++++++++++++ .../CodeFixes/CodeFixTestFramework.fs | 46 ++++++++++++ .../ConvertToAnonymousRecordTests.fs | 31 ++++++++ .../FSharp.Editor.Tests.fsproj | 5 ++ .../Helpers/RoslynHelpers.fs | 10 +++ 11 files changed, 286 insertions(+), 58 deletions(-) create mode 100644 vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs index 28915bd0af7..e7378074e48 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs @@ -9,17 +9,25 @@ open System.Collections.Immutable open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes +open CancellableTasks + [] type internal FSharpAddInstanceMemberParameterCodeFixProvider() = inherit CodeFixProvider() - static let title = SR.AddMissingInstanceMemberParameter() + interface IFSharpCodeFix with + member _.GetChangesAsync _ span = + cancellableTask { + let title = SR.AddMissingInstanceMemberParameter() + let changes = [ TextChange(TextSpan(span.Start, 0), "x.") ] + return title, changes + } override _.FixableDiagnosticIds = ImmutableArray.Create("FS0673") - override _.RegisterCodeFixesAsync context : Task = - asyncMaybe { - do context.RegisterFsharpFix(CodeFix.AddInstanceMemberParameter, title, [| TextChange(TextSpan(context.Span.Start, 0), "x.") |]) + override this.RegisterCodeFixesAsync context : Task = + cancellableTask { + let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span + context.RegisterFsharpFix(CodeFix.AddInstanceMemberParameter, title, changes) } - |> Async.Ignore - |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) + |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs b/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs index 39279cde5d5..572fcd3f62c 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs @@ -9,40 +9,53 @@ open System.Collections.Immutable open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes +open CancellableTasks + [] type internal FSharpChangeToUpcastCodeFixProvider() = inherit CodeFixProvider() override _.FixableDiagnosticIds = ImmutableArray.Create("FS3198") + interface IFSharpCodeFix with + member _.GetChangesAsync document span = + cancellableTask { + let! cancellationToken = CancellableTask.getCurrentCancellationToken () + + let! sourceText = document.GetTextAsync(cancellationToken) + let text = sourceText.GetSubText(span).ToString() + + // Only works if it's one or the other + let isDowncastOperator = text.Contains(":?>") + let isDowncastKeyword = text.Contains("downcast") + + let changes = + Option.guard ( + (isDowncastOperator || isDowncastKeyword) + && not (isDowncastOperator && isDowncastKeyword) + ) + |> Option.map (fun _ -> + let replacement = + if isDowncastOperator then + text.Replace(":?>", ":>") + else + text.Replace("downcast", "upcast") + + [ TextChange(span, replacement) ]) + |> Option.defaultValue [] + + let title = + if isDowncastOperator then + SR.UseUpcastOperator() + else + SR.UseUpcastKeyword() + + return title, changes + } + override this.RegisterCodeFixesAsync context : Task = - asyncMaybe { - let! sourceText = context.Document.GetTextAsync(context.CancellationToken) - let text = sourceText.GetSubText(context.Span).ToString() - - // Only works if it's one or the other - let isDowncastOperator = text.Contains(":?>") - let isDowncastKeyword = text.Contains("downcast") - - do! - Option.guard ( - (isDowncastOperator || isDowncastKeyword) - && not (isDowncastOperator && isDowncastKeyword) - ) - - let replacement = - if isDowncastOperator then - text.Replace(":?>", ":>") - else - text.Replace("downcast", "upcast") - - let title = - if isDowncastOperator then - SR.UseUpcastOperator() - else - SR.UseUpcastKeyword() - - do context.RegisterFsharpFix(CodeFix.ChangeToUpcast, title, [| TextChange(context.Span, replacement) |]) + cancellableTask { + let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span + context.RegisterFsharpFix(CodeFix.ChangeToUpcast, title, changes) } - |> Async.Ignore - |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) + |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs index 808dcac5ef9..71b96edefc7 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs @@ -3,45 +3,49 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Threading open System.Threading.Tasks open System.Collections.Immutable open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes -open Microsoft.CodeAnalysis.CodeActions + +open CancellableTasks [] type internal FSharpConvertToAnonymousRecordCodeFixProvider [] () = inherit CodeFixProvider() - static let title = SR.ConvertToAnonymousRecord() - override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039") - override _.RegisterCodeFixesAsync context : Task = - asyncMaybe { - let document = context.Document + interface IFSharpCodeFix with + member _.GetChangesAsync document span = + cancellableTask { + let! cancellationToken = CancellableTask.getCurrentCancellationToken () - let! parseResults = - document.GetFSharpParseResultsAsync(nameof (FSharpConvertToAnonymousRecordCodeFixProvider)) - |> liftAsync + let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpConvertToAnonymousRecordCodeFixProvider)) - let! sourceText = context.Document.GetTextAsync(context.CancellationToken) + let! sourceText = document.GetTextAsync(cancellationToken) - let errorRange = - RoslynHelpers.TextSpanToFSharpRange(document.FilePath, context.Span, sourceText) + let errorRange = + RoslynHelpers.TextSpanToFSharpRange(document.FilePath, span, sourceText) - let! recordRange = parseResults.TryRangeOfRecordExpressionContainingPos errorRange.Start - let! recordSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, recordRange) + let changes = + parseResults.TryRangeOfRecordExpressionContainingPos errorRange.Start + |> Option.bind (fun range -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range)) + |> Option.map (fun span -> + [ + TextChange(TextSpan(span.Start + 1, 0), "|") + TextChange(TextSpan(span.End - 1, 0), "|") + ]) + |> Option.defaultValue [] - let changes = - [ - TextChange(TextSpan(recordSpan.Start + 1, 0), "|") - TextChange(TextSpan(recordSpan.End, 0), "|") - ] + let title = SR.ConvertToAnonymousRecord() + return title, changes + } - context.RegisterFsharpFix(CodeFix.ConvertToAnonymousRecord, title, changes) + override this.RegisterCodeFixesAsync context : Task = + cancellableTask { + let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span + return context.RegisterFsharpFix(CodeFix.ConvertToAnonymousRecord, title, changes) } - |> Async.Ignore - |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) + |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs b/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs new file mode 100644 index 00000000000..e0a4fa23a6e --- /dev/null +++ b/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Microsoft.VisualStudio.FSharp.Editor + +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Text + +open CancellableTasks + +type IFSharpCodeFix = + abstract member GetChangesAsync: document: Document -> span: TextSpan -> CancellableTask diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index 598c72145aa..a2673c2d559 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -101,6 +101,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs new file mode 100644 index 00000000000..1fbd6b4a39d --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.AddInstanceMemberParameterTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = FSharpAddInstanceMemberParameterCodeFixProvider() +let private diagnostic = 0673 // This instance member needs a parameter to represent the object being invoked... + +[] +let ``Fixes FS0673`` () = + let code = + """ +type UsefulTestHarness() = + member FortyTwo = 42 +""" + + let expected = + { + Title = "Add missing instance member parameter" + FixedCode = + """ +type UsefulTestHarness() = + member x.FortyTwo = 42 +""" + } + + let actual = codeFix |> fix code diagnostic + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs new file mode 100644 index 00000000000..9b0ead993d7 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.ChangeToUpcastTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = FSharpChangeToUpcastCodeFixProvider() +let private diagnostic = 3198 // The conversion is an upcast, not a downcast... + +// Test cases are taken from the original PR: +// https://github.com/dotnet/fsharp/pull/10463 + +[] +let ``Fixes FS3198 - operator`` () = + let code = + """ +type IFoo = abstract member Bar : unit -> unit +type Foo() = interface IFoo with member __.Bar () = () + +let Thing : IFoo = Foo() :?> IFoo +""" + + let expected = + { + Title = "Use ':>' operator" + FixedCode = + """ +type IFoo = abstract member Bar : unit -> unit +type Foo() = interface IFoo with member __.Bar () = () + +let Thing : IFoo = Foo() :> IFoo +""" + } + + let actual = codeFix |> fix code diagnostic + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS3198 - keyword`` () = + let code = + """ +type IFoo = abstract member Bar : unit -> unit +type Foo() = interface IFoo with member __.Bar () = () + +let Thing : IFoo = downcast Foo() +""" + + let expected = + { + Title = "Use 'upcast'" + FixedCode = + """ +type IFoo = abstract member Bar : unit -> unit +type Foo() = interface IFoo with member __.Bar () = () + +let Thing : IFoo = upcast Foo() +""" + } + + let actual = codeFix |> fix code diagnostic + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs new file mode 100644 index 00000000000..04998935ea8 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.CodeFixTestFramework + +open System.Threading + +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Text +open Microsoft.VisualStudio.FSharp.Editor +open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks + +open FSharp.Editor.Tests.Helpers + +type TestCodeFix = { Title: string; FixedCode: string } + +let getRelevantDiagnostic (document: Document) errorNumber = + cancellableTask { + let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync "test" + + return + checkFileResults.Diagnostics + |> Seq.where (fun d -> d.ErrorNumber = errorNumber) + |> Seq.exactlyOne + } + +let fix (code: string) diagnostic (fixProvider: IFSharpCodeFix) = + cancellableTask { + let sourceText = SourceText.From code + let document = RoslynTestHelpers.GetFsDocument code + + let! diagnostic = getRelevantDiagnostic document diagnostic + + let diagnosticSpan = + RoslynHelpers.FSharpRangeToTextSpan(sourceText, diagnostic.Range) + + let! title, changes = fixProvider.GetChangesAsync document diagnosticSpan + let fixedSourceText = sourceText.WithChanges changes + + return + { + Title = title + FixedCode = fixedSourceText.ToString() + } + } + |> CancellableTask.start CancellationToken.None + |> fun task -> task.Result diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs new file mode 100644 index 00000000000..9f8008356bb --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.ConvertToAnonymousRecordTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = FSharpConvertToAnonymousRecordCodeFixProvider() +let private diagnostic = 0039 // The record label is not defined... + +[] +let ``Fixes FS0039`` () = + let code = + """ +let band = { Name = "The Velvet Underground" } +""" + + let expected = + { + Title = "Convert to Anonymous Record" + FixedCode = + """ +let band = {| Name = "The Velvet Underground" |} +""" + } + + let actual = codeFix |> fix code diagnostic + + 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 e1aae0a95a1..232e394455c 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -32,6 +32,10 @@ + + + + @@ -55,6 +59,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 17ab1d90f4c..3fed33dc983 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -337,3 +337,13 @@ type RoslynTestHelpers private () = options |> RoslynTestHelpers.SetProjectOptions projId solution solution, checker + + static member GetFsDocument code = + // without this lib some symbols are not loaded + let options = + { RoslynTestHelpers.DefaultProjectOptions with + OtherOptions = [| "--targetprofile:netcore" |] + } + + RoslynTestHelpers.CreateSolution(code, options = options) + |> RoslynTestHelpers.GetSingleDocument From 7555fe491a1e01418c74d5f544f503af5042f246 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 13 Jun 2023 14:33:29 +0200 Subject: [PATCH 02/10] Up --- .../src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs | 3 ++- .../src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs index e7378074e48..db2cba010f4 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs @@ -15,10 +15,11 @@ open CancellableTasks type internal FSharpAddInstanceMemberParameterCodeFixProvider() = inherit CodeFixProvider() + static let title = SR.AddMissingInstanceMemberParameter() + interface IFSharpCodeFix with member _.GetChangesAsync _ span = cancellableTask { - let title = SR.AddMissingInstanceMemberParameter() let changes = [ TextChange(TextSpan(span.Start, 0), "x.") ] return title, changes } diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs index 71b96edefc7..da39b93e9c3 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs @@ -15,6 +15,8 @@ open CancellableTasks type internal FSharpConvertToAnonymousRecordCodeFixProvider [] () = inherit CodeFixProvider() + static let title = SR.ConvertToAnonymousRecord() + override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039") interface IFSharpCodeFix with @@ -39,7 +41,6 @@ type internal FSharpConvertToAnonymousRecordCodeFixProvider [ Option.defaultValue [] - let title = SR.ConvertToAnonymousRecord() return title, changes } From 399d78afb76f05877071a2a543b97c71c94e5711 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 13 Jun 2023 16:49:20 +0200 Subject: [PATCH 03/10] Update AddInstanceMemberParameter.fs --- .../src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs index db2cba010f4..e98976184a0 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs @@ -19,10 +19,8 @@ type internal FSharpAddInstanceMemberParameterCodeFixProvider() = interface IFSharpCodeFix with member _.GetChangesAsync _ span = - cancellableTask { - let changes = [ TextChange(TextSpan(span.Start, 0), "x.") ] - return title, changes - } + let changes = [ TextChange(TextSpan(span.Start, 0), "x.") ] + CancellableTask.singleton (title, changes) override _.FixableDiagnosticIds = ImmutableArray.Create("FS0673") From 517debf4461ccbd9a9c2938a0b17d270cec87e53 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 14:13:10 +0200 Subject: [PATCH 04/10] Update ChangeToUpcast.fs --- .../FSharp.Editor/CodeFix/ChangeToUpcast.fs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs b/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs index 572fcd3f62c..7136009a928 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs @@ -30,19 +30,19 @@ type internal FSharpChangeToUpcastCodeFixProvider() = let isDowncastKeyword = text.Contains("downcast") let changes = - Option.guard ( - (isDowncastOperator || isDowncastKeyword) - && not (isDowncastOperator && isDowncastKeyword) - ) - |> Option.map (fun _ -> - let replacement = - if isDowncastOperator then - text.Replace(":?>", ":>") - else - text.Replace("downcast", "upcast") - - [ TextChange(span, replacement) ]) - |> Option.defaultValue [] + [ + if + (isDowncastOperator || isDowncastKeyword) + && not (isDowncastOperator && isDowncastKeyword) + then + let replacement = + if isDowncastOperator then + text.Replace(":?>", ":>") + else + text.Replace("downcast", "upcast") + + TextChange(span, replacement) + ] let title = if isDowncastOperator then From f1c472740902d2273d81ad0c0b005dc3ade977ca Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 14:41:08 +0200 Subject: [PATCH 05/10] Update AddMissingRecToMutuallyRecFunctions.fs --- .../CodeFix/AddMissingRecToMutuallyRecFunctions.fs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs index ec9c7dad660..e26595ae40f 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs @@ -5,17 +5,10 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System open System.Collections.Immutable open System.Composition -open System.Threading open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes -open FSharp.Compiler -open FSharp.Compiler.CodeAnalysis - -open Microsoft.CodeAnalysis -open Microsoft.CodeAnalysis.CodeActions - [] type internal FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider [] () = inherit CodeFixProvider() From f65ec3e0f4f8e365227a8dca9a8654fa25f9d894 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 14:53:13 +0200 Subject: [PATCH 06/10] tests --- ...ddMissingRecToMutuallyRecFunctionsTests.fs | 47 +++++++++++++++++++ .../FSharp.Editor.Tests.fsproj | 1 + 2 files changed, 48 insertions(+) create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs new file mode 100644 index 00000000000..bdd0aa5e558 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.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.AddMissingRecToMutuallyRecFunctionsTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider() +let private diagnostic = 0576 // The declaration form 'let ... and ...' for non-recursive bindings is not used in F# code... + +[] +let ``Fixes FS0576`` () = + let code = + """ +let isEven n = + match n with + | 0 -> true + | _ -> isOdd (n - 1) + +and isOdd n = + match n with + | 0 -> false + | _ -> isEven (n - 1) +""" + + let expected = + { + Title = "Make 'isEven' recursive" + FixedCode = + """ +let rec isEven n = + match n with + | 0 -> true + | _ -> isOdd (n - 1) + +and isOdd n = + match n with + | 0 -> false + | _ -> isEven (n - 1) +""" + } + + let actual = failwith "" // codeFix |> fix code diagnostic + + Assert.Equal(expected, actual) \ No newline at end of file diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index 232e394455c..758a979246c 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -36,6 +36,7 @@ + From 7e438b2130494e50d92e91279b41d892e8186654 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 15:28:04 +0200 Subject: [PATCH 07/10] cool - tests pass --- .../AddMissingRecToMutuallyRecFunctions.fs | 143 ++++++++++++------ ...ddMissingRecToMutuallyRecFunctionsTests.fs | 2 +- 2 files changed, 100 insertions(+), 45 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs index e26595ae40f..01aca735ebd 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs @@ -9,6 +9,8 @@ open System.Composition open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes +open CancellableTasks + [] type internal FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider [] () = inherit CodeFixProvider() @@ -17,48 +19,101 @@ type internal FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider [ liftAsync - - let! sourceText = context.Document.GetTextAsync(context.CancellationToken) - - let funcStartPos = - let rec loop ch pos = - if not (Char.IsWhiteSpace(ch)) then - pos - else - loop sourceText.[pos + 1] (pos + 1) - - loop sourceText.[context.Span.End + 1] (context.Span.End + 1) - - let! funcLexerSymbol = - Tokenizer.getSymbolAtPosition ( - context.Document.Id, - sourceText, - funcStartPos, - context.Document.FilePath, - defines, - SymbolLookupKind.Greedy, - false, - false, - Some langVersion, - context.CancellationToken - ) - - let! funcNameSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, funcLexerSymbol.Range) - let funcName = sourceText.GetSubText(funcNameSpan).ToString() - - do - context.RegisterFsharpFix( - CodeFix.AddMissingRecToMutuallyRecFunctions, - String.Format(titleFormat, funcName), - [| TextChange(TextSpan(context.Span.End, 0), " rec") |] - ) + interface IFSharpCodeFix with + member _.GetChangesAsync document span = + cancellableTask { + let! cancellationToken = CancellableTask.getCurrentCancellationToken () + + let! defines, langVersion = + document.GetFSharpCompilationDefinesAndLangVersionAsync( + nameof (FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider) + ) + + let! sourceText = document.GetTextAsync(cancellationToken) + + let funcStartPos = + let rec loop ch pos = + if not (Char.IsWhiteSpace(ch)) then + pos + else + loop sourceText.[pos + 1] (pos + 1) + + loop sourceText.[span.End + 1] (span.End + 1) + + let funcLexerSymbol = + Tokenizer.getSymbolAtPosition ( + document.Id, + sourceText, + funcStartPos, + document.FilePath, + defines, + SymbolLookupKind.Greedy, + false, + false, + Some langVersion, + cancellationToken + ) + + let funcNameOpt = + funcLexerSymbol + |> Option.bind (fun symbol -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, symbol.Range)) + |> Option.map (fun funcNameSpan -> sourceText.GetSubText(funcNameSpan).ToString()) + + return match funcNameOpt with + | Some funcName -> + String.Format(titleFormat, funcName), [ TextChange(TextSpan(span.End, 0), " rec") ] + | None -> + "", [] + } + + override this.RegisterCodeFixesAsync context = + //asyncMaybe { + // let! defines, langVersion = + // context.Document.GetFSharpCompilationDefinesAndLangVersionAsync( + // nameof (FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider) + // ) + // |> liftAsync + + // let! sourceText = context.Document.GetTextAsync(context.CancellationToken) + + // let funcStartPos = + // let rec loop ch pos = + // if not (Char.IsWhiteSpace(ch)) then + // pos + // else + // loop sourceText.[pos + 1] (pos + 1) + + // loop sourceText.[context.Span.End + 1] (context.Span.End + 1) + + // let! funcLexerSymbol = + // Tokenizer.getSymbolAtPosition ( + // context.Document.Id, + // sourceText, + // funcStartPos, + // context.Document.FilePath, + // defines, + // SymbolLookupKind.Greedy, + // false, + // false, + // Some langVersion, + // context.CancellationToken + // ) + + // let! funcNameSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, funcLexerSymbol.Range) + // let funcName = sourceText.GetSubText(funcNameSpan).ToString() + + // do + // context.RegisterFsharpFix( + // CodeFix.AddMissingRecToMutuallyRecFunctions, + // String.Format(titleFormat, funcName), + // [| TextChange(TextSpan(context.Span.End, 0), " rec") |] + // ) + //} + cancellableTask { + let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span + context.RegisterFsharpFix(CodeFix.AddMissingRecToMutuallyRecFunctions, title, changes) } - |> Async.Ignore - |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) + |> CancellableTask.startAsTask context.CancellationToken + + //|> Async.Ignore + //|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs index bdd0aa5e558..277313546a2 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs @@ -42,6 +42,6 @@ and isOdd n = """ } - let actual = failwith "" // codeFix |> fix code diagnostic + let actual = codeFix |> fix code diagnostic Assert.Equal(expected, actual) \ No newline at end of file From f9c3023506dff17417fd9cecb9990e41f844699e Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 15:55:10 +0200 Subject: [PATCH 08/10] Up --- .../CodeFix/AddInstanceMemberParameter.fs | 24 ++-- .../AddMissingRecToMutuallyRecFunctions.fs | 81 +++---------- .../FSharp.Editor/CodeFix/ChangeToUpcast.fs | 64 +++++------ .../FSharp.Editor/CodeFix/CodeFixHelpers.fs | 12 +- .../CodeFix/ConvertToAnonymousRecord.fs | 32 +++--- .../FSharp.Editor/CodeFix/IFSharpCodeFix.fs | 11 +- .../CodeFix/RemoveReturnOrYield.fs | 56 ++++----- .../CodeFix/WrapExpressionInParentheses.fs | 33 +++--- .../AddInstanceMemberParameterTests.fs | 13 ++- ...ddMissingRecToMutuallyRecFunctionsTests.fs | 17 +-- .../CodeFixes/ChangeToUpcastTests.fs | 43 +++++-- .../CodeFixes/CodeFixTestFramework.fs | 17 +-- .../ConvertToAnonymousRecordTests.fs | 30 +++-- ...ExpressionInParenthesesFixProviderTests.fs | 39 +++++++ .../CodeFixes/RemoveReturnOrYieldTests.fs | 106 ++++++++++++++++++ .../FSharp.Editor.Tests.fsproj | 2 + 16 files changed, 366 insertions(+), 214 deletions(-) create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FSharpWrapExpressionInParenthesesFixProviderTests.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs index e98976184a0..5c7ebfe5995 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs @@ -3,7 +3,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Threading.Tasks open System.Collections.Immutable open Microsoft.CodeAnalysis.Text @@ -17,16 +16,17 @@ type internal FSharpAddInstanceMemberParameterCodeFixProvider() = static let title = SR.AddMissingInstanceMemberParameter() - interface IFSharpCodeFix with - member _.GetChangesAsync _ span = - let changes = [ TextChange(TextSpan(span.Start, 0), "x.") ] - CancellableTask.singleton (title, changes) - override _.FixableDiagnosticIds = ImmutableArray.Create("FS0673") - override this.RegisterCodeFixesAsync context : Task = - cancellableTask { - let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span - context.RegisterFsharpFix(CodeFix.AddInstanceMemberParameter, title, changes) - } - |> CancellableTask.startAsTask context.CancellationToken + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIsAppliesAsync _ span = + let codeFix = + { + Name = CodeFix.AddInstanceMemberParameter + Message = title + Changes = [ TextChange(TextSpan(span.Start, 0), "x.") ] + } + + CancellableTask.singleton (Some codeFix) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs index 01aca735ebd..3d8b7d45814 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs @@ -19,11 +19,13 @@ type internal FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider [ Option.bind (fun symbol -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, symbol.Range)) + |> Option.bind (fun funcLexerSymbol -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, funcLexerSymbol.Range)) |> Option.map (fun funcNameSpan -> sourceText.GetSubText(funcNameSpan).ToString()) - - return match funcNameOpt with - | Some funcName -> - String.Format(titleFormat, funcName), [ TextChange(TextSpan(span.End, 0), " rec") ] - | None -> - "", [] + |> Option.map (fun funcName -> + { + Name = CodeFix.AddMissingRecToMutuallyRecFunctions + Message = String.Format(titleFormat, funcName) + Changes = [ TextChange(TextSpan(span.End, 0), " rec") ] + }) } - - override this.RegisterCodeFixesAsync context = - //asyncMaybe { - // let! defines, langVersion = - // context.Document.GetFSharpCompilationDefinesAndLangVersionAsync( - // nameof (FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider) - // ) - // |> liftAsync - - // let! sourceText = context.Document.GetTextAsync(context.CancellationToken) - - // let funcStartPos = - // let rec loop ch pos = - // if not (Char.IsWhiteSpace(ch)) then - // pos - // else - // loop sourceText.[pos + 1] (pos + 1) - - // loop sourceText.[context.Span.End + 1] (context.Span.End + 1) - - // let! funcLexerSymbol = - // Tokenizer.getSymbolAtPosition ( - // context.Document.Id, - // sourceText, - // funcStartPos, - // context.Document.FilePath, - // defines, - // SymbolLookupKind.Greedy, - // false, - // false, - // Some langVersion, - // context.CancellationToken - // ) - - // let! funcNameSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, funcLexerSymbol.Range) - // let funcName = sourceText.GetSubText(funcNameSpan).ToString() - - // do - // context.RegisterFsharpFix( - // CodeFix.AddMissingRecToMutuallyRecFunctions, - // String.Format(titleFormat, funcName), - // [| TextChange(TextSpan(context.Span.End, 0), " rec") |] - // ) - //} - cancellableTask { - let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span - context.RegisterFsharpFix(CodeFix.AddMissingRecToMutuallyRecFunctions, title, changes) - } - |> CancellableTask.startAsTask context.CancellationToken - - //|> Async.Ignore - //|> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs b/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs index 7136009a928..70dd7381ab4 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs @@ -3,7 +3,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Threading.Tasks open System.Collections.Immutable open Microsoft.CodeAnalysis.Text @@ -17,8 +16,10 @@ type internal FSharpChangeToUpcastCodeFixProvider() = override _.FixableDiagnosticIds = ImmutableArray.Create("FS3198") - interface IFSharpCodeFix with - member _.GetChangesAsync document span = + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIsAppliesAsync document span = cancellableTask { let! cancellationToken = CancellableTask.getCurrentCancellationToken () @@ -29,33 +30,32 @@ type internal FSharpChangeToUpcastCodeFixProvider() = let isDowncastOperator = text.Contains(":?>") let isDowncastKeyword = text.Contains("downcast") - let changes = - [ - if - (isDowncastOperator || isDowncastKeyword) - && not (isDowncastOperator && isDowncastKeyword) - then - let replacement = - if isDowncastOperator then - text.Replace(":?>", ":>") - else - text.Replace("downcast", "upcast") - - TextChange(span, replacement) - ] - - let title = - if isDowncastOperator then - SR.UseUpcastOperator() - else - SR.UseUpcastKeyword() - - return title, changes + if + (isDowncastOperator || isDowncastKeyword) + && not (isDowncastOperator && isDowncastKeyword) + then + let replacement = + if isDowncastOperator then + text.Replace(":?>", ":>") + else + text.Replace("downcast", "upcast") + + let changes = [ TextChange(span, replacement) ] + + let title = + if isDowncastOperator then + SR.UseUpcastOperator() + else + SR.UseUpcastKeyword() + + let codeFix = + { + Name = CodeFix.ChangeToUpcast + Message = title + Changes = changes + } + + return Some codeFix + else + return None } - - override this.RegisterCodeFixesAsync context : Task = - cancellableTask { - let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span - context.RegisterFsharpFix(CodeFix.ChangeToUpcast, title, changes) - } - |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs index 924bf6d38fb..bc5b4848354 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs @@ -2,9 +2,7 @@ namespace Microsoft.VisualStudio.FSharp.Editor -open System open System.Threading -open System.Threading.Tasks open System.Collections.Immutable open System.Diagnostics @@ -15,6 +13,8 @@ open Microsoft.CodeAnalysis.CodeFixes open Microsoft.CodeAnalysis.CodeActions open Microsoft.VisualStudio.FSharp.Editor.Telemetry +open CancellableTasks + [] module internal CodeFixHelpers = let private reportCodeFixTelemetry @@ -80,3 +80,11 @@ module internal CodeFixExtensions = let diag = diagnostics |> Option.defaultValue ctx.Diagnostics ctx.RegisterCodeFix(codeAction, diag) + + member ctx.RegisterFsharpFix(codeFix: IFSharpCodeFixProvider) = + cancellableTask { + match! codeFix.GetCodeFixIsAppliesAsync ctx.Document ctx.Span with + | Some codeFix -> ctx.RegisterFsharpFix(codeFix.Name, codeFix.Message, codeFix.Changes) + | None -> () + } + |> CancellableTask.startAsTask ctx.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs index da39b93e9c3..23c8ef922b2 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs @@ -3,7 +3,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Threading.Tasks open System.Collections.Immutable open Microsoft.CodeAnalysis.Text @@ -19,8 +18,10 @@ type internal FSharpConvertToAnonymousRecordCodeFixProvider [ Option.bind (fun range -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range)) |> Option.map (fun span -> - [ - TextChange(TextSpan(span.Start + 1, 0), "|") - TextChange(TextSpan(span.End - 1, 0), "|") - ]) - |> Option.defaultValue [] - - return title, changes + { + Name = CodeFix.ConvertToAnonymousRecord + Message = title + Changes = + [ + TextChange(TextSpan(span.Start + 1, 0), "|") + TextChange(TextSpan(span.End - 1, 0), "|") + ] + }) } - - override this.RegisterCodeFixesAsync context : Task = - cancellableTask { - let! title, changes = (this :> IFSharpCodeFix).GetChangesAsync context.Document context.Span - return context.RegisterFsharpFix(CodeFix.ConvertToAnonymousRecord, title, changes) - } - |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs b/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs index e0a4fa23a6e..0b2f785d064 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs @@ -7,5 +7,12 @@ open Microsoft.CodeAnalysis.Text open CancellableTasks -type IFSharpCodeFix = - abstract member GetChangesAsync: document: Document -> span: TextSpan -> CancellableTask +type FSharpCodeFix = + { + Name: string + Message: string + Changes: TextChange list + } + +type IFSharpCodeFixProvider = + abstract member GetCodeFixIsAppliesAsync: document: Document -> span: TextSpan -> CancellableTask diff --git a/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs b/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs index f69688fde13..d7f0d5ab8e8 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs @@ -8,40 +8,44 @@ open System.Collections.Immutable open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes +open CancellableTasks + [] type internal FSharpRemoveReturnOrYieldCodeFixProvider [] () = inherit CodeFixProvider() - override _.FixableDiagnosticIds = ImmutableArray.Create("FS0748", "FS0747") + override _.FixableDiagnosticIds = ImmutableArray.Create("FS0747", "FS0748") + + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) - override _.RegisterCodeFixesAsync context = - asyncMaybe { - let! parseResults = - context.Document.GetFSharpParseResultsAsync(nameof (FSharpRemoveReturnOrYieldCodeFixProvider)) - |> liftAsync + interface IFSharpCodeFixProvider with + member _.GetCodeFixIsAppliesAsync document span = + cancellableTask { + let! cancellationToken = CancellableTask.getCurrentCancellationToken () - let! sourceText = context.Document.GetTextAsync(context.CancellationToken) + let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpRemoveReturnOrYieldCodeFixProvider)) - let errorRange = - RoslynHelpers.TextSpanToFSharpRange(context.Document.FilePath, context.Span, sourceText) + let! sourceText = document.GetTextAsync(cancellationToken) - let! exprRange = parseResults.TryRangeOfExprInYieldOrReturn errorRange.Start - let! exprSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, exprRange) + let errorRange = + RoslynHelpers.TextSpanToFSharpRange(document.FilePath, span, sourceText) - let title = - let text = sourceText.GetSubText(context.Span).ToString() + return + parseResults.TryRangeOfExprInYieldOrReturn errorRange.Start + |> Option.bind (fun exprRange -> RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, exprRange)) + |> Option.map (fun exprSpan -> [ TextChange(span, sourceText.GetSubText(exprSpan).ToString()) ]) + |> Option.map (fun changes -> + let title = + let text = sourceText.GetSubText(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() + if text.StartsWith("return!") then SR.RemoveReturnBang() + elif text.StartsWith("return") then SR.RemoveReturn() + elif text.StartsWith("yield!") then SR.RemoveYieldBang() + else SR.RemoveYield() - do - context.RegisterFsharpFix( - CodeFix.RemoveReturnOrYield, - title, - [| TextChange(context.Span, sourceText.GetSubText(exprSpan).ToString()) |] - ) - } - |> Async.Ignore - |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) + { + Name = CodeFix.RemoveReturnOrYield + Message = title + Changes = changes + }) + } diff --git a/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs b/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs index 70ed64f5054..a9c0e12f5b2 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs @@ -3,13 +3,12 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Threading -open System.Threading.Tasks open System.Collections.Immutable -open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes -open Microsoft.CodeAnalysis.CodeActions +open Microsoft.CodeAnalysis.Text + +open CancellableTasks [] type internal FSharpWrapExpressionInParenthesesFixProvider() = @@ -19,13 +18,19 @@ type internal FSharpWrapExpressionInParenthesesFixProvider() = override _.FixableDiagnosticIds = ImmutableArray.Create("FS0597") - override this.RegisterCodeFixesAsync context : Task = - backgroundTask { - let changes = - [ - TextChange(TextSpan(context.Span.Start, 0), "(") - TextChange(TextSpan(context.Span.End + 1, 0), ")") - ] - - context.RegisterFsharpFix(CodeFix.AddParentheses, title, changes) - } + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIsAppliesAsync _ span = + let codeFix = + { + Name = CodeFix.AddParentheses + Message = title + Changes = + [ + TextChange(TextSpan(span.Start, 0), "(") + TextChange(TextSpan(span.End, 0), ")") + ] + } + + CancellableTask.singleton (Some codeFix) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs index 1fbd6b4a39d..6737045f7f1 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddInstanceMemberParameterTests.fs @@ -19,15 +19,16 @@ type UsefulTestHarness() = """ let expected = - { - Title = "Add missing instance member parameter" - FixedCode = - """ + Some + { + Message = "Add missing instance member parameter" + FixedCode = + """ type UsefulTestHarness() = member x.FortyTwo = 42 """ - } + } - let actual = codeFix |> fix code diagnostic + let actual = codeFix |> tryFix code diagnostic Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs index 277313546a2..04e1bbd4929 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddMissingRecToMutuallyRecFunctionsTests.fs @@ -10,6 +10,8 @@ open CodeFixTestFramework let private codeFix = FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider() let private diagnostic = 0576 // The declaration form 'let ... and ...' for non-recursive bindings is not used in F# code... +// TODO: write some negative test cases here + [] let ``Fixes FS0576`` () = let code = @@ -26,10 +28,11 @@ and isOdd n = """ let expected = - { - Title = "Make 'isEven' recursive" - FixedCode = - """ + Some + { + Message = "Make 'isEven' recursive" + FixedCode = + """ let rec isEven n = match n with | 0 -> true @@ -40,8 +43,8 @@ and isOdd n = | 0 -> false | _ -> isEven (n - 1) """ - } + } - let actual = codeFix |> fix code diagnostic + let actual = codeFix |> tryFix code diagnostic - Assert.Equal(expected, actual) \ No newline at end of file + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs index 9b0ead993d7..d47f320d284 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ChangeToUpcastTests.fs @@ -24,18 +24,19 @@ let Thing : IFoo = Foo() :?> IFoo """ let expected = - { - Title = "Use ':>' operator" - FixedCode = - """ + Some + { + Message = "Use ':>' operator" + FixedCode = + """ type IFoo = abstract member Bar : unit -> unit type Foo() = interface IFoo with member __.Bar () = () let Thing : IFoo = Foo() :> IFoo """ - } + } - let actual = codeFix |> fix code diagnostic + let actual = codeFix |> tryFix code diagnostic Assert.Equal(expected, actual) @@ -50,17 +51,35 @@ let Thing : IFoo = downcast Foo() """ let expected = - { - Title = "Use 'upcast'" - FixedCode = - """ + Some + { + Message = "Use 'upcast'" + FixedCode = + """ type IFoo = abstract member Bar : unit -> unit type Foo() = interface IFoo with member __.Bar () = () let Thing : IFoo = upcast Foo() """ - } + } - let actual = codeFix |> fix code diagnostic + let actual = codeFix |> tryFix code diagnostic + + Assert.Equal(expected, actual) + +[] +// TODO: that's a weird thing, we should rather rewrite the code of the code fix +let ``Doesn't fix FS3198 when both`` () = + let code = + """ +type IdowncastFoo = abstract member Bar : unit -> unit +type Foo() = interface IdowncastFoo with member __.Bar () = () + +let Thing : IdowncastFoo = Foo() :?> IdowncastFoo +""" + + let expected = None + + let actual = codeFix |> tryFix code diagnostic Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs index 04998935ea8..e93a99df084 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs @@ -11,7 +11,7 @@ open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks open FSharp.Editor.Tests.Helpers -type TestCodeFix = { Title: string; FixedCode: string } +type TestCodeFix = { Message: string; FixedCode: string } let getRelevantDiagnostic (document: Document) errorNumber = cancellableTask { @@ -23,7 +23,7 @@ let getRelevantDiagnostic (document: Document) errorNumber = |> Seq.exactlyOne } -let fix (code: string) diagnostic (fixProvider: IFSharpCodeFix) = +let tryFix (code: string) diagnostic (fixProvider: IFSharpCodeFixProvider) = cancellableTask { let sourceText = SourceText.From code let document = RoslynTestHelpers.GetFsDocument code @@ -33,14 +33,15 @@ let fix (code: string) diagnostic (fixProvider: IFSharpCodeFix) = let diagnosticSpan = RoslynHelpers.FSharpRangeToTextSpan(sourceText, diagnostic.Range) - let! title, changes = fixProvider.GetChangesAsync document diagnosticSpan - let fixedSourceText = sourceText.WithChanges changes + let! result = fixProvider.GetCodeFixIsAppliesAsync document diagnosticSpan return - { - Title = title - FixedCode = fixedSourceText.ToString() - } + (result + |> Option.map (fun codeFix -> + { + Message = codeFix.Message + FixedCode = (sourceText.WithChanges codeFix.Changes).ToString() + })) } |> CancellableTask.start CancellationToken.None |> fun task -> task.Result diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs index 9f8008356bb..33207bddaab 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/ConvertToAnonymousRecordTests.fs @@ -8,24 +8,38 @@ open Xunit open CodeFixTestFramework let private codeFix = FSharpConvertToAnonymousRecordCodeFixProvider() -let private diagnostic = 0039 // The record label is not defined... +let private diagnostic = 0039 // ... is not defined... [] -let ``Fixes FS0039`` () = +let ``Fixes FS0039 for records`` () = let code = """ let band = { Name = "The Velvet Underground" } """ let expected = - { - Title = "Convert to Anonymous Record" - FixedCode = - """ + Some + { + Message = "Convert to Anonymous Record" + FixedCode = + """ let band = {| Name = "The Velvet Underground" |} """ - } + } - let actual = codeFix |> fix code diagnostic + let actual = codeFix |> tryFix code diagnostic + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS0039 for random undefined identifiers`` () = + let code = + """ +let x = someUndefinedFunction 42 +""" + + let expected = None + + let actual = codeFix |> tryFix code diagnostic Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FSharpWrapExpressionInParenthesesFixProviderTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FSharpWrapExpressionInParenthesesFixProviderTests.fs new file mode 100644 index 00000000000..32009583e06 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/FSharpWrapExpressionInParenthesesFixProviderTests.fs @@ -0,0 +1,39 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.FSharpWrapExpressionInParenthesesFixProviderTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = FSharpWrapExpressionInParenthesesFixProvider() +let private diagnostic = 0597 // ... arguments involving function or method applications should be parenthesized + +// Test case is taken from the original PR: +// https://github.com/dotnet/fsharp/pull/10460 + +[] +let ``Fixes FS0597`` () = + let code = + """ +let rng = System.Random() + +printfn "Hello %d" rng.Next(5) +""" + + let expected = + Some + { + Message = "Wrap expression in parentheses" + FixedCode = + """ +let rng = System.Random() + +printfn "Hello %d" (rng.Next(5)) +""" + } + + let actual = codeFix |> tryFix code diagnostic + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs new file mode 100644 index 00000000000..83c841f198e --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RemoveReturnOrYieldTests.fs @@ -0,0 +1,106 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.RemoveReturnOrYieldTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = FSharpRemoveReturnOrYieldCodeFixProvider() +let private yieldDiagnostic = 0747 // This construct may only be used within list, array and sequence expressions... +let private returnDiagnostic = 0748 // This construct may only be used with computation expressions... + +// TODO: write some negative tests here + +[] +let ``Fixes FS0747 - yield`` () = + let code = + """ +let answer question = + yield 42 +""" + + let expected = + Some + { + Message = "Remove 'yield'" + FixedCode = + """ +let answer question = + 42 +""" + } + + let actual = codeFix |> tryFix code yieldDiagnostic + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0747 - yield!`` () = + let code = + """ +let answer question = + yield! 42 +""" + + let expected = + Some + { + Message = "Remove 'yield!'" + FixedCode = + """ +let answer question = + 42 +""" + } + + let actual = codeFix |> tryFix code yieldDiagnostic + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0748 - return`` () = + let code = + """ +let answer question = + return 42 +""" + + let expected = + Some + { + Message = "Remove 'return'" + FixedCode = + """ +let answer question = + 42 +""" + } + + let actual = codeFix |> tryFix code returnDiagnostic + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0748 - return!`` () = + let code = + """ +let answer question = + return! 42 +""" + + let expected = + Some + { + Message = "Remove 'return!'" + FixedCode = + """ +let answer question = + 42 +""" + } + + let actual = codeFix |> tryFix code returnDiagnostic + + 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 758a979246c..f539937e07b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -37,6 +37,8 @@ + + From 35357f6d74c864d93ba5da96fc6410b838add5dc Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 22:42:22 +0200 Subject: [PATCH 09/10] up --- .../src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs | 2 +- .../CodeFix/AddMissingRecToMutuallyRecFunctions.fs | 2 +- vsintegration/src/FSharp.Editor/CodeFix/ChangeToUpcast.fs | 2 +- vsintegration/src/FSharp.Editor/CodeFix/CodeFixHelpers.fs | 2 +- .../src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs | 2 +- vsintegration/src/FSharp.Editor/CodeFix/IFSharpCodeFix.fs | 2 +- vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs | 2 +- .../src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs index 5c7ebfe5995..36c0ad6f593 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddInstanceMemberParameter.fs @@ -21,7 +21,7 @@ type internal FSharpAddInstanceMemberParameterCodeFixProvider() = override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) interface IFSharpCodeFixProvider with - member _.GetCodeFixIsAppliesAsync _ span = + member _.GetCodeFixIfAppliesAsync _ span = let codeFix = { Name = CodeFix.AddInstanceMemberParameter diff --git a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs index 3d8b7d45814..73c7b9c7413 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/AddMissingRecToMutuallyRecFunctions.fs @@ -22,7 +22,7 @@ type internal FSharpAddMissingRecToMutuallyRecFunctionsCodeFixProvider [ ctx.RegisterFsharpFix(codeFix.Name, codeFix.Message, codeFix.Changes) | None -> () } diff --git a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs index 23c8ef922b2..1b2fbe49211 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/ConvertToAnonymousRecord.fs @@ -21,7 +21,7 @@ type internal FSharpConvertToAnonymousRecordCodeFixProvider [ span: TextSpan -> CancellableTask + abstract member GetCodeFixIfAppliesAsync: document: Document -> span: TextSpan -> CancellableTask diff --git a/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs b/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs index d7f0d5ab8e8..16f33212ef7 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/RemoveReturnOrYield.fs @@ -19,7 +19,7 @@ type internal FSharpRemoveReturnOrYieldCodeFixProvider [] override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) interface IFSharpCodeFixProvider with - member _.GetCodeFixIsAppliesAsync document span = + member _.GetCodeFixIfAppliesAsync document span = cancellableTask { let! cancellationToken = CancellableTask.getCurrentCancellationToken () diff --git a/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs b/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs index a9c0e12f5b2..2c04415d2b2 100644 --- a/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs +++ b/vsintegration/src/FSharp.Editor/CodeFix/WrapExpressionInParentheses.fs @@ -21,7 +21,7 @@ type internal FSharpWrapExpressionInParenthesesFixProvider() = override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix(this) interface IFSharpCodeFixProvider with - member _.GetCodeFixIsAppliesAsync _ span = + member _.GetCodeFixIfAppliesAsync _ span = let codeFix = { Name = CodeFix.AddParentheses From 01334ce1da1c9af0abcf0c6d1a4ce23324311de9 Mon Sep 17 00:00:00 2001 From: Petr Date: Wed, 14 Jun 2023 22:48:56 +0200 Subject: [PATCH 10/10] doh --- .../tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs index e93a99df084..616bc907fe5 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs @@ -33,7 +33,7 @@ let tryFix (code: string) diagnostic (fixProvider: IFSharpCodeFixProvider) = let diagnosticSpan = RoslynHelpers.FSharpRangeToTextSpan(sourceText, diagnostic.Range) - let! result = fixProvider.GetCodeFixIsAppliesAsync document diagnosticSpan + let! result = fixProvider.GetCodeFixIfAppliesAsync document diagnosticSpan return (result