Repository: rust-analyzer/rust-analyzer
Pull request #11216 (2022-01-06 13:57:01)

internal: Support registered tools and attributes in ide layer
bors r+


Patch:
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 9a1a893650..ee423573d5 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -2064,37 +2064,75 @@ impl Local {
     }
 }
 
+// FIXME: Wrong name? This is could also be a registered attribute
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-pub struct BuiltinAttr(usize);
+pub struct BuiltinAttr {
+    krate: Option<CrateId>,
+    idx: usize,
+}
 
 impl BuiltinAttr {
-    pub(crate) fn by_name(name: &str) -> Option<Self> {
-        // FIXME: def maps registered attrs?
-        hir_def::builtin_attr::find_builtin_attr_idx(name).map(Self)
+    // FIXME: consider crates\hir_def\src\nameres\attr_resolution.rs?
+    pub(crate) fn by_name(db: &dyn HirDatabase, krate: Crate, name: &str) -> Option<Self> {
+        if let builtin @ Some(_) = Self::builtin(name) {
+            return builtin;
+        }
+        let idx = db.crate_def_map(krate.id).registered_attrs().iter().position(|it| it == name)?;
+        Some(BuiltinAttr { krate: Some(krate.id), idx })
+    }
+
+    pub(crate) fn builtin(name: &str) -> Option<Self> {
+        hir_def::builtin_attr::INERT_ATTRIBUTES
+            .iter()
+            .position(|tool| tool.name == name)
+            .map(|idx| BuiltinAttr { krate: None, idx })
     }
 
-    pub fn name(&self, _: &dyn HirDatabase) -> &str {
+    pub fn name(&self, db: &dyn HirDatabase) -> SmolStr {
         // FIXME: Return a `Name` here
-        hir_def::builtin_attr::INERT_ATTRIBUTES[self.0].name
+        match self.krate {
+            Some(krate) => db.crate_def_map(krate).registered_attrs()[self.idx].clone(),
+            None => SmolStr::new(hir_def::builtin_attr::INERT_ATTRIBUTES[self.idx].name),
+        }
     }
 
-    pub fn template(&self, _: &dyn HirDatabase) -> AttributeTemplate {
-        hir_def::builtin_attr::INERT_ATTRIBUTES[self.0].template
+    pub fn template(&self, _: &dyn HirDatabase) -> Option<AttributeTemplate> {
+        match self.krate {
+            Some(_) => None,
+            None => Some(hir_def::builtin_attr::INERT_ATTRIBUTES[self.idx].template),
+        }
     }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-pub struct ToolModule(usize);
+pub struct ToolModule {
+    krate: Option<CrateId>,
+    idx: usize,
+}
 
 impl ToolModule {
-    pub(crate) fn by_name(name: &str) -> Option<Self> {
-        // FIXME: def maps registered tools
-        hir_def::builtin_attr::TOOL_MODULES.iter().position(|&tool| tool == name).map(Self)
+    // FIXME: consider crates\hir_def\src\nameres\attr_resolution.rs?
+    pub(crate) fn by_name(db: &dyn HirDatabase, krate: Crate, name: &str) -> Option<Self> {
+        if let builtin @ Some(_) = Self::builtin(name) {
+            return builtin;
+        }
+        let idx = db.crate_def_map(krate.id).registered_tools().iter().position(|it| it == name)?;
+        Some(ToolModule { krate: Some(krate.id), idx })
     }
 
-    pub fn name(&self, _: &dyn HirDatabase) -> &str {
+    pub(crate) fn builtin(name: &str) -> Option<Self> {
+        hir_def::builtin_attr::TOOL_MODULES
+            .iter()
+            .position(|&tool| tool == name)
+            .map(|idx| ToolModule { krate: None, idx })
+    }
+
+    pub fn name(&self, db: &dyn HirDatabase) -> SmolStr {
         // FIXME: Return a `Name` here
-        hir_def::builtin_attr::TOOL_MODULES[self.0]
+        match self.krate {
+            Some(krate) => db.crate_def_map(krate).registered_tools()[self.idx].clone(),
+            None => SmolStr::new(hir_def::builtin_attr::TOOL_MODULES[self.idx]),
+        }
     }
 }
 
diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs
index 233c814809..ef3dfa1f33 100644
--- a/crates/hir/src/source_analyzer.rs
+++ b/crates/hir/src/source_analyzer.rs
@@ -346,7 +346,11 @@ impl SourceAnalyzer {
             return match resolve_hir_path_qualifier(db, &self.resolver, &hir_path) {
                 None if is_path_of_attr => {
                     path.first_segment().and_then(|it| it.name_ref()).and_then(|name_ref| {
-                        ToolModule::by_name(&name_ref.text()).map(PathResolution::ToolModule)
+                        match self.resolver.krate() {
+                            Some(krate) => ToolModule::by_name(db, krate.into(), &name_ref.text()),
+                            None => ToolModule::builtin(&name_ref.text()),
+                        }
+                        .map(PathResolution::ToolModule)
                     })
                 }
                 res => res,
@@ -356,8 +360,10 @@ impl SourceAnalyzer {
             // in this case we have to check for inert/builtin attributes and tools and prioritize
             // resolution of attributes over other namespaces
             let name_ref = path.as_single_name_ref();
-            let builtin =
-                name_ref.as_ref().map(ast::NameRef::text).as_deref().and_then(BuiltinAttr::by_name);
+            let builtin = name_ref.as_ref().and_then(|name_ref| match self.resolver.krate() {
+                Some(krate) => BuiltinAttr::by_name(db, krate.into(), &name_ref.text()),
+                None => BuiltinAttr::builtin(&name_ref.text()),
+            });
             if let builtin @ Some(_) = builtin {
                 return builtin.map(PathResolution::BuiltinAttr);
             }
@@ -366,7 +372,11 @@ impl SourceAnalyzer {
                 // this labels any path that starts with a tool module as the tool itself, this is technically wrong
                 // but there is no benefit in differentiating these two cases for the time being
                 _ => path.first_segment().and_then(|it| it.name_ref()).and_then(|name_ref| {
-                    ToolModule::by_name(&name_ref.text()).map(PathResolution::ToolModule)
+                    match self.resolver.krate() {
+                        Some(krate) => ToolModule::by_name(db, krate.into(), &name_ref.text()),
+                        None => ToolModule::builtin(&name_ref.text()),
+                    }
+                    .map(PathResolution::ToolModule)
                 }),
             };
         }
diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs
index f793f0e002..64929c02b4 100644
--- a/crates/hir_def/src/nameres.rs
+++ b/crates/hir_def/src/nameres.rs
@@ -296,6 +296,12 @@ impl DefMap {
     pub fn exported_proc_macros(&self) -> impl Iterator<Item = (MacroDefId, Name)> + '_ {
         self.exported_proc_macros.iter().map(|(id, def)| (*id, def.name.clone()))
     }
+    pub fn registered_tools(&self) -> &[SmolStr] {
+        &self.registered_tools
+    }
+    pub fn registered_attrs(&self) -> &[SmolStr] {
+        &self.registered_attrs
+    }
     pub fn root(&self) -> LocalModuleId {
         self.root
     }
diff --git a/crates/ide/src/hover/render.rs b/crates/ide/src/hover/render.rs
index 9ae122d8a4..d29833a65b 100644
--- a/crates/ide/src/hover/render.rs
+++ b/crates/ide/src/hover/render.rs
@@ -393,7 +393,10 @@ fn render_builtin_attr(db: &RootDatabase, attr: hir::BuiltinAttr) -> Option<Mark
     let name = attr.name(db);
     let desc = format!("#[{}]", name);
 
-    let AttributeTemplate { word, list, name_value_str } = attr.template(db);
+    let AttributeTemplate { word, list, name_value_str } = match attr.template(db) {
+        Some(template) => template,
+        None => return Some(Markup::fenced_block(&attr.name(db))),
+    };
     let mut docs = "Valid forms are:".to_owned();
     if word {
         format_to!(docs, "\n - #\\[{}]", name);
diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs
index 9a2e435591..567cdc48ef 100644
--- a/crates/rust-analyzer/src/bin/main.rs
+++ b/crates/rust-analyzer/src/bin/main.rs
@@ -150,7 +150,7 @@ fn run_server() -> Result<()> {
 
     let mut config = Config::new(root_path, initialize_params.capabilities);
     if let Some(json) = initialize_params.initialization_options {
-        let _ = config.update(json);
+        config.update(json);
     }
 
     let server_capabilities = rust_analyzer::server_capabilities(&config);
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index de0aba0caa..a704139255 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -341,7 +341,7 @@ config_data! {
 
 impl Default for ConfigData {
     fn default() -> Self {
-        ConfigData::from_json(serde_json::Value::Null, &mut Vec::new())
+        ConfigData::from_json(serde_json::Value::Null)
     }
 }
 
@@ -492,21 +492,16 @@ impl Config {
             snippets: Default::default(),
         }
     }
-    pub fn update(
-        &mut self,
-        mut json: serde_json::Value,
-    ) -> Result<(), Vec<(String, serde_json::Error)>> {
+    pub fn update(&mut self, mut json: serde_json::Value) {
         tracing::info!("updating config from JSON: {:#}", json);
         if json.is_null() || json.as_object().map_or(false, |it| it.is_empty()) {
-            return Ok(());
+            return;
         }
-        let mut errors = Vec::new();
-        self.detached_files =
-            get_field::<Vec<PathBuf>>(&mut json, &mut errors, "detachedFiles", None, "[]")
-                .into_iter()
-                .map(AbsPathBuf::assert)
-                .collect();
-        self.data = ConfigData::from_json(json, &mut errors);
+        self.detached_files = get_field::<Vec<PathBuf>>(&mut json, "detachedFiles", None, "[]")
+            .into_iter()
+            .map(AbsPathBuf::assert)
+            .collect();
+        self.data = ConfigData::from_json(json);
         self.snippets.clear();
         for (name, def) in self.data.completion_snippets.iter() {
             if def.prefix.is_empty() && def.postfix.is_empty() {
@@ -529,11 +524,6 @@ impl Config {
                 None => tracing::info!("Invalid snippet {}", name),
             }
         }
-        if errors.is_empty() {
-            Ok(())
-        } else {
-            Err(errors)
-        }
     }
 
     pub fn json_schema() -> serde_json::Value {
@@ -1126,11 +1116,10 @@ macro_rules! _config_data {
         #[derive(Debug, Clone)]
         struct $name { $($field: $ty,)* }
         impl $name {
-            fn from_json(mut json: serde_json::Value, error_sink: &mut Vec<(String, serde_json::Error)>) -> $name {
+            fn from_json(mut json: serde_json::Value) -> $name {
                 $name {$(
                     $field: get_field(
                         &mut json,
-                        error_sink,
                         stringify!($field),
                         None$(.or(Some(stringify!($alias))))*,
                         $default,
@@ -1167,7 +1156,6 @@ use _config_data as config_data;
 
 fn get_field<T: DeserializeOwned>(
     json: &mut serde_json::Value,
-    error_sink: &mut Vec<(String, serde_json::Error)>,
     field: &'static str,
     alias: Option<&'static str>,
     default: &str,
@@ -1186,7 +1174,6 @@ fn get_field<T: DeserializeOwned>(
                 Ok(it) => Some(it),
                 Err(e) => {
                     tracing::warn!("Failed to deserialize config field at {}: {:?}", pointer, e);
-                    error_sink.push((pointer, e));
                     None
                 }
             })
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index af987230de..65f9c87367 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -9,7 +9,6 @@ use std::{
 use always_assert::always;
 use crossbeam_channel::{select, Receiver};
 use ide_db::base_db::{SourceDatabaseExt, VfsPath};
-use itertools::Itertools;
 use lsp_server::{Connection, Notification, Request};
 use lsp_types::notification::Notification as _;
 use vfs::{ChangeKind, FileId};
@@ -732,17 +731,7 @@ impl GlobalState {
                                     // Note that json can be null according to the spec if the client can't
                                     // provide a configuration. This is handled in Config::update below.
                                     let mut config = Config::clone(&*this.config);
-                                    if let Err(errors) = config.update(json.take()) {
-                                        let errors = errors
-                                            .iter()
-                                            .format_with("\n", |(key, e),f| {
-                                                f(key)?;
-                                                f(&": ")?;
-                                                f(e)
-                                            });
-                                        let msg= format!("Failed to deserialize config key(s):\n{}", errors);
-                                        this.show_message(lsp_types::MessageType::WARNING, msg);
-                                    }
+                                    config.update(json.take());
                                     this.update_configuration(config);
                                 }
                             }


Test patch:
diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs
index 78aa411915..6e7ecec5b2 100644
--- a/crates/rust-analyzer/tests/slow-tests/support.rs
+++ b/crates/rust-analyzer/tests/slow-tests/support.rs
@@ -137,7 +137,7 @@ impl<'a> Project<'a> {
             },
         );
         config.discovered_projects = Some(discovered_projects);
-        let _ = config.update(self.config);
+        config.update(self.config);
 
         Server::new(tmp_dir, config)
     }