From 0d64c481131451f985ca84b33bcfdb18170d1c5a Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Tue, 25 Feb 2025 09:01:36 +0100 Subject: [PATCH 1/8] Implement workspace watcher Fixes + test Fix Windows tests (no need for snapshots) --- Cargo.lock | 102 +++++++- crates/biome_cli/src/commands/daemon.rs | 11 +- crates/biome_cli/src/commands/mod.rs | 1 + crates/biome_cli/src/execute/migrate.rs | 2 +- crates/biome_cli/src/execute/mod.rs | 2 +- .../execute/process_file/workspace_file.rs | 2 +- crates/biome_cli/src/execute/std_in.rs | 4 +- crates/biome_cli/tests/main.rs | 4 +- crates/biome_fs/src/fs.rs | 12 + crates/biome_fs/src/fs/os.rs | 2 +- crates/biome_lsp/Cargo.toml | 1 + .../biome_lsp/src/handlers/text_document.rs | 2 +- crates/biome_lsp/src/server.rs | 58 +++-- crates/biome_lsp/src/server.tests.rs | 244 +++++++++++++++++- crates/biome_lsp/src/session.rs | 1 + crates/biome_service/Cargo.toml | 1 + crates/biome_service/src/diagnostics.rs | 15 ++ crates/biome_service/src/lib.rs | 4 +- crates/biome_service/src/projects.rs | 75 +++--- crates/biome_service/src/workspace.rs | 35 +-- crates/biome_service/src/workspace.tests.rs | 33 +-- crates/biome_service/src/workspace/scanner.rs | 16 +- crates/biome_service/src/workspace/server.rs | 132 +++++++--- .../src/workspace/server.tests.rs | 5 +- crates/biome_service/src/workspace/watcher.rs | 190 ++++++++++++++ crates/biome_service/src/workspace_watcher.rs | 174 +++++++++++++ .../@biomejs/backend-jsonrpc/src/workspace.ts | 2 +- .../backend-jsonrpc/tests/workspace.test.mjs | 1 - packages/@biomejs/js-api/src/index.ts | 1 - 29 files changed, 985 insertions(+), 147 deletions(-) create mode 100644 crates/biome_service/src/workspace/watcher.rs create mode 100644 crates/biome_service/src/workspace_watcher.rs diff --git a/Cargo.lock b/Cargo.lock index 5b62eb2db24b..d32ff4be0fb5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1072,6 +1072,7 @@ dependencies = [ "biome_service", "biome_text_edit", "camino", + "crossbeam", "futures", "papaya", "rustc-hash 2.1.1", @@ -1302,6 +1303,7 @@ dependencies = [ "getrandom 0.2.15", "ignore", "insta", + "notify", "papaya", "rayon", "regex", @@ -2099,6 +2101,18 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d7e9bc68be4cdabbb8938140b01a8b5bc1191937f2c7e7ecc2fcebbe2d749df" +[[package]] +name = "filetime" +version = "0.2.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35c0522e981e68cbfa8c3f978441a5f34b30b96e146b33cd3359176b50fe8586" +dependencies = [ + "cfg-if", + "libc", + "libredox", + "windows-sys 0.59.0", +] + [[package]] name = "flate2" version = "1.0.35" @@ -2133,6 +2147,15 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fsevent-sys" +version = "4.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76ee7a02da4d231650c7cea31349b889be2f45ddb3ef3032d2ec8185f6313fd2" +dependencies = [ + "libc", +] + [[package]] name = "fst" version = "0.4.7" @@ -2575,6 +2598,26 @@ dependencies = [ "web-time", ] +[[package]] +name = "inotify" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f37dccff2791ab604f9babef0ba14fbe0be30bd368dc541e2b08d07c8aa908f3" +dependencies = [ + "bitflags 2.8.0", + "inotify-sys", + "libc", +] + +[[package]] +name = "inotify-sys" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e05c02b5e89bff3b946cedeca278abc628fe811e604f027c45a8aa3cf793d0eb" +dependencies = [ + "libc", +] + [[package]] name = "insta" version = "1.42.1" @@ -2664,6 +2707,26 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9dbbfed4e59ba9750e15ba154fdfd9329cee16ff3df539c2666b70f58cc32105" +[[package]] +name = "kqueue" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7447f1ca1b7b563588a205fe93dea8df60fd981423a768bc1c0ded35ed147d0c" +dependencies = [ + "kqueue-sys", + "libc", +] + +[[package]] +name = "kqueue-sys" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed9625ffda8729b85e45cf04090035ac368927b8cebc34898e7c120f52e4838b" +dependencies = [ + "bitflags 1.3.2", + "libc", +] + [[package]] name = "lazy_static" version = "1.5.0" @@ -2706,6 +2769,7 @@ checksum = "c0ff37bd590ca25063e35af745c343cb7a0271906fb7b37e4813e8f79f00268d" dependencies = [ "bitflags 2.8.0", "libc", + "redox_syscall 0.5.9", ] [[package]] @@ -2849,6 +2913,7 @@ checksum = "4569e456d394deccd22ce1c1913e6ea0e54519f577285001215d33557431afe4" dependencies = [ "hermit-abi", "libc", + "log", "wasi 0.11.0+wasi-snapshot-preview1", "windows-sys 0.52.0", ] @@ -2897,6 +2962,32 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "notify" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fee8403b3d66ac7b26aee6e40a897d85dc5ce26f44da36b8b73e987cc52e943" +dependencies = [ + "bitflags 2.8.0", + "crossbeam-channel", + "filetime", + "fsevent-sys", + "inotify", + "kqueue", + "libc", + "log", + "mio", + "notify-types", + "walkdir", + "windows-sys 0.59.0", +] + +[[package]] +name = "notify-types" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e0826a989adedc2a244799e823aece04662b66609d96af8dff7ac6df9a8925d" + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -3024,7 +3115,7 @@ checksum = "4c42a9226546d68acdd9c0a280d17ce19bfe27a46bf68784e4066115788d008e" dependencies = [ "cfg-if", "libc", - "redox_syscall", + "redox_syscall 0.4.1", "smallvec", "windows-targets 0.48.5", ] @@ -3397,6 +3488,15 @@ dependencies = [ "bitflags 1.3.2", ] +[[package]] +name = "redox_syscall" +version = "0.5.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82b568323e98e49e2a0899dcee453dd679fae22d69adf9b11dd508d1549b7e2f" +dependencies = [ + "bitflags 2.8.0", +] + [[package]] name = "redox_users" version = "0.5.0" diff --git a/crates/biome_cli/src/commands/daemon.rs b/crates/biome_cli/src/commands/daemon.rs index f946d1b1e466..5dbfe8a8c145 100644 --- a/crates/biome_cli/src/commands/daemon.rs +++ b/crates/biome_cli/src/commands/daemon.rs @@ -6,7 +6,7 @@ use crate::{ use biome_console::{markup, ConsoleExt}; use biome_fs::OsFileSystem; use biome_lsp::ServerFactory; -use biome_service::{workspace::WorkspaceClient, TransportError, WorkspaceError}; +use biome_service::{workspace::WorkspaceClient, TransportError, WorkspaceError, WorkspaceWatcher}; use camino::{Utf8Path, Utf8PathBuf}; use std::{env, fs}; use tokio::io; @@ -80,8 +80,10 @@ pub(crate) fn run_server( ) -> Result<(), CliDiagnostic> { setup_tracing_subscriber(log_path.as_deref(), log_file_name_prefix.as_deref()); + let (mut watcher, instruction_channel) = WorkspaceWatcher::new()?; + let rt = Runtime::new()?; - let factory = ServerFactory::new(stop_on_disconnect); + let factory = ServerFactory::new(stop_on_disconnect, instruction_channel.sender.clone()); let cancellation = factory.cancellation(); let span = debug_span!("Running Server", pid = std::process::id(), @@ -90,6 +92,11 @@ pub(crate) fn run_server( log_file_name_prefix = &log_file_name_prefix.as_deref(), ); + let workspace = factory.workspace(); + rt.spawn_blocking(move || { + watcher.run(workspace.as_ref()); + }); + rt.block_on(async move { tokio::select! { res = run_daemon(factory, config_path).instrument(span) => { diff --git a/crates/biome_cli/src/commands/mod.rs b/crates/biome_cli/src/commands/mod.rs index 7cd7ef828072..00972c0edc36 100644 --- a/crates/biome_cli/src/commands/mod.rs +++ b/crates/biome_cli/src/commands/mod.rs @@ -811,6 +811,7 @@ pub(crate) trait CommandRunner: Sized { let result = workspace.scan_project_folder(ScanProjectFolderParams { project_key, path: Some(project_path), + watch: cli_options.use_server, })?; for diagnostic in result.diagnostics { if diagnostic.severity() >= Severity::Error { diff --git a/crates/biome_cli/src/execute/migrate.rs b/crates/biome_cli/src/execute/migrate.rs index 5323551dfb5a..8a57a806c543 100644 --- a/crates/biome_cli/src/execute/migrate.rs +++ b/crates/biome_cli/src/execute/migrate.rs @@ -79,7 +79,7 @@ pub(crate) fn run(migrate_payload: MigratePayload) -> Result<(), CliDiagnostic> project_key, path: biome_path.clone(), content: FileContent::FromClient(biome_config_content.to_string()), - version: 0, + version: Some(0), document_file_source: Some(JsonFileSource::json().into()), persist_node_cache: false, })?; diff --git a/crates/biome_cli/src/execute/mod.rs b/crates/biome_cli/src/execute/mod.rs index 3c6d76756865..8ea42c109f57 100644 --- a/crates/biome_cli/src/execute/mod.rs +++ b/crates/biome_cli/src/execute/mod.rs @@ -600,7 +600,7 @@ pub fn execute_mode( project_key, content: FileContent::FromClient(content), path: report_file.clone(), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, })?; diff --git a/crates/biome_cli/src/execute/process_file/workspace_file.rs b/crates/biome_cli/src/execute/process_file/workspace_file.rs index 1a3a8d9ac1d8..cb24e320667b 100644 --- a/crates/biome_cli/src/execute/process_file/workspace_file.rs +++ b/crates/biome_cli/src/execute/process_file/workspace_file.rs @@ -37,7 +37,7 @@ impl<'ctx, 'app> WorkspaceFile<'ctx, 'app> { project_key: ctx.project_key, document_file_source: None, path: path.clone(), - version: 0, + version: None, content: FileContent::FromClient(input.clone()), persist_node_cache: false, }, diff --git a/crates/biome_cli/src/execute/std_in.rs b/crates/biome_cli/src/execute/std_in.rs index a7bba8656a4c..8d2f2c8d80b4 100644 --- a/crates/biome_cli/src/execute/std_in.rs +++ b/crates/biome_cli/src/execute/std_in.rs @@ -50,7 +50,7 @@ pub(crate) fn run<'a>( workspace.open_file(OpenFileParams { project_key, path: biome_path.clone(), - version: 0, + version: None, content: FileContent::FromClient(content.into()), document_file_source: None, persist_node_cache: false, @@ -85,7 +85,7 @@ pub(crate) fn run<'a>( workspace.open_file(OpenFileParams { project_key, path: biome_path.clone(), - version: 0, + version: None, content: FileContent::FromClient(content.into()), document_file_source: None, persist_node_cache: false, diff --git a/crates/biome_cli/tests/main.rs b/crates/biome_cli/tests/main.rs index 7f4397734c46..9d3e7e3b0e07 100644 --- a/crates/biome_cli/tests/main.rs +++ b/crates/biome_cli/tests/main.rs @@ -9,7 +9,7 @@ use snap_test::assert_cli_snapshot; use biome_cli::{biome_command, CliDiagnostic, CliSession}; use biome_console::{markup, BufferConsole, Console, ConsoleExt}; -use biome_fs::{FileSystem, MemoryFileSystem}; +use biome_fs::{FileSystem, MemoryFileSystem, OsFileSystem}; use biome_service::App; use bpaf::ParseFailure; @@ -359,7 +359,7 @@ pub(crate) fn run_cli_with_dyn_fs( runtime::Runtime, }; - let factory = ServerFactory::default(); + let factory = ServerFactory::new_with_fs(Box::new(OsFileSystem::default())); let connection = factory.create(None); let runtime = Runtime::new().expect("failed to create runtime"); diff --git a/crates/biome_fs/src/fs.rs b/crates/biome_fs/src/fs.rs index 308bc84932e1..1d5acc755da9 100644 --- a/crates/biome_fs/src/fs.rs +++ b/crates/biome_fs/src/fs.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use std::collections::BTreeSet; use std::fmt::{Debug, Display, Formatter}; use std::panic::RefUnwindSafe; +use std::path::Path; use std::sync::Arc; use std::{fmt, io}; use tracing::{error, info}; @@ -450,6 +451,17 @@ impl Display for FileSystemDiagnostic { } } +impl FileSystemDiagnostic { + pub fn non_utf8_path(path: &Path) -> Self { + Self { + severity: Severity::Error, + path: path.display().to_string(), + error_kind: FsErrorKind::NonUtf8Path, + source: None, + } + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub enum FsErrorKind { /// File not found diff --git a/crates/biome_fs/src/fs/os.rs b/crates/biome_fs/src/fs/os.rs index 4c8a265cd8cb..adeebfe0ee6d 100644 --- a/crates/biome_fs/src/fs/os.rs +++ b/crates/biome_fs/src/fs/os.rs @@ -501,7 +501,7 @@ impl From for FsErrorKind { pub struct TemporaryFs { /// The current working directory. It's the OS temporary folder joined with a file /// name passed in the [TemporaryFs::new] function - working_directory: Utf8PathBuf, + pub working_directory: Utf8PathBuf, files: Vec<(Utf8PathBuf, String)>, } diff --git a/crates/biome_lsp/Cargo.toml b/crates/biome_lsp/Cargo.toml index c3c3b126a60f..97e552c7a1e9 100644 --- a/crates/biome_lsp/Cargo.toml +++ b/crates/biome_lsp/Cargo.toml @@ -26,6 +26,7 @@ biome_rowan = { workspace = true } biome_service = { workspace = true } biome_text_edit = { workspace = true } camino = { workspace = true } +crossbeam = { workspace = true } futures = "0.3.31" papaya = { workspace = true } rustc-hash = { workspace = true } diff --git a/crates/biome_lsp/src/handlers/text_document.rs b/crates/biome_lsp/src/handlers/text_document.rs index bbbb1fbae230..db99dfac3d85 100644 --- a/crates/biome_lsp/src/handlers/text_document.rs +++ b/crates/biome_lsp/src/handlers/text_document.rs @@ -51,8 +51,8 @@ pub(crate) async fn did_open( session.workspace.open_file(OpenFileParams { project_key, path, - version, content: FileContent::FromClient(content), + version: Some(version), document_file_source: Some(language_hint), persist_node_cache: true, })?; diff --git a/crates/biome_lsp/src/server.rs b/crates/biome_lsp/src/server.rs index 07046de82e49..e41a86038383 100644 --- a/crates/biome_lsp/src/server.rs +++ b/crates/biome_lsp/src/server.rs @@ -8,19 +8,20 @@ use crate::utils::{into_lsp_error, panic_to_lsp_error}; use crate::{handlers, requests}; use biome_console::markup; use biome_diagnostics::panic::PanicError; -use biome_fs::{ConfigName, FileSystem, OsFileSystem}; +use biome_fs::{ConfigName, FileSystem, MemoryFileSystem, OsFileSystem}; use biome_service::workspace::{ CloseProjectParams, OpenProjectParams, RageEntry, RageParams, RageResult, }; -use biome_service::{workspace, Workspace}; +use biome_service::{WatcherInstruction, WorkspaceServer}; use camino::Utf8PathBuf; +use crossbeam::channel::{bounded, Sender}; use futures::future::ready; use futures::FutureExt; use rustc_hash::FxHashMap; use serde_json::json; use std::panic::RefUnwindSafe; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; -use std::sync::{Arc, Mutex, OnceLock}; +use std::sync::{Arc, Mutex}; use tokio::io::{AsyncRead, AsyncWrite}; use tokio::sync::Notify; use tokio::task::spawn_blocking; @@ -530,16 +531,13 @@ macro_rules! workspace_method { /// Factory data structure responsible for creating [ServerConnection] handles /// for each incoming connection accepted by the server -#[derive(Default)] pub struct ServerFactory { /// Synchronization primitive used to broadcast a shutdown signal to all /// active connections cancellation: Arc, /// [Workspace] instance shared between all clients. - /// - /// Initialized when the first connection is created. - workspace: OnceLock>, + workspace: Arc, /// The sessions of the connected clients indexed by session key. sessions: Sessions, @@ -550,16 +548,27 @@ pub struct ServerFactory { /// If this is true the server will broadcast a shutdown signal once the /// last client disconnected stop_on_disconnect: bool, + /// This shared flag is set to true once at least one sessions has been /// initialized on this server instance is_initialized: Arc, } +impl Default for ServerFactory { + fn default() -> Self { + Self::new_with_fs(Box::new(MemoryFileSystem::default())) + } +} + impl ServerFactory { - pub fn new(stop_on_disconnect: bool) -> Self { + /// Regular constructor for use in the daemon. + pub fn new(stop_on_disconnect: bool, instruction_tx: Sender) -> Self { Self { cancellation: Arc::default(), - workspace: OnceLock::default(), + workspace: Arc::new(WorkspaceServer::new( + Box::new(OsFileSystem::default()), + instruction_tx, + )), sessions: Sessions::default(), next_session_key: AtomicU64::new(0), stop_on_disconnect, @@ -567,20 +576,22 @@ impl ServerFactory { } } - pub fn create(&self, config_path: Option) -> ServerConnection { - self.create_with_fs(config_path, Box::new(OsFileSystem::default())) + /// Constructor for use in tests. + pub fn new_with_fs(fs: Box) -> Self { + let (tx, _) = bounded(0); + Self { + cancellation: Arc::default(), + workspace: Arc::new(WorkspaceServer::new(fs, tx)), + sessions: Sessions::default(), + next_session_key: AtomicU64::new(0), + stop_on_disconnect: true, + is_initialized: Arc::default(), + } } - /// Create a new [ServerConnection] from this factory - pub fn create_with_fs( - &self, - config_path: Option, - fs: Box, - ) -> ServerConnection { - let workspace = self - .workspace - .get_or_init(|| workspace::server_sync(fs)) - .clone(); + /// Creates a new [ServerConnection] from this factory. + pub fn create(&self, config_path: Option) -> ServerConnection { + let workspace = self.workspace.clone(); let session_key = SessionKey(self.next_session_key.fetch_add(1, Ordering::Relaxed)); @@ -647,6 +658,11 @@ impl ServerFactory { pub fn cancellation(&self) -> Arc { self.cancellation.clone() } + + /// Returns the workspace used by this server. + pub fn workspace(&self) -> Arc { + self.workspace.clone() + } } /// Handle type created by the server for each incoming connection diff --git a/crates/biome_lsp/src/server.tests.rs b/crates/biome_lsp/src/server.tests.rs index 2f88dbe705bf..feff3a5c4e21 100644 --- a/crates/biome_lsp/src/server.tests.rs +++ b/crates/biome_lsp/src/server.tests.rs @@ -5,10 +5,15 @@ use std::slice; use std::time::Duration; use anyhow::{bail, Context, Error, Result}; -use biome_fs::{BiomePath, MemoryFileSystem}; +use biome_analyze::RuleCategories; +use biome_configuration::analyzer::RuleSelector; +use biome_diagnostics::PrintDescription; +use biome_fs::{BiomePath, MemoryFileSystem, TemporaryFs}; use biome_service::workspace::{ GetFileContentParams, GetSyntaxTreeParams, GetSyntaxTreeResult, OpenProjectParams, + PullDiagnosticsParams, PullDiagnosticsResult, ScanProjectFolderParams, ScanProjectFolderResult, }; +use biome_service::WorkspaceWatcher; use camino::Utf8PathBuf; use futures::channel::mpsc::{channel, Sender}; use futures::{Sink, SinkExt, Stream, StreamExt}; @@ -172,12 +177,12 @@ impl Server { Ok(()) } - /// It creates two workspaces, one at folder `test_one` and the other in `test_two`. + /// It creates two projects, one at folder `test_one` and the other in `test_two`. /// /// Hence, the two roots will be `/workspace/test_one` and `/workspace/test_two` // The `root_path` field is deprecated, but we still need to specify it #[expect(deprecated)] - async fn initialize_workspaces(&mut self) -> Result<()> { + async fn initialize_projects(&mut self) -> Result<()> { let _res: InitializeResult = self .request( "initialize", @@ -1581,7 +1586,6 @@ async fn pull_diagnostics_for_rome_json() -> Result<()> { #[tokio::test] async fn pull_diagnostics_for_css_files() -> Result<()> { - let factory = ServerFactory::default(); let mut fs = MemoryFileSystem::default(); let config = r#"{ "css": { @@ -1596,7 +1600,9 @@ async fn pull_diagnostics_for_css_files() -> Result<()> { Utf8PathBuf::from_path_buf(url!("biome.json").to_file_path().unwrap()).unwrap(), config, ); - let (service, client) = factory.create_with_fs(None, Box::new(fs)).into_inner(); + + let factory = ServerFactory::new_with_fs(Box::new(fs)); + let (service, client) = factory.create(None).into_inner(); let (stream, sink) = client.split(); let mut server = Server::new(service); @@ -1931,7 +1937,6 @@ if(a === -0) {} #[tokio::test] async fn does_not_pull_action_for_disabled_rule_in_override_issue_2782() -> Result<()> { - let factory = ServerFactory::default(); let mut fs = MemoryFileSystem::default(); let config = r#"{ "$schema": "https://biomejs.dev/schemas/1.7.3/schema.json", @@ -1963,7 +1968,9 @@ async fn does_not_pull_action_for_disabled_rule_in_override_issue_2782() -> Resu Utf8PathBuf::from_path_buf(url!("biome.json").to_file_path().unwrap()).unwrap(), config, ); - let (service, client) = factory.create_with_fs(None, Box::new(fs)).into_inner(); + + let factory = ServerFactory::new_with_fs(Box::new(fs)); + let (service, client) = factory.create(None).into_inner(); let (stream, sink) = client.split(); let mut server = Server::new(service); @@ -2450,7 +2457,6 @@ async fn format_jsx_in_javascript_file() -> Result<()> { #[tokio::test] #[ignore = "flaky, it times out on CI"] async fn does_not_format_ignored_files() -> Result<()> { - let factory = ServerFactory::default(); let mut fs = MemoryFileSystem::default(); let config = r#"{ "files": { @@ -2462,7 +2468,9 @@ async fn does_not_format_ignored_files() -> Result<()> { Utf8PathBuf::from_path_buf(url!("biome.json").to_file_path().unwrap()).unwrap(), config, ); - let (service, client) = factory.create_with_fs(None, Box::new(fs)).into_inner(); + + let factory = ServerFactory::new_with_fs(Box::new(fs)); + let (service, client) = factory.create(None).into_inner(); let (stream, sink) = client.split(); let mut server = Server::new(service); @@ -2657,7 +2665,7 @@ async fn multiple_projects() -> Result<()> { let (sender, _) = channel(CHANNEL_BUFFER_SIZE); let reader = tokio::spawn(client_handler(stream, sink, sender)); - server.initialize_workspaces().await?; + server.initialize_projects().await?; server.initialized().await?; let config_only_formatter = r#"{ @@ -2768,7 +2776,6 @@ async fn multiple_projects() -> Result<()> { #[tokio::test] async fn pull_source_assist_action() -> Result<()> { - let factory = ServerFactory::default(); let mut fs = MemoryFileSystem::default(); let config = r#"{ "assist": { @@ -2785,7 +2792,9 @@ async fn pull_source_assist_action() -> Result<()> { Utf8PathBuf::from_path_buf(url!("biome.json").to_file_path().unwrap()).unwrap(), config, ); - let (service, client) = factory.create_with_fs(None, Box::new(fs)).into_inner(); + + let factory = ServerFactory::new_with_fs(Box::new(fs)); + let (service, client) = factory.create(None).into_inner(); let (stream, sink) = client.split(); let mut server = Server::new(service); @@ -2941,3 +2950,214 @@ async fn pull_source_assist_action() -> Result<()> { Ok(()) } + +#[tokio::test] +async fn watcher_updates_dependency_graph() -> Result<()> { + // ARRANGE: Set up FS and LSP connection in order to test import cycles. + let mut fs = TemporaryFs::new("watcher_updates_dependency_graph"); + fs.create_file( + "biome.json", + r#"{ + "linter": { + "enabled": true, + "rules": { + "nursery": { + "noImportCycles": "error" + } + } + } +} +"#, + ); + + fs.create_file( + "foo.ts", + r#"import { bar } from "./bar.ts"; + +export function foo() { + bar(); +} +"#, + ); + + fs.create_file( + "bar.ts", + r#"import { foo } from "./foo.ts"; + +export function bar() { + foo(); +} +"#, + ); + + let (mut watcher, instruction_channel) = WorkspaceWatcher::new()?; + + let factory = ServerFactory::new(true, instruction_channel.sender.clone()); + + let workspace = factory.workspace(); + tokio::task::spawn_blocking(move || { + watcher.run(workspace.as_ref()); + }); + + let (service, client) = factory.create(None).into_inner(); + let (stream, sink) = client.split(); + let mut server = Server::new(service); + + let (sender, _) = channel(CHANNEL_BUFFER_SIZE); + let reader = tokio::spawn(client_handler(stream, sink, sender)); + + server.initialize().await?; + + let project_key = server + .request( + "biome/open_project", + "open_project", + OpenProjectParams { + path: fs.working_directory.clone().into(), + open_uninitialized: true, + }, + ) + .await? + .expect("open_project returned an error"); + + // ARRANGE: Scanning the project folder initializes the service data. + let result: ScanProjectFolderResult = server + .request( + "biome/scan_project_folder", + "scan_project_folder", + ScanProjectFolderParams { + project_key, + path: None, + watch: true, + }, + ) + .await? + .expect("scan_project_folder returned an error"); + assert_eq!(result.diagnostics.len(), 0); + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::all(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: One diagnostic should be emitted for the cyclic dependency. + assert_eq!(result.diagnostics.len(), 1); + assert_eq!( + PrintDescription(&result.diagnostics[0]).to_string(), + "This import is part of a cycle." + ); + + // ARRANGE: Remove `bar.ts`. + std::fs::remove_file(fs.working_directory.join("bar.ts")).expect("Cannot remove bar.ts"); + + // FIXME: If this test is unstable, we may need to wait here. + // Right now, we don't know if the watcher already processed the + // removal. It seems everything works fine though :D + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::empty(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: Diagnostic should've disappeared because `bar.ts` is removed. + assert_eq!(result.diagnostics.len(), 0); + + // ARRANGE: Recreate `bar.ts`. + fs.create_file( + "bar.ts", + r#"import { foo } from "./foo.ts"; + + export function bar() { + foo(); + } + "#, + ); + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::all(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: Diagnostic is expected to reappear. + assert_eq!(result.diagnostics.len(), 1); + assert_eq!( + PrintDescription(&result.diagnostics[0]).to_string(), + "This import is part of a cycle." + ); + + // ARRANGE: Fix `bar.ts`. + fs.create_file( + "bar.ts", + r#"import { foo } from "./shared.ts"; + + export function bar() { + foo(); + } + "#, + ); + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::all(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: Diagnostic should disappear again with a fixed `bar.ts`. + assert_eq!(result.diagnostics.len(), 0); + + server.shutdown().await?; + reader.abort(); + + Ok(()) +} diff --git a/crates/biome_lsp/src/session.rs b/crates/biome_lsp/src/session.rs index ca912deaf03d..d764e2db0e94 100644 --- a/crates/biome_lsp/src/session.rs +++ b/crates/biome_lsp/src/session.rs @@ -545,6 +545,7 @@ impl Session { .scan_project_folder(ScanProjectFolderParams { project_key, path: Some(project_path), + watch: true, }); match result { diff --git a/crates/biome_service/Cargo.toml b/crates/biome_service/Cargo.toml index 0a54bc35d3a6..5351bbf42f3c 100644 --- a/crates/biome_service/Cargo.toml +++ b/crates/biome_service/Cargo.toml @@ -61,6 +61,7 @@ crossbeam = { workspace = true } enumflags2 = { workspace = true, features = ["serde"] } getrandom = { workspace = true, features = ["js"] } ignore = { workspace = true } +notify = { version = "8.0", features = ["crossbeam-channel"] } papaya = { workspace = true } rayon = { workspace = true } regex = { workspace = true } diff --git a/crates/biome_service/src/diagnostics.rs b/crates/biome_service/src/diagnostics.rs index 9ae6975c3fb8..7e715b27e990 100644 --- a/crates/biome_service/src/diagnostics.rs +++ b/crates/biome_service/src/diagnostics.rs @@ -59,6 +59,8 @@ pub enum WorkspaceError { ProtectedFile(ProtectedFile), /// Error when searching for a pattern SearchError(SearchError), + /// Error in the workspace watcher. + WatchError(WatchError), } impl WorkspaceError { @@ -589,6 +591,19 @@ impl Advices for ProtectedFileAdvice { } } +#[derive(Debug, Deserialize, Diagnostic, Serialize)] +#[diagnostic( + category = "project", + severity = Error, + message( + message("Biome cannot watch files on disk: "{self.reason}), + description = "Biome cannot watch files on disk: {reason}", + ), +)] +pub struct WatchError { + pub reason: String, +} + #[cfg(test)] mod test { use crate::diagnostics::{CantReadFile, FileIgnored, NotFound, SourceFileNotSupported}; diff --git a/crates/biome_service/src/lib.rs b/crates/biome_service/src/lib.rs index 57d26b1d9ec5..59302490df84 100644 --- a/crates/biome_service/src/lib.rs +++ b/crates/biome_service/src/lib.rs @@ -4,6 +4,7 @@ pub mod file_handlers; pub mod projects; pub mod settings; pub mod workspace; +mod workspace_watcher; pub mod configuration; pub mod diagnostics; @@ -20,7 +21,8 @@ use biome_fs::{FileSystem, OsFileSystem}; pub use diagnostics::{extension_error, TransportError, WorkspaceError}; pub use file_handlers::JsFormatterSettings; -pub use workspace::Workspace; +pub use workspace::{Workspace, WorkspaceServer}; +pub use workspace_watcher::{WatcherInstruction, WorkspaceWatcher}; /// This is the main entrypoint of the application. pub struct App<'app> { diff --git a/crates/biome_service/src/projects.rs b/crates/biome_service/src/projects.rs index d872d726ce27..0e8cbe0f51a8 100644 --- a/crates/biome_service/src/projects.rs +++ b/crates/biome_service/src/projects.rs @@ -10,38 +10,6 @@ use std::num::NonZeroUsize; use std::sync::atomic::{AtomicUsize, Ordering}; use tracing::{debug, instrument}; -/// Type that holds all the settings and information for different projects -/// inside the workspace. -/// -/// ## Terminology -/// -/// Every project within a Biome workspace correlates with a single -/// **top-level** `biome.json`. This means that if the `biome.json` is at the -/// root of a monorepo, multiple packages (or "JavaScript projects") may reside -/// within a single project. -#[derive(Debug, Default)] -pub struct Projects(HashMap); - -#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] -#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] -#[repr(transparent)] -pub struct ProjectKey(NonZeroUsize); - -impl Display for ProjectKey { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "ProjectKey {}", self.0.get()) - } -} - -impl ProjectKey { - #[expect(clippy::new_without_default)] - pub fn new() -> Self { - static KEY: AtomicUsize = AtomicUsize::new(1); - let key = KEY.fetch_add(1, Ordering::Relaxed); - Self(NonZeroUsize::new(key).unwrap()) - } -} - /// The information tracked for each project. #[derive(Debug, Default)] struct ProjectData { @@ -55,6 +23,18 @@ struct ProjectData { settings: Settings, } +/// Type that holds all the settings and information for different projects +/// inside the workspace. +/// +/// ## Terminology +/// +/// Every project within a Biome workspace correlates with a single +/// **top-level** `biome.json`. This means that if the `biome.json` is at the +/// root of a monorepo, multiple packages (or "JavaScript projects") may reside +/// within a single project. +#[derive(Debug, Default)] +pub struct Projects(HashMap); + impl Projects { /// Inserts a new project with the given root path. /// @@ -134,6 +114,15 @@ impl Projects { self.0.pin().get(&project_key).map(|data| data.path.clone()) } + /// Finds the key of the project to which a given path belongs, if any. + pub fn find_project_for_path(&self, path: &Utf8Path) -> Option { + self.0 + .pin() + .iter() + .find(|(_, project_data)| path.starts_with(&project_data.path)) + .map(|(key, _)| *key) + } + /// Checks whether the given `path` belongs to project with the given path /// and no other project. pub fn path_belongs_only_to_project_with_path( @@ -144,7 +133,7 @@ impl Projects { let mut belongs_to_project = false; let mut belongs_to_other = false; for project_data in self.0.pin().values() { - if path.strip_prefix(project_data.path.as_path()).is_ok() { + if path.starts_with(&project_data.path) { if project_data.path.as_path() == project_path { belongs_to_project = true; } else { @@ -212,3 +201,23 @@ impl Projects { !is_feature_included } } + +#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[repr(transparent)] +pub struct ProjectKey(NonZeroUsize); + +impl Display for ProjectKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "ProjectKey {}", self.0.get()) + } +} + +impl ProjectKey { + #[expect(clippy::new_without_default)] + pub fn new() -> Self { + static KEY: AtomicUsize = AtomicUsize::new(1); + let key = KEY.fetch_add(1, Ordering::Relaxed); + Self(NonZeroUsize::new(key).unwrap()) + } +} diff --git a/crates/biome_service/src/workspace.rs b/crates/biome_service/src/workspace.rs index e0c872811a3f..16f3f7fb381e 100644 --- a/crates/biome_service/src/workspace.rs +++ b/crates/biome_service/src/workspace.rs @@ -54,15 +54,13 @@ mod client; mod scanner; mod server; +mod watcher; -pub use self::client::{TransportRequest, WorkspaceClient, WorkspaceTransport}; -use crate::file_handlers::Capabilities; pub use crate::file_handlers::DocumentFileSource; -use crate::projects::ProjectKey; -use crate::settings::WorkspaceSettingsHandle; -use crate::{Deserialize, Serialize, WorkspaceError}; -use biome_analyze::ActionCategory; -pub use biome_analyze::RuleCategories; +pub use client::{TransportRequest, WorkspaceClient, WorkspaceTransport}; +pub use server::WorkspaceServer; + +use biome_analyze::{ActionCategory, RuleCategories}; use biome_configuration::analyzer::RuleSelector; use biome_configuration::Configuration; use biome_console::{markup, Markup, MarkupBuf}; @@ -75,15 +73,21 @@ use biome_js_syntax::{TextRange, TextSize}; use biome_text_edit::TextEdit; use camino::Utf8Path; use core::str; +use crossbeam::channel::bounded; use enumflags2::{bitflags, BitFlags}; #[cfg(feature = "schema")] use schemars::{gen::SchemaGenerator, schema::Schema}; use smallvec::SmallVec; use std::collections::HashMap; use std::time::Duration; -use std::{borrow::Cow, panic::RefUnwindSafe, sync::Arc}; +use std::{borrow::Cow, panic::RefUnwindSafe}; use tracing::{debug, instrument}; +use crate::file_handlers::Capabilities; +use crate::projects::ProjectKey; +use crate::settings::WorkspaceSettingsHandle; +use crate::{Deserialize, Serialize, WorkspaceError}; + #[derive(Debug, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] @@ -564,7 +568,7 @@ pub struct OpenFileParams { pub project_key: ProjectKey, pub path: BiomePath, pub content: FileContent, - pub version: i32, + pub version: Option, pub document_file_source: Option, /// Set to `true` to persist the node cache used during parsing, in order to @@ -999,6 +1003,11 @@ pub struct ScanProjectFolderParams { /// which part of the project they are interested in. The server may or may /// not use this to avoid scanning parts that are irrelevant to clients. pub path: Option, + + /// Should the watcher be instructed to start watching this path? + /// + /// Does nothing if the watcher is already watching this path. + pub watch: bool, } #[derive(Debug, serde::Serialize, serde::Deserialize)] @@ -1208,12 +1217,8 @@ pub trait Workspace: Send + Sync + RefUnwindSafe { /// Convenience function for constructing a server instance of [Workspace] pub fn server(fs: Box) -> Box { - Box::new(server::WorkspaceServer::new(fs)) -} - -/// Convenience function for constructing a server instance of [Workspace] -pub fn server_sync(fs: Box) -> Arc { - Arc::new(server::WorkspaceServer::new(fs)) + let (tx, _) = bounded(0); + Box::new(WorkspaceServer::new(fs, tx)) } /// Convenience function for constructing a client instance of [Workspace] diff --git a/crates/biome_service/src/workspace.tests.rs b/crates/biome_service/src/workspace.tests.rs index 98f796d10506..75c66886c130 100644 --- a/crates/biome_service/src/workspace.tests.rs +++ b/crates/biome_service/src/workspace.tests.rs @@ -44,7 +44,7 @@ fn debug_control_flow() { project_key, path: BiomePath::new("file.js"), content: FileContent::FromClient(SOURCE.into()), - version: 0, + version: None, document_file_source: Some(DocumentFileSource::from(JsFileSource::default())), persist_node_cache: false, }, @@ -67,7 +67,7 @@ fn recognize_typescript_definition_file() { path: BiomePath::new("file.d.ts"), // the following code snippet can be correctly parsed in .d.ts file but not in .ts file content: FileContent::FromClient("export const foo: number".into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -88,7 +88,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("a.json"), content: FileContent::FromClient(r#"{"a": 42}"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -103,7 +103,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("b.json"), content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -118,7 +118,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("c.json"), content: FileContent::FromClient(r#"{"a": 42,}"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -133,7 +133,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("d.jsonc"), content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -148,7 +148,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("e.jsonc"), content: FileContent::FromClient(r#"{"a": 42,}"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -163,7 +163,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new(".eslintrc.json"), content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -178,7 +178,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("project/.vscode/settings.json"), content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -193,7 +193,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("dir/.eslintrc.json"), content: FileContent::FromClient(r#"{"a": 42,}"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -210,7 +210,7 @@ fn correctly_handle_json_files() { project_key, path: BiomePath::new("tsconfig.json"), content: FileContent::FromClient(r#"{"a": 42,}//comment"#.into()), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -241,7 +241,7 @@ type User { }"# .into(), ), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -269,7 +269,7 @@ fn correctly_pulls_lint_diagnostics() { }"# .into(), ), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -304,7 +304,7 @@ fn pull_grit_debug_info() { }"# .into(), ), - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }, @@ -338,6 +338,7 @@ fn files_loaded_by_the_scanner_are_only_unloaded_when_the_project_is_unregistere .scan_project_folder(ScanProjectFolderParams { project_key, path: None, + watch: false, }) .unwrap(); @@ -362,7 +363,7 @@ fn files_loaded_by_the_scanner_are_only_unloaded_when_the_project_is_unregistere project_key, path: BiomePath::new("/project/a.ts"), content: FileContent::FromServer, - version: 0, + version: None, document_file_source: None, persist_node_cache: false, }) @@ -424,6 +425,7 @@ fn too_large_files_are_tracked_but_not_parsed() { .scan_project_folder(ScanProjectFolderParams { project_key, path: None, + watch: false, }) .unwrap(); @@ -477,6 +479,7 @@ fn plugins_are_loaded_and_used_during_analysis() { .scan_project_folder(ScanProjectFolderParams { project_key, path: None, + watch: false, }) .unwrap(); diff --git a/crates/biome_service/src/workspace/scanner.rs b/crates/biome_service/src/workspace/scanner.rs index c729c3f20697..52ecfb3c9483 100644 --- a/crates/biome_service/src/workspace/scanner.rs +++ b/crates/biome_service/src/workspace/scanner.rs @@ -1,3 +1,13 @@ +//! Scanner for crawling the file system and loading documents into projects. +//! +//! Conceptually, the scanner is more than just the scanning logic in this +//! module. Rather, we also consider the [watcher](crate::WorkspaceWatcher) +//! (which is partly implemented outside the workspace because of its +//! threading requirements) to be a part of the scanner. +//! +//! In other words, the scanner is both the scanning logic in this module as +//! well as the watcher to allow continuous scanning. + use biome_diagnostics::serde::Diagnostic; use biome_diagnostics::{Diagnostic as _, Error, Severity}; use biome_fs::{BiomePath, PathInterner, TraversalContext, TraversalScope}; @@ -26,6 +36,8 @@ pub(crate) struct ScanResult { } impl WorkspaceServer { + /// Performs a file system scan and loads all supported documents into the + /// project. #[instrument(level = "debug", skip(self))] pub(super) fn scan( &self, @@ -128,7 +140,7 @@ fn scan_folder(folder: &Utf8Path, ctx: ScanContext) -> Duration { })); ctx.workspace - .update_dependency_graph_for_paths(&handleable_paths); + .update_dependency_graph_for_paths(&handleable_paths, &[]); start.elapsed() } @@ -234,7 +246,7 @@ fn open_file(ctx: &ScanContext, path: &BiomePath) { path: path.clone(), content: FileContent::FromServer, document_file_source: None, - version: 0, + version: None, persist_node_cache: false, }) }) { diff --git a/crates/biome_service/src/workspace/server.rs b/crates/biome_service/src/workspace/server.rs index a618af6d0d12..e97d091137c1 100644 --- a/crates/biome_service/src/workspace/server.rs +++ b/crates/biome_service/src/workspace/server.rs @@ -12,7 +12,6 @@ use crate::diagnostics::FileTooLarge; use crate::file_handlers::{ Capabilities, CodeActionsParams, DocumentFileSource, FixAllParams, LintParams, ParseResult, }; -use crate::is_dir; use crate::projects::Projects; use crate::settings::WorkspaceSettingsHandle; use crate::workspace::{ @@ -20,6 +19,7 @@ use crate::workspace::{ RageResult, ServerInfo, }; use crate::{file_handlers::Features, Workspace, WorkspaceError}; +use crate::{is_dir, WatcherInstruction}; use append_only_vec::AppendOnlyVec; use biome_analyze::AnalyzerPluginVec; use biome_configuration::plugins::{PluginConfiguration, Plugins}; @@ -43,6 +43,7 @@ use biome_plugin_loader::{BiomePlugin, PluginCache, PluginDiagnostic}; use biome_project_layout::ProjectLayout; use biome_rowan::NodeCache; use camino::{Utf8Path, Utf8PathBuf}; +use crossbeam::channel::Sender; use papaya::HashMap; use rustc_hash::{FxBuildHasher, FxHashMap}; use std::panic::RefUnwindSafe; @@ -50,13 +51,13 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use tracing::{error, info, instrument, warn}; -pub(super) struct WorkspaceServer { +pub struct WorkspaceServer { /// features available throughout the application features: Features, /// Open projects, including their settings, nested packages, and other /// metadata. - projects: Projects, + pub(super) projects: Projects, /// The layout of projects and their internal packages. project_layout: Arc, @@ -68,7 +69,7 @@ pub(super) struct WorkspaceServer { plugin_caches: Arc>, /// Stores the document (text content + version number) associated with a URL - documents: HashMap, + pub(super) documents: HashMap, /// Stores the document sources used across the workspace file_sources: AppendOnlyVec, @@ -94,10 +95,13 @@ pub(super) struct WorkspaceServer { /// anticipated. For other documents, the performance degradation due to /// lock contention would not be worth the potential of faster reparsing /// that may never actually happen. - node_cache: Mutex>, + pub(super) node_cache: Mutex>, /// File system implementation. - fs: Box, + pub(super) fs: Box, + + /// Channel sender for instructions to the [crate::WorkspaceWatcher]. + watcher_tx: Sender, } /// The `Workspace` object is long-lived, so we want it to be able to cross @@ -130,16 +134,12 @@ pub(crate) struct Document { /// Note it doesn't matter if the file is *also* opened explicitly through /// the LSP Proxy, for instance. In such a case, the scanner's "claim" on /// the file should be considered leading. - opened_by_scanner: bool, + pub(super) opened_by_scanner: bool, } impl WorkspaceServer { /// Creates a new [Workspace]. - /// - /// This is implemented as a crate-private method instead of using - /// [Default] to disallow instances of [Workspace] from being created - /// outside a [crate::App] - pub(crate) fn new(fs: Box) -> Self { + pub fn new(fs: Box, watcher_tx: Sender) -> Self { init_thread_pool(); Self { @@ -153,6 +153,7 @@ impl WorkspaceServer { patterns: Default::default(), node_cache: Default::default(), fs, + watcher_tx, } } @@ -290,7 +291,7 @@ impl WorkspaceServer { #[tracing::instrument(level = "debug", skip(self, params), fields( project_key = display(params.project_key), path = display(params.path.as_path()), - version = display(params.version), + version = debug(params.version), ))] fn open_file_internal( &self, @@ -306,6 +307,7 @@ impl WorkspaceServer { persist_node_cache, } = params; let path: Utf8PathBuf = path.into(); + let version = version.unwrap_or_default(); if document_file_source.is_none() && !DocumentFileSource::can_read(path.as_path()) { return Ok(()); @@ -581,7 +583,7 @@ impl WorkspaceServer { // TODO: add documentation for this method pub(super) fn update_project_layout_for_paths(&self, paths: &[BiomePath]) { for path in paths { - if let Err(error) = self.update_project_layout_for_path(path) { + if let Err(error) = self.update_project_layout_for_added_or_changed_path(path) { error!("Error while updating project layout: {error}"); } } @@ -628,7 +630,10 @@ impl WorkspaceServer { } // TODO: add documentation for this method - fn update_project_layout_for_path(&self, path: &BiomePath) -> Result<(), WorkspaceError> { + pub(super) fn update_project_layout_for_added_or_changed_path( + &self, + path: &Utf8Path, + ) -> Result<(), WorkspaceError> { if path .file_name() .is_some_and(|filename| filename == "package.json") @@ -645,12 +650,34 @@ impl WorkspaceServer { Ok(()) } - pub(super) fn update_dependency_graph_for_paths(&self, paths: &[BiomePath]) { + pub(super) fn update_project_layout_for_removed_path( + &self, + path: &Utf8Path, + ) -> Result<(), WorkspaceError> { + if path + .file_name() + .is_some_and(|filename| filename == "package.json") + { + let package_path = path + .parent() + .map(|parent| parent.to_path_buf()) + .ok_or_else(WorkspaceError::not_found)?; + self.project_layout.remove_package(&package_path); + } + + Ok(()) + } + + pub(super) fn update_dependency_graph_for_paths( + &self, + added_or_changed_paths: &[BiomePath], + removed_paths: &[BiomePath], + ) { self.dependency_graph.update_imports_for_js_paths( self.fs.as_ref(), &self.project_layout, - paths, - &[], + added_or_changed_paths, + removed_paths, |path| { let documents = self.documents.pin(); let doc = documents.get(path)?; @@ -666,6 +693,34 @@ impl WorkspaceServer { }, ); } + + pub(super) fn update_service_data_with_added_or_updated_path( + &self, + path: &Utf8Path, + ) -> Result<(), WorkspaceError> { + let path = BiomePath::from(path); + if path.is_manifest() { + self.update_project_layout_for_added_or_changed_path(&path)?; + } + + self.update_dependency_graph_for_paths(&[path], &[]); + + Ok(()) + } + + pub(super) fn update_service_data_with_removed_path( + &self, + path: &Utf8Path, + ) -> Result<(), WorkspaceError> { + let path = BiomePath::from(path); + if path.is_manifest() { + self.update_project_layout_for_removed_path(&path)?; + } + + self.update_dependency_graph_for_paths(&[], &[path]); + + Ok(()) + } } impl Workspace for WorkspaceServer { @@ -810,6 +865,12 @@ impl Workspace for WorkspaceServer { .or_else(|| self.projects.get_project_path(params.project_key)) .ok_or_else(WorkspaceError::no_project)?; + if params.watch { + let _ = self + .watcher_tx + .try_send(WatcherInstruction::WatchFolder(path.clone())); + } + let result = self.scan(params.project_key, &path)?; Ok(ScanProjectFolderResult { @@ -824,6 +885,10 @@ impl Workspace for WorkspaceServer { .get_project_path(params.project_key) .ok_or_else(WorkspaceError::no_project)?; + let _ = self + .watcher_tx + .try_send(WatcherInstruction::UnwatchFolder(project_path.clone())); + // Limit the scope of the pin and the lock inside. { let documents = self.documents.pin(); @@ -967,22 +1032,23 @@ impl Workspace for WorkspaceServer { /// opened by the scanner as well. If the scanner has opened the file, it /// may still be required for multi-file analysis. fn close_file(&self, params: CloseFileParams) -> Result<(), WorkspaceError> { - { + let path = params.path.as_path(); + + let result = { let documents = self.documents.pin(); - let document = documents - .get(params.path.as_path()) - .ok_or_else(WorkspaceError::not_found)?; - if !document.opened_by_scanner { - documents.remove(params.path.as_path()); + let document = documents.get(path).ok_or_else(WorkspaceError::not_found)?; + if document.opened_by_scanner { + Ok(()) // We may need the file for multi-file analysis still. + } else { + documents.remove(path); + + self.update_service_data_with_removed_path(path) } - } + }; - self.node_cache - .lock() - .unwrap() - .remove(params.path.as_path()); + self.node_cache.lock().unwrap().remove(path); - Ok(()) + result } /// Retrieves the list of diagnostics associated with a file @@ -1012,8 +1078,10 @@ impl Workspace for WorkspaceServer { enabled_rules, } = params; let parse = self.get_parse(&path)?; + let language = self.get_file_source(&path); + let capabilities = self.features.get_capabilities(language); let (diagnostics, errors, skipped_diagnostics) = - if let Some(lint) = self.get_file_capabilities(&path).analyzer.lint { + if let Some(lint) = capabilities.analyzer.lint { let settings = self .projects .get_settings(project_key) @@ -1025,7 +1093,7 @@ impl Workspace for WorkspaceServer { path: &path, only, skip, - language: self.get_file_source(&path), + language, categories, dependency_graph: self.dependency_graph.clone(), project_layout: self.project_layout.clone(), diff --git a/crates/biome_service/src/workspace/server.tests.rs b/crates/biome_service/src/workspace/server.tests.rs index 50129106bd30..500b4ebcb2e9 100644 --- a/crates/biome_service/src/workspace/server.tests.rs +++ b/crates/biome_service/src/workspace/server.tests.rs @@ -1,4 +1,5 @@ use biome_fs::MemoryFileSystem; +use crossbeam::channel::bounded; use super::*; @@ -11,7 +12,8 @@ fn commonjs_file_rejects_import_statement() { fs.insert(Utf8PathBuf::from("/project/a.js"), FILE_CONTENT); fs.insert(Utf8PathBuf::from("/project/package.json"), MANIFEST_CONTENT); - let workspace = WorkspaceServer::new(Box::new(fs)); + let (tx, _) = bounded(0); + let workspace = WorkspaceServer::new(Box::new(fs), tx); let project_key = workspace .open_project(OpenProjectParams { path: BiomePath::new("/"), @@ -23,6 +25,7 @@ fn commonjs_file_rejects_import_statement() { .scan_project_folder(ScanProjectFolderParams { project_key, path: Some(BiomePath::new("/")), + watch: false, }) .unwrap(); diff --git a/crates/biome_service/src/workspace/watcher.rs b/crates/biome_service/src/workspace/watcher.rs new file mode 100644 index 000000000000..9f69d3fca20c --- /dev/null +++ b/crates/biome_service/src/workspace/watcher.rs @@ -0,0 +1,190 @@ +//! Watcher that helps the scanner to keep scanned files up-to-date with the +//! file system. +//! +//! This module only contains the methods on the +//! [workspace server](crate::WorkspaceServer) that facilitate the watcher. +//! The heart of the watcher is implemented inside [crate::WorkspaceWatcher]. +//! +//! All the *public* methods in this module are intended to be called by the +//! watcher. Apart from updating open documents, they are also responsible for +//! updating service data such as the dependency graph. + +use std::path::{Path, PathBuf}; + +use biome_fs::{FileSystemDiagnostic, PathKind}; +use camino::Utf8Path; + +use crate::WorkspaceError; + +use super::{ + server::Document, FileContent, OpenFileParams, ScanProjectFolderParams, Workspace, + WorkspaceServer, +}; + +impl WorkspaceServer { + /// Used by the watcher to open one or more files. + /// + /// This method can also be used if we don't know whether the paths are + /// files or folders, because [Self::open_file_through_watcher()] checks the + /// kind internally. + pub fn open_files_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { + for path in paths { + self.open_file_through_watcher( + path.as_path() + .try_into() + .map_err(|_| FileSystemDiagnostic::non_utf8_path(&path))?, + )?; + } + + Ok(()) + } + + /// Used by the watcher to open one or more folders. + pub fn open_folders_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { + for path in paths { + self.open_folder_through_watcher( + path.as_path() + .try_into() + .map_err(|_| FileSystemDiagnostic::non_utf8_path(&path))?, + )?; + } + + Ok(()) + } + + /// Used indirectly by the watcher to open an individual file. + /// + /// Note that we don't always know whether the path belongs to a file, so we + /// explicitly need to check the kind here. + fn open_file_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + if let PathKind::Directory { .. } = self.fs.path_kind(path)? { + return self.open_folder_through_watcher(path); + } + + let Some(project_key) = self.projects.find_project_for_path(path) else { + return Ok(()); // file events outside our projects can be safely ignored. + }; + + self.open_file_by_scanner(OpenFileParams { + project_key, + path: path.into(), + content: FileContent::FromServer, + version: None, + document_file_source: None, + persist_node_cache: false, + })?; + + self.update_service_data_with_added_or_updated_path(path) + } + + /// Used indirectly by the watcher to open an individual folder. + fn open_folder_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + let Some(project_key) = self.projects.find_project_for_path(path) else { + return Ok(()); // file events outside our projects can be safely ignored. + }; + + self.scan_project_folder(ScanProjectFolderParams { + project_key, + path: Some(path.into()), + watch: false, // It's already being watched. + }) + .map(|_| ()) + } + + /// Used by the watcher to close one or more files. + pub fn close_files_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { + for path in paths { + self.close_file_through_watcher( + path.as_path() + .try_into() + .map_err(|_| FileSystemDiagnostic::non_utf8_path(&path))?, + )?; + } + + Ok(()) + } + + /// Used by the watcher to close one or more folders. + pub fn close_folders_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { + for path in &paths { + self.close_folder_through_watcher( + path.as_path() + .try_into() + .map_err(|_| FileSystemDiagnostic::non_utf8_path(path))?, + )?; + } + + Ok(()) + } + + /// Used indirectly by the watcher to close an individual file. + fn close_file_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + // Assign to a temporary to release the lock ASAP. + let has_node_cache = self.node_cache.lock().unwrap().contains_key(path); + + // This one is a bit tricky: If the user has the path open in an editor, + // we don't want to unload the document even though it was removed from + // the file system. But otherwise we do. + // + // Fortunately we have a great hueristic to determine whether a document + // is opened by an editor: the node cache. Node caches are only kept for + // editor documents. So if one is present for this path, we only unflag + // the document as being opened by the scanner. + let documents = self.documents.pin(); + if has_node_cache { + documents.update(path.to_path_buf(), |document| Document { + content: document.content.clone(), + file_source_index: document.file_source_index, + syntax: document.syntax.clone(), + version: document.version, + opened_by_scanner: false, + }); + } else { + documents.remove(path); + + self.update_service_data_with_removed_path(path)?; + } + + Ok(()) + } + + /// Used indirectly by the watcher to close an individual folder. + /// + /// Note that we don't really have a concept of open folders in the + /// workspace, so instead we just iterate the documents to find paths that + /// would be inside the closed folder. + /// + /// This method is also used when closing an individual file or folder if we + /// don't know which kind it is. This is because the watcher would only + /// attempt to close a file or folder after it has been removed, so asking + /// the file system for the kind wouldn't work anymore. + fn close_folder_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + for document_path in self.documents.pin().keys() { + if document_path.starts_with(path) { + self.close_file_through_watcher(document_path)?; + } + } + + Ok(()) + } + + /// Used by the watcher to rename a path if it knows both the from and to + /// paths. + pub fn rename_path_through_watcher( + &self, + from: &Path, + to: &Path, + ) -> Result<(), WorkspaceError> { + let from = from + .try_into() + .map_err(|_| FileSystemDiagnostic::non_utf8_path(from))?; + self.close_folder_through_watcher(from)?; + + let to = to + .try_into() + .map_err(|_| FileSystemDiagnostic::non_utf8_path(to))?; + self.open_file_through_watcher(to)?; + + Ok(()) + } +} diff --git a/crates/biome_service/src/workspace_watcher.rs b/crates/biome_service/src/workspace_watcher.rs new file mode 100644 index 000000000000..d2079101e604 --- /dev/null +++ b/crates/biome_service/src/workspace_watcher.rs @@ -0,0 +1,174 @@ +use camino::Utf8PathBuf; +use crossbeam::channel::{bounded, unbounded, Receiver, Sender}; +use notify::{ + event::{CreateKind, ModifyKind, RemoveKind, RenameMode}, + recommended_watcher, Error as NotifyError, Event as NotifyEvent, EventKind, RecursiveMode, + Result as NotifyResult, Watcher, +}; +use tracing::warn; + +use crate::{diagnostics::WatchError, WorkspaceError, WorkspaceServer}; + +/// Instructions to let the watcher either watch or unwatch a given folder. +pub enum WatcherInstruction { + WatchFolder(Utf8PathBuf), + UnwatchFolder(Utf8PathBuf), + + /// Stops the watcher entirely. + Stop, +} + +/// Channel for sending instructions to the watcher. +/// +/// Only exposes the sender of the channel. +/// +/// Implements [Drop] so that the watcher is stopped when the channel goes out +/// of scope. +pub struct WatcherInstructionChannel { + pub sender: Sender, +} + +impl Drop for WatcherInstructionChannel { + fn drop(&mut self) { + let _ = self.sender.send(WatcherInstruction::Stop); + } +} + +/// Watcher to keep the [WorkspaceServer] in sync with the filesystem state. +/// +/// Conceptually, it helps to think of the watcher as a helper to the scanner. +/// The watcher watches the same directories as those scanned by the scanner, so +/// the watcher is also instructed to watch folders that were scanned through +/// [WorkspaceServer::scan_project_folder()]. +/// +/// When watch events are received, they are handed back to the workspace. If +/// this results in opening new documents, we say they were opened by the +/// scanner, because the end result should be the same. And if we treat the +/// watcher as part of the scanner, it's not even a contradiction :) +pub struct WorkspaceWatcher { + /// Internal [`notify::Watcher`] instance. + // Note: The `Watcher` trait doesn't require its implementations to be + // `Send`, but it appears all platform implementations are. + watcher: Box, + + /// Channel receiver for the events from our + /// [internal watcher](Self::watcher). + notify_rx: Receiver>, + + /// Channel receiver for watch instructions. + instruction_rx: Receiver, +} + +impl WorkspaceWatcher { + /// Constructor. + /// + /// Returns both the watcher as well as the channel for sending instructions + /// to the watcher. + pub fn new() -> Result<(Self, WatcherInstructionChannel), WorkspaceError> { + // We use a bounded channel, because watchers are + // [intrinsically unreliable](https://docs.rs/notify/latest/notify/#watching-large-directories). + // If we block the sender, some events may get dropped, but that was + // already a possibility. So there doesn't really seem to be a + // justification for using an unbounded sender, which could end up + // consuming an ever-increasing amount of memory. + let (tx, rx) = bounded::>(128); + + let watcher = recommended_watcher(tx)?; + + let (instruction_tx, instruction_rx) = unbounded(); + let instruction_channel = WatcherInstructionChannel { + sender: instruction_tx, + }; + let watcher = Self { + watcher: Box::new(watcher), + notify_rx: rx, + instruction_rx, + }; + + Ok((watcher, instruction_channel)) + } + + /// Runs the watcher. + /// + /// This function is expected to run continuously until either the workspace + /// is dropped (because the workspace server is the one holding the sending + /// end of the instructions channel) or the watcher (unexpectedly) stops. + /// Under normal operation, neither should happen before the daemon + /// terminates. + pub fn run(&mut self, workspace: &WorkspaceServer) { + loop { + crossbeam::channel::select! { + recv(self.notify_rx) -> event => match event { + Ok(Ok(event)) => { + let result = match event.kind { + EventKind::Access(_) => Ok(()), + EventKind::Create(create_kind) => match create_kind { + CreateKind::Folder => workspace.open_folders_through_watcher(event.paths), + _ => workspace.open_files_through_watcher(event.paths), + }, + EventKind::Modify(modify_kind) => match modify_kind { + ModifyKind::Data(_) => { + workspace.open_files_through_watcher(event.paths) + }, + ModifyKind::Name(RenameMode::From) => { + workspace.close_folders_through_watcher(event.paths) + }, + ModifyKind::Name(RenameMode::To) => { + workspace.open_files_through_watcher(event.paths) + }, + ModifyKind::Name(RenameMode::Both) => { + workspace.rename_path_through_watcher(&event.paths[0], &event.paths[1]) + }, + _ => Ok(()), + }, + EventKind::Remove(remove_kind) => match remove_kind { + RemoveKind::File => workspace.close_files_through_watcher(event.paths), + _ => workspace.close_folders_through_watcher(event.paths), + }, + EventKind::Any | EventKind::Other => Ok(()), + }; + if let Err(error) = result { + // TODO: Improve error propagation. + warn!("Error processing watch event: {error}"); + } + }, + Ok(Err(error)) => { + // TODO: Improve error propagation. + warn!("Watcher error: {error}"); + break; + }, + Err(_) => { + // TODO: Improve error propagation. + warn!("Watcher stopped unexpectedly"); + break; + } + }, + recv(self.instruction_rx) -> instruction => match instruction { + Ok(WatcherInstruction::WatchFolder(path)) => { + if let Err(error) = self.watcher.watch(path.as_std_path(), RecursiveMode::Recursive) { + // TODO: Improve error propagation. + warn!("Error watching path {path}: {error}"); + } + } + Ok(WatcherInstruction::UnwatchFolder(path)) => { + if let Err(error) = self.watcher.unwatch(path.as_std_path()) { + // TODO: Improve error propagation. + warn!("Error unwatching path {path}: {error}"); + } + } + Ok(WatcherInstruction::Stop) | Err(_) => { + break; // Received stop instruction or workspace dropped. + } + } + } + } + } +} + +impl From for WorkspaceError { + fn from(error: NotifyError) -> Self { + Self::WatchError(WatchError { + reason: error.to_string(), + }) + } +} diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index 81c57f5b9242..9be3b4db2d2a 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -3526,7 +3526,7 @@ This should only be enabled if reparsing is to be expected, such as when the fil */ persistNodeCache?: boolean; projectKey: ProjectKey; - version: number; + version?: number; } export type FileContent = | { content: string; type: "fromClient" } diff --git a/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs b/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs index d7c2e9404248..dbdfb058775e 100644 --- a/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs +++ b/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs @@ -22,7 +22,6 @@ describe("Workspace API", () => { projectKey, path: "test.js", content: { type: "fromClient", content: "statement()" }, - version: 0, }); const printed = await workspace.formatFile({ diff --git a/packages/@biomejs/js-api/src/index.ts b/packages/@biomejs/js-api/src/index.ts index 01b50f1439f0..0ee5d246fbb1 100644 --- a/packages/@biomejs/js-api/src/index.ts +++ b/packages/@biomejs/js-api/src/index.ts @@ -168,7 +168,6 @@ export class Biome { this.workspace.openFile({ projectKey, content: { type: "fromClient", content }, - version: 0, path, }); From 04b0c692271062bb55fdce9877eae818795c3b3a Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 10:56:42 +0100 Subject: [PATCH 2/8] Add test case for moving directories --- crates/biome_lsp/src/server.tests.rs | 180 +++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/crates/biome_lsp/src/server.tests.rs b/crates/biome_lsp/src/server.tests.rs index feff3a5c4e21..465f0fe576a3 100644 --- a/crates/biome_lsp/src/server.tests.rs +++ b/crates/biome_lsp/src/server.tests.rs @@ -3161,3 +3161,183 @@ export function bar() { Ok(()) } + +#[tokio::test] +async fn watcher_updates_dependency_graph_with_directories() -> Result<()> { + // ARRANGE: Set up FS and LSP connection in order to test import cycles. + let mut fs = TemporaryFs::new("watcher_updates_dependency_graph_with_directories"); + fs.create_file( + "biome.json", + r#"{ + "linter": { + "enabled": true, + "rules": { + "nursery": { + "noImportCycles": "error" + } + } + } +} +"#, + ); + + fs.create_file( + "foo.ts", + r#"import { bar } from "./utils/bar.ts"; + +export function foo() { + bar(); +} +"#, + ); + + fs.create_file( + "utils/bar.ts", + r#"import { foo } from "../foo.ts"; + +export function bar() { + foo(); +} +"#, + ); + + let (mut watcher, instruction_channel) = WorkspaceWatcher::new()?; + + let factory = ServerFactory::new(true, instruction_channel.sender.clone()); + + let workspace = factory.workspace(); + tokio::task::spawn_blocking(move || { + watcher.run(workspace.as_ref()); + }); + + let (service, client) = factory.create(None).into_inner(); + let (stream, sink) = client.split(); + let mut server = Server::new(service); + + let (sender, _) = channel(CHANNEL_BUFFER_SIZE); + let reader = tokio::spawn(client_handler(stream, sink, sender)); + + server.initialize().await?; + + let project_key = server + .request( + "biome/open_project", + "open_project", + OpenProjectParams { + path: fs.working_directory.clone().into(), + open_uninitialized: true, + }, + ) + .await? + .expect("open_project returned an error"); + + // ARRANGE: Scanning the project folder initializes the service data. + let result: ScanProjectFolderResult = server + .request( + "biome/scan_project_folder", + "scan_project_folder", + ScanProjectFolderParams { + project_key, + path: None, + watch: true, + }, + ) + .await? + .expect("scan_project_folder returned an error"); + assert_eq!(result.diagnostics.len(), 0); + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::all(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: One diagnostic should be emitted for the cyclic dependency. + assert_eq!(result.diagnostics.len(), 1); + assert_eq!( + PrintDescription(&result.diagnostics[0]).to_string(), + "This import is part of a cycle." + ); + + // ARRANGE: Move `utils` directory. + std::fs::rename( + fs.working_directory.join("utils"), + fs.working_directory.join("bin"), + ) + .expect("Cannot move utils"); + + // FIXME: If this test is unstable, we may need to wait here. + // Right now, we don't know if the watcher already processed the + // move. It seems everything works fine though :D + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::empty(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: Diagnostic should've disappeared because `utils/bar.ts` is no + // longer there. + assert_eq!(result.diagnostics.len(), 0); + + // ARRANGE: Move `utils` back. + std::fs::rename( + fs.working_directory.join("bin"), + fs.working_directory.join("utils"), + ) + .expect("Cannot restore utils"); + + // ACT: Pull diagnostics. + let result: PullDiagnosticsResult = server + .request( + "biome/pull_diagnostics", + "pull_diagnostics", + PullDiagnosticsParams { + project_key, + path: fs.working_directory.join("foo.ts").into(), + categories: RuleCategories::all(), + max_diagnostics: 10, + only: Vec::new(), + skip: Vec::new(), + enabled_rules: vec![RuleSelector::Rule("nursery", "noImportCycles")], + }, + ) + .await? + .expect("pull_diagnostics returned an error"); + + // ASSERT: Diagnostic is expected to reappear. + assert_eq!(result.diagnostics.len(), 1); + assert_eq!( + PrintDescription(&result.diagnostics[0]).to_string(), + "This import is part of a cycle." + ); + + server.shutdown().await?; + reader.abort(); + + Ok(()) +} From bb1f572073e86d8c3ab541ffecf5d68f12e6c9af Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 12:17:36 +0100 Subject: [PATCH 3/8] PR feedback --- crates/biome_cli/src/execute/migrate.rs | 3 +- crates/biome_cli/src/execute/mod.rs | 3 +- .../execute/process_file/workspace_file.rs | 3 +- crates/biome_cli/src/execute/std_in.rs | 6 +-- .../biome_lsp/src/handlers/text_document.rs | 3 +- crates/biome_service/src/workspace.rs | 16 ++++-- crates/biome_service/src/workspace.tests.rs | 52 ++++++------------- crates/biome_service/src/workspace/scanner.rs | 1 - crates/biome_service/src/workspace/server.rs | 9 ++-- crates/biome_service/src/workspace/watcher.rs | 49 +++++++++-------- crates/biome_service/src/workspace_watcher.rs | 11 ++-- .../@biomejs/backend-jsonrpc/src/workspace.ts | 3 +- packages/@biomejs/js-api/src/index.ts | 2 +- 13 files changed, 72 insertions(+), 89 deletions(-) diff --git a/crates/biome_cli/src/execute/migrate.rs b/crates/biome_cli/src/execute/migrate.rs index 8a57a806c543..04507fb9594e 100644 --- a/crates/biome_cli/src/execute/migrate.rs +++ b/crates/biome_cli/src/execute/migrate.rs @@ -78,8 +78,7 @@ pub(crate) fn run(migrate_payload: MigratePayload) -> Result<(), CliDiagnostic> workspace.open_file(OpenFileParams { project_key, path: biome_path.clone(), - content: FileContent::FromClient(biome_config_content.to_string()), - version: Some(0), + content: FileContent::from_client(&biome_config_content), document_file_source: Some(JsonFileSource::json().into()), persist_node_cache: false, })?; diff --git a/crates/biome_cli/src/execute/mod.rs b/crates/biome_cli/src/execute/mod.rs index 8ea42c109f57..658796b1c1b5 100644 --- a/crates/biome_cli/src/execute/mod.rs +++ b/crates/biome_cli/src/execute/mod.rs @@ -598,9 +598,8 @@ pub fn execute_mode( let report_file = BiomePath::new("_report_output.json"); session.app.workspace.open_file(OpenFileParams { project_key, - content: FileContent::FromClient(content), + content: FileContent::from_client(content), path: report_file.clone(), - version: None, document_file_source: None, persist_node_cache: false, })?; diff --git a/crates/biome_cli/src/execute/process_file/workspace_file.rs b/crates/biome_cli/src/execute/process_file/workspace_file.rs index cb24e320667b..8d35bbc2fdce 100644 --- a/crates/biome_cli/src/execute/process_file/workspace_file.rs +++ b/crates/biome_cli/src/execute/process_file/workspace_file.rs @@ -37,8 +37,7 @@ impl<'ctx, 'app> WorkspaceFile<'ctx, 'app> { project_key: ctx.project_key, document_file_source: None, path: path.clone(), - version: None, - content: FileContent::FromClient(input.clone()), + content: FileContent::from_client(&input), persist_node_cache: false, }, ) diff --git a/crates/biome_cli/src/execute/std_in.rs b/crates/biome_cli/src/execute/std_in.rs index 8d2f2c8d80b4..c60d7e2baf10 100644 --- a/crates/biome_cli/src/execute/std_in.rs +++ b/crates/biome_cli/src/execute/std_in.rs @@ -50,8 +50,7 @@ pub(crate) fn run<'a>( workspace.open_file(OpenFileParams { project_key, path: biome_path.clone(), - version: None, - content: FileContent::FromClient(content.into()), + content: FileContent::from_client(content), document_file_source: None, persist_node_cache: false, })?; @@ -85,8 +84,7 @@ pub(crate) fn run<'a>( workspace.open_file(OpenFileParams { project_key, path: biome_path.clone(), - version: None, - content: FileContent::FromClient(content.into()), + content: FileContent::from_client(content), document_file_source: None, persist_node_cache: false, })?; diff --git a/crates/biome_lsp/src/handlers/text_document.rs b/crates/biome_lsp/src/handlers/text_document.rs index db99dfac3d85..2e75d5e0b423 100644 --- a/crates/biome_lsp/src/handlers/text_document.rs +++ b/crates/biome_lsp/src/handlers/text_document.rs @@ -51,8 +51,7 @@ pub(crate) async fn did_open( session.workspace.open_file(OpenFileParams { project_key, path, - content: FileContent::FromClient(content), - version: Some(version), + content: FileContent::FromClient { content, version }, document_file_source: Some(language_hint), persist_node_cache: true, })?; diff --git a/crates/biome_service/src/workspace.rs b/crates/biome_service/src/workspace.rs index 16f3f7fb381e..ca8e5d6305fa 100644 --- a/crates/biome_service/src/workspace.rs +++ b/crates/biome_service/src/workspace.rs @@ -568,7 +568,6 @@ pub struct OpenFileParams { pub project_key: ProjectKey, pub path: BiomePath, pub content: FileContent, - pub version: Option, pub document_file_source: Option, /// Set to `true` to persist the node cache used during parsing, in order to @@ -582,15 +581,24 @@ pub struct OpenFileParams { #[derive(Debug, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] -#[serde(rename_all = "camelCase", tag = "type", content = "content")] +#[serde(rename_all = "camelCase", tag = "type")] pub enum FileContent { /// The client has loaded the content and submits it to the server. - FromClient(String), + FromClient { content: String, version: i32 }, /// The server will be responsible for loading the content from the file system. FromServer, } +impl FileContent { + pub fn from_client(content: impl Into) -> Self { + Self::FromClient { + content: content.into(), + version: 0, + } + } +} + #[derive(Debug, serde::Serialize, serde::Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase")] @@ -1004,7 +1012,7 @@ pub struct ScanProjectFolderParams { /// not use this to avoid scanning parts that are irrelevant to clients. pub path: Option, - /// Should the watcher be instructed to start watching this path? + /// Whether the watcher should watch this path. /// /// Does nothing if the watcher is already watching this path. pub watch: bool, diff --git a/crates/biome_service/src/workspace.tests.rs b/crates/biome_service/src/workspace.tests.rs index 75c66886c130..0c6bda3a5389 100644 --- a/crates/biome_service/src/workspace.tests.rs +++ b/crates/biome_service/src/workspace.tests.rs @@ -43,8 +43,7 @@ fn debug_control_flow() { OpenFileParams { project_key, path: BiomePath::new("file.js"), - content: FileContent::FromClient(SOURCE.into()), - version: None, + content: FileContent::from_client(SOURCE), document_file_source: Some(DocumentFileSource::from(JsFileSource::default())), persist_node_cache: false, }, @@ -66,8 +65,7 @@ fn recognize_typescript_definition_file() { project_key, path: BiomePath::new("file.d.ts"), // the following code snippet can be correctly parsed in .d.ts file but not in .ts file - content: FileContent::FromClient("export const foo: number".into()), - version: None, + content: FileContent::from_client("export const foo: number"), document_file_source: None, persist_node_cache: false, }, @@ -87,8 +85,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("a.json"), - content: FileContent::FromClient(r#"{"a": 42}"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42}"#), document_file_source: None, persist_node_cache: false, }, @@ -102,8 +99,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("b.json"), - content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42}//comment"#), document_file_source: None, persist_node_cache: false, }, @@ -117,8 +113,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("c.json"), - content: FileContent::FromClient(r#"{"a": 42,}"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42,}"#), document_file_source: None, persist_node_cache: false, }, @@ -132,8 +127,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("d.jsonc"), - content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42}//comment"#), document_file_source: None, persist_node_cache: false, }, @@ -147,8 +141,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("e.jsonc"), - content: FileContent::FromClient(r#"{"a": 42,}"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42,}"#), document_file_source: None, persist_node_cache: false, }, @@ -162,8 +155,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new(".eslintrc.json"), - content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42}//comment"#), document_file_source: None, persist_node_cache: false, }, @@ -177,8 +169,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("project/.vscode/settings.json"), - content: FileContent::FromClient(r#"{"a": 42}//comment"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42}//comment"#), document_file_source: None, persist_node_cache: false, }, @@ -192,8 +183,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("dir/.eslintrc.json"), - content: FileContent::FromClient(r#"{"a": 42,}"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42,}"#), document_file_source: None, persist_node_cache: false, }, @@ -209,8 +199,7 @@ fn correctly_handle_json_files() { OpenFileParams { project_key, path: BiomePath::new("tsconfig.json"), - content: FileContent::FromClient(r#"{"a": 42,}//comment"#.into()), - version: None, + content: FileContent::from_client(r#"{"a": 42,}//comment"#), document_file_source: None, persist_node_cache: false, }, @@ -230,7 +219,7 @@ fn correctly_parses_graphql_files() { OpenFileParams { project_key, path: BiomePath::new("file.graphql"), - content: FileContent::FromClient( + content: FileContent::from_client( r#"type Query { me: User } @@ -238,10 +227,8 @@ fn correctly_parses_graphql_files() { type User { id: ID name: String -}"# - .into(), +}"#, ), - version: None, document_file_source: None, persist_node_cache: false, }, @@ -263,13 +250,11 @@ fn correctly_pulls_lint_diagnostics() { OpenFileParams { project_key, path: BiomePath::new("file.graphql"), - content: FileContent::FromClient( + content: FileContent::from_client( r#"query { member @deprecated(abc: 123) -}"# - .into(), +}"#, ), - version: None, document_file_source: None, persist_node_cache: false, }, @@ -298,13 +283,11 @@ fn pull_grit_debug_info() { OpenFileParams { project_key, path: BiomePath::new("file.grit"), - content: FileContent::FromClient( + content: FileContent::from_client( r#"`function ($args) { $body }` where { $args <: contains `x` -}"# - .into(), +}"#, ), - version: None, document_file_source: None, persist_node_cache: false, }, @@ -363,7 +346,6 @@ fn files_loaded_by_the_scanner_are_only_unloaded_when_the_project_is_unregistere project_key, path: BiomePath::new("/project/a.ts"), content: FileContent::FromServer, - version: None, document_file_source: None, persist_node_cache: false, }) diff --git a/crates/biome_service/src/workspace/scanner.rs b/crates/biome_service/src/workspace/scanner.rs index 52ecfb3c9483..a506b0f32650 100644 --- a/crates/biome_service/src/workspace/scanner.rs +++ b/crates/biome_service/src/workspace/scanner.rs @@ -246,7 +246,6 @@ fn open_file(ctx: &ScanContext, path: &BiomePath) { path: path.clone(), content: FileContent::FromServer, document_file_source: None, - version: None, persist_node_cache: false, }) }) { diff --git a/crates/biome_service/src/workspace/server.rs b/crates/biome_service/src/workspace/server.rs index e97d091137c1..abfaa7998298 100644 --- a/crates/biome_service/src/workspace/server.rs +++ b/crates/biome_service/src/workspace/server.rs @@ -291,7 +291,6 @@ impl WorkspaceServer { #[tracing::instrument(level = "debug", skip(self, params), fields( project_key = display(params.project_key), path = display(params.path.as_path()), - version = debug(params.version), ))] fn open_file_internal( &self, @@ -302,12 +301,10 @@ impl WorkspaceServer { project_key, path, content, - version, document_file_source, persist_node_cache, } = params; let path: Utf8PathBuf = path.into(); - let version = version.unwrap_or_default(); if document_file_source.is_none() && !DocumentFileSource::can_read(path.as_path()) { return Ok(()); @@ -324,9 +321,9 @@ impl WorkspaceServer { } } - let content = match content { - FileContent::FromClient(content) => content, - FileContent::FromServer => self.fs.read_file_from_path(&path)?, + let (content, version) = match content { + FileContent::FromClient { content, version } => (content, version), + FileContent::FromServer => (self.fs.read_file_from_path(&path)?, 0), }; let mut index = self.insert_source(source); diff --git a/crates/biome_service/src/workspace/watcher.rs b/crates/biome_service/src/workspace/watcher.rs index 9f69d3fca20c..c5128ebcfd81 100644 --- a/crates/biome_service/src/workspace/watcher.rs +++ b/crates/biome_service/src/workspace/watcher.rs @@ -22,14 +22,13 @@ use super::{ }; impl WorkspaceServer { - /// Used by the watcher to open one or more files. + /// Used by the watcher to open one or more files or folders. /// - /// This method can also be used if we don't know whether the paths are - /// files or folders, because [Self::open_file_through_watcher()] checks the - /// kind internally. - pub fn open_files_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { + /// If you already know the paths are folders, use + /// [Self::open_folders_through_watcher()] instead. + pub fn open_paths_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { for path in paths { - self.open_file_through_watcher( + self.open_path_through_watcher( path.as_path() .try_into() .map_err(|_| FileSystemDiagnostic::non_utf8_path(&path))?, @@ -52,11 +51,11 @@ impl WorkspaceServer { Ok(()) } - /// Used indirectly by the watcher to open an individual file. + /// Used indirectly by the watcher to open an individual file or folder. /// - /// Note that we don't always know whether the path belongs to a file, so we - /// explicitly need to check the kind here. - fn open_file_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + /// If you already know the path is a folder, use + /// [Self::open_folder_through_watcher()] instead. + fn open_path_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { if let PathKind::Directory { .. } = self.fs.path_kind(path)? { return self.open_folder_through_watcher(path); } @@ -69,7 +68,6 @@ impl WorkspaceServer { project_key, path: path.into(), content: FileContent::FromServer, - version: None, document_file_source: None, persist_node_cache: false, })?; @@ -104,10 +102,13 @@ impl WorkspaceServer { Ok(()) } - /// Used by the watcher to close one or more folders. - pub fn close_folders_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { + /// Used by the watcher to close one or more files or folders. + /// + /// If you know the paths are files, use + /// [Self::close_files_through_watcher()] instead. + pub fn close_paths_through_watcher(&self, paths: Vec) -> Result<(), WorkspaceError> { for path in &paths { - self.close_folder_through_watcher( + self.close_path_through_watcher( path.as_path() .try_into() .map_err(|_| FileSystemDiagnostic::non_utf8_path(path))?, @@ -119,7 +120,7 @@ impl WorkspaceServer { /// Used indirectly by the watcher to close an individual file. fn close_file_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { - // Assign to a temporary to release the lock ASAP. + // Assign to a variable outside the `if` condition to release the lock ASAP. let has_node_cache = self.node_cache.lock().unwrap().contains_key(path); // This one is a bit tricky: If the user has the path open in an editor, @@ -148,17 +149,19 @@ impl WorkspaceServer { Ok(()) } - /// Used indirectly by the watcher to close an individual folder. + /// Used indirectly by the watcher to close an individual file or folder. /// /// Note that we don't really have a concept of open folders in the /// workspace, so instead we just iterate the documents to find paths that /// would be inside the closed folder. /// - /// This method is also used when closing an individual file or folder if we - /// don't know which kind it is. This is because the watcher would only - /// attempt to close a file or folder after it has been removed, so asking - /// the file system for the kind wouldn't work anymore. - fn close_folder_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + /// If you already know the path is a file, use + /// [Self::close_file_through_watcher()] instead. + fn close_path_through_watcher(&self, path: &Utf8Path) -> Result<(), WorkspaceError> { + // Note that we cannot check the kind of the path, because the watcher + // would only attempt to close a file or folder after it has been + // removed. So asking the file system wouldn't work anymore. + for document_path in self.documents.pin().keys() { if document_path.starts_with(path) { self.close_file_through_watcher(document_path)?; @@ -178,12 +181,12 @@ impl WorkspaceServer { let from = from .try_into() .map_err(|_| FileSystemDiagnostic::non_utf8_path(from))?; - self.close_folder_through_watcher(from)?; + self.close_path_through_watcher(from)?; let to = to .try_into() .map_err(|_| FileSystemDiagnostic::non_utf8_path(to))?; - self.open_file_through_watcher(to)?; + self.open_path_through_watcher(to)?; Ok(()) } diff --git a/crates/biome_service/src/workspace_watcher.rs b/crates/biome_service/src/workspace_watcher.rs index d2079101e604..75f58ecff3fc 100644 --- a/crates/biome_service/src/workspace_watcher.rs +++ b/crates/biome_service/src/workspace_watcher.rs @@ -95,6 +95,7 @@ impl WorkspaceWatcher { /// end of the instructions channel) or the watcher (unexpectedly) stops. /// Under normal operation, neither should happen before the daemon /// terminates. + #[tracing::instrument(level = "debug", skip(self, workspace))] pub fn run(&mut self, workspace: &WorkspaceServer) { loop { crossbeam::channel::select! { @@ -104,17 +105,17 @@ impl WorkspaceWatcher { EventKind::Access(_) => Ok(()), EventKind::Create(create_kind) => match create_kind { CreateKind::Folder => workspace.open_folders_through_watcher(event.paths), - _ => workspace.open_files_through_watcher(event.paths), + _ => workspace.open_paths_through_watcher(event.paths), }, EventKind::Modify(modify_kind) => match modify_kind { ModifyKind::Data(_) => { - workspace.open_files_through_watcher(event.paths) + workspace.open_paths_through_watcher(event.paths) }, ModifyKind::Name(RenameMode::From) => { - workspace.close_folders_through_watcher(event.paths) + workspace.close_paths_through_watcher(event.paths) }, ModifyKind::Name(RenameMode::To) => { - workspace.open_files_through_watcher(event.paths) + workspace.open_paths_through_watcher(event.paths) }, ModifyKind::Name(RenameMode::Both) => { workspace.rename_path_through_watcher(&event.paths[0], &event.paths[1]) @@ -123,7 +124,7 @@ impl WorkspaceWatcher { }, EventKind::Remove(remove_kind) => match remove_kind { RemoveKind::File => workspace.close_files_through_watcher(event.paths), - _ => workspace.close_folders_through_watcher(event.paths), + _ => workspace.close_paths_through_watcher(event.paths), }, EventKind::Any | EventKind::Other => Ok(()), }; diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index 9be3b4db2d2a..b5ab3a6d4722 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -3526,10 +3526,9 @@ This should only be enabled if reparsing is to be expected, such as when the fil */ persistNodeCache?: boolean; projectKey: ProjectKey; - version?: number; } export type FileContent = - | { content: string; type: "fromClient" } + | { content: string; type: "fromClient"; version: number } | { type: "fromServer" }; export type DocumentFileSource = | "Ignore" diff --git a/packages/@biomejs/js-api/src/index.ts b/packages/@biomejs/js-api/src/index.ts index 0ee5d246fbb1..f4b69f25ad9c 100644 --- a/packages/@biomejs/js-api/src/index.ts +++ b/packages/@biomejs/js-api/src/index.ts @@ -167,7 +167,7 @@ export class Biome { return this.tryCatchWrapper(() => { this.workspace.openFile({ projectKey, - content: { type: "fromClient", content }, + content: { type: "fromClient", content, version: 0 }, path, }); From 05f9b45c3ede83cfefe57f47159bbb8f3291278d Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 12:46:14 +0100 Subject: [PATCH 4/8] Fix JS API --- packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs b/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs index dbdfb058775e..b3d0d288814a 100644 --- a/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs +++ b/packages/@biomejs/backend-jsonrpc/tests/workspace.test.mjs @@ -21,7 +21,7 @@ describe("Workspace API", () => { await workspace.openFile({ projectKey, path: "test.js", - content: { type: "fromClient", content: "statement()" }, + content: { type: "fromClient", content: "statement()", version: 0 }, }); const printed = await workspace.formatFile({ From 2f4c77ffb26d44cc536f6b2a9c54992d10062c55 Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 12:51:24 +0100 Subject: [PATCH 5/8] Add comment --- crates/biome_service/src/workspace_watcher.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/biome_service/src/workspace_watcher.rs b/crates/biome_service/src/workspace_watcher.rs index 75f58ecff3fc..f225c55d2d6e 100644 --- a/crates/biome_service/src/workspace_watcher.rs +++ b/crates/biome_service/src/workspace_watcher.rs @@ -71,6 +71,8 @@ impl WorkspaceWatcher { // already a possibility. So there doesn't really seem to be a // justification for using an unbounded sender, which could end up // consuming an ever-increasing amount of memory. + // The actual size of the buffer is an arbitrary choice that we can + // tweak if we find a need for it. let (tx, rx) = bounded::>(128); let watcher = recommended_watcher(tx)?; From 9dd75206730434366156e7e7512aca0fdbde1731 Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 14:29:40 +0100 Subject: [PATCH 6/8] Add CONTRIBUTING guide for biome_service --- crates/biome_service/CONTRIBUTING.md | 44 +++++++++++++++++++ crates/biome_service/src/workspace_watcher.rs | 9 +++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 crates/biome_service/CONTRIBUTING.md diff --git a/crates/biome_service/CONTRIBUTING.md b/crates/biome_service/CONTRIBUTING.md new file mode 100644 index 000000000000..9ea4f6b2e539 --- /dev/null +++ b/crates/biome_service/CONTRIBUTING.md @@ -0,0 +1,44 @@ +# Biome Service + +The Biome Service is where we implement the [Workspace](src/workspace.rs). + +The workspace is where Biome keeps all internal state of projects, such as open +documents, but also more advanced service data, such as the instances of our +[project layout](../biome_project_layout/) and our +[dependency graph](../biome_dependency_graph/). + +Note that there are two implementations of the `Workspace` trait: + +* [`WorkspaceServer`](src/workspace/server.rs) maintains the state itself, and + is used both inside the daemon as well as in the CLI when it runs in + daemonless mode. +* [`WorkspaceClient`](src/workspace/client.rs) is used for creating connections + to the daemon and communicating with the `WorkspaceServer`. + +## Watcher + +The state inside our workspace is kept in sync with the file system using the +[`WorkspaceWatcher`](src/workspace_watcher.rs). The watcher is only active in +daemon mode and not used by the CLI. + +### Debugging + +Debugging the watcher can be tricky, because you need to run the daemon, +interact with the file system, and observe the daemon's state somehow. + +Debugging is possible with the VS Code extension, but for an easier experience, +you can use these CLI commands: + +1. Start the daemon using `cargo run --bin=biome -- start`. Note there won't be + much output to inspect, but the daemon will write its logs in your + [cache folder](../../crates/biome_fs/src/dir.rs). +2. Run commands against the daemon, such as + `cargo run --bin=biome -- lint --use-server `. + +The rule `noImportCycles` is currently the best candidate to observe the state +in the project layout and the dependency graph. + +### Tests + +Tests for the watcher are inside the +[LSP tests](../biome_lsp/src/server.tests.rs). diff --git a/crates/biome_service/src/workspace_watcher.rs b/crates/biome_service/src/workspace_watcher.rs index f225c55d2d6e..c9b3016e92bb 100644 --- a/crates/biome_service/src/workspace_watcher.rs +++ b/crates/biome_service/src/workspace_watcher.rs @@ -5,7 +5,7 @@ use notify::{ recommended_watcher, Error as NotifyError, Event as NotifyEvent, EventKind, RecursiveMode, Result as NotifyResult, Watcher, }; -use tracing::warn; +use tracing::{debug, warn}; use crate::{diagnostics::WatchError, WorkspaceError, WorkspaceServer}; @@ -103,6 +103,10 @@ impl WorkspaceWatcher { crossbeam::channel::select! { recv(self.notify_rx) -> event => match event { Ok(Ok(event)) => { + if !matches!(event.kind, EventKind::Access(_)) { + debug!(event = debug(&event), "watcher_event"); + } + let result = match event.kind { EventKind::Access(_) => Ok(()), EventKind::Create(create_kind) => match create_kind { @@ -148,18 +152,21 @@ impl WorkspaceWatcher { }, recv(self.instruction_rx) -> instruction => match instruction { Ok(WatcherInstruction::WatchFolder(path)) => { + debug!(%path, "watch_folder"); if let Err(error) = self.watcher.watch(path.as_std_path(), RecursiveMode::Recursive) { // TODO: Improve error propagation. warn!("Error watching path {path}: {error}"); } } Ok(WatcherInstruction::UnwatchFolder(path)) => { + debug!(%path, "unwatch_folder"); if let Err(error) = self.watcher.unwatch(path.as_std_path()) { // TODO: Improve error propagation. warn!("Error unwatching path {path}: {error}"); } } Ok(WatcherInstruction::Stop) | Err(_) => { + debug!("stop"); break; // Received stop instruction or workspace dropped. } } From e8c0a9c2361a5871a38c9ad6f8718a8eab06419f Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 14:36:34 +0100 Subject: [PATCH 7/8] Update crates/biome_service/CONTRIBUTING.md Co-authored-by: Emanuele Stoppa --- crates/biome_service/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_service/CONTRIBUTING.md b/crates/biome_service/CONTRIBUTING.md index 9ea4f6b2e539..5b0c1668e067 100644 --- a/crates/biome_service/CONTRIBUTING.md +++ b/crates/biome_service/CONTRIBUTING.md @@ -26,7 +26,7 @@ daemon mode and not used by the CLI. Debugging the watcher can be tricky, because you need to run the daemon, interact with the file system, and observe the daemon's state somehow. -Debugging is possible with the VS Code extension, but for an easier experience, +Debugging is possible with the VS Code extension or Zed extension, but for an easier experience, you can use these CLI commands: 1. Start the daemon using `cargo run --bin=biome -- start`. Note there won't be From c0bf7f8957b7dd9f6995902feb1f2f054569613c Mon Sep 17 00:00:00 2001 From: "Arend van Beelen jr." Date: Thu, 27 Feb 2025 15:01:41 +0100 Subject: [PATCH 8/8] Add WatcherSignalKind --- crates/biome_service/src/workspace/scanner.rs | 6 +- crates/biome_service/src/workspace/server.rs | 83 +++++++++---------- crates/biome_service/src/workspace/watcher.rs | 6 +- crates/biome_service/src/workspace_watcher.rs | 7 ++ 4 files changed, 51 insertions(+), 51 deletions(-) diff --git a/crates/biome_service/src/workspace/scanner.rs b/crates/biome_service/src/workspace/scanner.rs index a506b0f32650..4ccb13afbbf2 100644 --- a/crates/biome_service/src/workspace/scanner.rs +++ b/crates/biome_service/src/workspace/scanner.rs @@ -23,6 +23,7 @@ use tracing::instrument; use crate::diagnostics::Panic; use crate::projects::ProjectKey; use crate::workspace::{DocumentFileSource, FileContent, OpenFileParams}; +use crate::workspace_watcher::WatcherSignalKind; use crate::{Workspace, WorkspaceError}; use super::server::WorkspaceServer; @@ -124,7 +125,8 @@ fn scan_folder(folder: &Utf8Path, ctx: ScanContext) -> Duration { let mut paths = configs; paths.append(&mut manifests); - ctx.workspace.update_project_layout_for_paths(&paths); + ctx.workspace + .update_project_layout_for_paths(WatcherSignalKind::AddedOrChanged, &paths); let result = ctx .workspace .update_project_ignore_files(ctx.project_key, &ignore_paths); @@ -140,7 +142,7 @@ fn scan_folder(folder: &Utf8Path, ctx: ScanContext) -> Duration { })); ctx.workspace - .update_dependency_graph_for_paths(&handleable_paths, &[]); + .update_dependency_graph(WatcherSignalKind::AddedOrChanged, &handleable_paths); start.elapsed() } diff --git a/crates/biome_service/src/workspace/server.rs b/crates/biome_service/src/workspace/server.rs index abfaa7998298..548a388e9a4b 100644 --- a/crates/biome_service/src/workspace/server.rs +++ b/crates/biome_service/src/workspace/server.rs @@ -18,6 +18,7 @@ use crate::workspace::{ FileFeaturesResult, GetFileContentParams, IsPathIgnoredParams, RageEntry, RageParams, RageResult, ServerInfo, }; +use crate::workspace_watcher::WatcherSignalKind; use crate::{file_handlers::Features, Workspace, WorkspaceError}; use crate::{is_dir, WatcherInstruction}; use append_only_vec::AppendOnlyVec; @@ -577,10 +578,14 @@ impl WorkspaceServer { .unwrap_or_default() } - // TODO: add documentation for this method - pub(super) fn update_project_layout_for_paths(&self, paths: &[BiomePath]) { + /// Updates the [ProjectLayout] for multiple `paths` at once. + pub(super) fn update_project_layout_for_paths( + &self, + signal_kind: WatcherSignalKind, + paths: &[BiomePath], + ) { for path in paths { - if let Err(error) = self.update_project_layout_for_added_or_changed_path(path) { + if let Err(error) = self.update_project_layout(signal_kind, path) { error!("Error while updating project layout: {error}"); } } @@ -626,9 +631,10 @@ impl WorkspaceServer { Ok(()) } - // TODO: add documentation for this method - pub(super) fn update_project_layout_for_added_or_changed_path( + /// Updates the [ProjectLayout] for the given `path`. + pub(super) fn update_project_layout( &self, + signal_kind: WatcherSignalKind, path: &Utf8Path, ) -> Result<(), WorkspaceError> { if path @@ -639,37 +645,34 @@ impl WorkspaceServer { .parent() .map(|parent| parent.to_path_buf()) .ok_or_else(WorkspaceError::not_found)?; - let parsed = self.get_parse(path)?; - self.project_layout - .insert_serialized_node_manifest(package_path, parsed); - } - Ok(()) - } - - pub(super) fn update_project_layout_for_removed_path( - &self, - path: &Utf8Path, - ) -> Result<(), WorkspaceError> { - if path - .file_name() - .is_some_and(|filename| filename == "package.json") - { - let package_path = path - .parent() - .map(|parent| parent.to_path_buf()) - .ok_or_else(WorkspaceError::not_found)?; - self.project_layout.remove_package(&package_path); + match signal_kind { + WatcherSignalKind::AddedOrChanged => { + let parsed = self.get_parse(path)?; + self.project_layout + .insert_serialized_node_manifest(package_path, parsed); + } + WatcherSignalKind::Removed => { + self.project_layout.remove_package(&package_path); + } + } } Ok(()) } - pub(super) fn update_dependency_graph_for_paths( + /// Updates the [DependencyGraph] for the given `paths`. + pub(super) fn update_dependency_graph( &self, - added_or_changed_paths: &[BiomePath], - removed_paths: &[BiomePath], + signal_kind: WatcherSignalKind, + paths: &[BiomePath], ) { + let no_paths: &[BiomePath] = &[]; + let (added_or_changed_paths, removed_paths) = match signal_kind { + WatcherSignalKind::AddedOrChanged => (paths, no_paths), + WatcherSignalKind::Removed => (no_paths, paths), + }; + self.dependency_graph.update_imports_for_js_paths( self.fs.as_ref(), &self.project_layout, @@ -691,30 +694,18 @@ impl WorkspaceServer { ); } - pub(super) fn update_service_data_with_added_or_updated_path( - &self, - path: &Utf8Path, - ) -> Result<(), WorkspaceError> { - let path = BiomePath::from(path); - if path.is_manifest() { - self.update_project_layout_for_added_or_changed_path(&path)?; - } - - self.update_dependency_graph_for_paths(&[path], &[]); - - Ok(()) - } - - pub(super) fn update_service_data_with_removed_path( + /// Updates the state of any services relevant to the given `path`. + pub(super) fn update_service_data( &self, + signal_kind: WatcherSignalKind, path: &Utf8Path, ) -> Result<(), WorkspaceError> { let path = BiomePath::from(path); if path.is_manifest() { - self.update_project_layout_for_removed_path(&path)?; + self.update_project_layout(signal_kind, &path)?; } - self.update_dependency_graph_for_paths(&[], &[path]); + self.update_dependency_graph(signal_kind, &[path]); Ok(()) } @@ -1039,7 +1030,7 @@ impl Workspace for WorkspaceServer { } else { documents.remove(path); - self.update_service_data_with_removed_path(path) + self.update_service_data(WatcherSignalKind::Removed, path) } }; diff --git a/crates/biome_service/src/workspace/watcher.rs b/crates/biome_service/src/workspace/watcher.rs index c5128ebcfd81..68fab0f14956 100644 --- a/crates/biome_service/src/workspace/watcher.rs +++ b/crates/biome_service/src/workspace/watcher.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; use biome_fs::{FileSystemDiagnostic, PathKind}; use camino::Utf8Path; -use crate::WorkspaceError; +use crate::{workspace_watcher::WatcherSignalKind, WorkspaceError}; use super::{ server::Document, FileContent, OpenFileParams, ScanProjectFolderParams, Workspace, @@ -72,7 +72,7 @@ impl WorkspaceServer { persist_node_cache: false, })?; - self.update_service_data_with_added_or_updated_path(path) + self.update_service_data(WatcherSignalKind::AddedOrChanged, path) } /// Used indirectly by the watcher to open an individual folder. @@ -143,7 +143,7 @@ impl WorkspaceServer { } else { documents.remove(path); - self.update_service_data_with_removed_path(path)?; + self.update_service_data(WatcherSignalKind::Removed, path)?; } Ok(()) diff --git a/crates/biome_service/src/workspace_watcher.rs b/crates/biome_service/src/workspace_watcher.rs index c9b3016e92bb..e573ae970133 100644 --- a/crates/biome_service/src/workspace_watcher.rs +++ b/crates/biome_service/src/workspace_watcher.rs @@ -34,6 +34,13 @@ impl Drop for WatcherInstructionChannel { } } +/// Kind of change being reported. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum WatcherSignalKind { + AddedOrChanged, + Removed, +} + /// Watcher to keep the [WorkspaceServer] in sync with the filesystem state. /// /// Conceptually, it helps to think of the watcher as a helper to the scanner.