From 070c8cec7d1babaaadee3ecd732b477391bf0712 Mon Sep 17 00:00:00 2001 From: Petr Date: Thu, 3 Aug 2023 16:47:56 +0200 Subject: [PATCH 1/4] Fixing signature-related code fixes --- FSharp.Editor.sln | 8 ++ .../ProjectGeneration.fs | 5 +- .../CodeFixes/RenameParamToMatchSignature.fs | 80 ++++++++----------- .../RenameParamToMatchSignatureTests.fs | 42 ++++++++++ .../FSharp.Editor.Tests.fsproj | 1 + .../Helpers/RoslynHelpers.fs | 27 ++++--- 6 files changed, 103 insertions(+), 60 deletions(-) create mode 100644 vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RenameParamToMatchSignatureTests.fs diff --git a/FSharp.Editor.sln b/FSharp.Editor.sln index 3b03857a6ca..1223a07c99a 100644 --- a/FSharp.Editor.sln +++ b/FSharp.Editor.sln @@ -19,6 +19,8 @@ Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "FSharp.VS.FSI", "vsintegrat EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "FSharp.Editor.IntegrationTests", "vsintegration\tests\FSharp.Editor.IntegrationTests\FSharp.Editor.IntegrationTests.csproj", "{42BE0F2F-BC45-437B-851D-E88A83201339}" EndProject +Project("{6EC3EE1D-3C4E-46DD-8F32-0CC8E7565705}") = "FSharp.Test.Utilities", "tests\FSharp.Test.Utilities\FSharp.Test.Utilities.fsproj", "{B7148170-93C5-4F2F-B31D-85316D3248CF}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -73,6 +75,12 @@ Global {42BE0F2F-BC45-437B-851D-E88A83201339}.Proto|Any CPU.Build.0 = Debug|Any CPU {42BE0F2F-BC45-437B-851D-E88A83201339}.Release|Any CPU.ActiveCfg = Release|Any CPU {42BE0F2F-BC45-437B-851D-E88A83201339}.Release|Any CPU.Build.0 = Release|Any CPU + {B7148170-93C5-4F2F-B31D-85316D3248CF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {B7148170-93C5-4F2F-B31D-85316D3248CF}.Debug|Any CPU.Build.0 = Debug|Any CPU + {B7148170-93C5-4F2F-B31D-85316D3248CF}.Proto|Any CPU.ActiveCfg = Debug|Any CPU + {B7148170-93C5-4F2F-B31D-85316D3248CF}.Proto|Any CPU.Build.0 = Debug|Any CPU + {B7148170-93C5-4F2F-B31D-85316D3248CF}.Release|Any CPU.ActiveCfg = Release|Any CPU + {B7148170-93C5-4F2F-B31D-85316D3248CF}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/tests/FSharp.Test.Utilities/ProjectGeneration.fs b/tests/FSharp.Test.Utilities/ProjectGeneration.fs index 0b4ea1e7aaa..2e0726bc662 100644 --- a/tests/FSharp.Test.Utilities/ProjectGeneration.fs +++ b/tests/FSharp.Test.Utilities/ProjectGeneration.fs @@ -752,10 +752,7 @@ let SaveAndCheckProject project checker = let options = project.GetProjectOptions checker - let! results = checker.ParseAndCheckProject(options) - - if not (Array.isEmpty results.Diagnostics) then - failwith $"Project {project.Name} failed initial check: \n%A{results.Diagnostics}" + let! _ = checker.ParseAndCheckProject(options) let! signatures = Async.Sequential diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/RenameParamToMatchSignature.fs b/vsintegration/src/FSharp.Editor/CodeFixes/RenameParamToMatchSignature.fs index 1dfc34e62bb..a6d8ca2867e 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/RenameParamToMatchSignature.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/RenameParamToMatchSignature.fs @@ -3,8 +3,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System.Composition -open System.Threading -open System.Threading.Tasks open System.Collections.Immutable open System.Text.RegularExpressions @@ -13,11 +11,13 @@ open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes open Microsoft.VisualStudio.FSharp.Editor.SymbolHelpers +open FSharp.Compiler.Diagnostics open FSharp.Compiler.Tokenization.FSharpKeywords +open CancellableTasks + [] type internal RenameParamToMatchSignatureCodeFixProvider [] () = - inherit CodeFixProvider() let getSuggestion (d: Diagnostic) = @@ -28,53 +28,41 @@ type internal RenameParamToMatchSignatureCodeFixProvider [ else ValueNone - override _.FixableDiagnosticIds = ImmutableArray.Create("FS3218") + override _.FixableDiagnosticIds = ImmutableArray.Create "FS3218" + + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this - member this.GetChanges(document: Document, diagnostics: ImmutableArray, ct: CancellationToken) = - backgroundTask { - let! sourceText = document.GetTextAsync(ct) + override this.GetFixAllProvider() = this.RegisterFsharpFixAll() - let! changes = - seq { - for d in diagnostics do - let suggestionOpt = getSuggestion d + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let! sourceText = context.GetSourceTextAsync() - match suggestionOpt with - | ValueSome suggestion -> - let replacement = NormalizeIdentifierBackticks suggestion + let suggestionOpt = getSuggestion context.Diagnostics[0] - async { - let! symbolUses = getSymbolUsesOfSymbolAtLocationInDocument (document, d.Location.SourceSpan.Start) - let symbolUses = symbolUses |> Option.defaultValue [||] + match suggestionOpt with + | ValueSome suggestion -> + let replacement = NormalizeIdentifierBackticks suggestion - return - [| - for symbolUse in symbolUses do - match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, symbolUse.Range) with - | None -> () - | Some span -> - let textSpan = Tokenizer.fixupSpan (sourceText, span) - yield TextChange(textSpan, replacement) - |] + let! symbolUses = getSymbolUsesOfSymbolAtLocationInDocument (context.Document, context.Span.Start) + let symbolUses = symbolUses |> Option.defaultValue [||] + let changes = + [ + for symbolUse in symbolUses do + let span = RoslynHelpers.FSharpRangeToTextSpan(sourceText, symbolUse.Range) + let textSpan = Tokenizer.fixupSpan (sourceText, span) + yield TextChange(textSpan, replacement) + ] + + return + ValueSome + { + Name = CodeFix.FSharpRenameParamToMatchSignature + Message = CompilerDiagnostics.GetErrorMessage(FSharpDiagnosticKind.ReplaceWithSuggestion suggestion) + Changes = changes } - | ValueNone -> () - } - |> Async.Parallel - - return (changes |> Seq.concat) - } - - override this.RegisterCodeFixesAsync ctx : Task = - backgroundTask { - let title = ctx.Diagnostics |> Seq.head |> getSuggestion - - match title with - | ValueSome title -> - let! changes = this.GetChanges(ctx.Document, ctx.Diagnostics, ctx.CancellationToken) - ctx.RegisterFsharpFix(CodeFix.FSharpRenameParamToMatchSignature, title, changes) - | ValueNone -> () - } - - override this.GetFixAllProvider() = - CodeFixHelpers.createFixAllProvider CodeFix.FSharpRenameParamToMatchSignature this.GetChanges + + | ValueNone -> return ValueNone + } diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RenameParamToMatchSignatureTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RenameParamToMatchSignatureTests.fs new file mode 100644 index 00000000000..5b606b2393f --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/RenameParamToMatchSignatureTests.fs @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.RenameParamToMatchSignatureTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = RenameParamToMatchSignatureCodeFixProvider() + +[] +let ``Fixes FS3218`` () = + let fsCode = + """ +module Library + +let id y = y +""" + + let fsiCode = + """ +module Library + +val id: x: 'a -> 'a +""" + + let expected = + Some + { + Message = "Replace with 'x'" + FixedCode = + """ +module Library + +let id x = x +""" + } + + let actual = codeFix |> tryFix fsCode (WithSignature fsiCode) + + 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 439045694d8..65851ab5b71 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -58,6 +58,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 36e70eae0d9..101fe2231e6 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -358,17 +358,24 @@ type RoslynTestHelpers private () = |> RoslynTestHelpers.GetSingleDocument static member GetFsiAndFsDocuments (fsiCode: string) (fsCode: string) = - let projectId = ProjectId.CreateNewId() - let projFilePath = "C:\\test.fsproj" - - let fsiDocInfo = - RoslynTestHelpers.CreateDocumentInfo projectId "C:\\test.fsi" fsiCode - - let fsDocInfo = RoslynTestHelpers.CreateDocumentInfo projectId "C:\\test.fs" fsCode - let projInfo = - RoslynTestHelpers.CreateProjectInfo projectId projFilePath [ fsiDocInfo; fsDocInfo ] + { SyntheticProject.Create( + { sourceFile "test" [] with + SignatureFile = Custom fsiCode + Source = fsCode + } + ) with + + AutoAddModules = false + OtherOptions = + [ + "--targetprofile:netcore" // without this lib some symbols are not loaded + "--nowarn:3384" // The .NET SDK for this script could not be determined + ] + } - let solution = RoslynTestHelpers.CreateSolution [ projInfo ] + let solution, _ = RoslynTestHelpers.CreateSolution projInfo let project = solution.Projects |> Seq.exactlyOne + project.Documents + |> Seq.sortWith (fun d1 _ -> if d1.IsFSharpSignatureFile then -1 else 1) From ee1014648e1a0f50d68acb6442484bda26563db2 Mon Sep 17 00:00:00 2001 From: Petr Date: Fri, 11 Aug 2023 12:59:50 +0200 Subject: [PATCH 2/4] fixing merge issues --- .../FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs index 8f7bbffe642..2098dc76958 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs @@ -20,6 +20,7 @@ type TestCodeFix = { Message: string; FixedCode: string } type Mode = | Auto | WithOption of CustomProjectOption: string + | WithSignature of FsiCode: string | Manual of Squiggly: string * Diagnostic: string let inline toOption o = @@ -41,6 +42,7 @@ let getDocument code mode = match mode with | Auto -> RoslynTestHelpers.GetFsDocument code | WithOption option -> RoslynTestHelpers.GetFsDocument(code, option) + | WithSignature fsiCode -> RoslynTestHelpers.GetFsiAndFsDocuments fsiCode code |> Seq.last | Manual _ -> RoslynTestHelpers.GetFsDocument code let getRelevantDiagnostics (document: Document) = @@ -64,6 +66,7 @@ let createTestCodeFixContext (code: string) document (mode: Mode) diagnosticIds getRelevantDiagnostics document |> Array.filter (fun d -> diagnosticIds |> Seq.contains d.ErrorNumberText) | WithOption _ -> getRelevantDiagnostics document + | WithSignature _ -> getRelevantDiagnostics document | Manual (squiggly, diagnostic) -> let spanStart = code.IndexOf squiggly let span = TextSpan(spanStart, squiggly.Length) From d872a01c0439e040a0dc32492f238114bee3fa96 Mon Sep 17 00:00:00 2001 From: Petr Date: Fri, 11 Aug 2023 13:04:39 +0200 Subject: [PATCH 3/4] Removing unused code --- .../FSharp.Editor/CodeFixes/CodeFixHelpers.fs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs index 6ea37164caa..dec879fd7d8 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs @@ -4,7 +4,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System open System.Threading -open System.Threading.Tasks open System.Collections.Immutable open System.Diagnostics @@ -73,24 +72,6 @@ module internal CodeFixHelpers = TelemetryReporter.ReportSingleEvent(TelemetryEvents.CodefixActivated, props) - let createFixAllProvider name getChanges = - FixAllProvider.Create(fun fixAllCtx doc allDiagnostics -> - backgroundTask { - let sw = Stopwatch.StartNew() - let! (changes: seq) = getChanges (doc, allDiagnostics, fixAllCtx.CancellationToken) - let! text = doc.GetTextAsync(fixAllCtx.CancellationToken) - let doc = doc.WithText(text.WithChanges(changes)) - - do - reportCodeFixTelemetry - allDiagnostics - doc - name - [| "scope", fixAllCtx.Scope.ToString(); "elapsedMs", sw.ElapsedMilliseconds |] - - return doc - }) - let createTextChangeCodeFix (name: string, title: string, context: CodeFixContext, changes: TextChange seq) = CodeAction.Create( title, From fa90465d0343166b89cbebe316406e55536fd41d Mon Sep 17 00:00:00 2001 From: 0101 <0101@innit.cz> Date: Fri, 11 Aug 2023 15:44:41 +0200 Subject: [PATCH 4/4] Returned SyntheticProject initial check and made it optional --- tests/FSharp.Test.Utilities/ProjectGeneration.fs | 12 +++++++++--- .../FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 1 + 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/FSharp.Test.Utilities/ProjectGeneration.fs b/tests/FSharp.Test.Utilities/ProjectGeneration.fs index 2e0726bc662..1d4c882a954 100644 --- a/tests/FSharp.Test.Utilities/ProjectGeneration.fs +++ b/tests/FSharp.Test.Utilities/ProjectGeneration.fs @@ -231,7 +231,9 @@ type SyntheticProject = OtherOptions: string list AutoAddModules: bool NugetReferences: Reference list - FrameworkReferences: Reference list } + FrameworkReferences: Reference list + /// If set to true this project won't cause an exception if there are errors in the initial check + SkipInitialCheck: bool } static member Create(?name: string) = let name = defaultArg name $"TestProject_{Guid.NewGuid().ToString()[..7]}" @@ -245,7 +247,8 @@ type SyntheticProject = OtherOptions = [] AutoAddModules = true NugetReferences = [] - FrameworkReferences = [] } + FrameworkReferences = [] + SkipInitialCheck = false } static member Create([] sourceFiles: SyntheticSourceFile[]) = { SyntheticProject.Create() with SourceFiles = sourceFiles |> List.ofArray } @@ -752,7 +755,10 @@ let SaveAndCheckProject project checker = let options = project.GetProjectOptions checker - let! _ = checker.ParseAndCheckProject(options) + let! results = checker.ParseAndCheckProject(options) + + if not (Array.isEmpty results.Diagnostics || project.SkipInitialCheck) then + failwith $"Project {project.Name} failed initial check: \n%A{results.Diagnostics}" let! signatures = Async.Sequential diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 101fe2231e6..ee069431c98 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -367,6 +367,7 @@ type RoslynTestHelpers private () = ) with AutoAddModules = false + SkipInitialCheck = true OtherOptions = [ "--targetprofile:netcore" // without this lib some symbols are not loaded