Repository: rust-lang/rust-analyzer
Pull request #16616 (2024-02-20 14:56:48)

fix: server hanging up on build script task
This should fix https://github.com/rust-lang/rust-analyzer/issues/16614, can't say for certain since it might be not 100% reproducible... We really need to replace the current workspace fetching logic, it is completely unreadable and incredibly difficult to follow. I don't really understand how the server even got to hang here honestly (I would expect it to loop re-fetching build scripts, but not hang).


Patch:
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 2d8811cf5e..beaa6dd4d6 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -2653,37 +2653,6 @@ impl ItemInNs {
     }
 }
 
-/// Invariant: `inner.as_extern_assoc_item(db).is_some()`
-/// We do not actively enforce this invariant.
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub enum ExternAssocItem {
-    Function(Function),
-    Static(Static),
-    TypeAlias(TypeAlias),
-}
-
-pub trait AsExternAssocItem {
-    fn as_extern_assoc_item(self, db: &dyn HirDatabase) -> Option<ExternAssocItem>;
-}
-
-impl AsExternAssocItem for Function {
-    fn as_extern_assoc_item(self, db: &dyn HirDatabase) -> Option<ExternAssocItem> {
-        as_extern_assoc_item(db, ExternAssocItem::Function, self.id)
-    }
-}
-
-impl AsExternAssocItem for Static {
-    fn as_extern_assoc_item(self, db: &dyn HirDatabase) -> Option<ExternAssocItem> {
-        as_extern_assoc_item(db, ExternAssocItem::Static, self.id)
-    }
-}
-
-impl AsExternAssocItem for TypeAlias {
-    fn as_extern_assoc_item(self, db: &dyn HirDatabase) -> Option<ExternAssocItem> {
-        as_extern_assoc_item(db, ExternAssocItem::TypeAlias, self.id)
-    }
-}
-
 /// Invariant: `inner.as_assoc_item(db).is_some()`
 /// We do not actively enforce this invariant.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
@@ -2758,63 +2727,6 @@ where
     }
 }
 
