fix: Fix build scripts not being rebuilt in some occasions

Veykril · Veykril · commit 39637d77d3dc · 2024-02-09T11:59:59.000+01:00
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
@@ -98,7 +98,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
                         };
                         (
                             name.as_name(),
-                            CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
+                            CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId::new(
                                 idx as u32,
                             )),
                         )
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
@@ -224,8 +224,8 @@ pub enum MacroCallKind {
     },
     Attr {
         ast_id: AstId<ast::Item>,
-        // FIXME: This is being interned, subtrees can vary quickly differ just slightly causing
-        // leakage problems here
+        // FIXME: This shouldn't be here, we can derive this from `invoc_attr_index`
+        // but we need to fix the `cfg_attr` handling first.
         attr_args: Option<Arc<tt::Subtree>>,
         /// Syntactical index of the invoking `#[attribute]`.
         ///
diff --git a/crates/hir-expand/src/proc_macro.rs b/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 {
diff --git a/crates/ide/src/parent_module.rs b/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()
diff --git a/crates/project-model/src/build_scripts.rs b/crates/project-model/src/build_scripts.rs
@@ -23,7 +23,7 @@ use serde::Deserialize;
 
 use crate::{
     cfg_flag::CfgFlag, utf8_stdout, CargoConfig, CargoFeatures, CargoWorkspace, InvocationLocation,
-    InvocationStrategy, Package,
+    InvocationStrategy, Package, TargetKind,
 };
 
 #[derive(Debug, Default, Clone, PartialEq, Eq)]
@@ -458,7 +458,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)
diff --git a/crates/project-model/src/cargo_workspace.rs b/crates/project-model/src/cargo_workspace.rs
@@ -188,8 +188,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>,
 }
@@ -198,7 +196,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,
@@ -215,8 +216,8 @@ impl TargetKind {
                 "bench" => TargetKind::Bench,
                 "example" => TargetKind::Example,
                 "custom-build" => TargetKind::BuildScript,
-                "proc-macro" => TargetKind::Lib,
-                _ if kind.contains("lib") => TargetKind::Lib,
+                "proc-macro" => TargetKind::Lib { is_proc_macro: true },
+                _ if kind.contains("lib") => TargetKind::Lib { is_proc_macro: false },
                 _ => continue,
             };
         }
@@ -368,7 +369,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);
diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs
@@ -586,7 +586,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));
@@ -949,17 +949,17 @@ 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.
                 // In fact, they can break quite badly if multiple client workspaces get merged:
                 // 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];
 
-            if kind == TargetKind::Lib
+            if matches!(kind, TargetKind::Lib { .. })
                 && sysroot.map_or(false, |sysroot| root.starts_with(sysroot.src_root()))
             {
                 if let Some(&(_, crate_id, _)) =
@@ -984,19 +984,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));
@@ -1194,9 +1199,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,
@@ -1206,7 +1211,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(),
@@ -1215,12 +1220,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(
-                            crate_graph,
-                            crate_id,
-                            proc_macro,
-                            rustc_workspace[tgt].is_proc_macro,
-                        );
+                        add_proc_macro_dep(crate_graph, crate_id, proc_macro, is_proc_macro);
                     }
                     rustc_pkg_crates.entry(pkg).or_insert_with(Vec::new).push(crate_id);
                 }
@@ -1282,7 +1282,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>,
@@ -1332,7 +1332,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 {
@@ -1343,7 +1343,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())),
diff --git a/crates/rust-analyzer/src/cargo_target_spec.rs b/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 => (),
diff --git a/crates/rust-analyzer/src/config.rs b/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",
diff --git a/crates/rust-analyzer/src/global_state.rs b/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) proc_macros_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([]),
 
+            proc_macros_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 =
-                            Some((path, self.crate_graph_file_dependencies.contains(vfs_path)));
+                        workspace_structure_change = Some((
+                            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,27 @@ impl GlobalState {
         };
 
         self.analysis_host.apply_change(change);
+
         {
-            let raw_database = self.analysis_host.raw_database();
             // 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)
-            });
+            _ = self
+                .deferred_task_queue
+                .sender
+                .send(crate::main_loop::QueuedTask::CheckProcMacroSources(modified_rust_files));
         }
 
         true
diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs
@@ -145,11 +145,9 @@ 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 {
-        // reset the flag
-        state.proc_macro_changed = false;
-        // rebuild the proc macros
-        state.fetch_build_data_queue.request_op("ScriptRebuildOnSave".to_owned(), ());
+    if state.config.script_rebuild_on_save() && state.proc_macros_changed {
+        state.proc_macros_changed = false;
+        state.fetch_build_data_queue.request_op("proc macros changed".to_owned(), ());
     }
 
     if let Ok(vfs_path) = from_proto::vfs_path(&params.text_document.uri) {
@@ -158,7 +156,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);
             }
         }
 
@@ -236,7 +234,7 @@ pub(crate) fn handle_did_change_workspace_folders(
 
     if !config.has_linked_projects() && config.detached_files().is_empty() {
         config.rediscover_workspaces();
-        state.fetch_workspaces_queue.request_op("client workspaces changed".to_string(), false)
+        state.fetch_workspaces_queue.request_op("workspaces folders changed".to_string(), false)
     }
 
     Ok(())
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs
@@ -52,15 +52,15 @@ 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.proc_macros_changed = false;
 
     state.fetch_workspaces_queue.request_op("reload workspace request".to_string(), false);
     Ok(())
 }
 
 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.proc_macros_changed = false;
 
     state.fetch_build_data_queue.request_op("rebuild proc macros request".to_string(), ());
     Ok(())
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs
diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc
diff --git a/editors/code/package.json b/editors/code/package.json

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI`
`98`	`98`	`};`
`99`	`99`	`(`
`100`	`100`	`name.as_name(),`
`101`		`- CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(`
	`101`	`+ CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId::new(`
`102`	`102`	`idx as u32,`
`103`	`103`	`)),`
`104`	`104`	`)`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ pub(crate) fn parent_module(db: &RootDatabase, position: FilePosition) -> Vec<Na`
`54`	`54`	`}`
`55`	`55`	`}`
`56`	`56`
`57`		-/// Returns `Vec` for the same reason as `parent_module`
	`57`	+/// This returns `Vec` because a module may be included from several places.
`58`	`58`	`pub(crate) fn crates_for(db: &RootDatabase, file_id: FileId) -> Vec<CrateId> {`
`59`	`59`	`db.relevant_crates(file_id)`
`60`	`60`	`.iter()`
Original file line number	Diff line number	Diff line change
`@@ -174,7 +174,7 @@ impl CargoTargetSpec {`
`174`	`174`	`buf.push("--example".to_owned());`
`175`	`175`	`buf.push(self.target);`
`176`	`176`	`}`
`177`		`- TargetKind::Lib => {`
	`177`	`+ TargetKind::Lib { is_proc_macro: _ } => {`
`178`	`178`	`buf.push("--lib".to_owned());`
`179`	`179`	`}`
`180`	`180`	`TargetKind::Other \| TargetKind::BuildScript => (),`