diff --git a/src/FsAutoComplete.Core/Commands.fs b/src/FsAutoComplete.Core/Commands.fs index 1ba7f3204..c3f23c582 100644 --- a/src/FsAutoComplete.Core/Commands.fs +++ b/src/FsAutoComplete.Core/Commands.fs @@ -1121,7 +1121,7 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers: let symbolFileText = state.TryGetFileSource(symbolFile) - |> Result.fold id (fun e -> failwith "blah blah") + |> Result.fold id (fun e -> failwith $"Unable to get file source for file '{symbolFile}'") let symbolText = symbolFileText[symbolRange] diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 457ff3951..4975f4d96 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -1252,16 +1252,24 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = asyncResult { let! documentsAndRanges = commands.RenameSymbol(pos, tyRes, lineStr, lines) + |> Async.Catch + |> Async.map (function + | Choice1Of2 v -> v + | Choice2Of2 err -> Error err.Message) |> AsyncResult.mapError (JsonRpc.Error.InternalErrorMessage) let documentChanges = documentsAndRanges |> Seq.map (fun (namedText, symbols) -> let edits = + let newName = + p.NewName + |> FSharp.Compiler.Syntax.PrettyNaming.AddBackticksToIdentifierIfNeeded + symbols |> Seq.map (fun sym -> let range = fcsRangeToLsp sym - { Range = range; NewText = p.NewName }) + { Range = range; NewText = newName }) |> Array.ofSeq { TextDocument = diff --git a/test/FsAutoComplete.Tests.Lsp/RenameTests.fs b/test/FsAutoComplete.Tests.Lsp/RenameTests.fs index 9179497e4..5be43ab78 100644 --- a/test/FsAutoComplete.Tests.Lsp/RenameTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/RenameTests.fs @@ -9,6 +9,8 @@ open Expecto open FsAutoComplete.Utils open Utils.ServerTests open Utils.Server +open Utils.Utils +open Utils.TextEdit let private normalizePathCasing = Path.FilePathToUri @@ -121,111 +123,97 @@ let tests state = ]) let sameScriptTests = - let server = - async { - let testDir = - Path.Combine(__SOURCE_DIRECTORY__, "TestCases", "RenameTest", "SameScript") - - let! (server, event) = serverInitialize testDir defaultConfigDto state - do! waitForWorkspaceFinishedParsing event - - return (server, testDir, event) + serverTestList "Within same script file" state defaultConfigDto None (fun server -> [ + let checkRename textWithCursor newName expectedText = async { + let (cursor, text) = + textWithCursor + |> Text.trimTripleQuotation + |> Cursor.assertExtractPosition + + let! (doc, diags) = server |> Server.createUntitledDocument text + use doc = doc + Expect.isEmpty diags "There should be no diags" + + let p: RenameParams = + { + TextDocument = doc.TextDocumentIdentifier + Position = cursor + NewName = newName + } + let! res = doc.Server.Server.TextDocumentRename p + let edits = + match res with + | Result.Error e -> failtestf "Request failed: %A" e + | Result.Ok None -> failtest "Request none" + | Result.Ok (Some { DocumentChanges = Some edits }) -> edits + | Result.Ok edits -> failtestf "got some unexpected edits: %A" edits + + Expect.hasLength edits 1 "should have just one file worth of edits" + let edit = edits.[0] + Expect.equal edit.TextDocument.Uri doc.Uri "should be for this file" + let edits = edit.Edits |> List.ofArray |> TextEdits.sortByRange + let actual = + text + |> TextEdits.applyWithErrorCheck edits + |> Flip.Expect.wantOk "TextEdits should be valid" + + let expected = expectedText |> Text.trimTripleQuotation + + Expect.equal actual expected "Text after TextEdits should be correct" } - - testSequenced - <| testList - "Within same script file" - [ testCaseAsync - "Rename from definition within script file" - (async { - let! server, testDir, events = server - let path = Path.Combine(testDir, "Script.fsx") - let tdop: DidOpenTextDocumentParams = { TextDocument = loadDocument path } - do! server.TextDocumentDidOpen tdop - - do! - waitForParseResultsForFile "Script.fsx" events - |> AsyncResult.foldResult id (fun e -> failtestf "%A" e) - - let newName = "afterwards" - let sourceFile: TextDocumentIdentifier = { Uri = normalizePathCasing path } - - let p: RenameParams = - { TextDocument = sourceFile - Position = { Line = 0; Character = 4 } // beginning of the 'initial' identifier - NewName = newName } - - let! res = server.TextDocumentRename p - - match res with - | Result.Error e -> failtestf "Request failed: %A" e - | Result.Ok None -> failtest "Request none" - | Result.Ok (Some { DocumentChanges = Some edits }) -> - Expect.hasLength edits 1 "should have just one file worth of edits" - let edit = edits.[0] - - Expect.equal edit.TextDocument.Uri sourceFile.Uri "should be for this file" - let edits = edit.Edits - - let expected: TextEdit[] = - [| { NewText = newName - Range = - { Start = { Line = 0; Character = 4 } - End = { Line = 0; Character = 11 } } } - { NewText = newName - Range = - { Start = { Line = 2; Character = 0 } - End = { Line = 2; Character = 7 } } } |] - - Expect.equal edits expected "Should change the two usages" - | Result.Ok edits -> failtestf "got some enexpected edits: %A" edits - }) - - testCaseAsync - "Rename from usage within script file" - (async { - let! server, testDir, events = server - let path = Path.Combine(testDir, "Script.fsx") - let tdop: DidOpenTextDocumentParams = { TextDocument = loadDocument path } - do! server.TextDocumentDidOpen tdop - - do! - waitForParseResultsForFile "Script.fsx" events - |> AsyncResult.foldResult id (fun e -> failtestf "%A" e) - - let newName = "afterwards" - let sourceFile: TextDocumentIdentifier = { Uri = normalizePathCasing path } - - let p: RenameParams = - { TextDocument = sourceFile - Position = { Line = 2; Character = 0 } // beginning of the 'initial' identifier usage - NewName = newName } - - let! res = server.TextDocumentRename p - - match res with - | Result.Error e -> failtestf "Request failed: %A" e - | Result.Ok None -> failtest "Request none" - | Result.Ok (Some { DocumentChanges = Some edits }) -> - Expect.hasLength edits 1 "should have just one file worth of edits" - let edit = edits.[0] - - Expect.equal edit.TextDocument.Uri sourceFile.Uri "should be for this file" - let edits = edit.Edits - - let expected: TextEdit[] = - [| { NewText = newName - Range = - { Start = { Line = 0; Character = 4 } - End = { Line = 0; Character = 11 } } } - { NewText = newName - Range = - { Start = { Line = 2; Character = 0 } - End = { Line = 2; Character = 7 } } } |] - - Expect.equal edits expected "Should change the two usages" - | Result.Ok edits -> failtestf "got some unexpected edits: %A" edits - }) ] + testCaseAsync "Rename from definition within script file" <| + checkRename + """ + let $0initial () = printfn "hi" + + initial () + """ + "afterwards" + """ + let afterwards () = printfn "hi" + + afterwards () + """ + testCaseAsync "Rename from usage within script file" <| + checkRename + """ + let initial () = printfn "hi" + + $0initial () + """ + "afterwards" + """ + let afterwards () = printfn "hi" + + afterwards () + """ + testCaseAsync "can add backticks to new name with space" <| + checkRename + """ + let initial () = printfn "hi" + + $0initial () + """ + "hello world" + """ + let ``hello world`` () = printfn "hi" + + ``hello world`` () + """ + testCaseAsync "doesn't add additional backticks to new name with backticks" <| + checkRename + """ + let initial () = printfn "hi" + + $0initial () + """ + "``hello world``" + """ + let ``hello world`` () = printfn "hi" + + ``hello world`` () + """ + ]) let crossProjectTests = let server = diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/RenameTest/SameScript/Script.fsx b/test/FsAutoComplete.Tests.Lsp/TestCases/RenameTest/SameScript/Script.fsx deleted file mode 100644 index 4e11db0fb..000000000 --- a/test/FsAutoComplete.Tests.Lsp/TestCases/RenameTest/SameScript/Script.fsx +++ /dev/null @@ -1,3 +0,0 @@ -let initial () = printfn "hi" - -initial ()