-fn as_extern_assoc_item<'db, ID, DEF, LOC>(
-    db: &(dyn HirDatabase + 'db),
-    ctor: impl FnOnce(DEF) -> ExternAssocItem,
-    id: ID,
-) -> Option<ExternAssocItem>
-where
-    ID: Lookup<Database<'db> = dyn DefDatabase + 'db, Data = AssocItemLoc<LOC>>,
-    DEF: From<ID>,
-    LOC: ItemTreeNode,
-{
-    match id.lookup(db.upcast()).container {
-        ItemContainerId::ExternBlockId(_) => Some(ctor(DEF::from(id))),
-        ItemContainerId::TraitId(_) | ItemContainerId::ImplId(_) | ItemContainerId::ModuleId(_) => {
-            None
-        }
-    }
-}
-
-impl ExternAssocItem {
-    pub fn name(self, db: &dyn HirDatabase) -> Name {
-        match self {
-            Self::Function(it) => it.name(db),
-            Self::Static(it) => it.name(db),
-            Self::TypeAlias(it) => it.name(db),
-        }
-    }
-
-    pub fn module(self, db: &dyn HirDatabase) -> Module {
-        match self {
-            Self::Function(f) => f.module(db),
-            Self::Static(c) => c.module(db),
-            Self::TypeAlias(t) => t.module(db),
-        }
-    }
-
-    pub fn as_function(self) -> Option<Function> {
-        match self {
-            Self::Function(v) => Some(v),
-            _ => None,
-        }
-    }
-
-    pub fn as_static(self) -> Option<Static> {
-        match self {
-            Self::Static(v) => Some(v),
-            _ => None,
-        }
-    }
-
-    pub fn as_type_alias(self) -> Option<TypeAlias> {
-        match self {
-            Self::TypeAlias(v) => Some(v),
-            _ => None,
-        }
-    }
-}
-
 impl AssocItem {
     pub fn name(self, db: &dyn HirDatabase) -> Option<Name> {
         match self {
diff --git a/crates/ide-db/src/defs.rs b/crates/ide-db/src/defs.rs
index 1b6ff8bad5..d95d94ec72 100644
--- a/crates/ide-db/src/defs.rs
+++ b/crates/ide-db/src/defs.rs
@@ -8,11 +8,11 @@
 use arrayvec::ArrayVec;
 use either::Either;
 use hir::{
-    Adt, AsAssocItem, AsExternAssocItem, AssocItem, AttributeTemplate, BuiltinAttr, BuiltinType,
-    Const, Crate, DefWithBody, DeriveHelper, DocLinkDef, ExternAssocItem, ExternCrateDecl, Field,
-    Function, GenericParam, HasVisibility, HirDisplay, Impl, Label, Local, Macro, Module,
-    ModuleDef, Name, PathResolution, Semantics, Static, ToolModule, Trait, TraitAlias, TupleField,
-    TypeAlias, Variant, VariantDef, Visibility,
+    Adt, AsAssocItem, AssocItem, AttributeTemplate, BuiltinAttr, BuiltinType, Const, Crate,
+    DefWithBody, DeriveHelper, DocLinkDef, ExternCrateDecl, Field, Function, GenericParam,
+    HasVisibility, HirDisplay, Impl, Label, Local, Macro, Module, ModuleDef, Name, PathResolution,
+    Semantics, Static, ToolModule, Trait, TraitAlias, TupleField, TypeAlias, Variant, VariantDef,
+    Visibility,
 };
 use stdx::{format_to, impl_from};
 use syntax::{
@@ -213,8 +213,8 @@ impl Definition {
         })
     }
 
-    pub fn label(&self, db: &RootDatabase) -> String {
-        match *self {
+    pub fn label(&self, db: &RootDatabase) -> Option<String> {
+        let label = match *self {
             Definition::Macro(it) => it.display(db).to_string(),
             Definition::Field(it) => it.display(db).to_string(),
             Definition::TupleField(it) => it.display(db).to_string(),
@@ -241,11 +241,7 @@ impl Definition {
                 }
             }
             Definition::SelfType(impl_def) => {
-                let self_ty = &impl_def.self_ty(db);
-                match self_ty.as_adt() {
-                    Some(it) => it.display(db).to_string(),
-                    None => self_ty.display(db).to_string(),
-                }
+                impl_def.self_ty(db).as_adt().and_then(|adt| Definition::Adt(adt).label(db))?
             }
             Definition::GenericParam(it) => it.display(db).to_string(),
             Definition::Label(it) => it.name(db).display(db).to_string(),
@@ -253,7 +249,8 @@ impl Definition {
             Definition::BuiltinAttr(it) => format!("#[{}]", it.name(db)),
             Definition::ToolModule(it) => it.name(db).to_string(),
             Definition::DeriveHelper(it) => format!("derive_helper {}", it.name(db).display(db)),
-        }
+        };
+        Some(label)
     }
 }
 
@@ -742,17 +739,6 @@ impl AsAssocItem for Definition {
     }
 }
 
-impl AsExternAssocItem for Definition {
-    fn as_extern_assoc_item(self, db: &dyn hir::db::HirDatabase) -> Option<ExternAssocItem> {
-        match self {
-            Definition::Function(it) => it.as_extern_assoc_item(db),
-            Definition::Static(it) => it.as_extern_assoc_item(db),
-            Definition::TypeAlias(it) => it.as_extern_assoc_item(db),
-            _ => None,
-        }
-    }
-}
-
 impl From<AssocItem> for Definition {
     fn from(assoc_item: AssocItem) -> Self {
         match assoc_item {
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index 4a7350feb3..19b181ae3b 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -147,7 +147,7 @@ fn hover_simple(
     if let Some(doc_comment) = token_as_doc_comment(&original_token) {
         cov_mark::hit!(no_highlight_on_comment_hover);
         return doc_comment.get_definition_with_descend_at(sema, offset, |def, node, range| {
-            let res = hover_for_definition(sema, file_id, def, &node, config);
+            let res = hover_for_definition(sema, file_id, def, &node, config)?;
             Some(RangeInfo::new(range, res))
         });
     }
@@ -161,7 +161,7 @@ fn hover_simple(
             Definition::from(resolution?),
             &original_token.parent()?,
             config,
-        );
+        )?;
         return Some(RangeInfo::new(range, res));
     }
 
@@ -215,7 +215,7 @@ fn hover_simple(
                 })
                 .flatten()
                 .unique_by(|&(def, _)| def)
-                .map(|(def, node)| hover_for_definition(sema, file_id, def, &node, config))
+                .filter_map(|(def, node)| hover_for_definition(sema, file_id, def, &node, config))
                 .reduce(|mut acc: HoverResult, HoverResult { markup, actions }| {
                     acc.actions.extend(actions);
                     acc.markup = Markup::from(format!("{}\n---\n{markup}", acc.markup));
@@ -373,9 +373,9 @@ pub(crate) fn hover_for_definition(
     def: Definition,
     scope_node: &SyntaxNode,
     config: &HoverConfig,
-) -> HoverResult {
+) -> Option<HoverResult> {
     let famous_defs = match &def {
-        Definition::BuiltinType(_) => sema.scope(scope_node).map(|it| FamousDefs(sema, it.krate())),
+        Definition::BuiltinType(_) => Some(FamousDefs(sema, sema.scope(scope_node)?.krate())),
         _ => None,
     };
 
@@ -396,19 +396,20 @@ pub(crate) fn hover_for_definition(
     };
     let notable_traits = def_ty.map(|ty| notable_traits(db, &ty)).unwrap_or_default();
 
-    let markup = render::definition(sema.db, def, famous_defs.as_ref(), &notable_traits, config);
-    HoverResult {
-        markup: render::process_markup(sema.db, def, &markup, config),
-        actions: [
-            show_implementations_action(sema.db, def),
-            show_fn_references_action(sema.db, def),
-            runnable_action(sema, def, file_id),
-            goto_type_action_for_def(sema.db, def, &notable_traits),
-        ]
-        .into_iter()
-        .flatten()
-        .collect(),
-    }
+    render::definition(sema.db, def, famous_defs.as_ref(), &notable_traits, config).map(|markup| {
+        HoverResult {
+            markup: render::process_markup(sema.db, def, &markup, config),
+            actions: [
+                show_implementations_action(sema.db, def),
+                show_fn_references_action(sema.db, def),
+                runnable_action(sema, def, file_id),
+                goto_type_action_for_def(sema.db, def, &notable_traits),
+            ]
+            .into_iter()
+            .flatten()
+            .collect(),
+        }
+    })
 }
 
 fn notable_traits(
diff --git a/crates/ide/src/hover/render.rs b/crates/ide/src/hover/render.rs
index 563e78253a..eff055c959 100644
--- a/crates/ide/src/hover/render.rs
+++ b/crates/ide/src/hover/render.rs
@@ -3,8 +3,8 @@ use std::{mem, ops::Not};
 
 use either::Either;
 use hir::{
-    Adt, AsAssocItem, AsExternAssocItem, CaptureKind, HasCrate, HasSource, HirDisplay, Layout,
-    LayoutError, Name, Semantics, Trait, Type, TypeInfo,
+    Adt, AsAssocItem, CaptureKind, HasCrate, HasSource, HirDisplay, Layout, LayoutError, Name,
+    Semantics, Trait, Type, TypeInfo,
 };
 use ide_db::{
     base_db::SourceDatabase,
@@ -264,7 +264,7 @@ pub(super) fn keyword(
     let markup = process_markup(
         sema.db,
         Definition::Module(doc_owner),
-        &markup(Some(docs.into()), description, None),
+        &markup(Some(docs.into()), description, None)?,
         config,
     );
     Some(HoverResult { markup, actions })
@@ -369,20 +369,12 @@ fn definition_owner_name(db: &RootDatabase, def: &Definition) -> Option<String>
     match def {
         Definition::Field(f) => Some(f.parent_def(db).name(db)),
         Definition::Local(l) => l.parent(db).name(db),
+        Definition::Function(f) => match f.as_assoc_item(db)?.container(db) {
+            hir::AssocItemContainer::Trait(t) => Some(t.name(db)),
+            hir::AssocItemContainer::Impl(i) => i.self_ty(db).as_adt().map(|adt| adt.name(db)),
+        },
         Definition::Variant(e) => Some(e.parent_enum(db).name(db)),
-
-        d => {
-            if let Some(assoc_item) = d.as_assoc_item(db) {
-                match assoc_item.container(db) {
-                    hir::AssocItemContainer::Trait(t) => Some(t.name(db)),
-                    hir::AssocItemContainer::Impl(i) => {
-                        i.self_ty(db).as_adt().map(|adt| adt.name(db))
-                    }
-                }
-            } else {
-                return d.as_extern_assoc_item(db).map(|_| "<extern>".to_owned());
-            }
-        }
+        _ => None,
     }
     .map(|name| name.display(db).to_string())
 }
@@ -404,11 +396,11 @@ pub(super) fn definition(
     famous_defs: Option<&FamousDefs<'_, '_>>,
     notable_traits: &[(Trait, Vec<(Option<Type>, Name)>)],
     config: &HoverConfig,
-) -> Markup {
+) -> Option<Markup> {
     let mod_path = definition_mod_path(db, &def);
-    let label = def.label(db);
+    let label = def.label(db)?;
     let docs = def.docs(db, famous_defs);
-    let value = (|| match def {
+    let value = match def {
         Definition::Variant(it) => {
             if !it.parent_enum(db).is_data_carrying(db) {
                 match it.eval(db) {
@@ -444,7 +436,7 @@ pub(super) fn definition(
             Some(body.to_string())
         }
         _ => None,
-    })();
+    };
 
     let layout_info = match def {
         Definition::Field(it) => render_memory_layout(
@@ -691,7 +683,7 @@ fn definition_mod_path(db: &RootDatabase, def: &Definition) -> Option<String> {
     def.module(db).map(|module| path(db, module, definition_owner_name(db, def)))
 }
 
-fn markup(docs: Option<String>, desc: String, mod_path: Option<String>) -> Markup {
+fn markup(docs: Option<String>, desc: String, mod_path: Option<String>) -> Option<Markup> {
     let mut buf = String::new();
 
     if let Some(mod_path) = mod_path {
@@ -704,7 +696,7 @@ fn markup(docs: Option<String>, desc: String, mod_path: Option<String>) -> Marku
     if let Some(doc) = docs {
         format_to!(buf, "\n___\n\n{}", doc);
     }
-    buf.into()
+    Some(buf.into())
 }
 
 fn find_std_module(famous_defs: &FamousDefs<'_, '_>, name: &str) -> Option<hir::Module> {
diff --git a/crates/ide/src/static_index.rs b/crates/ide/src/static_index.rs
index 2929a7522e..5feaf21aa9 100644
--- a/crates/ide/src/static_index.rs
+++ b/crates/ide/src/static_index.rs
@@ -186,7 +186,7 @@ impl StaticIndex<'_> {
             } else {
                 let it = self.tokens.insert(TokenStaticData {
                     documentation: documentation_for_definition(&sema, def, &node),
-                    hover: Some(hover_for_definition(&sema, file_id, def, &node, &hover_config)),
+                    hover: hover_for_definition(&sema, file_id, def, &node, &hover_config),
                     definition: def.try_to_nav(self.db).map(UpmappingResult::call_site).map(|it| {
                         FileRange { file_id: it.file_id, range: it.focus_or_full_range() }
                     }),
@@ -196,7 +196,7 @@ impl StaticIndex<'_> {
                     enclosing_moniker: current_crate
                         .zip(def.enclosing_definition(self.db))
                         .and_then(|(cc, enclosing_def)| def_to_moniker(self.db, enclosing_def, cc)),
-                    signature: Some(def.label(self.db)),
+                    signature: def.label(self.db),
                     kind: def_to_kind(self.db, def),
                 });
                 self.def_map.insert(def, it);
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 293807a383..b2d507491b 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -301,19 +301,12 @@ 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,
-                            AsRef::<std::path::Path>::as_ref(&path).ends_with("build.rs"),
-                        ));
+                        workspace_structure_change = Some((path.clone(), false));
                     }
                     if file.is_created_or_deleted() {
                         has_structure_changes = true;
-                        workspace_structure_change = Some((
-                            path,
-                            self.crate_graph_file_dependencies.contains(vfs_path),
-                            false,
-                        ));
+                        workspace_structure_change =
+                            Some((path, self.crate_graph_file_dependencies.contains(vfs_path)));
                     } else if path.extension() == Some("rs".as_ref()) {
                         modified_rust_files.push(file.file_id);
                     }
@@ -365,16 +358,11 @@ impl GlobalState {
             // 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, build_scripts_touched)) =
-                workspace_structure_change
-            {
+            if let Some((path, force_crate_graph_reload)) = 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}"), ());
-                }
             }
         }
 
diff --git a/crates/rust-analyzer/src/lsp/utils.rs b/crates/rust-analyzer/src/lsp/utils.rs
index 10335cb145..800c0eee53 100644
--- a/crates/rust-analyzer/src/lsp/utils.rs
+++ b/crates/rust-analyzer/src/lsp/utils.rs
@@ -134,6 +134,7 @@ impl GlobalState {
         let token = lsp_types::ProgressToken::String(
             cancel_token.unwrap_or_else(|| format!("rustAnalyzer/{title}")),
         );
+        tracing::debug!(?token, ?state, "report_progress {message:?}");
         let work_done_progress = match state {
             Progress::Begin => {
                 self.send_request::<lsp_types::request::WorkDoneProgressCreate>(
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
index 5895459d1f..00494ca5ba 100644
--- a/crates/rust-analyzer/src/reload.rs
+++ b/crates/rust-analyzer/src/reload.rs
@@ -411,10 +411,7 @@ 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;
@@ -424,7 +421,7 @@ impl GlobalState {
 
             // Here, we completely changed the workspace (Cargo.toml edit), so
             // we don't care about build-script results, they are stale.
-            // FIXME: can we abort the build scripts here?
+            // FIXME: can we abort the build scripts here if they are already running?
             self.workspaces = Arc::new(workspaces);
 
             if self.config.run_build_scripts() {


Test patch:
diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs
index ead4f91595..69ddc1e45e 100644
--- a/crates/ide/src/hover/tests.rs
+++ b/crates/ide/src/hover/tests.rs
@@ -1202,7 +1202,7 @@ fn main() {
             *C*
 
             ```rust
-            test::X
+            test
             ```
 
             ```rust
@@ -1279,11 +1279,11 @@ impl Thing {
     );
     check(
         r#"
-enum Thing { A }
-impl Thing {
-    pub fn thing(a: Self$0) {}
-}
-"#,
+        enum Thing { A }
+        impl Thing {
+            pub fn thing(a: Self$0) {}
+        }
+        "#,
         expect![[r#"
                 *Self*
 
@@ -1298,42 +1298,6 @@ impl Thing {
                 ```
             "#]],
     );
-    check(
-        r#"
-impl usize {
-    pub fn thing(a: Self$0) {}
-}
-"#,
-        expect![[r#"
-            *Self*
-
-            ```rust
-            test
-            ```
-
-            ```rust
-            usize
-            ```
-        "#]],
-    );
-    check(
-        r#"
-impl fn() -> usize {
-    pub fn thing(a: Self$0) {}
-}
-"#,
-        expect![[r#"
-            *Self*
-
-            ```rust
-            test
-            ```
-
-            ```rust
-            fn() -> usize
-            ```
-        "#]],
-    );
 }
 
 #[test]
@@ -2277,7 +2241,7 @@ fn main() { let foo_test = unsafe { fo$0o(1, 2, 3); } }
             *foo*
 
             ```rust
-            test::<extern>
+            test
             ```
 
             ```rust
@@ -4266,7 +4230,7 @@ fn main() {
             *B*
 
             ```rust
-            test::T
+            test
             ```
 
             ```rust
@@ -4295,7 +4259,7 @@ fn main() {
             *B*
 
             ```rust
-            test::T
+            test
             ```
 
             ```rust
@@ -4327,7 +4291,7 @@ fn main() {
             *B*
 
             ```rust
-            test::T
+            test
             ```
 
             ```rust
@@ -4919,7 +4883,7 @@ fn test() {
             *FOO*
 
             ```rust
-            test::S
+            test
             ```
 
             ```rust
@@ -5284,7 +5248,7 @@ impl T1 for Foo {
             *Bar*
 
             ```rust
-            test::t2::T2
+            test::t2
             ```
 
             ```rust
@@ -5306,7 +5270,7 @@ trait A {
             *Assoc*
 
             ```rust
-            test::A
+            test
             ```
 
             ```rust
@@ -5327,7 +5291,7 @@ trait A {
             *Assoc*
 
             ```rust
-            test::A
+            test
             ```
 
             ```rust
@@ -5346,7 +5310,7 @@ trait A where
             *Assoc*
 
             ```rust
-            test::A
+            test
             ```
 
             ```rust
@@ -6632,7 +6596,7 @@ fn test() {
             *A*
 
             ```rust
-            test::S
+            test
             ```
 
             ```rust
@@ -6661,7 +6625,7 @@ fn test() {
             *A*
 
             ```rust
-            test::S
+            test
             ```
 
             ```rust
@@ -6691,7 +6655,7 @@ mod m {
             *A*
 
             ```rust
-            test::S
+            test
             ```
 
             ```rust
@@ -7237,65 +7201,6 @@ impl Iterator for S {
     );
 }
 
-#[test]
-fn extern_items() {
-    check(
-        r#"
-extern "C" {
-    static STATIC$0: ();
-}
-"#,
-        expect![[r#"
-            *STATIC*
-
-            ```rust
-            test::<extern>
-            ```
-
-            ```rust
-            static STATIC: ()
-            ```
-        "#]],
-    );
-    check(
-        r#"
-extern "C" {
-    fn fun$0();
-}
-"#,
-        expect![[r#"
-            *fun*
-
-            ```rust
-            test::<extern>
-            ```
-
-            ```rust
-            unsafe fn fun()
-            ```
-        "#]],
-    );
-    check(
-        r#"
-extern "C" {
-    type Ty$0;
-}
-"#,
-        expect![[r#"
-            *Ty*
-
-            ```rust
-            test::<extern>
-            ```
-
-            ```rust
-             // size = 0, align = 1
-            type Ty
-            ```
-        "#]],
-    );
-}
-
 #[test]
 fn notable_ranged() {
     check_hover_range(