Synchronize code_lens request & provide better UX for forc-fmt <> `…

…sway-lsp` interaction. (#5094)

## Description
This PR adds 3 things:

1. Adds a benchmark for the format LSP request.

2. Calls `session.wait_for_parsing();` before computing the `code_lens`
request. This fixes the original issue reported in #4893 where run
buttons where being placed incorrectly after formatting.

3. If a file open in a code editor contains unsaved changes we write a
lock file to `.forc/lsp_locks/`. This file is removed when the file is
saved and there are no pending changes. If `forc-fmt` is run in a
terminal we check if the path has a lock file associated with it. If
unsaved changes are detected we bail from formatting with an error
message instructing the user to save changes before continuing. See
video below.


https://github.com/FuelLabs/sway/assets/1289413/75e6fddc-adbc-4796-aeb9-985574ae8dcc


closes #4893

forc-pkg/Cargo.toml

@@ -12,7 +12,6 @@ repository.workspace = true
 ansi_term = "0.12"
 anyhow = "1"
 cid = "0.10"
-fd-lock = "3.0"
 forc-tracing = { version = "0.46.0", path = "../forc-tracing" }
 forc-util = { version = "0.46.0", path = "../forc-util" }
 fuel-abi-types = "0.1"

forc-pkg/src/pkg.rs

@@ -7,7 +7,7 @@ use crate::{
 use anyhow::{anyhow, bail, Context, Error, Result};
 use forc_util::{
     default_output_directory, find_file_name, kebab_to_snake_case, print_compiling,
-    print_on_failure, print_warnings, user_forc_directory,
+    print_on_failure, print_warnings,
 };
 use fuel_abi_types::program_abi;
 use petgraph::{
@@ -26,7 +26,6 @@ use std::{
     str::FromStr,
     sync::Arc,
 };
-use sway_core::fuel_prelude::fuel_types::ChainId;
 pub use sway_core::Programs;
 use sway_core::{
     abi_generation::{
@@ -38,6 +37,7 @@ use sway_core::{
     fuel_prelude::{
         fuel_crypto,
         fuel_tx::{self, Contract, ContractId, StorageSlot},
+        fuel_types::ChainId,
     },
     language::{parsed::TreeType, Visibility},
     semantic_analysis::namespace,
@@ -1516,48 +1516,6 @@ fn fetch_deps(
     Ok(added)
 }
 
-/// Given a path to a directory we wish to lock, produce a path for an associated lock file.
-///
-/// Note that the lock file itself is simply a placeholder for co-ordinating access. As a result,
-/// we want to create the lock file if it doesn't exist, but we can never reliably remove it
-/// without risking invalidation of an existing lock. As a result, we use a dedicated, hidden
-/// directory with a lock file named after the checkout path.
-///
-/// Note: This has nothing to do with `Forc.lock` files, rather this is about fd locks for
-/// coordinating access to particular paths (e.g. git checkout directories).
-fn fd_lock_path(path: &Path) -> PathBuf {
-    const LOCKS_DIR_NAME: &str = ".locks";
-    const LOCK_EXT: &str = "forc-lock";
-
-    // Hash the path to produce a file-system friendly lock file name.
-    // Append the file stem for improved readability.
-    let mut hasher = hash_map::DefaultHasher::default();
-    path.hash(&mut hasher);
-    let hash = hasher.finish();
-    let file_name = match path.file_stem().and_then(|s| s.to_str()) {
-        None => format!("{hash:X}"),
-        Some(stem) => format!("{hash:X}-{stem}"),
-    };
-
-    user_forc_directory()
-        .join(LOCKS_DIR_NAME)
-        .join(file_name)
-        .with_extension(LOCK_EXT)
-}
-
-/// Create an advisory lock over the given path.
-///
-/// See [fd_lock_path] for details.
-pub(crate) fn path_lock(path: &Path) -> Result<fd_lock::RwLock<File>> {
-    let lock_path = fd_lock_path(path);
-    let lock_dir = lock_path
-        .parent()
-        .expect("lock path has no parent directory");
-    std::fs::create_dir_all(lock_dir).context("failed to create forc advisory lock directory")?;
-    let lock_file = File::create(&lock_path).context("failed to create advisory lock file")?;
-    Ok(fd_lock::RwLock::new(lock_file))
-}
-
 /// Given a `forc_pkg::BuildProfile`, produce the necessary `sway_core::BuildConfig` required for
 /// compilation.
 pub fn sway_build_config(

forc-pkg/src/source/git/mod.rs

@@ -185,7 +185,7 @@ impl source::Pin for Source {
 impl source::Fetch for Pinned {
     fn fetch(&self, ctx: source::PinCtx, repo_path: &Path) -> Result<PackageManifestFile> {
         // Co-ordinate access to the git checkout directory using an advisory file lock.
-        let mut lock = crate::pkg::path_lock(repo_path)?;
+        let mut lock = forc_util::path_lock(repo_path)?;
         // TODO: Here we assume that if the local path already exists, that it contains the
         // full and correct source for that commit and hasn't been tampered with. This is
         // probably fine for most cases as users should never be touching these
@@ -217,7 +217,7 @@ impl source::DepPath for Pinned {
     fn dep_path(&self, name: &str) -> anyhow::Result<source::DependencyPath> {
         let repo_path = commit_path(name, &self.source.repo, &self.commit_hash);
         // Co-ordinate access to the git checkout directory using an advisory file lock.
-        let lock = crate::pkg::path_lock(&repo_path)?;
+        let lock = forc_util::path_lock(&repo_path)?;
         let _guard = lock.read()?;
         let path = manifest::find_within(&repo_path, name)
             .ok_or_else(|| anyhow!("failed to find package `{}` in {}", name, self))?;

forc-pkg/src/source/ipfs.rs

@@ -62,7 +62,7 @@ impl source::Fetch for Pinned {
             anyhow::bail!("offline fetching for IPFS sources is not supported")
         }
 
-        let mut lock = crate::pkg::path_lock(repo_path)?;
+        let mut lock = forc_util::path_lock(repo_path)?;
         {
             let _guard = lock.write()?;
             if !repo_path.exists() {
@@ -111,7 +111,7 @@ impl source::DepPath for Pinned {
     fn dep_path(&self, name: &str) -> anyhow::Result<source::DependencyPath> {
         let repo_path = pkg_cache_dir(&self.0);
         // Co-ordinate access to the ipfs checkout directory using an advisory file lock.
-        let lock = crate::pkg::path_lock(&repo_path)?;
+        let lock = forc_util::path_lock(&repo_path)?;
         let _guard = lock.read()?;
         let path = manifest::find_within(&repo_path, name)
             .ok_or_else(|| anyhow::anyhow!("failed to find package `{}` in {}", name, self))?;

forc-plugins/forc-fmt/src/main.rs

@@ -78,7 +78,6 @@ fn run() -> Result<()> {
     };
 
     let manifest_file = forc_pkg::manifest::ManifestFile::from_dir(&dir)?;
-
     match manifest_file {
         ManifestFile::Workspace(ws) => {
             format_workspace_at_dir(&app, &ws, &dir)?;
@@ -87,10 +86,19 @@ fn run() -> Result<()> {
             format_pkg_at_dir(&app, &dir, &mut formatter)?;
         }
     }
-
     Ok(())
 }
 
+/// Checks if the specified file is marked as "dirty".
+/// This is used to prevent formatting files that are currently open in an editor
+/// with unsaved changes.
+///
+/// Returns `true` if a corresponding "dirty" flag file exists, `false` otherwise.
+fn is_file_dirty(path: &Path) -> bool {
+    let dirty_file_path = forc_util::is_dirty_path(path);
+    dirty_file_path.exists()
+}
+
 /// Recursively get a Vec<PathBuf> of subdirectories that contains a Forc.toml.
 fn get_sway_dirs(workspace_dir: PathBuf) -> Vec<PathBuf> {
     let mut dirs_to_format = vec![];
@@ -126,6 +134,14 @@ fn format_file(
     formatter: &mut Formatter,
 ) -> Result<bool> {
     let file = file.canonicalize()?;
+    if is_file_dirty(&file) {
+        bail!(
+            "The below file is open in an editor and contains unsaved changes.\n       \
+             Please save it before formatting.\n       \
+             {}",
+            file.display()
+        );
+    }
     if let Ok(file_content) = fs::read_to_string(&file) {
         let mut edited = false;
         let file_content: Arc<str> = Arc::from(file_content);

forc-util/Cargo.toml

@@ -14,6 +14,7 @@ ansi_term = "0.12"
 anyhow = "1"
 clap = { version = "3.1", features = ["cargo", "derive", "env"] }
 dirs = "3.0.2"
+fd-lock = "4.0"
 forc-tracing = { version = "0.46.0", path = "../forc-tracing" }
 fuel-tx = { workspace = true, features = ["serde"], optional = true }
 hex = "0.4.3"

forc-util/src/lib.rs

@@ -5,12 +5,17 @@ use annotate_snippets::{
     snippet::{Annotation, AnnotationType, Slice, Snippet, SourceAnnotation},
 };
 use ansi_term::Colour;
-use anyhow::{bail, Result};
+use anyhow::{bail, Context, Result};
 use forc_tracing::{println_red_err, println_yellow_err};
-use std::{collections::HashSet, str};
+use std::{
+    collections::{hash_map, HashSet},
+    str,
+};
 use std::{ffi::OsStr, process::Termination};
 use std::{
     fmt::Display,
+    fs::File,
+    hash::{Hash, Hasher},
     path::{Path, PathBuf},
 };
 use sway_core::language::parsed::TreeType;
@@ -349,6 +354,64 @@ pub fn git_checkouts_directory() -> PathBuf {
     user_forc_directory().join("git").join("checkouts")
 }
 
+/// Given a path to a directory we wish to lock, produce a path for an associated lock file.
+///
+/// Note that the lock file itself is simply a placeholder for co-ordinating access. As a result,
+/// we want to create the lock file if it doesn't exist, but we can never reliably remove it
+/// without risking invalidation of an existing lock. As a result, we use a dedicated, hidden
+/// directory with a lock file named after the checkout path.
+///
+/// Note: This has nothing to do with `Forc.lock` files, rather this is about fd locks for
+/// coordinating access to particular paths (e.g. git checkout directories).
+fn fd_lock_path(path: &Path) -> PathBuf {
+    const LOCKS_DIR_NAME: &str = ".locks";
+    const LOCK_EXT: &str = "forc-lock";
+    let file_name = hash_path(path);
+    user_forc_directory()
+        .join(LOCKS_DIR_NAME)
+        .join(file_name)
+        .with_extension(LOCK_EXT)
+}
+
+/// Constructs the path for the "dirty" flag file corresponding to the specified file.
+///
+/// This function uses a hashed representation of the original path for uniqueness.
+pub fn is_dirty_path(path: &Path) -> PathBuf {
+    const LOCKS_DIR_NAME: &str = ".lsp-locks";
+    const LOCK_EXT: &str = "dirty";
+    let file_name = hash_path(path);
+    user_forc_directory()
+        .join(LOCKS_DIR_NAME)
+        .join(file_name)
+        .with_extension(LOCK_EXT)
+}
+
+/// Hash the path to produce a file-system friendly file name.
+/// Append the file stem for improved readability.
+fn hash_path(path: &Path) -> String {
+    let mut hasher = hash_map::DefaultHasher::default();
+    path.hash(&mut hasher);
+    let hash = hasher.finish();
+    let file_name = match path.file_stem().and_then(|s| s.to_str()) {
+        None => format!("{hash:X}"),
+        Some(stem) => format!("{hash:X}-{stem}"),
+    };
+    file_name
+}
+
+/// Create an advisory lock over the given path.
+///
+/// See [fd_lock_path] for details.
+pub fn path_lock(path: &Path) -> Result<fd_lock::RwLock<File>> {
+    let lock_path = fd_lock_path(path);
+    let lock_dir = lock_path
+        .parent()
+        .expect("lock path has no parent directory");
+    std::fs::create_dir_all(lock_dir).context("failed to create forc advisory lock directory")?;
+    let lock_file = File::create(&lock_path).context("failed to create advisory lock file")?;
+    Ok(fd_lock::RwLock::new(lock_file))
+}
+
 pub fn program_type_str(ty: &TreeType) -> &'static str {
     match ty {
         TreeType::Script {} => "script",

sway-lsp/Cargo.toml

@@ -11,8 +11,10 @@ repository.workspace = true
 [dependencies]
 anyhow = "1.0.41"
 dashmap = "5.4"
+fd-lock = "4.0"
 forc-pkg = { version = "0.46.0", path = "../forc-pkg" }
 forc-tracing = { version = "0.46.0", path = "../forc-tracing" }
+forc-util = { version = "0.46.0", path = "../forc-util" }
 lsp-types = { version = "0.94", features = ["proposed"] }
 notify = "5.0.0"
 notify-debouncer-mini = { version = "0.2.0" }

sway-lsp/benches/lsp_benchmarks/requests.rs

@@ -93,6 +93,8 @@ fn benchmarks(c: &mut Criterion) {
         };
         b.iter(|| capabilities::on_enter::on_enter(&config.on_enter, &session, &uri, &params))
     });
+
+    c.bench_function("format", |b| b.iter(|| session.format_text(&uri)));
 }
 
 criterion_group! {

sway-lsp/src/core/document.rs

@@ -1,9 +1,11 @@
 #![allow(dead_code)]
-use lsp_types::{Position, Range, TextDocumentContentChangeEvent};
+use crate::{
+    error::{DirectoryError, DocumentError, LanguageServerError},
+    utils::document,
+};
+use lsp_types::{Position, Range, TextDocumentContentChangeEvent, Url};
 use ropey::Rope;
 
-use crate::error::DocumentError;
-
 #[derive(Debug, Clone)]
 pub struct TextDocument {
     #[allow(dead_code)]
@@ -104,6 +106,40 @@ impl TextDocument {
     }
 }
 
+/// Marks the specified file as "dirty" by creating a corresponding flag file.
+///
+/// This function ensures the necessary directory structure exists before creating the flag file.
+pub fn mark_file_as_dirty(uri: &Url) -> Result<(), LanguageServerError> {
+    let path = document::get_path_from_url(uri)?;
+    let dirty_file_path = forc_util::is_dirty_path(&path);
+    if let Some(dir) = dirty_file_path.parent() {
+        // Ensure the directory exists
+        std::fs::create_dir_all(dir).map_err(|_| DirectoryError::LspLocksDirFailed)?;
+    }
+    // Create an empty "dirty" file
+    std::fs::File::create(&dirty_file_path).map_err(|err| DocumentError::UnableToCreateFile {
+        path: uri.path().to_string(),
+        err: err.to_string(),
+    })?;
+    Ok(())
+}
+
+/// Removes the corresponding flag file for the specifed Url.
+///
+/// If the flag file does not exist, this function will do nothing.
+pub fn remove_dirty_flag(uri: &Url) -> Result<(), LanguageServerError> {
+    let path = document::get_path_from_url(uri)?;
+    let dirty_file_path = forc_util::is_dirty_path(&path);
+    if dirty_file_path.exists() {
+        // Remove the "dirty" file
+        std::fs::remove_file(dirty_file_path).map_err(|err| DocumentError::UnableToRemoveFile {
+            path: uri.path().to_string(),
+            err: err.to_string(),
+        })?;
+    }
+    Ok(())
+}
+
 #[derive(Debug)]
 struct EditText<'text> {
     start_index: usize,

sway-lsp/src/error.rs

@@ -40,6 +40,8 @@ pub enum DocumentError {
     UnableToCreateFile { path: String, err: String },
     #[error("Unable to write string to file at {:?} : {:?}", path, err)]
     UnableToWriteFile { path: String, err: String },
+    #[error("File wasn't able to be removed at path {:?} : {:?}", path, err)]
+    UnableToRemoveFile { path: String, err: String },
 }
 
 #[derive(Debug, Error, PartialEq, Eq)]
@@ -50,6 +52,8 @@ pub enum DirectoryError {
     ManifestDirNotFound,
     #[error("Can't extract project name from {:?}", dir)]
     CantExtractProjectName { dir: String },
+    #[error("Failed to create hidden .lsp_locks directory")]
+    LspLocksDirFailed,
     #[error("Failed to create temp directory")]
     TempDirFailed,
     #[error("Failed to canonicalize path")]