From 8347ea1ba1cac6e7f4a5fc20352f4212a344bf5e Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Sep 2023 11:01:42 +0200 Subject: [PATCH 01/11] Initial scaffold of implementation file. --- build/Program.fs | 33 ++-- build/ScaffoldCodeFix.fs | 164 ++++++++++++++++++ build/ScaffoldCodeFix.fsi | 9 + build/build.fsproj | 2 + .../LspServers/AdaptiveFSharpLspServer.fs | 3 +- 5 files changed, 196 insertions(+), 15 deletions(-) create mode 100644 build/ScaffoldCodeFix.fs create mode 100644 build/ScaffoldCodeFix.fsi diff --git a/build/Program.fs b/build/Program.fs index 9584e62ca..0941ad313 100644 --- a/build/Program.fs +++ b/build/Program.fs @@ -71,14 +71,13 @@ let init args = "reportgenerator" "-reports:test/FsAutoComplete.Tests.Lsp/coverage.xml -reporttypes:Html;HtmlSummary -targetdir:./coverage" |> fun r -> - if not r.OK then - failwithf "Errors while generating coverage report: %A" r.Errors) + if not r.OK then + failwithf "Errors while generating coverage report: %A" r.Errors) Target.create "ReleaseArchive" (fun _ -> Directory.ensure pkgsDir - !!(toolsDir "fsautocomplete.*.nupkg") - |> Shell.copy pkgsDir) + !!(toolsDir "fsautocomplete.*.nupkg") |> Shell.copy pkgsDir) Target.create "LocalRelease" (fun _ -> Directory.ensure toolsDir @@ -89,7 +88,9 @@ let init args = { p with OutputPath = Some(__SOURCE_DIRECTORY__ ".." toolsDir) Configuration = DotNet.BuildConfiguration.fromString configuration - MSBuildParams = { MSBuild.CliArguments.Create() with Properties = [ packAsToolProp ] } }) + MSBuildParams = + { MSBuild.CliArguments.Create() with + Properties = [ packAsToolProp ] } }) "src/FsAutoComplete") Target.create "Clean" (fun _ -> Shell.cleanDirs [ buildDir; buildReleaseDir; pkgsDir; toolsDir ]) @@ -98,7 +99,9 @@ let init args = Target.create "Build" (fun _ -> DotNet.build - (fun p -> { p with Configuration = DotNet.BuildConfiguration.fromString configuration }) + (fun p -> + { p with + Configuration = DotNet.BuildConfiguration.fromString configuration }) "FsAutoComplete.sln") Target.create "EnsureRepoConfig" (fun _ -> @@ -170,25 +173,27 @@ let init args = Target.create "Promote" ignore + Target.create "ScaffoldCodeFix" (fun ctx -> + let codeFixName = ctx.Context.Arguments |> List.tryHead + + match codeFixName with + | None -> failwith "Usage: dotnet run --project ./build/build.fsproj -- -t ScaffoldCodeFix " + | Some codeFixName -> ScaffoldCodeFix.scaffold codeFixName) + "PromoteUnreleasedToVersion" ==> "CreateVersionTag" ==> "Promote" |> ignore "Restore" ==> "Build" |> ignore - "Build" ==> "LspTest" ==> "Coverage" ==> "Test" ==> "All" - |> ignore + "Build" ==> "LspTest" ==> "Coverage" ==> "Test" ==> "All" |> ignore - "Clean" - ==> "LocalRelease" - ==> "ReleaseArchive" - ==> "Release" - |> ignore + "Clean" ==> "LocalRelease" ==> "ReleaseArchive" ==> "Release" |> ignore "ReleaseArchive" ==> "All" |> ignore [] let main args = - init ((args |> List.ofArray)) + init (args |> List.ofArray) try Target.runOrDefaultWithArguments "ReleaseArchive" diff --git a/build/ScaffoldCodeFix.fs b/build/ScaffoldCodeFix.fs new file mode 100644 index 000000000..2f232b6f1 --- /dev/null +++ b/build/ScaffoldCodeFix.fs @@ -0,0 +1,164 @@ +module ScaffoldCodeFix + +open System +open System.IO +open Fake.Core +open Fake.IO.FileSystemOperators + +let repositoryRoot = __SOURCE_DIRECTORY__ ".." + +let mkCodeFixImplementation codeFixName = + let path = + repositoryRoot + "src" + "FsAutoComplete" + "CodeFixes" + $"{codeFixName}.fs" + + let content = + $"""module FsAutoComplete.CodeFix.%s{codeFixName} + +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax +open FsToolkit.ErrorHandling +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete.CodeFix.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +// 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`. +let visitSyntaxTree + (cursor: FSharp.Compiler.Text.pos) + (tree: ParsedInput) + = + // We will use a syntax visitor to traverse the tree from the top to the node of interest. + // See https://github.com/dotnet/fsharp/blob/main/src/Compiler/Service/ServiceParseTreeWalk.fsi + // We implement the different members of interest and allow the default traversal to move to the lower levels we care about. + let visitor = + // A visitor will report the first item it finds. + // Think of it as `List.tryPick` + // It is not an ideal solution to find all nodes inside a tree, be aware of that. + // For example finding all function names. + {{ new SyntaxVisitorBase() with + // We know that `a` will be part of a `SynPat.LongIdent` + // This was visible in the online tool. + member _.VisitPat(path, defaultTraverse, synPat) = + match synPat with + | SynPat.LongIdent(longDotId = SynLongIdent(id = [ functionNameIdent ])) -> + // 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. + if FSharp.Compiler.Text.Range.rangeContainsPos functionNameIdent.idRange cursor then + Some functionNameIdent.idRange + else + None + | _ -> None }} + + // Invoke the visitor and kick off the traversal. + SyntaxTraversal.Traverse(cursor, tree, visitor) + +// TODO: add proper title for code fix +let title = "%s{codeFixName} Codefix" + +let fix + (getParseResultsForFile: GetParseResultsForFile) + : CodeFix = + fun (codeActionParams: 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 + + // As an example, we want to check whether the users cursor is inside a function definition name. + // We will traverse the syntax tree to verify this is the case. + match visitSyntaxTree fcsPos parseAndCheckResults.GetParseResults.ParseTree 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 mBindingName -> + // 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! functionName = sourceText.GetText mBindingName + // 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(mBindingName.EndLine, mBindingName.EndColumn, line, [ functionName ]) + + let hasFunctionDefinitionSymbol = + 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 hasFunctionDefinitionSymbol 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 mBindingName + NewText = "Text replaced by %s{codeFixName}" + }} + |] + Kind = FixKind.Fix + }} + ] + }} +""" + + File.WriteAllText(path, content) + +let updateProjectFiles () = + let fsAutoCompleteProject = + repositoryRoot "src" "FsAutoComplete" "FsAutoComplete.fsproj" + + File.SetLastWriteTime(fsAutoCompleteProject, DateTime.Now) + + let fsAutoCompleteTestsLsp = + repositoryRoot + "test" + "FsAutoComplete.Tests.Lsp" + "FsAutoComplete.Tests.Lsp.fsproj" + + File.SetLastWriteTime(fsAutoCompleteTestsLsp, DateTime.Now) + +let scaffold (codeFixName: string) : unit = + // generate files in src/CodeFixes/ + // .fs + // .fsi + mkCodeFixImplementation codeFixName + + // wire up codefix in + // - AdaptiveFSharpLspServer.fs + // - FsAutoComplete.Lsp.fs + + // Add test file in test/FsAutoComplete.Tests.Lsp/CodeFixTests + + // Wire up tests in test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs + + updateProjectFiles () + Trace.tracefn $"Scaffolding %s{codeFixName} complete!" diff --git a/build/ScaffoldCodeFix.fsi b/build/ScaffoldCodeFix.fsi new file mode 100644 index 000000000..3329cb701 --- /dev/null +++ b/build/ScaffoldCodeFix.fsi @@ -0,0 +1,9 @@ +module ScaffoldCodeFix + +/// Scaffold a new CodeFix by: +/// - Generating the implementation and signature files. +/// - Wire up the codefix AdaptiveFSharpLspServer.fs and FsAutoComplete.Lsp.fs. +/// - Generate a tests file with a focused test. +/// - Wire up the tests file. +/// - Update the last write time the project files. +val scaffold: codeFixName: string -> unit diff --git a/build/build.fsproj b/build/build.fsproj index 5525ec55c..129f74bc8 100644 --- a/build/build.fsproj +++ b/build/build.fsproj @@ -5,6 +5,8 @@ net7.0 + + diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index 49cd59c15..9a82aa413 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -1835,7 +1835,8 @@ type AdaptiveFSharpLspServer UseTripleQuotedInterpolation.fix tryGetParseResultsForFile getRangeText RenameParamToMatchSignature.fix tryGetParseResultsForFile RemovePatternArgument.fix tryGetParseResultsForFile - ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion |]) + ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion + UpdateFooBar.fix tryGetParseResultsForFile |]) let forgetDocument (uri: DocumentUri) = async { From c53088927e86b125d65f3b840f26ac9fdca5b2ba Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Sep 2023 11:11:19 +0200 Subject: [PATCH 02/11] Remove temporary code fix addition. --- src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index 9a82aa413..49cd59c15 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -1835,8 +1835,7 @@ type AdaptiveFSharpLspServer UseTripleQuotedInterpolation.fix tryGetParseResultsForFile getRangeText RenameParamToMatchSignature.fix tryGetParseResultsForFile RemovePatternArgument.fix tryGetParseResultsForFile - ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion - UpdateFooBar.fix tryGetParseResultsForFile |]) + ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion |]) let forgetDocument (uri: DocumentUri) = async { From 7d51a7c5c897d92a6e8eeeba4fe62b9a3fa31554 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Sep 2023 13:15:46 +0200 Subject: [PATCH 03/11] Wire up codefixes to LSP servers. --- build/ScaffoldCodeFix.fs | 215 ++++++++++++++++++++++++++++++++++++++- build/paket.references | 1 + paket.dependencies | 1 + paket.lock | 17 +++- 4 files changed, 228 insertions(+), 6 deletions(-) diff --git a/build/ScaffoldCodeFix.fs b/build/ScaffoldCodeFix.fs index 2f232b6f1..1d2a9a944 100644 --- a/build/ScaffoldCodeFix.fs +++ b/build/ScaffoldCodeFix.fs @@ -4,6 +4,7 @@ open System open System.IO open Fake.Core open Fake.IO.FileSystemOperators +open Fantomas.Core.SyntaxOak let repositoryRoot = __SOURCE_DIRECTORY__ ".." @@ -131,6 +132,27 @@ let fix """ File.WriteAllText(path, content) + Trace.tracefn $"Generated %s{Path.GetRelativePath(repositoryRoot, path)}" + +let mkCodeFixSignature codeFixName = + let path = + repositoryRoot + "src" + "FsAutoComplete" + "CodeFixes" + $"{codeFixName}.fsi" + + let content = + $"""module FsAutoComplete.CodeFix.%s{codeFixName} + +open FsAutoComplete.CodeFix.Types + +val title: string +val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix +""" + + File.WriteAllText(path, content) + Trace.tracefn $"Generated %s{Path.GetRelativePath(repositoryRoot, path)}" let updateProjectFiles () = let fsAutoCompleteProject = @@ -146,15 +168,195 @@ let updateProjectFiles () = File.SetLastWriteTime(fsAutoCompleteTestsLsp, DateTime.Now) +let (|IdentName|_|) (name: string) (identListNode: IdentListNode) = + match identListNode.Content with + | [ IdentifierOrDot.Ident stn ] when stn.Text = name -> Some() + | _ -> None + +let getOakFor path = + let content = File.ReadAllText path + + Fantomas.Core.CodeFormatter.ParseOakAsync(false, content) + |> Async.RunSynchronously + |> Array.head + |> fst + +let appendItemToArray codeFixName path (array: ExprArrayOrListNode) = + let lastElement = array.Elements |> List.last |> Expr.Node + let startIndent = lastElement.Range.StartColumn + let lineIdx = lastElement.Range.EndLine - 1 + let arrayEndsOnLastElement = array.Range.EndLine = lastElement.Range.EndLine + + let updatedLines = + let lines = File.ReadAllLines path + let currentLastLine = lines.[lineIdx] + let spaces = String.replicate startIndent " " + + if arrayEndsOnLastElement then + let endOfLastElement = currentLastLine.Substring(0, lastElement.Range.EndColumn) + let endOfArray = currentLastLine.Substring(lastElement.Range.EndColumn) + + lines + |> Array.updateAt + lineIdx + $"{endOfLastElement}\n%s{spaces}%s{codeFixName}.fix tryGetParseResultsForFile%s{endOfArray}" + else + lines + |> Array.insertAt (lineIdx + 1) $"%s{spaces}%s{codeFixName}.fix tryGetParseResultsForFile" + + File.WriteAllLines(path, updatedLines) + +let wireCodeFixInAdaptiveFSharpLspServer codeFixName = + let path = + repositoryRoot + "src" + "FsAutoComplete" + "LspServers" + "AdaptiveFSharpLspServer.fs" + + try + let oak = getOakFor path + + // namespace FsAutoComplete.Lsp + let ns = oak.ModulesOrNamespaces |> List.exactlyOne + + // type AdaptiveFSharpLspServer + let t = + ns.Declarations + |> List.pick (function + | ModuleDecl.TypeDefn t -> + let tdn = TypeDefn.TypeDefnNode t + + match tdn.TypeName.Identifier with + | IdentName "AdaptiveFSharpLspServer" -> Some tdn + | _ -> None + | _ -> None) + + // let codefixes = + let codefixesValue = + t.Members + |> List.pick (function + | MemberDefn.LetBinding bindingList -> + match bindingList.Bindings with + | bindings -> + bindings + |> List.tryPick (fun binding -> + match binding.FunctionName with + | Choice1Of2(IdentName "codefixes") -> Some binding + | _ -> None) + | _ -> None) + + let infixApp = + match codefixesValue.Expr with + | Expr.CompExprBody body -> + match List.last body.Statements with + | ComputationExpressionStatement.OtherStatement other -> + match other with + | Expr.InfixApp infixApp -> infixApp + | _ -> raise (exn "Expected |> infix operator") + | _ -> raise (exn "Expected |> infix operator") + | _ -> raise (exn "Expected |> infix operator") + + let appWithLambda = + match infixApp.RightHandSide with + | Expr.AppWithLambda appWithLambda -> appWithLambda + | _ -> raise (exn "Expected function with lambda") + + let lambda = + match appWithLambda.Lambda with + | Choice1Of2 lambda -> lambda + | Choice2Of2 _ -> raise (exn "Expected lambda") + + let array = + match lambda.Expr with + | Expr.ArrayOrList array -> array + | _ -> raise (exn "Expected array") + + appendItemToArray codeFixName path array + with ex -> + Trace.traceException ex + Trace.traceError $"Unable to find array of codefixes in %s{path}.\nDid the code structure change?" + +let wireCodeFixInFsAutoCompleteLsp codeFixName = + let path = + repositoryRoot + "src" + "FsAutoComplete" + "LspServers" + "FsAutoComplete.Lsp.fs" + + try + let oak = getOakFor path + // namespace FsAutoComplete.Lsp + let ns = oak.ModulesOrNamespaces |> List.exactlyOne + + // type AdaptiveFSharpLspServer + let t = + ns.Declarations + |> List.pick (function + | ModuleDecl.TypeDefn t -> + let tdn = TypeDefn.TypeDefnNode t + + match tdn.TypeName.Identifier with + | IdentName "FSharpLspServer" -> Some tdn + | _ -> None + | _ -> None) + + // interface IFSharpLspServer with + let iFSharpLspServer = + t.Members + |> List.pick (function + | MemberDefn.Interface i -> Some i + | _ -> None) + + // override _.Initialize(p: InitializeParams) = + let overrideMember = + iFSharpLspServer.Members + |> List.pick (function + | MemberDefn.Member mb -> + match mb.FunctionName with + | Choice1Of2 iln -> + match iln.Content with + | [ _; _; IdentifierOrDot.Ident ident ] when ident.Text = "Initialize" -> Some mb + | _ -> None + | Choice2Of2 _ -> None + | _ -> None) + + let asyncComp = + match overrideMember.Expr with + | Expr.NamedComputation namedComputation -> namedComputation + | e -> failwithf "Expected Expr.NamedComputation, got %A" e + + let compBody = + match asyncComp.Body with + | Expr.CompExprBody body -> body + | e -> failwithf "Expected Expr.CompExprBody, got %A" e + + let array = + compBody.Statements + |> List.pick (function + | ComputationExpressionStatement.OtherStatement(Expr.LongIdentSet longIdentSet) -> + match longIdentSet.Identifier with + | IdentName "codefixes" -> + match longIdentSet.Expr with + | Expr.ArrayOrList array -> Some array + | _ -> None + | _ -> None + | _ -> None) + + appendItemToArray codeFixName path array + with ex -> + Trace.traceException ex + Trace.traceError $"Unable to find array of codefixes in %s{path}.\nDid the code structure change?" + let scaffold (codeFixName: string) : unit = // generate files in src/CodeFixes/ - // .fs - // .fsi mkCodeFixImplementation codeFixName + mkCodeFixSignature codeFixName - // wire up codefix in - // - AdaptiveFSharpLspServer.fs - // - FsAutoComplete.Lsp.fs + // Wire up codefix to LSP servers + wireCodeFixInAdaptiveFSharpLspServer codeFixName + wireCodeFixInFsAutoCompleteLsp codeFixName // Add test file in test/FsAutoComplete.Tests.Lsp/CodeFixTests @@ -162,3 +364,6 @@ let scaffold (codeFixName: string) : unit = updateProjectFiles () Trace.tracefn $"Scaffolding %s{codeFixName} complete!" + +// TODO: introduce a target that verifies the codefix can still be added. +// By checking the AST can still reach the entry points of the lists. diff --git a/build/paket.references b/build/paket.references index 84895ac7b..aa0513d00 100644 --- a/build/paket.references +++ b/build/paket.references @@ -15,3 +15,4 @@ group Build Microsoft.Build MSBuild.StructuredLogger Octokit + Fantomas.Core diff --git a/paket.dependencies b/paket.dependencies index e34f8d8f7..a604c2537 100644 --- a/paket.dependencies +++ b/paket.dependencies @@ -82,3 +82,4 @@ group Build nuget Microsoft.Build nuget MSBuild.StructuredLogger nuget Octokit 0.48 // there's an API break in 0.50, so we need to pin this to prevent errors + nuget Fantomas.Core 6.2.0 diff --git a/paket.lock b/paket.lock index 5e908fc45..91ba0bff7 100644 --- a/paket.lock +++ b/paket.lock @@ -613,6 +613,14 @@ NUGET Fake.Core.Trace (>= 5.23.1) Fake.IO.FileSystem (>= 5.23.1) FSharp.Core (>= 6.0) + Fantomas.Core (6.2) + Fantomas.FCS (>= 6.2) + FSharp.Core (>= 6.0.1) + Fantomas.FCS (6.2) + FSharp.Core (>= 6.0.1) + System.Diagnostics.DiagnosticSource (>= 7.0) + System.Memory (>= 4.5.5) + System.Runtime (>= 4.3.1) FParsec (1.1.1) FSharp.Core (>= 4.3.4) FSharp.Control.Reactive (5.0.5) @@ -647,6 +655,8 @@ NUGET System.Security.Principal.Windows (>= 5.0) - restriction: || (== net6.0) (&& (== net7.0) (>= net472)) System.Text.Encoding.CodePages (>= 7.0) - restriction: == net6.0 Microsoft.NET.StringTools (17.6.3) + Microsoft.NETCore.Platforms (7.0.4) + Microsoft.NETCore.Targets (5.0) Microsoft.VisualStudio.Setup.Configuration.Interop (3.6.2115) - restriction: || (&& (== net6.0) (>= net472)) (&& (== net6.0) (>= net7.0)) (== net7.0) Microsoft.Win32.Registry (5.0) System.Security.AccessControl (>= 5.0) @@ -678,17 +688,22 @@ NUGET System.Diagnostics.EventLog (>= 7.0) - restriction: || (&& (== net6.0) (>= net7.0)) (== net7.0) System.Security.Cryptography.ProtectedData (>= 7.0) System.Security.Permissions (>= 7.0) + System.Diagnostics.DiagnosticSource (7.0.2) + System.Runtime.CompilerServices.Unsafe (>= 6.0) - restriction: || (== net6.0) (&& (== net7.0) (>= net462)) (&& (== net7.0) (< net6.0)) System.Diagnostics.EventLog (7.0) - restriction: || (&& (== net6.0) (>= net7.0)) (== net7.0) System.Drawing.Common (7.0) Microsoft.Win32.SystemEvents (>= 7.0) System.Formats.Asn1 (6.0) - System.Memory (4.5.5) - restriction: || (== net6.0) (&& (== net7.0) (>= net472)) + System.Memory (4.5.5) System.Reactive (5.0) System.Reflection.Metadata (6.0.1) - restriction: || (&& (== net6.0) (>= net7.0)) (== net7.0) System.Collections.Immutable (>= 6.0) System.Reflection.MetadataLoadContext (6.0) - restriction: || (&& (== net6.0) (>= net472)) (&& (== net6.0) (>= net7.0)) (== net7.0) System.Collections.Immutable (>= 6.0) System.Reflection.Metadata (>= 6.0) + System.Runtime (4.3.1) + Microsoft.NETCore.Platforms (>= 1.1.1) + Microsoft.NETCore.Targets (>= 1.1.3) System.Runtime.CompilerServices.Unsafe (6.0) System.Security.AccessControl (6.0) - restriction: == net6.0 System.Security.Cryptography.Cng (5.0) From ff611809ad26322876cf25348b62d60b2bbeb480 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Sep 2023 17:53:22 +0200 Subject: [PATCH 04/11] Add dedicated test file and wire tests. --- build/ScaffoldCodeFix.fs | 100 ++++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 13 deletions(-) diff --git a/build/ScaffoldCodeFix.fs b/build/ScaffoldCodeFix.fs index 1d2a9a944..e3e36bbb9 100644 --- a/build/ScaffoldCodeFix.fs +++ b/build/ScaffoldCodeFix.fs @@ -8,6 +8,8 @@ open Fantomas.Core.SyntaxOak let repositoryRoot = __SOURCE_DIRECTORY__ ".." +let removeReturnCarriage (v: string) = v.Replace("\r", "") + let mkCodeFixImplementation codeFixName = let path = repositoryRoot @@ -131,7 +133,7 @@ let fix }} """ - File.WriteAllText(path, content) + File.WriteAllText(path, removeReturnCarriage content) Trace.tracefn $"Generated %s{Path.GetRelativePath(repositoryRoot, path)}" let mkCodeFixSignature codeFixName = @@ -151,7 +153,7 @@ val title: string val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix """ - File.WriteAllText(path, content) + File.WriteAllText(path, removeReturnCarriage content) Trace.tracefn $"Generated %s{Path.GetRelativePath(repositoryRoot, path)}" let updateProjectFiles () = @@ -181,11 +183,11 @@ let getOakFor path = |> Array.head |> fst -let appendItemToArray codeFixName path (array: ExprArrayOrListNode) = - let lastElement = array.Elements |> List.last |> Expr.Node +let appendItemToArrayOrList item path (node: ExprArrayOrListNode) = + let lastElement = node.Elements |> List.last |> Expr.Node let startIndent = lastElement.Range.StartColumn let lineIdx = lastElement.Range.EndLine - 1 - let arrayEndsOnLastElement = array.Range.EndLine = lastElement.Range.EndLine + let arrayEndsOnLastElement = node.Range.EndLine = lastElement.Range.EndLine let updatedLines = let lines = File.ReadAllLines path @@ -197,14 +199,12 @@ let appendItemToArray codeFixName path (array: ExprArrayOrListNode) = let endOfArray = currentLastLine.Substring(lastElement.Range.EndColumn) lines - |> Array.updateAt - lineIdx - $"{endOfLastElement}\n%s{spaces}%s{codeFixName}.fix tryGetParseResultsForFile%s{endOfArray}" + |> Array.updateAt lineIdx $"{endOfLastElement}\n%s{spaces}%s{item}%s{endOfArray}" else - lines - |> Array.insertAt (lineIdx + 1) $"%s{spaces}%s{codeFixName}.fix tryGetParseResultsForFile" + lines |> Array.insertAt (lineIdx + 1) $"%s{spaces}%s{item}" File.WriteAllLines(path, updatedLines) + Trace.tracefn $"Added \"%s{item}\" to %s{Path.GetRelativePath(repositoryRoot, path)}" let wireCodeFixInAdaptiveFSharpLspServer codeFixName = let path = @@ -272,7 +272,7 @@ let wireCodeFixInAdaptiveFSharpLspServer codeFixName = | Expr.ArrayOrList array -> array | _ -> raise (exn "Expected array") - appendItemToArray codeFixName path array + appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" path array with ex -> Trace.traceException ex Trace.traceError $"Unable to find array of codefixes in %s{path}.\nDid the code structure change?" @@ -344,11 +344,83 @@ let wireCodeFixInFsAutoCompleteLsp codeFixName = | _ -> None | _ -> None) - appendItemToArray codeFixName path array + appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" path array with ex -> Trace.traceException ex Trace.traceError $"Unable to find array of codefixes in %s{path}.\nDid the code structure change?" +let mkCodeFixTests codeFixName = + let path = + repositoryRoot + "test" + "FsAutoComplete.Tests.Lsp" + "CodeFixTests" + $"%s{codeFixName}Tests.fs" + + let contents = + $"module private FsAutoComplete.Tests.CodeFixTests.%s{codeFixName}Tests + +open Expecto +open Helpers +open Utils.ServerTests +open Utils.CursorbasedTests +open FsAutoComplete.CodeFix + +let tests state = + serverTestList (nameof %s{codeFixName}) state defaultConfigDto None (fun server -> + [ let selectCodeFix = CodeFix.withTitle %s{codeFixName}.title + + ftestCaseAsync \"first unit test for %s{codeFixName}\" + <| CodeFix.check + server + \"let a$0 b c = ()\" + Diagnostics.acceptAll + selectCodeFix + \"let Text replaced by %s{codeFixName} b c = ()\" + ]) +" + + File.WriteAllText(path, removeReturnCarriage contents) + Trace.tracefn $"Generated %s{Path.GetRelativePath(repositoryRoot, path)}" + +let wireCodeFixTests codeFixName = + let path = + repositoryRoot + "test" + "FsAutoComplete.Tests.Lsp" + "CodeFixTests" + "Tests.fs" + + try + let oak = getOakFor path + // module FsAutoComplete.Tests.CodeFixTests.Tests + let testsModule = oak.ModulesOrNamespaces |> List.exactlyOne + + // let tests state = + let testBinding = + testsModule.Declarations + |> List.pick (function + | ModuleDecl.TopLevelBinding binding -> + match binding.FunctionName with + | Choice1Of2(IdentName "tests") -> Some binding + | _ -> None + | _ -> None) + + let appNode = + match testBinding.Expr with + | Expr.App appNode -> appNode + | e -> failwithf "Expected Expr.App, got %A" e + + let list = + match List.last appNode.Arguments with + | Expr.ArrayOrList list -> list + | e -> failwithf "Expected Expr.ArrayOrList, got %A" e + + appendItemToArrayOrList $"%s{codeFixName}Tests.tests state" path list + with ex -> + Trace.traceException ex + Trace.traceError $"Unable to find array of tests in %s{path}.\nDid the code structure change?" + let scaffold (codeFixName: string) : unit = // generate files in src/CodeFixes/ mkCodeFixImplementation codeFixName @@ -358,9 +430,11 @@ let scaffold (codeFixName: string) : unit = wireCodeFixInAdaptiveFSharpLspServer codeFixName wireCodeFixInFsAutoCompleteLsp codeFixName - // Add test file in test/FsAutoComplete.Tests.Lsp/CodeFixTests + // Add test file + mkCodeFixTests codeFixName // Wire up tests in test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs + wireCodeFixTests codeFixName updateProjectFiles () Trace.tracefn $"Scaffolding %s{codeFixName} complete!" From 194fe5d90916b1097b7135ca5de542041b6172ca Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Sep 2023 17:53:52 +0200 Subject: [PATCH 05/11] Sample of usage --- src/FsAutoComplete/CodeFixes/UpdateFooBar.fs | 112 ++++++++++++++++++ src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi | 6 + .../LspServers/AdaptiveFSharpLspServer.fs | 3 +- .../LspServers/FsAutoComplete.Lsp.fs | 1 + .../CodeFixTests/Tests.fs | 5 +- .../CodeFixTests/UpdateFooBarTests.fs | 20 ++++ 6 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/UpdateFooBar.fs create mode 100644 src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi create mode 100644 test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs diff --git a/src/FsAutoComplete/CodeFixes/UpdateFooBar.fs b/src/FsAutoComplete/CodeFixes/UpdateFooBar.fs new file mode 100644 index 000000000..e0d600eb6 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/UpdateFooBar.fs @@ -0,0 +1,112 @@ +module FsAutoComplete.CodeFix.UpdateFooBar + +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax +open FsToolkit.ErrorHandling +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete.CodeFix.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +// 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`. +let visitSyntaxTree + (cursor: FSharp.Compiler.Text.pos) + (tree: ParsedInput) + = + // We will use a syntax visitor to traverse the tree from the top to the node of interest. + // See https://github.com/dotnet/fsharp/blob/main/src/Compiler/Service/ServiceParseTreeWalk.fsi + // We implement the different members of interest and allow the default traversal to move to the lower levels we care about. + let visitor = + // A visitor will report the first item it finds. + // Think of it as `List.tryPick` + // It is not an ideal solution to find all nodes inside a tree, be aware of that. + // For example finding all function names. + { new SyntaxVisitorBase() with + // We know that `a` will be part of a `SynPat.LongIdent` + // This was visible in the online tool. + member _.VisitPat(path, defaultTraverse, synPat) = + match synPat with + | SynPat.LongIdent(longDotId = SynLongIdent(id = [ functionNameIdent ])) -> + // 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. + if FSharp.Compiler.Text.Range.rangeContainsPos functionNameIdent.idRange cursor then + Some functionNameIdent.idRange + else + None + | _ -> None } + + // Invoke the visitor and kick off the traversal. + SyntaxTraversal.Traverse(cursor, tree, visitor) + +// TODO: add proper title for code fix +let title = "UpdateFooBar Codefix" + +let fix + (getParseResultsForFile: GetParseResultsForFile) + : CodeFix = + fun (codeActionParams: 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 + + // As an example, we want to check whether the users cursor is inside a function definition name. + // We will traverse the syntax tree to verify this is the case. + match visitSyntaxTree fcsPos parseAndCheckResults.GetParseResults.ParseTree 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 mBindingName -> + // 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! functionName = sourceText.GetText mBindingName + // 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(mBindingName.EndLine, mBindingName.EndColumn, line, [ functionName ]) + + let hasFunctionDefinitionSymbol = + 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 hasFunctionDefinitionSymbol 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 mBindingName + NewText = "Text replaced by UpdateFooBar" + } + |] + Kind = FixKind.Fix + } + ] + } diff --git a/src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi b/src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi new file mode 100644 index 000000000..9e7029d69 --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi @@ -0,0 +1,6 @@ +module FsAutoComplete.CodeFix.UpdateFooBar + +open FsAutoComplete.CodeFix.Types + +val title: string +val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index 49cd59c15..9a82aa413 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -1835,7 +1835,8 @@ type AdaptiveFSharpLspServer UseTripleQuotedInterpolation.fix tryGetParseResultsForFile getRangeText RenameParamToMatchSignature.fix tryGetParseResultsForFile RemovePatternArgument.fix tryGetParseResultsForFile - ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion |]) + ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion + UpdateFooBar.fix tryGetParseResultsForFile |]) let forgetDocument (uri: DocumentUri) = async { diff --git a/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs index b11db9eee..29d9e85f0 100644 --- a/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs @@ -1173,6 +1173,7 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient, sourceTextFactory RenameParamToMatchSignature.fix tryGetParseResultsForFile RemovePatternArgument.fix tryGetParseResultsForFile ToInterpolatedString.fix tryGetParseResultsForFile tryGetLanguageVersion + UpdateFooBar.fix tryGetParseResultsForFile |] diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs index 9230d36a8..3e95152ce 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs @@ -3333,7 +3333,7 @@ let tests state = convertTripleSlashCommentToXmlTaggedDocTests state addPrivateAccessModifierTests state GenerateAbstractClassStubTests.tests state - generateRecordStubTests state + // generateRecordStubTests state generateUnionCasesTests state generateXmlDocumentationTests state ImplementInterfaceTests.tests state @@ -3351,4 +3351,5 @@ let tests state = useTripleQuotedInterpolationTests state wrapExpressionInParenthesesTests state removeRedundantAttributeSuffixTests state - removePatternArgumentTests state ] + removePatternArgumentTests state + UpdateFooBarTests.tests state ] diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs new file mode 100644 index 000000000..dc1d91eff --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs @@ -0,0 +1,20 @@ +module private FsAutoComplete.Tests.CodeFixTests.UpdateFooBarTests + +open Expecto +open Helpers +open Utils.ServerTests +open Utils.CursorbasedTests +open FsAutoComplete.CodeFix + +let tests state = + serverTestList (nameof UpdateFooBar) state defaultConfigDto None (fun server -> + [ let selectCodeFix = CodeFix.withTitle UpdateFooBar.title + + ftestCaseAsync "first unit test for UpdateFooBar" + <| CodeFix.check + server + "let a$0 b c = ()" + Diagnostics.acceptAll + selectCodeFix + "let Text replaced by UpdateFooBar b c = ()" + ]) From 3da98e3086b4237e4166511d67ca2b980b866367 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 4 Sep 2023 17:54:04 +0200 Subject: [PATCH 06/11] Revert "Sample of usage" This reverts commit 194fe5d90916b1097b7135ca5de542041b6172ca. --- src/FsAutoComplete/CodeFixes/UpdateFooBar.fs | 112 ------------------ src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi | 6 - .../LspServers/AdaptiveFSharpLspServer.fs | 3 +- .../LspServers/FsAutoComplete.Lsp.fs | 1 - .../CodeFixTests/Tests.fs | 5 +- .../CodeFixTests/UpdateFooBarTests.fs | 20 ---- 6 files changed, 3 insertions(+), 144 deletions(-) delete mode 100644 src/FsAutoComplete/CodeFixes/UpdateFooBar.fs delete mode 100644 src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi delete mode 100644 test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs diff --git a/src/FsAutoComplete/CodeFixes/UpdateFooBar.fs b/src/FsAutoComplete/CodeFixes/UpdateFooBar.fs deleted file mode 100644 index e0d600eb6..000000000 --- a/src/FsAutoComplete/CodeFixes/UpdateFooBar.fs +++ /dev/null @@ -1,112 +0,0 @@ -module FsAutoComplete.CodeFix.UpdateFooBar - -open FSharp.Compiler.Symbols -open FSharp.Compiler.Syntax -open FsToolkit.ErrorHandling -open Ionide.LanguageServerProtocol.Types -open FsAutoComplete.CodeFix.Types -open FsAutoComplete -open FsAutoComplete.LspHelpers - -// 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`. -let visitSyntaxTree - (cursor: FSharp.Compiler.Text.pos) - (tree: ParsedInput) - = - // We will use a syntax visitor to traverse the tree from the top to the node of interest. - // See https://github.com/dotnet/fsharp/blob/main/src/Compiler/Service/ServiceParseTreeWalk.fsi - // We implement the different members of interest and allow the default traversal to move to the lower levels we care about. - let visitor = - // A visitor will report the first item it finds. - // Think of it as `List.tryPick` - // It is not an ideal solution to find all nodes inside a tree, be aware of that. - // For example finding all function names. - { new SyntaxVisitorBase() with - // We know that `a` will be part of a `SynPat.LongIdent` - // This was visible in the online tool. - member _.VisitPat(path, defaultTraverse, synPat) = - match synPat with - | SynPat.LongIdent(longDotId = SynLongIdent(id = [ functionNameIdent ])) -> - // 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. - if FSharp.Compiler.Text.Range.rangeContainsPos functionNameIdent.idRange cursor then - Some functionNameIdent.idRange - else - None - | _ -> None } - - // Invoke the visitor and kick off the traversal. - SyntaxTraversal.Traverse(cursor, tree, visitor) - -// TODO: add proper title for code fix -let title = "UpdateFooBar Codefix" - -let fix - (getParseResultsForFile: GetParseResultsForFile) - : CodeFix = - fun (codeActionParams: 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 - - // As an example, we want to check whether the users cursor is inside a function definition name. - // We will traverse the syntax tree to verify this is the case. - match visitSyntaxTree fcsPos parseAndCheckResults.GetParseResults.ParseTree 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 mBindingName -> - // 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! functionName = sourceText.GetText mBindingName - // 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(mBindingName.EndLine, mBindingName.EndColumn, line, [ functionName ]) - - let hasFunctionDefinitionSymbol = - 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 hasFunctionDefinitionSymbol 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 mBindingName - NewText = "Text replaced by UpdateFooBar" - } - |] - Kind = FixKind.Fix - } - ] - } diff --git a/src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi b/src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi deleted file mode 100644 index 9e7029d69..000000000 --- a/src/FsAutoComplete/CodeFixes/UpdateFooBar.fsi +++ /dev/null @@ -1,6 +0,0 @@ -module FsAutoComplete.CodeFix.UpdateFooBar - -open FsAutoComplete.CodeFix.Types - -val title: string -val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index 9a82aa413..49cd59c15 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -1835,8 +1835,7 @@ type AdaptiveFSharpLspServer UseTripleQuotedInterpolation.fix tryGetParseResultsForFile getRangeText RenameParamToMatchSignature.fix tryGetParseResultsForFile RemovePatternArgument.fix tryGetParseResultsForFile - ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion - UpdateFooBar.fix tryGetParseResultsForFile |]) + ToInterpolatedString.fix tryGetParseResultsForFile getLanguageVersion |]) let forgetDocument (uri: DocumentUri) = async { diff --git a/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs index 29d9e85f0..b11db9eee 100644 --- a/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs @@ -1173,7 +1173,6 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient, sourceTextFactory RenameParamToMatchSignature.fix tryGetParseResultsForFile RemovePatternArgument.fix tryGetParseResultsForFile ToInterpolatedString.fix tryGetParseResultsForFile tryGetLanguageVersion - UpdateFooBar.fix tryGetParseResultsForFile |] diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs index 3e95152ce..9230d36a8 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs @@ -3333,7 +3333,7 @@ let tests state = convertTripleSlashCommentToXmlTaggedDocTests state addPrivateAccessModifierTests state GenerateAbstractClassStubTests.tests state - // generateRecordStubTests state + generateRecordStubTests state generateUnionCasesTests state generateXmlDocumentationTests state ImplementInterfaceTests.tests state @@ -3351,5 +3351,4 @@ let tests state = useTripleQuotedInterpolationTests state wrapExpressionInParenthesesTests state removeRedundantAttributeSuffixTests state - removePatternArgumentTests state - UpdateFooBarTests.tests state ] + removePatternArgumentTests state ] diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs deleted file mode 100644 index dc1d91eff..000000000 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateFooBarTests.fs +++ /dev/null @@ -1,20 +0,0 @@ -module private FsAutoComplete.Tests.CodeFixTests.UpdateFooBarTests - -open Expecto -open Helpers -open Utils.ServerTests -open Utils.CursorbasedTests -open FsAutoComplete.CodeFix - -let tests state = - serverTestList (nameof UpdateFooBar) state defaultConfigDto None (fun server -> - [ let selectCodeFix = CodeFix.withTitle UpdateFooBar.title - - ftestCaseAsync "first unit test for UpdateFooBar" - <| CodeFix.check - server - "let a$0 b c = ()" - Diagnostics.acceptAll - selectCodeFix - "let Text replaced by UpdateFooBar b c = ()" - ]) From de67a6dd5d3f9eca4bee517bdb007e9179c27820 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 5 Sep 2023 11:45:49 +0200 Subject: [PATCH 07/11] Improve syntax tree traversal code. --- build/ScaffoldCodeFix.fs | 95 +++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 44 deletions(-) diff --git a/build/ScaffoldCodeFix.fs b/build/ScaffoldCodeFix.fs index e3e36bbb9..46695b7c9 100644 --- a/build/ScaffoldCodeFix.fs +++ b/build/ScaffoldCodeFix.fs @@ -203,9 +203,35 @@ let appendItemToArrayOrList item path (node: ExprArrayOrListNode) = else lines |> Array.insertAt (lineIdx + 1) $"%s{spaces}%s{item}" - File.WriteAllLines(path, updatedLines) + let content = String.concat "\n" updatedLines + File.WriteAllText(path, content) Trace.tracefn $"Added \"%s{item}\" to %s{Path.GetRelativePath(repositoryRoot, path)}" +module List = + let exactlyOneOrFail (message: string) (items: 'T list) : 'T = + if items.Length = 1 then items.Head else failwith message + + let pickOrFail (message: string) (chooser: 'T -> 'U option) (items: 'T list) : 'U = + match List.tryPick chooser items with + | None -> failwith message + | Some u -> u + +let findArrayOrListOfFail (e: Expr) = + match e with + | Expr.ArrayOrList array -> array + | e -> failwithf $"Expected to find Expr.ArrayOrList, got %A{e}" + +let findTypeWithNameOfFail (typeName: string) (mn: ModuleOrNamespaceNode) : ITypeDefn = + mn.Declarations + |> List.pickOrFail $"Expected to find ModuleDecl.TypeDefn for %s{typeName}" (function + | ModuleDecl.TypeDefn t -> + let tdn = TypeDefn.TypeDefnNode t + + match tdn.TypeName.Identifier with + | IdentName typeName -> Some tdn + | _ -> None + | _ -> None) + let wireCodeFixInAdaptiveFSharpLspServer codeFixName = let path = repositoryRoot @@ -218,24 +244,17 @@ let wireCodeFixInAdaptiveFSharpLspServer codeFixName = let oak = getOakFor path // namespace FsAutoComplete.Lsp - let ns = oak.ModulesOrNamespaces |> List.exactlyOne + let ns = + oak.ModulesOrNamespaces + |> List.exactlyOneOrFail "Expected a single namespace in Oak." // type AdaptiveFSharpLspServer - let t = - ns.Declarations - |> List.pick (function - | ModuleDecl.TypeDefn t -> - let tdn = TypeDefn.TypeDefnNode t - - match tdn.TypeName.Identifier with - | IdentName "AdaptiveFSharpLspServer" -> Some tdn - | _ -> None - | _ -> None) + let t = findTypeWithNameOfFail "AdaptiveFSharpLspServer" ns // let codefixes = let codefixesValue = t.Members - |> List.pick (function + |> List.pickOrFail "Expected to find MemberDefn.LetBinding for codefixes" (function | MemberDefn.LetBinding bindingList -> match bindingList.Bindings with | bindings -> @@ -253,24 +272,21 @@ let wireCodeFixInAdaptiveFSharpLspServer codeFixName = | ComputationExpressionStatement.OtherStatement other -> match other with | Expr.InfixApp infixApp -> infixApp - | _ -> raise (exn "Expected |> infix operator") - | _ -> raise (exn "Expected |> infix operator") - | _ -> raise (exn "Expected |> infix operator") + | e -> failwithf $"Expected to find Expr.InfixApp, got %A{e}" + | ces -> failwithf $"Expected to find ComputationExpressionStatement.OtherStatement, got %A{ces}" + | e -> failwithf $"Expected to find Expr.CompExprBody, got %A{e}" let appWithLambda = match infixApp.RightHandSide with | Expr.AppWithLambda appWithLambda -> appWithLambda - | _ -> raise (exn "Expected function with lambda") + | e -> failwithf $"Expected to find Expr.AppWithLambda, got %A{e}" let lambda = match appWithLambda.Lambda with | Choice1Of2 lambda -> lambda - | Choice2Of2 _ -> raise (exn "Expected lambda") + | Choice2Of2 ml -> failwithf $"Expected to find ExprLambdaNode, got %A{ml}" - let array = - match lambda.Expr with - | Expr.ArrayOrList array -> array - | _ -> raise (exn "Expected array") + let array = findArrayOrListOfFail lambda.Expr appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" path array with ex -> @@ -288,19 +304,12 @@ let wireCodeFixInFsAutoCompleteLsp codeFixName = try let oak = getOakFor path // namespace FsAutoComplete.Lsp - let ns = oak.ModulesOrNamespaces |> List.exactlyOne + let ns = + oak.ModulesOrNamespaces + |> List.exactlyOneOrFail "Expected a single namespace in Oak." // type AdaptiveFSharpLspServer - let t = - ns.Declarations - |> List.pick (function - | ModuleDecl.TypeDefn t -> - let tdn = TypeDefn.TypeDefnNode t - - match tdn.TypeName.Identifier with - | IdentName "FSharpLspServer" -> Some tdn - | _ -> None - | _ -> None) + let t = findTypeWithNameOfFail "FSharpLspServer" ns // interface IFSharpLspServer with let iFSharpLspServer = @@ -325,16 +334,16 @@ let wireCodeFixInFsAutoCompleteLsp codeFixName = let asyncComp = match overrideMember.Expr with | Expr.NamedComputation namedComputation -> namedComputation - | e -> failwithf "Expected Expr.NamedComputation, got %A" e + | e -> failwithf $"Expected Expr.NamedComputation, got %A{e}" let compBody = match asyncComp.Body with | Expr.CompExprBody body -> body - | e -> failwithf "Expected Expr.CompExprBody, got %A" e + | e -> failwithf $"Expected Expr.CompExprBody, got %A{e}" let array = compBody.Statements - |> List.pick (function + |> List.pickOrFail "Expected to find ComputationExpressionStatement.OtherStatement" (function | ComputationExpressionStatement.OtherStatement(Expr.LongIdentSet longIdentSet) -> match longIdentSet.Identifier with | IdentName "codefixes" -> @@ -394,12 +403,14 @@ let wireCodeFixTests codeFixName = try let oak = getOakFor path // module FsAutoComplete.Tests.CodeFixTests.Tests - let testsModule = oak.ModulesOrNamespaces |> List.exactlyOne + let testsModule = + oak.ModulesOrNamespaces + |> List.exactlyOneOrFail "Expected a single module in Oak." // let tests state = let testBinding = testsModule.Declarations - |> List.pick (function + |> List.pickOrFail "Expected to find ModuleDecl.TopLevelBinding for tests" (function | ModuleDecl.TopLevelBinding binding -> match binding.FunctionName with | Choice1Of2(IdentName "tests") -> Some binding @@ -409,13 +420,9 @@ let wireCodeFixTests codeFixName = let appNode = match testBinding.Expr with | Expr.App appNode -> appNode - | e -> failwithf "Expected Expr.App, got %A" e - - let list = - match List.last appNode.Arguments with - | Expr.ArrayOrList list -> list - | e -> failwithf "Expected Expr.ArrayOrList, got %A" e + | e -> failwithf $"Expected Expr.App, got %A{e}" + let list = findArrayOrListOfFail (List.last appNode.Arguments) appendItemToArrayOrList $"%s{codeFixName}Tests.tests state" path list with ex -> Trace.traceException ex From f931fb7685cf41a589f5b896b99b35d49d3f9e9b Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 5 Sep 2023 11:47:07 +0200 Subject: [PATCH 08/11] Add documentation. --- README.md | 4 +++ docs/Creating a new code fix.md | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 docs/Creating a new code fix.md diff --git a/README.md b/README.md index c7479c425..ec68375c3 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,10 @@ This repository is prepared to use Gitpod for a web-based VSCode-style IDE. Clic [![Gitpod Ready-to-Code](https://img.shields.io/badge/Gitpod-ready--to--code-blue?logo=gitpod)](https://gitpod.io/#https://github.com/fsharp/fsautocomplete) +### Creating a new code fix + +Checkout [this guide](./docs/Creating%20a%20new%20code%20fix.md) to start with a new code fix. + ## Releasing * Update CHANGELOG.md with the release notes from the current release in the `Unreleased` section. Use section headings like `Added`, `Fixed`, etc from keepachangelog.com. diff --git a/docs/Creating a new code fix.md b/docs/Creating a new code fix.md new file mode 100644 index 000000000..3f5b71317 --- /dev/null +++ b/docs/Creating a new code fix.md @@ -0,0 +1,46 @@ +# Creating a New Code Fix + +A code fix, also referred to as a quick fix or code action, serves as a mechanism within the editor to propose and implement code changes within the current file. +This functionality is facilitated through the [Code Action Request](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction) provided by the Language Server Protocol (LSP). +You might associate code fixes with the concept of "light bulbs" found in certain integrated development environments (IDEs). + +To introduce a new code fix within the context of FSAutocomplete, there are several essential components to consider: + +1. **Code Fix File**: This pertains to the actual implementation of the code fix. + +2. **Registration in LSP Servers**: Registration of the code fix is required in both of the associated LSP servers. + +3. **Unit Test Setup**: Proper unit tests need to be established to ensure the correctness and effectiveness of the new code fix. + +To streamline the process of creating a new code fix, a convenient `FAKE` target has been provided. By executing the following command: + +```bash +dotnet run --project ./build/build.fsproj -- -t ScaffoldCodeFix YourCodeFixName +``` + +The above command accomplishes the following tasks: + +- Generation of three files: + - The implementation file for your code fix. + - A signature file associated with your code fix. + - A dedicated standalone unit test file. + +Furthermore, this command updates the following files to properly register the new code fix: + +- `src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs` +- `src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs` +- `test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs` + +The unit test file contains a single focused test, allowing you to promptly verify the functionality. To run this initial test, you have two options: + +1. Using the `dotnet test` command: + ```bash + dotnet test -f net6.0 ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj + ``` + +2. Alternatively, using the `dotnet run` command: + ```bash + dotnet run -f net6.0 --project ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj + ``` + +This comprehensive approach ensures that the newly introduced code fix is properly integrated, tested, and ready for seamless integration into the FSAutocomplete environment. From 8d7aff5431557ca0418d7933c0260d159bb43d8f Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 5 Sep 2023 12:01:04 +0200 Subject: [PATCH 09/11] Ensure ScaffoldCodeFix keeps working. --- build/Program.fs | 10 +- build/ScaffoldCodeFix.fs | 309 ++++++++++++++++++++------------------ build/ScaffoldCodeFix.fsi | 5 + 3 files changed, 174 insertions(+), 150 deletions(-) diff --git a/build/Program.fs b/build/Program.fs index 0941ad313..55afc5000 100644 --- a/build/Program.fs +++ b/build/Program.fs @@ -180,12 +180,20 @@ let init args = | None -> failwith "Usage: dotnet run --project ./build/build.fsproj -- -t ScaffoldCodeFix " | Some codeFixName -> ScaffoldCodeFix.scaffold codeFixName) + Target.create "EnsureCanScaffoldCodeFix" (fun _ -> ScaffoldCodeFix.ensureScaffoldStillWorks ()) + "PromoteUnreleasedToVersion" ==> "CreateVersionTag" ==> "Promote" |> ignore "Restore" ==> "Build" |> ignore - "Build" ==> "LspTest" ==> "Coverage" ==> "Test" ==> "All" |> ignore + "Build" + ==> "EnsureCanScaffoldCodeFix" + ==> "LspTest" + ==> "Coverage" + ==> "Test" + ==> "All" + |> ignore "Clean" ==> "LocalRelease" ==> "ReleaseArchive" ==> "Release" |> ignore diff --git a/build/ScaffoldCodeFix.fs b/build/ScaffoldCodeFix.fs index 46695b7c9..86e671107 100644 --- a/build/ScaffoldCodeFix.fs +++ b/build/ScaffoldCodeFix.fs @@ -8,6 +8,27 @@ open Fantomas.Core.SyntaxOak let repositoryRoot = __SOURCE_DIRECTORY__ ".." +let AdaptiveFSharpLspServerPath = + repositoryRoot + "src" + "FsAutoComplete" + "LspServers" + "AdaptiveFSharpLspServer.fs" + +let FsAutoCompleteLspPath = + repositoryRoot + "src" + "FsAutoComplete" + "LspServers" + "FsAutoComplete.Lsp.fs" + +let TestsPath = + repositoryRoot + "test" + "FsAutoComplete.Tests.Lsp" + "CodeFixTests" + "Tests.fs" + let removeReturnCarriage (v: string) = v.Replace("\r", "") let mkCodeFixImplementation codeFixName = @@ -232,131 +253,123 @@ let findTypeWithNameOfFail (typeName: string) (mn: ModuleOrNamespaceNode) : ITyp | _ -> None | _ -> None) -let wireCodeFixInAdaptiveFSharpLspServer codeFixName = - let path = - repositoryRoot - "src" - "FsAutoComplete" - "LspServers" - "AdaptiveFSharpLspServer.fs" +let findArrayInAdaptiveFSharpLspServer () : ExprArrayOrListNode = + let oak = getOakFor AdaptiveFSharpLspServerPath + + // namespace FsAutoComplete.Lsp + let ns = + oak.ModulesOrNamespaces + |> List.exactlyOneOrFail "Expected a single namespace in Oak." + + // type AdaptiveFSharpLspServer + let t = findTypeWithNameOfFail "AdaptiveFSharpLspServer" ns + + // let codefixes = + let codefixesValue = + t.Members + |> List.pickOrFail "Expected to find MemberDefn.LetBinding for codefixes" (function + | MemberDefn.LetBinding bindingList -> + match bindingList.Bindings with + | bindings -> + bindings + |> List.tryPick (fun binding -> + match binding.FunctionName with + | Choice1Of2(IdentName "codefixes") -> Some binding + | _ -> None) + | _ -> None) + + let infixApp = + match codefixesValue.Expr with + | Expr.CompExprBody body -> + match List.last body.Statements with + | ComputationExpressionStatement.OtherStatement other -> + match other with + | Expr.InfixApp infixApp -> infixApp + | e -> failwithf $"Expected to find Expr.InfixApp, got %A{e}" + | ces -> failwithf $"Expected to find ComputationExpressionStatement.OtherStatement, got %A{ces}" + | e -> failwithf $"Expected to find Expr.CompExprBody, got %A{e}" + + let appWithLambda = + match infixApp.RightHandSide with + | Expr.AppWithLambda appWithLambda -> appWithLambda + | e -> failwithf $"Expected to find Expr.AppWithLambda, got %A{e}" + + let lambda = + match appWithLambda.Lambda with + | Choice1Of2 lambda -> lambda + | Choice2Of2 ml -> failwithf $"Expected to find ExprLambdaNode, got %A{ml}" + + findArrayOrListOfFail lambda.Expr +let wireCodeFixInAdaptiveFSharpLspServer codeFixName = try - let oak = getOakFor path - - // namespace FsAutoComplete.Lsp - let ns = - oak.ModulesOrNamespaces - |> List.exactlyOneOrFail "Expected a single namespace in Oak." - - // type AdaptiveFSharpLspServer - let t = findTypeWithNameOfFail "AdaptiveFSharpLspServer" ns - - // let codefixes = - let codefixesValue = - t.Members - |> List.pickOrFail "Expected to find MemberDefn.LetBinding for codefixes" (function - | MemberDefn.LetBinding bindingList -> - match bindingList.Bindings with - | bindings -> - bindings - |> List.tryPick (fun binding -> - match binding.FunctionName with - | Choice1Of2(IdentName "codefixes") -> Some binding - | _ -> None) - | _ -> None) - - let infixApp = - match codefixesValue.Expr with - | Expr.CompExprBody body -> - match List.last body.Statements with - | ComputationExpressionStatement.OtherStatement other -> - match other with - | Expr.InfixApp infixApp -> infixApp - | e -> failwithf $"Expected to find Expr.InfixApp, got %A{e}" - | ces -> failwithf $"Expected to find ComputationExpressionStatement.OtherStatement, got %A{ces}" - | e -> failwithf $"Expected to find Expr.CompExprBody, got %A{e}" - - let appWithLambda = - match infixApp.RightHandSide with - | Expr.AppWithLambda appWithLambda -> appWithLambda - | e -> failwithf $"Expected to find Expr.AppWithLambda, got %A{e}" - - let lambda = - match appWithLambda.Lambda with - | Choice1Of2 lambda -> lambda - | Choice2Of2 ml -> failwithf $"Expected to find ExprLambdaNode, got %A{ml}" - - let array = findArrayOrListOfFail lambda.Expr - - appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" path array + let array = findArrayInAdaptiveFSharpLspServer () + + appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" AdaptiveFSharpLspServerPath array with ex -> Trace.traceException ex - Trace.traceError $"Unable to find array of codefixes in %s{path}.\nDid the code structure change?" -let wireCodeFixInFsAutoCompleteLsp codeFixName = - let path = - repositoryRoot - "src" - "FsAutoComplete" - "LspServers" - "FsAutoComplete.Lsp.fs" - - try - let oak = getOakFor path - // namespace FsAutoComplete.Lsp - let ns = - oak.ModulesOrNamespaces - |> List.exactlyOneOrFail "Expected a single namespace in Oak." - - // type AdaptiveFSharpLspServer - let t = findTypeWithNameOfFail "FSharpLspServer" ns - - // interface IFSharpLspServer with - let iFSharpLspServer = - t.Members - |> List.pick (function - | MemberDefn.Interface i -> Some i - | _ -> None) - - // override _.Initialize(p: InitializeParams) = - let overrideMember = - iFSharpLspServer.Members - |> List.pick (function - | MemberDefn.Member mb -> - match mb.FunctionName with - | Choice1Of2 iln -> - match iln.Content with - | [ _; _; IdentifierOrDot.Ident ident ] when ident.Text = "Initialize" -> Some mb - | _ -> None - | Choice2Of2 _ -> None - | _ -> None) - - let asyncComp = - match overrideMember.Expr with - | Expr.NamedComputation namedComputation -> namedComputation - | e -> failwithf $"Expected Expr.NamedComputation, got %A{e}" - - let compBody = - match asyncComp.Body with - | Expr.CompExprBody body -> body - | e -> failwithf $"Expected Expr.CompExprBody, got %A{e}" - - let array = - compBody.Statements - |> List.pickOrFail "Expected to find ComputationExpressionStatement.OtherStatement" (function - | ComputationExpressionStatement.OtherStatement(Expr.LongIdentSet longIdentSet) -> - match longIdentSet.Identifier with - | IdentName "codefixes" -> - match longIdentSet.Expr with - | Expr.ArrayOrList array -> Some array - | _ -> None + Trace.traceError + $"Unable to find array of codefixes in %s{AdaptiveFSharpLspServerPath}.\nDid the code structure change?" + +let findArrayInFsAutoCompleteLsp () = + let oak = getOakFor FsAutoCompleteLspPath + // namespace FsAutoComplete.Lsp + let ns = + oak.ModulesOrNamespaces + |> List.exactlyOneOrFail "Expected a single namespace in Oak." + + // type AdaptiveFSharpLspServer + let t = findTypeWithNameOfFail "FSharpLspServer" ns + + // interface IFSharpLspServer with + let iFSharpLspServer = + t.Members + |> List.pick (function + | MemberDefn.Interface i -> Some i + | _ -> None) + + // override _.Initialize(p: InitializeParams) = + let overrideMember = + iFSharpLspServer.Members + |> List.pick (function + | MemberDefn.Member mb -> + match mb.FunctionName with + | Choice1Of2 iln -> + match iln.Content with + | [ _; _; IdentifierOrDot.Ident ident ] when ident.Text = "Initialize" -> Some mb | _ -> None - | _ -> None) + | Choice2Of2 _ -> None + | _ -> None) + + let asyncComp = + match overrideMember.Expr with + | Expr.NamedComputation namedComputation -> namedComputation + | e -> failwithf $"Expected Expr.NamedComputation, got %A{e}" + + let compBody = + match asyncComp.Body with + | Expr.CompExprBody body -> body + | e -> failwithf $"Expected Expr.CompExprBody, got %A{e}" + + compBody.Statements + |> List.pickOrFail "Expected to find ComputationExpressionStatement.OtherStatement" (function + | ComputationExpressionStatement.OtherStatement(Expr.LongIdentSet longIdentSet) -> + match longIdentSet.Identifier with + | IdentName "codefixes" -> + match longIdentSet.Expr with + | Expr.ArrayOrList array -> Some array + | _ -> None + | _ -> None + | _ -> None) - appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" path array +let wireCodeFixInFsAutoCompleteLsp codeFixName = + try + let array = findArrayInFsAutoCompleteLsp () + appendItemToArrayOrList $"%s{codeFixName}.fix tryGetParseResultsForFile" FsAutoCompleteLspPath array with ex -> Trace.traceException ex - Trace.traceError $"Unable to find array of codefixes in %s{path}.\nDid the code structure change?" + Trace.traceError $"Unable to find array of codefixes in %s{FsAutoCompleteLspPath}.\nDid the code structure change?" let mkCodeFixTests codeFixName = let path = @@ -392,41 +405,37 @@ let tests state = File.WriteAllText(path, removeReturnCarriage contents) Trace.tracefn $"Generated %s{Path.GetRelativePath(repositoryRoot, path)}" -let wireCodeFixTests codeFixName = - let path = - repositoryRoot - "test" - "FsAutoComplete.Tests.Lsp" - "CodeFixTests" - "Tests.fs" +let findListInTests () = + let oak = getOakFor TestsPath + // module FsAutoComplete.Tests.CodeFixTests.Tests + let testsModule = + oak.ModulesOrNamespaces + |> List.exactlyOneOrFail "Expected a single module in Oak." + + // let tests state = + let testBinding = + testsModule.Declarations + |> List.pickOrFail "Expected to find ModuleDecl.TopLevelBinding for tests" (function + | ModuleDecl.TopLevelBinding binding -> + match binding.FunctionName with + | Choice1Of2(IdentName "tests") -> Some binding + | _ -> None + | _ -> None) + + let appNode = + match testBinding.Expr with + | Expr.App appNode -> appNode + | e -> failwithf $"Expected Expr.App, got %A{e}" + + findArrayOrListOfFail (List.last appNode.Arguments) +let wireCodeFixTests codeFixName = try - let oak = getOakFor path - // module FsAutoComplete.Tests.CodeFixTests.Tests - let testsModule = - oak.ModulesOrNamespaces - |> List.exactlyOneOrFail "Expected a single module in Oak." - - // let tests state = - let testBinding = - testsModule.Declarations - |> List.pickOrFail "Expected to find ModuleDecl.TopLevelBinding for tests" (function - | ModuleDecl.TopLevelBinding binding -> - match binding.FunctionName with - | Choice1Of2(IdentName "tests") -> Some binding - | _ -> None - | _ -> None) - - let appNode = - match testBinding.Expr with - | Expr.App appNode -> appNode - | e -> failwithf $"Expected Expr.App, got %A{e}" - - let list = findArrayOrListOfFail (List.last appNode.Arguments) - appendItemToArrayOrList $"%s{codeFixName}Tests.tests state" path list + let list = findListInTests () + appendItemToArrayOrList $"%s{codeFixName}Tests.tests state" TestsPath list with ex -> Trace.traceException ex - Trace.traceError $"Unable to find array of tests in %s{path}.\nDid the code structure change?" + Trace.traceError $"Unable to find array of tests in %s{TestsPath}.\nDid the code structure change?" let scaffold (codeFixName: string) : unit = // generate files in src/CodeFixes/ @@ -446,5 +455,7 @@ let scaffold (codeFixName: string) : unit = updateProjectFiles () Trace.tracefn $"Scaffolding %s{codeFixName} complete!" -// TODO: introduce a target that verifies the codefix can still be added. -// By checking the AST can still reach the entry points of the lists. +let ensureScaffoldStillWorks () = + findArrayInAdaptiveFSharpLspServer () |> ignore + findArrayInFsAutoCompleteLsp () |> ignore + findListInTests () |> ignore diff --git a/build/ScaffoldCodeFix.fsi b/build/ScaffoldCodeFix.fsi index 3329cb701..fc48f497d 100644 --- a/build/ScaffoldCodeFix.fsi +++ b/build/ScaffoldCodeFix.fsi @@ -7,3 +7,8 @@ module ScaffoldCodeFix /// - Wire up the tests file. /// - Update the last write time the project files. val scaffold: codeFixName: string -> unit + +/// Verifies that the code fix scaffold target can still wire up a new codefix to the existing list. +/// Throws when any expected AST nodes can no longer be found. +/// If this code throws, you may need to revisit ScaffoldCodeFix.fs to tweak any recent changes. +val ensureScaffoldStillWorks: unit -> unit From 61a37c71984802f98abedadd47602450aa5fc4d6 Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 5 Sep 2023 12:02:58 +0200 Subject: [PATCH 10/11] Don't indent code snippet. --- docs/Creating a new code fix.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/Creating a new code fix.md b/docs/Creating a new code fix.md index 3f5b71317..94c1ab8bd 100644 --- a/docs/Creating a new code fix.md +++ b/docs/Creating a new code fix.md @@ -34,13 +34,13 @@ Furthermore, this command updates the following files to properly register the n The unit test file contains a single focused test, allowing you to promptly verify the functionality. To run this initial test, you have two options: 1. Using the `dotnet test` command: - ```bash - dotnet test -f net6.0 ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj - ``` + ```bash +dotnet test -f net6.0 ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj + ``` 2. Alternatively, using the `dotnet run` command: - ```bash - dotnet run -f net6.0 --project ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj - ``` + ```bash +dotnet run -f net6.0 --project ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj + ``` This comprehensive approach ensures that the newly introduced code fix is properly integrated, tested, and ready for seamless integration into the FSAutocomplete environment. From 8660e2e26d1cdc8526601a0178f3cb667426e4b2 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 6 Sep 2023 10:48:44 +0200 Subject: [PATCH 11/11] Add pointers for PR. --- docs/Creating a new code fix.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/Creating a new code fix.md b/docs/Creating a new code fix.md index 94c1ab8bd..75778bb71 100644 --- a/docs/Creating a new code fix.md +++ b/docs/Creating a new code fix.md @@ -31,7 +31,7 @@ Furthermore, this command updates the following files to properly register the n - `src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs` - `test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs` -The unit test file contains a single focused test, allowing you to promptly verify the functionality. To run this initial test, you have two options: +The unit test file contains a [single focused test](https://github.com/haf/expecto#focusing-tests), allowing you to promptly verify the functionality. To run this initial test, you have two options: 1. Using the `dotnet test` command: ```bash @@ -44,3 +44,9 @@ dotnet run -f net6.0 --project ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Te ``` This comprehensive approach ensures that the newly introduced code fix is properly integrated, tested, and ready for seamless integration into the FSAutocomplete environment. + +When preparing to submit a pull request, please consider the following guidelines: + +- Eliminate any extraneous code or comments that may remain from the sample code. +- Ensure proper source code formatting by running the command `dotnet fantomas src`. +- Avoid including focused tests, as they can cause the continuous integration build to fail.