From 4bb43bd834808310d71ff9f883017cd9a90e547d Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Sat, 21 Dec 2024 23:12:21 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Add=20a=20code=20fix=20for=20adding=20missi?= =?UTF-8?q?ng=20seq=20before=20{=E2=80=A6}?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/FsAutoComplete/CodeFixes/AddMissingSeq.fs | 96 +++++++++++++++++++ .../CodeFixes/AddMissingSeq.fsi | 6 ++ .../LspServers/AdaptiveServerState.fs | 5 +- .../CodeFixTests/AddMissingSeqTests.fs | 20 ++++ .../CodeFixTests/Tests.fs | 3 +- 5 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/AddMissingSeq.fs create mode 100644 src/FsAutoComplete/CodeFixes/AddMissingSeq.fsi create mode 100644 test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs diff --git a/src/FsAutoComplete/CodeFixes/AddMissingSeq.fs b/src/FsAutoComplete/CodeFixes/AddMissingSeq.fs new file mode 100644 index 000000000..eb241ef43 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/AddMissingSeq.fs @@ -0,0 +1,96 @@ +module FsAutoComplete.CodeFix.AddMissingSeq + +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax +open FSharp.Compiler.Text +open FsToolkit.ErrorHandling +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete.CodeFix.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +let title = "Add missing 'seq'" + +let fix + (getParseResultsForFile: GetParseResultsForFile) + : CodeFix = + Run.ifDiagnosticByCode (Set.ofList [ "3873"; "740" ]) (fun _ codeActionParams -> + asyncResult { + // Most code fixes have some general setup. + // We initially want to detect the state of the current code and whether we can propose any text edits to the user. + + let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + // The converted LSP start position to an FCS start position. + let fcsPos = protocolPosToPos codeActionParams.Range.Start + // The syntax tree and typed tree, current line and sourceText of the current file. + let! (parseAndCheckResults:ParseAndCheckResults, _line:string, sourceText:IFSACSourceText) = + getParseResultsForFile fileName fcsPos + + // The syntax tree can be an intimidating set of types to work with. + // It is a tree structure but it consists out of many different types. + // See https://fsharp.github.io/fsharp-compiler-docs/reference/fsharp-compiler-syntax.html + // It can be useful to inspect a syntax tree via a code sample using https://fsprojects.github.io/fantomas-tools/#/ast + // For example `let a b c = ()` in + // https://fsprojects.github.io/fantomas-tools/#/ast?data=N4KABGBEAmCmBmBLAdrAzpAXFSAacUiaAYmolmPAIYA2as%2BEkAxgPZwWQ2wAuYVYAEZhmYALxgAFAEo8BSLAAeAByrJoFHgCcArrBABfIA + // Let's say we want to find the (FCS) range for identifier `a` if the user's cursor is inside the function name. + // We will query the syntax tree to verify this is the case. + let maybeCERange = + (fcsPos, parseAndCheckResults.GetParseResults.ParseTree) + ||> ParsedInput.tryPick (fun _path node -> + match node with + // We know that `a` will be part of a `SynPat.LongIdent` + // This was visible in the online tool. + | SyntaxNode.SynExpr(SynExpr.ComputationExpr _) as e when + // When our code fix operates on the user's code there is no way of knowing what will be inside the syntax tree. + // So we need to be careful and verify that the pattern is indeed matching the position of the cursor. + Range.rangeContainsPos e.Range fcsPos + -> + Some e.Range + | _ -> None) + + match maybeCERange with + | None -> + // The cursor is not in a position we are interested in. + // This code fix should not trigger any suggestions so we return an empty list. + return [] + | Some ceExprRange -> + // It turns out we are inside a let binding and we have the range of the function name. + // Just for fun, we want to detect if there is a matching typed tree symbol present for the current name. + // We could have passed the function name from the syntax visitor, instead will we grab it from the source text. + let! sourceText = sourceText.GetText ceExprRange + // FSharpSymbolUse is reflecting the typed tree. + // See https://fsharp.github.io/fsharp-compiler-docs/fcs/symbols.html + // let symbolUse: FSharp.Compiler.CodeAnalysis.FSharpSymbolUse option = + // parseAndCheckResults.GetCheckResults.GetSymbolUseAtLocation(ceExprRange.EndLine, ceExprRange.EndColumn, line, [ functionName ]) + + // let hasCEExprDefinitionSymbol = + // match symbolUse with + // | None -> false + // | Some symbolUse -> + // // We want to verify the found symbol is indeed a definition of a function + // match symbolUse.Symbol with + // | :? FSharpMemberOrFunctionOrValue -> true + // | _ -> false + + // if not hasCEExprDefinitionSymbol then + // return [] + // else + // Return a list of Fix records for when the code fix is applicable. + return [ + { + SourceDiagnostic = None + Title = title + File = codeActionParams.TextDocument + // Based on conditional logic, you typically want to suggest a text edit to the user. + Edits = [| + { + // When dealing with FCS, we typically want to use the FCS flavour of range. + // However, to interact correctly with the LSP protocol, we need to return an LSP range. + Range = fcsRangeToLsp ceExprRange + NewText = $"seq { sourceText }" + } + |] + Kind = FixKind.Fix + } + ] + }) diff --git a/src/FsAutoComplete/CodeFixes/AddMissingSeq.fsi b/src/FsAutoComplete/CodeFixes/AddMissingSeq.fsi new file mode 100644 index 000000000..af063fdce --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/AddMissingSeq.fsi @@ -0,0 +1,6 @@ +module FsAutoComplete.CodeFix.AddMissingSeq + +open FsAutoComplete.CodeFix.Types + +val title: string +val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index ebb226282..e8a96ca1e 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -2194,7 +2194,8 @@ type AdaptiveState AddBindingToSignatureFile.fix forceGetFSharpProjectOptions tryGetParseAndCheckResultsForFile ReplaceLambdaWithDotLambda.fix getLanguageVersion tryGetParseAndCheckResultsForFile IgnoreExpression.fix tryGetParseAndCheckResultsForFile - ExprTypeMismatch.fix tryGetParseAndCheckResultsForFile |]) + ExprTypeMismatch.fix tryGetParseAndCheckResultsForFile + AddMissingSeq.fix tryGetParseAndCheckResultsForFile |]) let forgetDocument (uri: DocumentUri) = async { @@ -2541,4 +2542,4 @@ type AdaptiveState member this.Dispose() = traceNotifications |> Option.iter (dispose) - disposables.Dispose() + disposables.Dispose() \ No newline at end of file diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs new file mode 100644 index 000000000..ce4ecfaf8 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs @@ -0,0 +1,20 @@ +module private FsAutoComplete.Tests.CodeFixTests.AddMissingSeqTests + +open Expecto +open Helpers +open Utils.ServerTests +open Utils.CursorbasedTests +open FsAutoComplete.CodeFix + +let tests state = + serverTestList (nameof AddMissingSeq) state defaultConfigDto None (fun server -> + [ let selectCodeFix = CodeFix.withTitle AddMissingSeq.title + + ftestCaseAsync "first unit test for AddMissingSeq" + <| CodeFix.check + server + "$0{ 1;10 }" + Diagnostics.acceptAll + selectCodeFix + "seq { 1;10 }" + ]) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs index b9ce0bd3e..92a12032d 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs @@ -3605,4 +3605,5 @@ let tests textFactory state = AddBindingToSignatureFileTests.tests state ReplaceLambdaWithDotLambdaTests.tests state IgnoreExpressionTests.tests state - ExprTypeMismatchTests.tests state ] + ExprTypeMismatchTests.tests state + AddMissingSeqTests.tests state ] \ No newline at end of file From aa92446e271b95fa455bbd6f958e68a318c16736 Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Thu, 26 Dec 2024 15:33:13 +0000 Subject: [PATCH 2/3] more tests --- .../CodeFixTests/AddMissingSeqTests.fs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs index ce4ecfaf8..131ffaeb2 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs @@ -10,11 +10,35 @@ let tests state = serverTestList (nameof AddMissingSeq) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle AddMissingSeq.title - ftestCaseAsync "first unit test for AddMissingSeq" + testCaseAsync "FS3873 — Adds missing seq before { start..finish } top level" + <| CodeFix.check + server + "$0{ 1..10 }" + Diagnostics.acceptAll + selectCodeFix + "seq { 1..10 }" + + testCaseAsync "FS3873 — Adds missing seq before { start..finish } binding" + <| CodeFix.check + server + "let xs = $0{ 1;10 }" + Diagnostics.acceptAll + selectCodeFix + "let xs = seq { 1;10 }" + + testCaseAsync "FS0740 — Adds missing seq before { x; y } top level" <| CodeFix.check server "$0{ 1;10 }" Diagnostics.acceptAll selectCodeFix "seq { 1;10 }" + + testCaseAsync "FS0740 — Adds missing seq before { x; y } binding" + <| CodeFix.check + server + "let xs = $0{ 1;10 }" + Diagnostics.acceptAll + selectCodeFix + "let xs = seq { 1;10 }" ]) From aa3d6c5c5cb9ba23197734362fd86bfcb2701b1c Mon Sep 17 00:00:00 2001 From: Edgar Gonzalez Date: Thu, 26 Dec 2024 15:43:45 +0000 Subject: [PATCH 3/3] { start..step..finish } --- .../CodeFixTests/AddMissingSeqTests.fs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs index 131ffaeb2..341406073 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddMissingSeqTests.fs @@ -10,6 +10,22 @@ let tests state = serverTestList (nameof AddMissingSeq) state defaultConfigDto None (fun server -> [ let selectCodeFix = CodeFix.withTitle AddMissingSeq.title + testCaseAsync "FS3873 — Adds missing seq before { start..step..finish } top level" + <| CodeFix.check + server + "$0{ 1..10..20 }" + Diagnostics.acceptAll + selectCodeFix + "seq { 1..10..20 }" + + testCaseAsync "FS3873 — Adds missing seq before { start..step..finish } binding" + <| CodeFix.check + server + "let xs = $0{ 1..10..20 }" + Diagnostics.acceptAll + selectCodeFix + "let xs = seq { 1..10..20 }" + testCaseAsync "FS3873 — Adds missing seq before { start..finish } top level" <| CodeFix.check server @@ -21,10 +37,10 @@ let tests state = testCaseAsync "FS3873 — Adds missing seq before { start..finish } binding" <| CodeFix.check server - "let xs = $0{ 1;10 }" + "let xs = $0{ 1..10 }" Diagnostics.acceptAll selectCodeFix - "let xs = seq { 1;10 }" + "let xs = seq { 1..10 }" testCaseAsync "FS0740 — Adds missing seq before { x; y } top level" <| CodeFix.check