Skip to content

Commit 836cb46

Browse files
committed
feat: enhance multi-root workspace robustness and cleanup
- Introduce a permanent fallback workspace to handle files outside defined roots. - Automatically migrate open documents to fallback on workspace removal. - Explicitly clear diagnostics for all files when a workspace folder is removed. - Clean up matching documents from the fallback workspace when a new folder is added. - Refactor get_workspace to rely on the fallback root and remove redundant null checks in handlers. - Add comprehensive test for fallback workspace document lifecycle.
1 parent 58a1245 commit 836cb46

4 files changed

Lines changed: 73 additions & 10 deletions

File tree

R/diagnostics.R

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ diagnose_file <- function(uri, content, is_rmarkdown = FALSE, globals = NULL, ca
130130

131131
diagnostics_callback <- function(self, uri, version, diagnostics) {
132132
workspace <- self$get_workspace(uri)
133-
if (is.null(diagnostics) || is.null(workspace) || !workspace$documents$has(uri) || !lsp_settings$get("diagnostics")) return(NULL)
133+
if (is.null(diagnostics) || !workspace$documents$has(uri) || !lsp_settings$get("diagnostics")) return(NULL)
134134

135135
logger$info("diagnostics_callback called:", list(
136136
uri = uri,
@@ -163,7 +163,7 @@ diagnostics_task <- function(self, uri, document, delay = 0) {
163163

164164
workspace <- self$get_workspace(uri)
165165

166-
if (cache_ttl > 0 && !is.null(workspace) && workspace$diagnostics_cache$has(cache_key)) {
166+
if (cache_ttl > 0 && workspace$diagnostics_cache$has(cache_key)) {
167167
cached_entry <- workspace$diagnostics_cache$get(cache_key)
168168
age <- as.numeric(difftime(Sys.time(), cached_entry$time, units = "secs"))
169169
if (!is.na(age) && age <= cache_ttl) {
@@ -173,7 +173,7 @@ diagnostics_task <- function(self, uri, document, delay = 0) {
173173
}
174174
}
175175

176-
is_package <- !is.null(workspace) && is_package(workspace$root)
176+
is_package <- is_package(workspace$root)
177177
globals <- NULL
178178

179179
if (is_package) {
@@ -199,7 +199,7 @@ diagnostics_task <- function(self, uri, document, delay = 0) {
199199
cache = lsp_settings$get("lint_cache")
200200
),
201201
callback = function(result) {
202-
if (cache_ttl > 0 && !is.null(workspace)) {
202+
if (cache_ttl > 0) {
203203
workspace$diagnostics_cache$set(cache_key, list(
204204
time = Sys.time(),
205205
diagnostics = result

R/document.R

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ parse_document <- function(uri, content) {
476476

477477
parse_callback <- function(self, uri, version, parse_data) {
478478
workspace <- self$get_workspace(uri)
479-
if (is.null(workspace) || is.null(parse_data) || !workspace$documents$has(uri)) return(NULL)
479+
if (is.null(parse_data) || !workspace$documents$has(uri)) return(NULL)
480480
logger$info("parse_callback called:", list(uri = uri, version = version))
481481
doc <- workspace$documents$get(uri)
482482

@@ -526,7 +526,7 @@ parse_task <- function(self, uri, document, delay = 0) {
526526

527527
# Check cache in the main process before spawning a child task
528528
workspace <- self$get_workspace(uri)
529-
if (!is.null(workspace) && workspace$parse_cache$has(content_hash)) {
529+
if (workspace$parse_cache$has(content_hash)) {
530530
logger$info("parse_task: cache hit for", uri)
531531
cached_entry <- workspace$parse_cache$get(content_hash)
532532
cached_env <- list2env(cached_entry, parent = .GlobalEnv)
@@ -545,7 +545,7 @@ parse_task <- function(self, uri, document, delay = 0) {
545545

546546
resolve_callback <- function(self, uri, version, packages) {
547547
workspace <- self$get_workspace(uri)
548-
if (is.null(workspace) || !workspace$documents$has(uri)) return(NULL)
548+
if (!workspace$documents$has(uri)) return(NULL)
549549
logger$info("resolve_callback called:", list(uri = uri, version = version))
550550
workspace$load_packages(packages)
551551
doc <- workspace$documents$get(uri)

R/languageserver.R

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ LanguageServer <- R6::R6Class("LanguageServer",
6868

6969
self$pending_replies <- collections::dict()
7070
self$workspaces <- collections::dict()
71+
self$workspaces$set("", Workspace$new(NULL))
7172

7273
super$initialize()
7374
},
@@ -86,21 +87,41 @@ LanguageServer <- R6::R6Class("LanguageServer",
8687
key <- if (length(uri) == 0) "" else uri
8788
if (!self$workspaces$has(key)) {
8889
path <- if (length(uri) == 0) NULL else path_from_uri(uri)
89-
self$workspaces$set(key, Workspace$new(path))
90+
new_workspace <- Workspace$new(path)
91+
self$workspaces$set(key, new_workspace)
92+
93+
# Remove documents from the fallback workspace if they belong here
94+
if (!is.null(path)) {
95+
fallback <- self$workspaces$get("")
96+
for (doc_uri in fallback$documents$keys()) {
97+
doc_path <- path_from_uri(doc_uri)
98+
if (path_has_parent(doc_path, path)) {
99+
fallback$documents$remove(doc_uri)
100+
}
101+
}
102+
}
90103
}
91104
},
92105
remove_workspace = function(uri) {
93106
key <- if (length(uri) == 0) "" else uri
107+
if (key == "") return(invisible(NULL))
94108
if (self$workspaces$has(key)) {
109+
workspace <- self$workspaces$get(key)
110+
for (doc_uri in workspace$documents$keys()) {
111+
diagnostics_callback(self, doc_uri, NULL, list())
112+
doc <- workspace$documents$get(doc_uri)
113+
if (isTRUE(doc$is_open)) {
114+
self$workspaces$get("")$documents$set(doc_uri, doc)
115+
}
116+
}
95117
self$workspaces$remove(key)
96118
}
97119
},
98120
get_workspace = function(uri) {
99121
workspaces_list <- self$workspaces$values()
100-
if (length(workspaces_list) == 0) return(NULL)
101122

102123
default_key <- if (length(self$rootUri) == 0) "" else self$rootUri
103-
default_workspace <- self$workspaces$get(default_key, workspaces_list[[1]])
124+
default_workspace <- self$workspaces$get(default_key, self$workspaces$get(""))
104125

105126
if (length(uri) == 0) return(default_workspace)
106127

tests/testthat/test-workspace.R

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,45 @@ test_that("Multiple workspace folders are loaded and handled", {
179179
})
180180
expect_equal(length(result), 3)
181181
})
182+
183+
test_that("Fallback workspace cleans up documents when directory is added", {
184+
skip_on_cran()
185+
186+
client <- language_client(NULL, workspace_folders = NULL)
187+
188+
dir1 <- tempfile()
189+
dir.create(dir1)
190+
dir.create(file.path(dir1, "R"))
191+
file.create(file.path(dir1, "DESCRIPTION"))
192+
file1 <- file.path(dir1, "R", "test1.R")
193+
writeLines(c("fun_wk1 <- function(x) { x }"), file1)
194+
195+
# Open the file, it goes to fallback workspace
196+
client %>% did_open(file1)
197+
198+
# Wait for file to be parsed
199+
Sys.sleep(0.5)
200+
201+
# Add a workspace that contains the file
202+
client %>% notify(
203+
"workspace/didChangeWorkspaceFolders", list(
204+
event = list(
205+
added = list(list(uri = path_to_uri(dir1), name = basename(dir1))),
206+
removed = list()
207+
)
208+
))
209+
210+
# Wait for workspace update to process
211+
Sys.sleep(0.5)
212+
213+
# Server should have loaded the file as part of the new workspace folder,
214+
# but the old reference in the fallback workspace should be deleted.
215+
# If we search for fun_wk1, it should only be found once (in the new workspace).
216+
result <- client %>% respond_workspace_symbol("fun_wk1",
217+
retry_when = function(result) {
218+
length(result) == 0
219+
})
220+
221+
expect_equal(length(result), 1)
222+
})
223+

0 commit comments

Comments
 (0)