Move testthat out of Document

Author: lionel-

crates/ark/src/lsp/diagnostics.rs

@@ -135,7 +135,11 @@ impl<'a> DiagnosticContext<'a> {
     }
 }
 
-pub(crate) fn generate_diagnostics(doc: Document, state: WorldState) -> Vec<Diagnostic> {
+pub(crate) fn generate_diagnostics(
+    doc: Document,
+    state: WorldState,
+    testthat: bool,
+) -> Vec<Diagnostic> {
     let mut diagnostics = Vec::new();
 
     if !state.config.diagnostics.enable {
@@ -190,7 +194,7 @@ pub(crate) fn generate_diagnostics(doc: Document, state: WorldState) -> Vec<Diag
     // don't special-case how workspace inclusion works for packages). We might
     // want to provide a mechanism for test packages to declare this sort of
     // test files setup.
-    if doc.testthat {
+    if testthat {
         if let Some(pkg) = state.library.get("testthat") {
             for export in &pkg.namespace.exports {
                 context.workspace_symbols.insert(export.clone());
@@ -1138,10 +1142,10 @@ mod tests {
 
     use harp::eval::RParseEvalOptions;
     use once_cell::sync::Lazy;
+    use tower_lsp::lsp_types;
     use tower_lsp::lsp_types::Position;
 
     use crate::interface::console_inputs;
-    use crate::lsp::diagnostics::generate_diagnostics;
     use crate::lsp::documents::Document;
     use crate::lsp::inputs::library::Library;
     use crate::lsp::inputs::package::Package;
@@ -1154,6 +1158,10 @@ mod tests {
     // Default state that includes installed packages and default scopes.
     static DEFAULT_STATE: Lazy<WorldState> = Lazy::new(|| current_state());
 
+    fn generate_diagnostics(doc: Document, state: WorldState) -> Vec<lsp_types::Diagnostic> {
+        super::generate_diagnostics(doc, state, false)
+    }
+
     fn current_state() -> WorldState {
         let inputs = console_inputs().unwrap();
 

crates/ark/src/lsp/documents.rs

@@ -48,10 +48,6 @@ pub struct Document {
 
     // Configuration of the document, such as indentation settings.
     pub config: DocumentConfig,
-
-    /// Whether the document is a testthat file. This property should not be
-    /// here, it's just a short term stopgap.
-    pub testthat: bool,
 }
 
 impl std::fmt::Debug for Document {
@@ -84,7 +80,6 @@ impl Document {
             version,
             ast,
             config: Default::default(),
-            testthat: false,
         }
     }
 

crates/ark/src/lsp/main_loop.rs

@@ -827,15 +827,11 @@ async fn process_diagnostics_batch(batch: Vec<RefreshDiagnosticsTask>) {
                 // stopgap approach that has some false positives (e.g. when we
                 // work on testthat itself the flag will always be true), but
                 // that shouldn't have much practical impact.
-                let mut doc = document.clone();
-                if Path::new(uri.path())
+                let testthat = Path::new(uri.path())
                     .components()
-                    .any(|c| c.as_os_str() == "testthat")
-                {
-                    doc.testthat = true;
-                };
+                    .any(|c| c.as_os_str() == "testthat");
 
-                let diagnostics = generate_diagnostics(doc, state.clone());
+                let diagnostics = generate_diagnostics(document.clone(), state.clone(), testthat);
                 Some(RefreshDiagnosticsResult {
                     uri,
                     diagnostics,