LS: Look for unmanaged core in Scarb

fixes #6235

commit-id:baf78a5f

crates/cairo-lang-language-server/Cargo.toml

@@ -33,6 +33,7 @@ scarb-metadata = "1.12"
 serde = { workspace = true, default-features = true }
 serde_json.workspace = true
 smol_str.workspace = true
+tempfile = "3"
 tokio.workspace = true
 tower-lsp = "0.20.0"
 tracing = "0.1"

crates/cairo-lang-language-server/src/lib.rs

@@ -571,21 +571,23 @@ impl Backend {
     async fn detect_crate_for(&self, db: &mut AnalysisDatabase, file_path: PathBuf) {
         match ProjectManifestPath::discover(&file_path) {
             Some(ProjectManifestPath::Scarb(manifest_path)) => {
-                let scarb = self.scarb_toolchain.clone();
-                let Ok(metadata) = spawn_blocking(move || {
-                    scarb
-                        .metadata(&manifest_path)
-                        .with_context(|| {
-                            format!(
-                                "failed to refresh scarb workspace: {}",
-                                manifest_path.display()
-                            )
-                        })
-                        .inspect_err(|e| {
-                            // TODO(mkaput): Send a notification to the language client.
-                            warn!("{e:?}");
-                        })
-                        .ok()
+                let Ok(metadata) = spawn_blocking({
+                    let scarb = self.scarb_toolchain.clone();
+                    move || {
+                        scarb
+                            .metadata(&manifest_path)
+                            .with_context(|| {
+                                format!(
+                                    "failed to refresh scarb workspace: {}",
+                                    manifest_path.display()
+                                )
+                            })
+                            .inspect_err(|e| {
+                                // TODO(mkaput): Send a notification to the language client.
+                                warn!("{e:?}");
+                            })
+                            .ok()
+                    }
                 })
                 .await
                 else {
@@ -597,7 +599,11 @@ impl Backend {
                     update_crate_roots(&metadata, db);
                 } else {
                     // Try to set up a corelib at least.
-                    try_to_init_unmanaged_core(&*self.config.read().await, db);
+                    try_to_init_unmanaged_core(
+                        db,
+                        &*self.config.read().await,
+                        &self.scarb_toolchain,
+                    );
                 }
 
                 if let Err(result) = validate_corelib(db) {
@@ -612,15 +618,15 @@ impl Backend {
                 // DB will also be absolute.
                 assert!(config_path.is_absolute());
 
-                try_to_init_unmanaged_core(&*self.config.read().await, db);
+                try_to_init_unmanaged_core(db, &*self.config.read().await, &self.scarb_toolchain);
 
                 if let Ok(config) = ProjectConfig::from_file(&config_path) {
                     update_crate_roots_from_project_config(db, &config);
                 };
             }
 
             None => {
-                try_to_init_unmanaged_core(&*self.config.read().await, db);
+                try_to_init_unmanaged_core(db, &*self.config.read().await, &self.scarb_toolchain);
 
                 if let Err(err) = setup_project(&mut *db, file_path.as_path()) {
                     let file_path_s = file_path.to_string_lossy();

crates/cairo-lang-language-server/src/project/unmanaged_core_crate.rs

@@ -1,17 +1,25 @@
-use std::path;
 use std::path::{Path, PathBuf};
+use std::sync::OnceLock;
+use std::{fs, path};
 
 use anyhow::Context;
-use cairo_lang_filesystem::db::init_dev_corelib;
+use cairo_lang_filesystem::db::{init_dev_corelib, CORELIB_CRATE_NAME};
+use indoc::indoc;
+use tempfile::tempdir;
 use tracing::{error, warn};
 
 use crate::config::Config;
 use crate::lang::db::AnalysisDatabase;
+use crate::toolchain::scarb::{ScarbToolchain, SCARB_TOML};
 
 /// Try to find a Cairo `core` crate (see [`find_unmanaged_core`]) and initialize it in the
 /// provided database.
-pub fn try_to_init_unmanaged_core(config: &Config, db: &mut AnalysisDatabase) {
-    if let Some(path) = find_unmanaged_core(config) {
+pub fn try_to_init_unmanaged_core(
+    db: &mut AnalysisDatabase,
+    config: &Config,
+    scarb: &ScarbToolchain,
+) {
+    if let Some(path) = find_unmanaged_core(config, scarb) {
         init_dev_corelib(db, path);
     } else {
         warn!("failed to find unmanaged core crate")
@@ -22,13 +30,16 @@ pub fn try_to_init_unmanaged_core(config: &Config, db: &mut AnalysisDatabase) {
 /// do not manage the `core` crate (i.e., anything non-Scarb).
 ///
 /// The path is guaranteed to be absolute, so it can be safely used as a `FileId` in LS Salsa DB.
-pub fn find_unmanaged_core(config: &Config) -> Option<PathBuf> {
-    // TODO(mkaput): First, try to find Scarb-managed `core` package if we have Scarb toolchain.
-    //   The most reliable way to do this is to create an empty Scarb package, and run
-    //   `scarb metadata` on it. The `core` package will be a component of this empty package.
-    //   For minimal packages, `scarb metadata` should be pretty fast.
+pub fn find_unmanaged_core(config: &Config, scarb: &ScarbToolchain) -> Option<PathBuf> {
     find_core_at_config_path(config)
-        .or_else(cairo_lang_filesystem::detect::detect_corelib)
+        .or_else(|| find_scarb_managed_core(scarb))
+        .or_else(|| {
+            if cfg!(feature = "testing") {
+                cairo_lang_filesystem::detect::detect_corelib()
+            } else {
+                None
+            }
+        })
         .and_then(ensure_absolute)
 }
 
@@ -39,8 +50,8 @@ fn find_core_at_config_path(config: &Config) -> Option<PathBuf> {
 
 /// Attempts to find the `core` crate source root at a given path.
 ///
-/// In the [starkware-libs/cairo] repository, the `core` crate sits in `./corelib/src`, this is the
-/// first place looked for.
+/// In the [starkware-libs/cairo] repository, the `core` crate sits in `./corelib/src`.
+/// This is the first place looked for.
 /// The `core` crate is a regular Scarb package, so it sounds obvious that the user may provide a
 /// path to the directory containing the manifest file, hence next this function looks for `./src`.
 /// Finally, the input path is considered as a candidate and is just checked for existence.
@@ -67,6 +78,62 @@ fn find_core_at_path(root_path: &Path) -> Option<PathBuf> {
     None
 }
 
+// TODO(#6246): Add tests for this logic. Pay attention to usage of silent mode in ScarbToolchain.
+/// Try to find a Scarb-managed `core` package if we have Scarb toolchain.
+///
+/// The easiest way to do this is to create an empty Scarb package and run `scarb metadata` on it.
+/// The `core` package will be a component of this empty package.
+/// For minimal packages, `scarb metadata` should be pretty fast.
+///
+/// Because CairoLS is tightly bound to Scarb (due to hard compiler version dependency),
+/// we can safely make this lookup once and keep it cached for the entire LS lifetime.
+#[tracing::instrument(level = "trace", skip_all)]
+fn find_scarb_managed_core(scarb: &ScarbToolchain) -> Option<PathBuf> {
+    let lookup = || {
+        let workspace = tempdir()
+            .context("failed to create temporary directory")
+            .inspect_err(|e| warn!("{e:?}"))
+            .ok()?;
+
+        let scarb_toml = workspace.path().join(SCARB_TOML);
+        fs::write(
+            &scarb_toml,
+            indoc! {r#"
+                [package]
+                name = "cairols_unmanaged_core_lookup"
+                version = "1.0.0"
+            "#},
+        )
+        .context("failed to write Scarb.toml")
+        .inspect_err(|e| warn!("{e:?}"))
+        .ok()?;
+
+        let metadata = scarb.silent().metadata(&scarb_toml).inspect_err(|e| warn!("{e:?}")).ok()?;
+
+        // Ensure the workspace directory is deleted after running Scarb.
+        // We are ignoring the error, leaving doing proper cleanup to the OS.
+        let _ = workspace
+            .close()
+            .context("failed to wipe temporary directory")
+            .inspect_err(|e| warn!("{e:?}"));
+
+        // Scarb is expected to generate only one compilation unit (for our stub package)
+        // that will consist of this package and the `core` crate.
+        // Therefore, we allow ourselves to liberally just look for any first usage of a package
+        // named `core` in all compilation units components we got.
+        metadata.compilation_units.into_iter().find_map(|compilation_unit| {
+            compilation_unit
+                .components
+                .iter()
+                .find(|component| component.name == CORELIB_CRATE_NAME)
+                .map(|component| component.source_root().to_path_buf().into_std_path_buf())
+        })
+    };
+
+    static CACHE: OnceLock<Option<PathBuf>> = OnceLock::new();
+    CACHE.get_or_init(lookup).clone()
+}
+
 /// Makes a path absolute, or logs an error.
 fn ensure_absolute(path: PathBuf) -> Option<PathBuf> {
     path::absolute(&path)

crates/cairo-lang-language-server/src/toolchain/scarb.rs

@@ -4,7 +4,7 @@ use std::sync::{Arc, OnceLock};
 use anyhow::{bail, Context, Result};
 use scarb_metadata::{Metadata, MetadataCommand};
 use tower_lsp::lsp_types::notification::Notification;
-use tracing::warn;
+use tracing::{error, warn};
 
 use crate::env_config;
 use crate::server::notifier::Notifier;
@@ -22,35 +22,85 @@ pub const SCARB_TOML: &str = "Scarb.toml";
 pub struct ScarbToolchain {
     /// Cached path to the `scarb` executable.
     scarb_path_cell: Arc<OnceLock<Option<PathBuf>>>,
+
     /// The notifier object used to send notifications to the language client.
     notifier: Notifier,
+
+    /// States whether this instance is in _silent mode_.
+    ///
+    /// See [`ScarbToolchain::silent`] for more info.
+    is_silent: bool,
 }
 
 impl ScarbToolchain {
     /// Constructs a new [`ScarbToolchain`].
     pub fn new(notifier: &Notifier) -> Self {
-        ScarbToolchain { scarb_path_cell: Default::default(), notifier: notifier.clone() }
+        ScarbToolchain {
+            scarb_path_cell: Default::default(),
+            notifier: notifier.clone(),
+            is_silent: false,
+        }
     }
 
     /// Finds the path to the `scarb` executable to use.
     ///
     /// This method may send notifications to the language client if there are any actionable issues
-    /// with the found `scarb` installation, or if it could not be found.
+    /// with the found `scarb` installation or if it could not be found.
     fn discover(&self) -> Option<&Path> {
         self.scarb_path_cell
             .get_or_init(|| {
                 let path = env_config::scarb_path();
                 // TODO(mkaput): Perhaps we should display this notification again after reloading?
                 if path.is_none() {
-                    warn!("attempt to use scarb without SCARB env being set");
-                    self.notifier.send_notification::<ScarbPathMissing>(());
+                    if self.is_silent {
+                        // If we are in silent mode, then missing Scarb is probably dealt with
+                        // at the caller site.
+                        warn!("attempt to use scarb without SCARB env being set");
+                    } else {
+                        error!("attempt to use scarb without SCARB env being set");
+                        self.notifier.send_notification::<ScarbPathMissing>(());
+                    }
                 }
                 path
             })
             .as_ref()
             .map(PathBuf::as_path)
     }
 
+    /// Creates a clone instance of this object that will be in _silent mode_.
+    ///
+    /// Silent mode means that any operations invoked through this instance should avoid performing
+    /// any user-visible actions.
+    pub fn silent(&self) -> Self {
+        if self.is_silent {
+            // Going silent from silent is noop, so skip any shenanigans we do here.
+            self.clone()
+        } else {
+            Self {
+                // Disassociate this instance from the shared path cell if it has not been
+                // initialized yet.
+                //
+                // This maintains a good UX for the following scenario (timeline):
+                // 1. CairoLS is started without a path to Scarb provided.
+                // 2. Some internal operation is silently attempting to query Scarb, which will
+                //    initialize the cell but only log a warning.
+                // 3. User-invoked operation makes an attempt to query Scarb.
+                //
+                // At this point we want to show missing Scarb notification,
+                // but without this trick we would never do
+                // as the path cell would be already initialized.
+                scarb_path_cell: match self.scarb_path_cell.get() {
+                    Some(_) => self.scarb_path_cell.clone(),
+                    None => Default::default(),
+                },
+
+                notifier: self.notifier.clone(),
+
+                is_silent: true,
+            }
+        }
+    }
+
     /// Calls `scarb metadata` for the given `Scarb.toml` and parse its output.
     ///
     /// This is a blocking operation that may be long-running. It should only be called from within
@@ -66,7 +116,9 @@ impl ScarbToolchain {
             bail!("could not find scarb executable");
         };
 
-        self.notifier.send_notification::<ScarbResolvingStart>(());
+        if !self.is_silent {
+            self.notifier.send_notification::<ScarbResolvingStart>(());
+        }
 
         let result = MetadataCommand::new()
             .scarb_path(scarb_path)
@@ -75,7 +127,9 @@ impl ScarbToolchain {
             .exec()
             .context("failed to execute: scarb metadata");
 
-        self.notifier.send_notification::<ScarbResolvingFinish>(());
+        if !self.is_silent {
+            self.notifier.send_notification::<ScarbResolvingFinish>(());
+        }
 
         result
     }