From ec8c22e53d5f03afca4d40baa83a73e17ce2a536 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Thu, 8 Sep 2022 20:50:25 +0200 Subject: [PATCH 1/2] Remove Cache when non-existing file or file outside of workspace gets closed --- src/FsAutoComplete.Core/State.fs | 7 ++ src/FsAutoComplete/FsAutoComplete.Lsp.fs | 73 ++++++++++++++++++- test/FsAutoComplete.Tests.Lsp/CoreTests.fs | 54 ++++++++++++++ test/FsAutoComplete.Tests.Lsp/Program.fs | 1 + .../InsideProjectOutsideWorkspace.fs | 3 + .../TestCases/CloseTests/Script.fsx | 1 + .../Workspace/InsideProjectInsideWorkspace.fs | 3 + .../TestCases/CloseTests/Workspace/Script.fsx | 1 + .../CloseTests/Workspace/Workspace.fsproj | 13 ++++ 9 files changed, 153 insertions(+), 3 deletions(-) create mode 100644 test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/InsideProjectOutsideWorkspace.fs create mode 100644 test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Script.fsx create mode 100644 test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/InsideProjectInsideWorkspace.fs create mode 100644 test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Script.fsx create mode 100644 test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Workspace.fsproj diff --git a/src/FsAutoComplete.Core/State.fs b/src/FsAutoComplete.Core/State.fs index f0b80ef8a..0acb9670f 100644 --- a/src/FsAutoComplete.Core/State.fs +++ b/src/FsAutoComplete.Core/State.fs @@ -292,3 +292,10 @@ type State = return (opts, text, line) } + + /// Removes `file` from all caches + member x.Forget(file: string) = + x.LastCheckedVersion.TryRemove(file) |> ignore + x.Files.TryRemove(file) |> ignore + x.ProjectController.RemoveProjectOptions(UMX.untag file) |> ignore + x.ScriptProjectOptions.TryRemove(file) |> ignore diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 0c3f3ca73..26d5876ab 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -612,6 +612,66 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = do commandDisposables.Add <| commands.Notify.Subscribe handleCommandEvents + /// Removes all caches (state & diagnostics) if: + /// * file doesn't exist on disk (untitled or deleted) + /// * file is outside of current workspace (and not part of any project) + let forgetDocument (uri: DocumentUri) = + let filePath = + uri + |> Path.FileUriToLocalPath + |> Utils.normalizePath + + // remove cached data for + // * non-existing files (untitled & deleted) + // * files outside of workspace (and not used by any project) + let doesNotExist (file: string) = + not (File.Exists (UMX.untag file)) + let isOutsideWorkspace (file: string) = + match rootPath with + | None -> + // no workspace specified + true + | Some rootPath -> + let rec isInside (rootDir: DirectoryInfo, dirToCheck: DirectoryInfo) = + if String.Equals(rootDir.FullName, dirToCheck.FullName, StringComparison.InvariantCultureIgnoreCase) then + true + else + match dirToCheck.Parent with + | null -> false + | parent -> + isInside (rootDir, parent) + + let rootDir = DirectoryInfo(rootPath) + let fileDir = FileInfo(UMX.untag file).Directory + + if isInside (rootDir, fileDir) then + false + else + // file might be outside of workspace but part of a project + match state.GetProjectOptions file with + | None -> true + | Some projOptions -> + if doesNotExist (UMX.tag projOptions.ProjectFileName) then + // case for script file + true + else + // issue: fs-file does never get removed from project options (-> requires reload of FSAC to register) + // -> don't know if file still part of project (file might have been removed from project) + // -> keep cache for file + false + + if + doesNotExist filePath + || + isOutsideWorkspace filePath + then + logger.info ( + Log.setMessage "Removing cached data for {file}" + >> Log.addContext "file" filePath + ) + state.Forget filePath + diagnosticCollections.ClearFor uri + ///Helper function for handling Position requests using **recent** type check results member x.positionHandler<'a, 'b when 'b :> ITextDocumentPositionParams> (f: 'b -> FcsPos -> ParseAndCheckResults -> string -> NamedText -> AsyncLspResult<'a>) @@ -1043,6 +1103,15 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = () } + override _.TextDocumentDidClose(p) = async { + logger.info ( + Log.setMessage "TextDocumentDidOpen Request: {parms}" + >> Log.addContextDestructured "parms" p + ) + + forgetDocument p.TextDocument.Uri + } + override __.TextDocumentCompletion(p: CompletionParams) = asyncResult { logger.info ( @@ -1941,9 +2010,7 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = p.Changes |> Array.iter (fun c -> if c.Type = FileChangeType.Deleted then - let uri = c.Uri - diagnosticCollections.ClearFor uri - + forgetDocument c.Uri ()) return () diff --git a/test/FsAutoComplete.Tests.Lsp/CoreTests.fs b/test/FsAutoComplete.Tests.Lsp/CoreTests.fs index 2ec3725b2..8e439f8db 100644 --- a/test/FsAutoComplete.Tests.Lsp/CoreTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CoreTests.fs @@ -21,6 +21,7 @@ open Utils open Utils.Utils open FsToolkit.ErrorHandling.Operator.AsyncResult open FSharpx.Control +open Utils.Tests ///Test for initialization of the server let initTests state = @@ -371,3 +372,56 @@ let tooltipTests state = "* `'a` is `int`" "* `'b` is `int`" "* `'c` is `int`" ] ] ] + +let closeTests state = + // Note: clear diagnostics also implies clear caches (-> remove file & project options from State). + let root = Path.Combine(__SOURCE_DIRECTORY__, "TestCases", "CloseTests") + let workspace = Path.Combine(root, "Workspace") + serverTestList "close tests" state defaultConfigDto (Some workspace) (fun server -> [ + testCaseAsync "closing untitled script file clears diagnostics" (async { + let source = + // The value or constructor 'untitled' is not defined. + "let foo = untitled" + let! (doc, diags) = server |> Server.createUntitledDocument source + Expect.isNonEmpty diags "There should be an error" + do! doc |> Document.close + + let! diags = doc |> Document.waitForLatestDiagnostics (TimeSpan.FromSeconds 5.0) + Expect.isEmpty diags "There should be a final publishDiagnostics without any diags" + }) + testCaseAsync "closing existing script file inside workspace doesn't clear diagnostics" (async { + let! (doc, diags) = server |> Server.openDocument "Script.fsx" + Expect.isNonEmpty diags "There should be an error" + do! doc |> Document.close + + let! diags = doc |> Document.waitForLatestDiagnostics (TimeSpan.FromSeconds 5.0) + Expect.isNonEmpty diags "There should be no publishDiagnostics without any diags after close" + }) + testCaseAsync "closing existing script file outside workspace clears diagnostics" (async { + let file = Path.Combine(root, "Script.fsx") + let! (doc, diags) = server |> Server.openDocument file + Expect.isNonEmpty diags "There should be an error" + do! doc |> Document.close + + let! diags = doc |> Document.waitForLatestDiagnostics (TimeSpan.FromSeconds 5.0) + Expect.isEmpty diags "There should be a final publishDiagnostics without any diags" + }) + + testCaseAsync "closing existing file inside project & workspace doesn't clear diagnostics" (async { + let! (doc, diags) = server |> Server.openDocument "InsideProjectInsideWorkspace.fs" + Expect.isNonEmpty diags "There should be an error" + do! doc |> Document.close + + let! diags = doc |> Document.waitForLatestDiagnostics (TimeSpan.FromSeconds 5.0) + Expect.isNonEmpty diags "There should be no publishDiagnostics without any diags after close" + }) + testCaseAsync "closing existing file inside project but outside workspace doesn't clear diagnostics" (async { + let file = Path.Combine(root, "InsideProjectOutsideWorkspace.fs") + let! (doc, diags) = server |> Server.openDocument file + Expect.isNonEmpty diags "There should be an error" + do! doc |> Document.close + + let! diags = doc |> Document.waitForLatestDiagnostics (TimeSpan.FromSeconds 5.0) + Expect.isNonEmpty diags "There should be no publishDiagnostics without any diags after close" + }) + ]) diff --git a/test/FsAutoComplete.Tests.Lsp/Program.fs b/test/FsAutoComplete.Tests.Lsp/Program.fs index e8b414a1c..9eece28af 100644 --- a/test/FsAutoComplete.Tests.Lsp/Program.fs +++ b/test/FsAutoComplete.Tests.Lsp/Program.fs @@ -51,6 +51,7 @@ let lspTests = FsAutoComplete.State.Initial toolsPath testRunDir workspaceLoaderFactory initTests state + closeTests state Utils.Tests.Server.tests state Utils.Tests.CursorbasedTests.tests state diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/InsideProjectOutsideWorkspace.fs b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/InsideProjectOutsideWorkspace.fs new file mode 100644 index 000000000..972d9acd6 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/InsideProjectOutsideWorkspace.fs @@ -0,0 +1,3 @@ +namespace MyProject + +let someValue = inProjectOutsideWorkspace \ No newline at end of file diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Script.fsx b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Script.fsx new file mode 100644 index 000000000..12bdaca8a --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Script.fsx @@ -0,0 +1 @@ +let foo = outsideWorkspace \ No newline at end of file diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/InsideProjectInsideWorkspace.fs b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/InsideProjectInsideWorkspace.fs new file mode 100644 index 000000000..a97a6d90a --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/InsideProjectInsideWorkspace.fs @@ -0,0 +1,3 @@ +namespace MyProject + +let someValue = inProjectInsideWorkspace \ No newline at end of file diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Script.fsx b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Script.fsx new file mode 100644 index 000000000..425bfe74b --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Script.fsx @@ -0,0 +1 @@ +let foo = insideWorkspace \ No newline at end of file diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Workspace.fsproj b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Workspace.fsproj new file mode 100644 index 000000000..1eb62e593 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CloseTests/Workspace/Workspace.fsproj @@ -0,0 +1,13 @@ + + + + net6.0 + true + + + + + + + + From fb6f6fd2db18494727ce863e053412337154bcf5 Mon Sep 17 00:00:00 2001 From: BooksBaum <15612932+Booksbaum@users.noreply.github.com> Date: Thu, 8 Sep 2022 21:11:48 +0200 Subject: [PATCH 2/2] Format Code --- src/FsAutoComplete/FsAutoComplete.Lsp.fs | 89 +++++++++++------------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/src/FsAutoComplete/FsAutoComplete.Lsp.fs b/src/FsAutoComplete/FsAutoComplete.Lsp.fs index 26d5876ab..2e7556738 100644 --- a/src/FsAutoComplete/FsAutoComplete.Lsp.fs +++ b/src/FsAutoComplete/FsAutoComplete.Lsp.fs @@ -616,59 +616,52 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = /// * file doesn't exist on disk (untitled or deleted) /// * file is outside of current workspace (and not part of any project) let forgetDocument (uri: DocumentUri) = - let filePath = - uri - |> Path.FileUriToLocalPath - |> Utils.normalizePath + let filePath = uri |> Path.FileUriToLocalPath |> Utils.normalizePath // remove cached data for // * non-existing files (untitled & deleted) // * files outside of workspace (and not used by any project) - let doesNotExist (file: string) = - not (File.Exists (UMX.untag file)) + let doesNotExist (file: string) = not (File.Exists(UMX.untag file)) + let isOutsideWorkspace (file: string) = match rootPath with - | None -> - // no workspace specified - true + | None -> + // no workspace specified + true | Some rootPath -> - let rec isInside (rootDir: DirectoryInfo, dirToCheck: DirectoryInfo) = - if String.Equals(rootDir.FullName, dirToCheck.FullName, StringComparison.InvariantCultureIgnoreCase) then + let rec isInside (rootDir: DirectoryInfo, dirToCheck: DirectoryInfo) = + if String.Equals(rootDir.FullName, dirToCheck.FullName, StringComparison.InvariantCultureIgnoreCase) then + true + else + match dirToCheck.Parent with + | null -> false + | parent -> isInside (rootDir, parent) + + let rootDir = DirectoryInfo(rootPath) + let fileDir = FileInfo(UMX.untag file).Directory + + if isInside (rootDir, fileDir) then + false + else + // file might be outside of workspace but part of a project + match state.GetProjectOptions file with + | None -> true + | Some projOptions -> + if doesNotExist (UMX.tag projOptions.ProjectFileName) then + // case for script file true else - match dirToCheck.Parent with - | null -> false - | parent -> - isInside (rootDir, parent) - - let rootDir = DirectoryInfo(rootPath) - let fileDir = FileInfo(UMX.untag file).Directory - - if isInside (rootDir, fileDir) then - false - else - // file might be outside of workspace but part of a project - match state.GetProjectOptions file with - | None -> true - | Some projOptions -> - if doesNotExist (UMX.tag projOptions.ProjectFileName) then - // case for script file - true - else - // issue: fs-file does never get removed from project options (-> requires reload of FSAC to register) - // -> don't know if file still part of project (file might have been removed from project) - // -> keep cache for file - false - - if - doesNotExist filePath - || - isOutsideWorkspace filePath - then + // issue: fs-file does never get removed from project options (-> requires reload of FSAC to register) + // -> don't know if file still part of project (file might have been removed from project) + // -> keep cache for file + false + + if doesNotExist filePath || isOutsideWorkspace filePath then logger.info ( Log.setMessage "Removing cached data for {file}" >> Log.addContext "file" filePath ) + state.Forget filePath diagnosticCollections.ClearFor uri @@ -1103,14 +1096,15 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = () } - override _.TextDocumentDidClose(p) = async { - logger.info ( - Log.setMessage "TextDocumentDidOpen Request: {parms}" - >> Log.addContextDestructured "parms" p - ) + override _.TextDocumentDidClose(p) = + async { + logger.info ( + Log.setMessage "TextDocumentDidOpen Request: {parms}" + >> Log.addContextDestructured "parms" p + ) - forgetDocument p.TextDocument.Uri - } + forgetDocument p.TextDocument.Uri + } override __.TextDocumentCompletion(p: CompletionParams) = asyncResult { @@ -2011,6 +2005,7 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) = |> Array.iter (fun c -> if c.Type = FileChangeType.Deleted then forgetDocument c.Uri + ()) return ()