Auto merge of #16247 - Veykril:opqueues, r=Veykril

fix: Fix build scripts not being rebuilt in some occasions

Also makes proc-macro changed flag setting async, we don't wanna block `process_changes` on the database as that is on the main thread!

crates/hir-def/src/nameres/collector.rs

@@ -101,9 +101,9 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
                             if it.disabled {
                                 CustomProcMacroExpander::disabled()
                             } else {
-                                CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
+                                CustomProcMacroExpander::new(
-                                    idx as u32,
+                                    hir_expand::proc_macro::ProcMacroId::new(idx as u32),
-                                ))
+                                )
                             },
                         )
                     })
@@ -2354,7 +2354,7 @@ impl ModCollector<'_, '_> {
                 resolved_res.resolved_def.take_macros().map(|it| db.macro_def(it))
             },
         ) {
-            // FIXME: if there were errors, this mightve been in the eager expansion from an
+            // FIXME: if there were errors, this might've been in the eager expansion from an
             // unresolved macro, so we need to push this into late macro resolution. see fixme above
             if res.err.is_none() {
                 // Legacy macros need to be expanded immediately, so that any macros they produce

crates/hir-expand/src/lib.rs

@@ -227,8 +227,8 @@ pub enum MacroCallKind {
     },
     Attr {
         ast_id: AstId<ast::Item>,
-        // FIXME: This is being interned, subtrees can vary quickly differ just slightly causing
+        // FIXME: This shouldn't be here, we can derive this from `invoc_attr_index`
-        // leakage problems here
+        // but we need to fix the `cfg_attr` handling first.
         attr_args: Option<Arc<tt::Subtree>>,
         /// Syntactical index of the invoking `#[attribute]`.
         ///

crates/hir-expand/src/proc_macro.rs

@@ -12,7 +12,13 @@ use syntax::SmolStr;
 use crate::{db::ExpandDatabase, tt, ExpandError, ExpandResult};
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub struct ProcMacroId(pub u32);
+pub struct ProcMacroId(u32);
+
+impl ProcMacroId {
+    pub fn new(u32: u32) -> Self {
+        ProcMacroId(u32)
+    }
+}
 
 #[derive(Copy, Clone, Eq, PartialEq, Debug, Hash)]
 pub enum ProcMacroKind {

crates/ide/src/parent_module.rs

@@ -54,7 +54,7 @@ pub(crate) fn parent_module(db: &RootDatabase, position: FilePosition) -> Vec<Na
     }
 }
 
-/// Returns `Vec` for the same reason as `parent_module`
+/// This returns `Vec` because a module may be included from several places.
 pub(crate) fn crates_for(db: &RootDatabase, file_id: FileId) -> Vec<CrateId> {
     db.relevant_crates(file_id)
         .iter()

crates/project-model/src/build_scripts.rs

@@ -24,7 +24,7 @@ use toolchain::Tool;
 
 use crate::{
     cfg_flag::CfgFlag, utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
-    InvocationStrategy, Package, Sysroot,
+    InvocationStrategy, Package, Sysroot, TargetKind,
 };
 
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
@@ -467,7 +467,11 @@ impl WorkspaceBuildScripts {
                 .collect();
             for p in rustc.packages() {
                 let package = &rustc[p];
-                if package.targets.iter().any(|&it| rustc[it].is_proc_macro) {
+                if package
+                    .targets
+                    .iter()
+                    .any(|&it| matches!(rustc[it].kind, TargetKind::Lib { is_proc_macro: true }))
+                {
                     if let Some((_, path)) = proc_macro_dylibs
                         .iter()
                         .find(|(name, _)| *name.trim_start_matches("lib") == package.name)

crates/project-model/src/cargo_workspace.rs

@@ -189,8 +189,6 @@ pub struct TargetData {
     pub root: AbsPathBuf,
     /// Kind of target
     pub kind: TargetKind,
-    /// Is this target a proc-macro
-    pub is_proc_macro: bool,
     /// Required features of the target without which it won't build
     pub required_features: Vec<String>,
 }
@@ -199,7 +197,10 @@ pub struct TargetData {
 pub enum TargetKind {
     Bin,
     /// Any kind of Cargo lib crate-type (dylib, rlib, proc-macro, ...).
-    Lib,
+    Lib {
+        /// Is this target a proc-macro
+        is_proc_macro: bool,
+    },
     Example,
     Test,
     Bench,
@@ -216,8 +217,8 @@ impl TargetKind {
                 "bench" => TargetKind::Bench,
                 "example" => TargetKind::Example,
                 "custom-build" => TargetKind::BuildScript,
-                "proc-macro" => TargetKind::Lib,
+                "proc-macro" => TargetKind::Lib { is_proc_macro: true },
-                _ if kind.contains("lib") => TargetKind::Lib,
+                _ if kind.contains("lib") => TargetKind::Lib { is_proc_macro: false },
                 _ => continue,
             };
         }
@@ -371,7 +372,6 @@ impl CargoWorkspace {
                     name,
                     root: AbsPathBuf::assert(src_path.into()),
                     kind: TargetKind::new(&kind),
-                    is_proc_macro: *kind == ["proc-macro"],
                     required_features,
                 });
                 pkg_data.targets.push(tgt);

crates/project-model/src/workspace.rs

@@ -625,7 +625,7 @@ impl ProjectWorkspace {
                         let extra_targets = cargo[pkg]
                             .targets
                             .iter()
-                            .filter(|&&tgt| cargo[tgt].kind == TargetKind::Lib)
+                            .filter(|&&tgt| matches!(cargo[tgt].kind, TargetKind::Lib { .. }))
                             .filter_map(|&tgt| cargo[tgt].root.parent())
                             .map(|tgt| tgt.normalize().to_path_buf())
                             .filter(|path| !path.starts_with(&pkg_root));
@@ -991,7 +991,7 @@ fn cargo_to_crate_graph(
 
         let mut lib_tgt = None;
         for &tgt in cargo[pkg].targets.iter() {
-            if cargo[tgt].kind != TargetKind::Lib && !cargo[pkg].is_member {
+            if !matches!(cargo[tgt].kind, TargetKind::Lib { .. }) && !cargo[pkg].is_member {
                 // For non-workspace-members, Cargo does not resolve dev-dependencies, so we don't
                 // add any targets except the library target, since those will not work correctly if
                 // they use dev-dependencies.
@@ -999,7 +999,7 @@ fn cargo_to_crate_graph(
                 // https://github.com/rust-lang/rust-analyzer/issues/11300
                 continue;
             }
-            let &TargetData { ref name, kind, is_proc_macro, ref root, .. } = &cargo[tgt];
+            let &TargetData { ref name, kind, ref root, .. } = &cargo[tgt];
 
             let Some(file_id) = load(root) else { continue };
 
@@ -1011,19 +1011,24 @@ fn cargo_to_crate_graph(
                 cfg_options.clone(),
                 file_id,
                 name,
-                is_proc_macro,
+                kind,
                 target_layout.clone(),
                 false,
                 toolchain.cloned(),
             );
-            if kind == TargetKind::Lib {
+            if let TargetKind::Lib { .. } = kind {
                 lib_tgt = Some((crate_id, name.clone()));
                 pkg_to_lib_crate.insert(pkg, crate_id);
             }
             // Even crates that don't set proc-macro = true are allowed to depend on proc_macro
             // (just none of the APIs work when called outside of a proc macro).
             if let Some(proc_macro) = libproc_macro {
-                add_proc_macro_dep(crate_graph, crate_id, proc_macro, is_proc_macro);
+                add_proc_macro_dep(
+                    crate_graph,
+                    crate_id,
+                    proc_macro,
+                    matches!(kind, TargetKind::Lib { is_proc_macro: true }),
+                );
             }
 
             pkg_crates.entry(pkg).or_insert_with(Vec::new).push((crate_id, kind));
@@ -1221,9 +1226,9 @@ fn handle_rustc_crates(
             };
 
             for &tgt in rustc_workspace[pkg].targets.iter() {
-                if rustc_workspace[tgt].kind != TargetKind::Lib {
+                let kind @ TargetKind::Lib { is_proc_macro } = rustc_workspace[tgt].kind else {
                     continue;
-                }
+                };
                 if let Some(file_id) = load(&rustc_workspace[tgt].root) {
                     let crate_id = add_target_crate_root(
                         crate_graph,
@@ -1233,7 +1238,7 @@ fn handle_rustc_crates(
                         cfg_options.clone(),
                         file_id,
                         &rustc_workspace[tgt].name,
-                        rustc_workspace[tgt].is_proc_macro,
+                        kind,
                         target_layout.clone(),
                         true,
                         toolchain.cloned(),
@@ -1242,12 +1247,7 @@ fn handle_rustc_crates(
                     // Add dependencies on core / std / alloc for this crate
                     public_deps.add_to_crate_graph(crate_graph, crate_id);
                     if let Some(proc_macro) = libproc_macro {
-                        add_proc_macro_dep(
+                        add_proc_macro_dep(crate_graph, crate_id, proc_macro, is_proc_macro);
-                            crate_graph,
-                            crate_id,
-                            proc_macro,
-                            rustc_workspace[tgt].is_proc_macro,
-                        );
                     }
                     rustc_pkg_crates.entry(pkg).or_insert_with(Vec::new).push(crate_id);
                 }
@@ -1309,7 +1309,7 @@ fn add_target_crate_root(
     cfg_options: CfgOptions,
     file_id: FileId,
     cargo_name: &str,
-    is_proc_macro: bool,
+    kind: TargetKind,
     target_layout: TargetLayoutLoadResult,
     rustc_crate: bool,
     toolchain: Option<Version>,
@@ -1359,7 +1359,7 @@ fn add_target_crate_root(
         cfg_options,
         potential_cfg_options,
         env,
-        is_proc_macro,
+        matches!(kind, TargetKind::Lib { is_proc_macro: true }),
         if rustc_crate {
             CrateOrigin::Rustc { name: pkg.name.clone() }
         } else if pkg.is_member {
@@ -1370,7 +1370,7 @@ fn add_target_crate_root(
         target_layout,
         toolchain,
     );
-    if is_proc_macro {
+    if let TargetKind::Lib { is_proc_macro: true } = kind {
         let proc_macro = match build_data.as_ref().map(|it| it.proc_macro_dylib_path.as_ref()) {
             Some(it) => it.cloned().map(|path| Ok((Some(cargo_name.to_owned()), path))),
             None => Some(Err("crate has not yet been built".to_owned())),

crates/rust-analyzer/src/cargo_target_spec.rs

@@ -174,7 +174,7 @@ impl CargoTargetSpec {
                 buf.push("--example".to_owned());
                 buf.push(self.target);
             }
-            TargetKind::Lib => {
+            TargetKind::Lib { is_proc_macro: _ } => {
                 buf.push("--lib".to_owned());
             }
             TargetKind::Other | TargetKind::BuildScript => (),

crates/rust-analyzer/src/config.rs

@@ -112,7 +112,7 @@ config_data! {
         cargo_buildScripts_overrideCommand: Option<Vec<String>> = "null",
         /// Rerun proc-macros building/build-scripts running when proc-macro
         /// or build-script sources change and are saved.
-        cargo_buildScripts_rebuildOnSave: bool = "false",
+        cargo_buildScripts_rebuildOnSave: bool = "true",
         /// Use `RUSTC_WRAPPER=rust-analyzer` when running build scripts to
         /// avoid checking unnecessary things.
         cargo_buildScripts_useRustcWrapper: bool = "true",

crates/rust-analyzer/src/global_state.rs

@@ -9,7 +9,7 @@ use crossbeam_channel::{unbounded, Receiver, Sender};
 use flycheck::FlycheckHandle;
 use hir::Change;
 use ide::{Analysis, AnalysisHost, Cancellable, FileId};
-use ide_db::base_db::{CrateId, FileLoader, ProcMacroPaths, SourceDatabase};
+use ide_db::base_db::{CrateId, ProcMacroPaths};
 use load_cargo::SourceRootConfig;
 use lsp_types::{SemanticTokens, Url};
 use nohash_hasher::IntMap;
@@ -74,8 +74,8 @@ pub(crate) struct GlobalState {
     pub(crate) last_reported_status: Option<lsp_ext::ServerStatusParams>,
 
     // proc macros
-    pub(crate) proc_macro_changed: bool,
     pub(crate) proc_macro_clients: Arc<[anyhow::Result<ProcMacroServer>]>,
+    pub(crate) build_deps_changed: bool,
 
     // Flycheck
     pub(crate) flycheck: Arc<[FlycheckHandle]>,
@@ -203,9 +203,10 @@ impl GlobalState {
             source_root_config: SourceRootConfig::default(),
             config_errors: Default::default(),
 
-            proc_macro_changed: false,
             proc_macro_clients: Arc::from_iter([]),
 
+            build_deps_changed: false,
+
             flycheck: Arc::from_iter([]),
             flycheck_sender,
             flycheck_receiver,
@@ -300,12 +301,19 @@ impl GlobalState {
                 if let Some(path) = vfs_path.as_path() {
                     let path = path.to_path_buf();
                     if reload::should_refresh_for_change(&path, file.kind()) {
-                        workspace_structure_change = Some((path.clone(), false));
+                        workspace_structure_change = Some((
+                            path.clone(),
+                            false,
+                            AsRef::<std::path::Path>::as_ref(&path).ends_with("build.rs"),
+                        ));
                     }
                     if file.is_created_or_deleted() {
                         has_structure_changes = true;
-                        workspace_structure_change =
+                        workspace_structure_change = Some((
-                            Some((path, self.crate_graph_file_dependencies.contains(vfs_path)));
+                            path,
+                            self.crate_graph_file_dependencies.contains(vfs_path),
+                            false,
+                        ));
                     } else if path.extension() == Some("rs".as_ref()) {
                         modified_rust_files.push(file.file_id);
                     }
@@ -346,23 +354,28 @@ impl GlobalState {
         };
 
         self.analysis_host.apply_change(change);
+
         {
-            let raw_database = self.analysis_host.raw_database();
+            if !matches!(&workspace_structure_change, Some((.., true))) {
+                _ = self
+                    .deferred_task_queue
+                    .sender
+                    .send(crate::main_loop::QueuedTask::CheckProcMacroSources(modified_rust_files));
+            }
             // FIXME: ideally we should only trigger a workspace fetch for non-library changes
             // but something's going wrong with the source root business when we add a new local
             // crate see https://github.com/rust-lang/rust-analyzer/issues/13029
-            if let Some((path, force_crate_graph_reload)) = workspace_structure_change {
+            if let Some((path, force_crate_graph_reload, build_scripts_touched)) =
+                workspace_structure_change
+            {
                 self.fetch_workspaces_queue.request_op(
                     format!("workspace vfs file change: {path}"),
                     force_crate_graph_reload,
                 );
+                if build_scripts_touched {
+                    self.fetch_build_data_queue.request_op(format!("build.rs changed: {path}"), ());
+                }
             }
-            self.proc_macro_changed = modified_rust_files.into_iter().any(|file_id| {
-                let crates = raw_database.relevant_crates(file_id);
-                let crate_graph = raw_database.crate_graph();
-
-                crates.iter().any(|&krate| crate_graph[krate].is_proc_macro)
-            });
         }
 
         true

crates/rust-analyzer/src/handlers/notification.rs

@@ -145,11 +145,11 @@ pub(crate) fn handle_did_save_text_document(
     state: &mut GlobalState,
     params: DidSaveTextDocumentParams,
 ) -> anyhow::Result<()> {
-    if state.config.script_rebuild_on_save() && state.proc_macro_changed {
+    if state.config.script_rebuild_on_save() && state.build_deps_changed {
-        // reset the flag
+        state.build_deps_changed = false;
-        state.proc_macro_changed = false;
+        state
-        // rebuild the proc macros
+            .fetch_build_data_queue
-        state.fetch_build_data_queue.request_op("ScriptRebuildOnSave".to_owned(), ());
+            .request_op("build_deps_changed - save notification".to_owned(), ());
     }
 
     if let Ok(vfs_path) = from_proto::vfs_path(&params.text_document.uri) {
@@ -158,7 +158,7 @@ pub(crate) fn handle_did_save_text_document(
             if reload::should_refresh_for_change(abs_path, ChangeKind::Modify) {
                 state
                     .fetch_workspaces_queue
-                    .request_op(format!("DidSaveTextDocument {abs_path}"), false);
+                    .request_op(format!("workspace vfs file change saved {abs_path}"), false);
             }
         }
 

crates/rust-analyzer/src/handlers/request.rs

@@ -52,7 +52,7 @@ use crate::{
 
 pub(crate) fn handle_workspace_reload(state: &mut GlobalState, _: ()) -> anyhow::Result<()> {
     state.proc_macro_clients = Arc::from_iter([]);
-    state.proc_macro_changed = false;
+    state.build_deps_changed = false;
 
     state.fetch_workspaces_queue.request_op("reload workspace request".to_owned(), false);
     Ok(())
@@ -60,7 +60,7 @@ pub(crate) fn handle_workspace_reload(state: &mut GlobalState, _: ()) -> anyhow:
 
 pub(crate) fn handle_proc_macros_rebuild(state: &mut GlobalState, _: ()) -> anyhow::Result<()> {
     state.proc_macro_clients = Arc::from_iter([]);
-    state.proc_macro_changed = false;
+    state.build_deps_changed = false;
 
     state.fetch_build_data_queue.request_op("rebuild proc macros request".to_owned(), ());
     Ok(())

crates/rust-analyzer/src/main_loop.rs

@@ -8,11 +8,10 @@ use std::{
 
 use always_assert::always;
 use crossbeam_channel::{select, Receiver};
-use ide_db::base_db::{SourceDatabaseExt, VfsPath};
+use ide_db::base_db::{SourceDatabase, SourceDatabaseExt, VfsPath};
 use lsp_server::{Connection, Notification, Request};
 use lsp_types::notification::Notification as _;
 use stdx::thread::ThreadIntent;
-use triomphe::Arc;
 use vfs::FileId;
 
 use crate::{
@@ -76,6 +75,7 @@ impl fmt::Display for Event {
 #[derive(Debug)]
 pub(crate) enum QueuedTask {
     CheckIfIndexed(lsp_types::Url),
+    CheckProcMacroSources(Vec<FileId>),
 }
 
 #[derive(Debug)]
@@ -88,6 +88,7 @@ pub(crate) enum Task {
     FetchWorkspace(ProjectWorkspaceProgress),
     FetchBuildData(BuildDataProgress),
     LoadProcMacros(ProcMacroProgress),
+    BuildDepsHaveChanged,
 }
 
 #[derive(Debug)]
@@ -357,9 +358,7 @@ impl GlobalState {
                 }
 
                 // Refresh inlay hints if the client supports it.
-                if (self.send_hint_refresh_query || self.proc_macro_changed)
+                if self.send_hint_refresh_query && self.config.inlay_hints_refresh() {
-                    && self.config.inlay_hints_refresh()
-                {
                     self.send_request::<lsp_types::request::InlayHintRefreshRequest>((), |_, _| ());
                     self.send_hint_refresh_query = false;
                 }
@@ -554,16 +553,7 @@ impl GlobalState {
                         if let Err(e) = self.fetch_workspace_error() {
                             tracing::error!("FetchWorkspaceError:\n{e}");
                         }
-
-                        let old = Arc::clone(&self.workspaces);
                         self.switch_workspaces("fetched workspace".to_owned());
-                        let workspaces_updated = !Arc::ptr_eq(&old, &self.workspaces);
-
-                        if self.config.run_build_scripts() && workspaces_updated {
-                            self.fetch_build_data_queue
-                                .request_op("workspace updated".to_owned(), ());
-                        }
-
                         (Progress::End, None)
                     }
                 };
@@ -607,6 +597,7 @@ impl GlobalState {
                     self.report_progress("Loading", state, msg, None, None);
                 }
             }
+            Task::BuildDepsHaveChanged => self.build_deps_changed = true,
         }
     }
 
@@ -685,6 +676,25 @@ impl GlobalState {
                     }
                 });
             }
+            QueuedTask::CheckProcMacroSources(modified_rust_files) => {
+                let crate_graph = self.analysis_host.raw_database().crate_graph();
+                let snap = self.snapshot();
+                self.task_pool.handle.spawn_with_sender(stdx::thread::ThreadIntent::Worker, {
+                    move |sender| {
+                        if modified_rust_files.into_iter().any(|file_id| {
+                            // FIXME: Check whether these files could be build script related
+                            match snap.analysis.crates_for(file_id) {
+                                Ok(crates) => {
+                                    crates.iter().any(|&krate| crate_graph[krate].is_proc_macro)
+                                }
+                                _ => false,
+                            }
+                        }) {
+                            sender.send(Task::BuildDepsHaveChanged).unwrap();
+                        }
+                    }
+                });
+            }
         }
     }
 

crates/rust-analyzer/src/reload.rs

@@ -83,7 +83,7 @@ impl GlobalState {
         }
         if self.config.linked_or_discovered_projects() != old_config.linked_or_discovered_projects()
         {
-            self.fetch_workspaces_queue.request_op("linked projects changed".to_owned(), false)
+            self.fetch_workspaces_queue.request_op("discovered projects changed".to_owned(), false)
         } else if self.config.flycheck() != old_config.flycheck() {
             self.reload_flycheck();
         }
@@ -106,9 +106,11 @@ impl GlobalState {
         };
         let mut message = String::new();
 
-        if self.proc_macro_changed {
+        if self.build_deps_changed {
             status.health = lsp_ext::Health::Warning;
-            message.push_str("Proc-macros have changed and need to be rebuilt.\n\n");
+            message.push_str(
+                "Proc-macros and/or build scripts have changed and need to be rebuilt.\n\n",
+            );
         }
         if self.fetch_build_data_error().is_err() {
             status.health = lsp_ext::Health::Warning;
@@ -408,6 +410,10 @@ impl GlobalState {
                 if *force_reload_crate_graph {
                     self.recreate_crate_graph(cause);
                 }
+                if self.build_deps_changed && self.config.run_build_scripts() {
+                    self.build_deps_changed = false;
+                    self.fetch_build_data_queue.request_op("build_deps_changed".to_owned(), ());
+                }
                 // Current build scripts do not match the version of the active
                 // workspace, so there's nothing for us to update.
                 return;
@@ -419,6 +425,11 @@ impl GlobalState {
             // we don't care about build-script results, they are stale.
             // FIXME: can we abort the build scripts here?
             self.workspaces = Arc::new(workspaces);
+
+            if self.config.run_build_scripts() {
+                self.build_deps_changed = false;
+                self.fetch_build_data_queue.request_op("workspace updated".to_owned(), ());
+            }
         }
 
         if let FilesWatcher::Client = self.config.files().watcher {

docs/user/generated_config.adoc

@@ -71,7 +71,7 @@ cargo check --quiet --workspace --message-format=json --all-targets
 ```
 .
 --
-[[rust-analyzer.cargo.buildScripts.rebuildOnSave]]rust-analyzer.cargo.buildScripts.rebuildOnSave (default: `false`)::
+[[rust-analyzer.cargo.buildScripts.rebuildOnSave]]rust-analyzer.cargo.buildScripts.rebuildOnSave (default: `true`)::
 +
 --
 Rerun proc-macros building/build-scripts running when proc-macro

editors/code/package.json

@@ -590,7 +590,7 @@
                 },
                 "rust-analyzer.cargo.buildScripts.rebuildOnSave": {
                     "markdownDescription": "Rerun proc-macros building/build-scripts running when proc-macro\nor build-script sources change and are saved.",
-                    "default": false,
+                    "default": true,
                     "type": "boolean"
                 },
                 "rust-analyzer.cargo.buildScripts.useRustcWrapper": {