Skip to content

Commit 804d6df

Browse files
authored
Replace Tokio Thread File Watcher with LSP Client File Watcher (#7147)
## Description This PR replaces our custom Tokio-based thread watcher for `Forc.toml` files with the built-in LSP `didChangeWatchedFiles` notification system. The changes: * Remove the Tokio thread-based file watching system and associated cleanup code * Implement client-side file watching registration during initialization * Handle `Forc.toml` file changes through the LSP notification protocol * Update tests to use the new notification system * Simplify manifest syncing with a direct approach This approach is more efficient as it leverages the client's native file watching capabilities rather than spawning a separate thread. The PR maintains the same functionality while reducing resource usage and complexity. Note: This doesn't yet implement the recompilation on `Forc.toml` changes as mentioned in issue #7103, but adds a TODO placeholder for that functionality. This PR is a precursor to #7139 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
1 parent 1c885cf commit 804d6df

File tree

11 files changed

+84
-164
lines changed

11 files changed

+84
-164
lines changed

Cargo.lock

Lines changed: 1 addition & 94 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,6 @@ mark-flaky-tests = "1.0"
180180
mdbook = { version = "0.4", default-features = false }
181181
minifier = "0.3"
182182
normalize-path = "0.2"
183-
notify = "6.1"
184-
notify-debouncer-mini = "0.4"
185183
num-bigint = { version = "0.4", features = ["serde"] }
186184
num-traits = "0.2"
187185
object = "0.36"

sway-lsp/Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ forc-pkg.workspace = true
1717
forc-tracing.workspace = true
1818
forc-util.workspace = true
1919
lsp-types = { workspace = true, features = ["proposed"] }
20-
notify.workspace = true
21-
notify-debouncer-mini.workspace = true
2220
parking_lot.workspace = true
2321
proc-macro2.workspace = true
2422
quote.workspace = true

sway-lsp/src/core/session.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,17 +107,11 @@ impl Session {
107107
self.sync.clone_manifest_dir_to_temp()?;
108108
// iterate over the project dir, parse all sway files
109109
let _ = self.store_sway_files(documents).await;
110-
self.sync.watch_and_sync_manifest();
110+
self.sync.sync_manifest();
111111
self.sync.manifest_dir().map_err(Into::into)
112112
}
113113

114114
pub fn shutdown(&self) {
115-
// shutdown the thread watching the manifest file
116-
let handle = self.sync.notify_join_handle.read();
117-
if let Some(join_handle) = &*handle {
118-
join_handle.abort();
119-
}
120-
121115
// Delete the temporary directory.
122116
self.sync.remove_temp_dir();
123117
}

sway-lsp/src/core/sync.rs

Lines changed: 4 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,16 @@ use dashmap::DashMap;
66
use forc_pkg::manifest::{GenericManifestFile, ManifestFile};
77
use forc_pkg::PackageManifestFile;
88
use lsp_types::Url;
9-
use notify::RecursiveMode;
10-
use notify_debouncer_mini::new_debouncer;
11-
use parking_lot::RwLock;
129
use std::{
1310
fs,
1411
path::{Path, PathBuf},
15-
time::Duration,
1612
};
1713
use sway_types::{SourceEngine, Span};
1814
use sway_utils::{
1915
constants::{LOCK_FILE_NAME, MANIFEST_FILE_NAME},
2016
SWAY_EXTENSION,
2117
};
2218
use tempfile::Builder;
23-
use tokio::task::JoinHandle;
24-
use tracing::error;
2519

2620
#[derive(Debug, Eq, PartialEq, Hash)]
2721
pub enum Directory {
@@ -32,7 +26,6 @@ pub enum Directory {
3226
#[derive(Debug)]
3327
pub struct SyncWorkspace {
3428
pub directories: DashMap<Directory, PathBuf>,
35-
pub notify_join_handle: RwLock<Option<JoinHandle<()>>>,
3629
}
3730

3831
impl SyncWorkspace {
@@ -41,21 +34,14 @@ impl SyncWorkspace {
4134
pub(crate) fn new() -> Self {
4235
Self {
4336
directories: DashMap::new(),
44-
notify_join_handle: RwLock::new(None),
4537
}
4638
}
4739

4840
/// Overwrite the contents of the tmp/folder with everything in
4941
/// the current workspace.
5042
pub fn resync(&self) -> Result<(), LanguageServerError> {
5143
self.clone_manifest_dir_to_temp()?;
52-
if let (Ok(manifest_dir), Some(manifest_path), Some(temp_manifest_path)) = (
53-
self.manifest_dir(),
54-
self.manifest_path(),
55-
self.temp_manifest_path(),
56-
) {
57-
edit_manifest_dependency_paths(&manifest_dir, &manifest_path, &temp_manifest_path)?;
58-
}
44+
self.sync_manifest();
5945
Ok(())
6046
}
6147

@@ -178,8 +164,8 @@ impl SyncWorkspace {
178164
.ok()
179165
}
180166

181-
/// Watch the manifest directory and check for any save events on Forc.toml
182-
pub(crate) fn watch_and_sync_manifest(&self) {
167+
/// Read the Forc.toml and convert relative paths to absolute. Save into our temp directory.
168+
pub(crate) fn sync_manifest(&self) {
183169
if let (Ok(manifest_dir), Some(manifest_path), Some(temp_manifest_path)) = (
184170
self.manifest_dir(),
185171
self.manifest_path(),
@@ -188,40 +174,7 @@ impl SyncWorkspace {
188174
if let Err(err) =
189175
edit_manifest_dependency_paths(&manifest_dir, &manifest_path, &temp_manifest_path)
190176
{
191-
error!("Failed to edit manifest dependency paths: {}", err);
192-
}
193-
194-
let handle = tokio::spawn(async move {
195-
let (tx, mut rx) = tokio::sync::mpsc::channel(10);
196-
// Setup debouncer. No specific tickrate, max debounce time 500 milliseconds
197-
let mut debouncer = new_debouncer(Duration::from_millis(500), move |event| {
198-
if let Ok(e) = event {
199-
let _ = tx.blocking_send(e);
200-
}
201-
})
202-
.unwrap();
203-
204-
debouncer
205-
.watcher()
206-
.watch(&manifest_dir, RecursiveMode::NonRecursive)
207-
.unwrap();
208-
while let Some(_events) = rx.recv().await {
209-
// Rescan the Forc.toml and convert
210-
// relative paths to absolute. Save into our temp directory.
211-
if let Err(err) = edit_manifest_dependency_paths(
212-
&manifest_dir,
213-
&manifest_path,
214-
&temp_manifest_path,
215-
) {
216-
error!("Failed to edit manifest dependency paths: {}", err);
217-
}
218-
}
219-
});
220-
221-
// Store the join handle so we can clean up the thread on shutdown
222-
{
223-
let mut join_handle = self.notify_join_handle.write();
224-
*join_handle = Some(handle);
177+
tracing::error!("Failed to edit manifest dependency paths: {}", err);
225178
}
226179
}
227180
}

sway-lsp/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ pub enum LanguageServerError {
2727
ProgramsIsNone,
2828
#[error("Unable to acquire a semaphore permit for parsing")]
2929
UnableToAcquirePermit,
30+
#[error("Client is not initialized")]
31+
ClientNotInitialized,
32+
#[error("Client request error: {0}")]
33+
ClientRequestError(String),
3034
}
3135

3236
#[derive(Debug, Error, PartialEq, Eq)]

sway-lsp/src/handlers/notification.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,23 @@ pub(crate) async fn handle_did_change_watched_files(
159159
params: DidChangeWatchedFilesParams,
160160
) -> Result<(), LanguageServerError> {
161161
for event in params.changes {
162-
let (uri, _) = state.uri_and_session_from_workspace(&event.uri).await?;
163-
if let FileChangeType::DELETED = event.typ {
164-
state.pid_locked_files.remove_dirty_flag(&event.uri)?;
165-
let _ = state.documents.remove_document(&uri);
162+
let (uri, session) = state.uri_and_session_from_workspace(&event.uri).await?;
163+
164+
match event.typ {
165+
FileChangeType::CHANGED => {
166+
if event.uri.to_string().contains("Forc.toml") {
167+
session.sync.sync_manifest();
168+
// TODO: Recompile the project | see https://github.com/FuelLabs/sway/issues/7103
169+
}
170+
}
171+
FileChangeType::DELETED => {
172+
state.pid_locked_files.remove_dirty_flag(&event.uri)?;
173+
let _ = state.documents.remove_document(&uri);
174+
}
175+
FileChangeType::CREATED => {
176+
// TODO: handle this case
177+
}
178+
_ => {}
166179
}
167180
}
168181
Ok(())

sway-lsp/src/server.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ impl LanguageServer for ServerState {
2727
}
2828

2929
async fn initialized(&self, _: InitializedParams) {
30-
let _p = tracing::trace_span!("parse_text").entered();
30+
// Register a file system watcher for Forc.toml files with the client.
31+
if let Err(err) = self.register_forc_toml_watcher().await {
32+
tracing::error!("Failed to register Forc.toml file watcher: {}", err);
33+
}
3134
tracing::info!("Sway Language Server Initialized");
3235
}
3336

sway-lsp/src/server_state.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ use crossbeam_channel::{Receiver, Sender};
1313
use dashmap::{mapref::multiple::RefMulti, DashMap};
1414
use forc_pkg::manifest::GenericManifestFile;
1515
use forc_pkg::PackageManifestFile;
16-
use lsp_types::{Diagnostic, Url};
16+
use lsp_types::{
17+
Diagnostic, DidChangeWatchedFilesRegistrationOptions, FileSystemWatcher, GlobPattern,
18+
Registration, Url, WatchKind,
19+
};
1720
use parking_lot::{Mutex, RwLock};
1821
use std::{
1922
collections::{BTreeMap, VecDeque},
@@ -115,6 +118,35 @@ impl ServerState {
115118
}
116119
}
117120

121+
/// Registers a file system watcher for Forc.toml files with the client.
122+
pub async fn register_forc_toml_watcher(&self) -> Result<(), LanguageServerError> {
123+
let client = self
124+
.client
125+
.as_ref()
126+
.ok_or(LanguageServerError::ClientNotInitialized)?;
127+
128+
let watchers = vec![FileSystemWatcher {
129+
glob_pattern: GlobPattern::String("**/Forc.toml".to_string()),
130+
kind: Some(WatchKind::Create | WatchKind::Change),
131+
}];
132+
let registration_options = DidChangeWatchedFilesRegistrationOptions { watchers };
133+
let registration = Registration {
134+
id: "forc-toml-watcher".to_string(),
135+
method: "workspace/didChangeWatchedFiles".to_string(),
136+
register_options: Some(
137+
serde_json::to_value(registration_options)
138+
.expect("Failed to serialize registration options"),
139+
),
140+
};
141+
142+
client
143+
.register_capability(vec![registration])
144+
.await
145+
.map_err(|err| LanguageServerError::ClientRequestError(err.to_string()))?;
146+
147+
Ok(())
148+
}
149+
118150
/// Spawns a new thread dedicated to handling compilation tasks. This thread listens for
119151
/// `TaskMessage` instances sent over a channel and processes them accordingly.
120152
///

0 commit comments

Comments
 (0)