From af4e75533f2c071330e740e2fa94b131e3a2b538 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 20 Oct 2020 15:38:11 +0200 Subject: Rename declaration_name -> display_name Declaration names sounds like a name of declaration -- something you can use for analysis. It empathically isn't, and is just a label displayed in various UI. It's important not to confuse the two, least we accidentally mix semantics with UI (I believe, there's already a case of this in the FamousDefs at least). --- crates/assists/src/utils.rs | 2 +- crates/base_db/src/input.rs | 20 +++++++++++--------- crates/hir/src/code_model.rs | 4 ++-- crates/hir_def/src/import_map.rs | 6 +++--- crates/hir_def/src/nameres.rs | 6 +----- crates/ide/src/doc_links.rs | 8 ++++---- crates/ide/src/hover.rs | 2 +- crates/ide/src/inlay_hints.rs | 2 +- crates/ide/src/prime_caches.rs | 3 +-- crates/ide/src/status.rs | 2 +- crates/rust-analyzer/src/cli/diagnostics.rs | 8 ++------ 11 files changed, 28 insertions(+), 35 deletions(-) diff --git a/crates/assists/src/utils.rs b/crates/assists/src/utils.rs index b37b0d2b6..4e89a7aed 100644 --- a/crates/assists/src/utils.rs +++ b/crates/assists/src/utils.rs @@ -406,7 +406,7 @@ pub use prelude::*; let std_crate = path.next()?; let std_crate = if self .1 - .declaration_name(db) + .display_name(db) .map(|name| name.to_string() == std_crate) .unwrap_or(false) { diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs index b870e2cee..eb3aac88d 100644 --- a/crates/base_db/src/input.rs +++ b/crates/base_db/src/input.rs @@ -127,11 +127,13 @@ impl PartialEq for ProcMacro { pub struct CrateData { pub root_file_id: FileId, pub edition: Edition, - /// A name used in the package's project declaration: for Cargo projects, it's [package].name, - /// can be different for other project types or even absent (a dummy crate for the code snippet, for example). - /// NOTE: The crate can be referenced as a dependency under a different name, - /// this one should be used when working with crate hierarchies. - pub declaration_name: Option, + /// A name used in the package's project declaration: for Cargo projects, + /// it's [package].name, can be different for other project types or even + /// absent (a dummy crate for the code snippet, for example). + /// + /// For purposes of analysis, crates are anonymous (only names in + /// `Dependency` matters), this name should only be used for UI. + pub display_name: Option, pub cfg_options: CfgOptions, pub env: Env, pub dependencies: Vec, @@ -160,7 +162,7 @@ impl CrateGraph { &mut self, file_id: FileId, edition: Edition, - declaration_name: Option, + display_name: Option, cfg_options: CfgOptions, env: Env, proc_macro: Vec<(SmolStr, Arc)>, @@ -171,7 +173,7 @@ impl CrateGraph { let data = CrateData { root_file_id: file_id, edition, - declaration_name, + display_name, cfg_options, env, proc_macro, @@ -310,8 +312,8 @@ impl CrateGraph { } } - fn hacky_find_crate(&self, declaration_name: &str) -> Option { - self.iter().find(|it| self[*it].declaration_name.as_deref() == Some(declaration_name)) + fn hacky_find_crate(&self, display_name: &str) -> Option { + self.iter().find(|it| self[*it].display_name.as_deref() == Some(display_name)) } } diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index b65be4fe1..64f3fbe31 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -103,8 +103,8 @@ impl Crate { db.crate_graph()[self.id].edition } - pub fn declaration_name(self, db: &dyn HirDatabase) -> Option { - db.crate_graph()[self.id].declaration_name.clone() + pub fn display_name(self, db: &dyn HirDatabase) -> Option { + db.crate_graph()[self.id].display_name.clone() } pub fn query_external_importables( diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 028cae2e7..1e24f29a8 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -356,7 +356,7 @@ mod tests { let krate = crate_graph .iter() .find(|krate| { - crate_graph[*krate].declaration_name.as_ref().map(|n| n.to_string()) + crate_graph[*krate].display_name.as_ref().map(|n| n.to_string()) == Some(crate_name.to_string()) }) .unwrap(); @@ -375,7 +375,7 @@ mod tests { let path = map.path_of(item).unwrap(); format!( "{}::{} ({})\n", - crate_graph[krate].declaration_name.as_ref().unwrap(), + crate_graph[krate].display_name.as_ref().unwrap(), path, mark ) @@ -416,7 +416,7 @@ mod tests { .iter() .filter_map(|krate| { let cdata = &crate_graph[krate]; - let name = cdata.declaration_name.as_ref()?; + let name = cdata.display_name.as_ref()?; let map = db.import_map(krate); diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 464ffef21..3d04f81c6 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -172,11 +172,7 @@ pub struct ModuleData { impl CrateDefMap { pub(crate) fn crate_def_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc { let _p = profile::span("crate_def_map_query").detail(|| { - db.crate_graph()[krate] - .declaration_name - .as_ref() - .map(ToString::to_string) - .unwrap_or_default() + db.crate_graph()[krate].display_name.as_deref().unwrap_or_default().to_string() }); let def_map = { let edition = db.crate_graph()[krate].edition; diff --git a/crates/ide/src/doc_links.rs b/crates/ide/src/doc_links.rs index d9dc63b33..b9d8b8a2b 100644 --- a/crates/ide/src/doc_links.rs +++ b/crates/ide/src/doc_links.rs @@ -130,7 +130,7 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option { let module = definition.module(db)?; let krate = module.krate(); let import_map = db.import_map(krate.into()); - let base = once(krate.declaration_name(db)?.to_string()) + let base = once(krate.display_name(db)?.to_string()) .chain(import_map.path_of(ns)?.segments.iter().map(|name| name.to_string())) .join("/"); @@ -188,7 +188,7 @@ fn rewrite_intra_doc_link( let krate = resolved.module(db)?.krate(); let canonical_path = resolved.canonical_path(db)?; let new_target = get_doc_url(db, &krate)? - .join(&format!("{}/", krate.declaration_name(db)?)) + .join(&format!("{}/", krate.display_name(db)?)) .ok()? .join(&canonical_path.replace("::", "/")) .ok()? @@ -208,7 +208,7 @@ fn rewrite_url_link(db: &RootDatabase, def: ModuleDef, target: &str) -> Option Option { // // FIXME: clicking on the link should just open the file in the editor, // instead of falling back to external urls. - Some(format!("https://docs.rs/{}/*/", krate.declaration_name(db)?)) + Some(format!("https://docs.rs/{}/*/", krate.display_name(db)?)) }) .and_then(|s| Url::parse(&s).ok()) } diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 845333e2a..6466422c5 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -300,7 +300,7 @@ fn definition_owner_name(db: &RootDatabase, def: &Definition) -> Option fn render_path(db: &RootDatabase, module: Module, item_name: Option) -> String { let crate_name = - db.crate_graph()[module.krate().into()].declaration_name.as_ref().map(ToString::to_string); + db.crate_graph()[module.krate().into()].display_name.as_ref().map(|it| it.to_string()); let module_path = module .path_to_root(db) .into_iter() diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index e2079bbcf..f5f366354 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -215,7 +215,7 @@ fn hint_iterator( .last() .and_then(|strukt| strukt.as_adt())?; let krate = strukt.krate(db)?; - if krate.declaration_name(db).as_deref() != Some("core") { + if krate.display_name(db).as_deref() != Some("core") { return None; } let iter_trait = FamousDefs(sema, krate).core_iter_Iterator()?; diff --git a/crates/ide/src/prime_caches.rs b/crates/ide/src/prime_caches.rs index 9687c2734..6944dbcd2 100644 --- a/crates/ide/src/prime_caches.rs +++ b/crates/ide/src/prime_caches.rs @@ -32,8 +32,7 @@ pub(crate) fn prime_caches(db: &RootDatabase, cb: &(dyn Fn(PrimeCachesProgress) // Unfortunately rayon prevents panics from propagation out of a `scope`, which breaks // cancellation, so we cannot use rayon. for (i, krate) in topo.iter().enumerate() { - let crate_name = - graph[*krate].declaration_name.as_ref().map(ToString::to_string).unwrap_or_default(); + let crate_name = graph[*krate].display_name.as_deref().unwrap_or_default().to_string(); cb(PrimeCachesProgress::StartedOnCrate { on_crate: crate_name, diff --git a/crates/ide/src/status.rs b/crates/ide/src/status.rs index f67f10491..0af84daa0 100644 --- a/crates/ide/src/status.rs +++ b/crates/ide/src/status.rs @@ -45,7 +45,7 @@ pub(crate) fn status(db: &RootDatabase, file_id: Option) -> String { match krate { Some(krate) => { let crate_graph = db.crate_graph(); - let display_crate = |krate: CrateId| match &crate_graph[krate].declaration_name { + let display_crate = |krate: CrateId| match &crate_graph[krate].display_name { Some(it) => format!("{}({:?})", it, krate), None => format!("{:?}", krate), }; diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs index d1d3b12f8..a89993a2b 100644 --- a/crates/rust-analyzer/src/cli/diagnostics.rs +++ b/crates/rust-analyzer/src/cli/diagnostics.rs @@ -36,12 +36,8 @@ pub fn diagnostics(path: &Path, load_output_dirs: bool, with_proc_macro: bool) - for module in work { let file_id = module.definition_source(db).file_id.original_file(db); if !visited_files.contains(&file_id) { - let crate_name = module - .krate() - .declaration_name(db) - .as_ref() - .map(ToString::to_string) - .unwrap_or_else(|| "unknown".to_string()); + let crate_name = + module.krate().display_name(db).as_deref().unwrap_or("unknown").to_string(); println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id)); for diagnostic in analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() { -- cgit v1.2.3 From a85c4280bf7af3ea25c34c0cd72d05c8de17454d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 20 Oct 2020 16:00:51 +0200 Subject: Introduce CrateDisplayName --- crates/base_db/src/input.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs index eb3aac88d..02c7348ff 100644 --- a/crates/base_db/src/input.rs +++ b/crates/base_db/src/input.rs @@ -102,7 +102,29 @@ impl fmt::Display for CrateName { impl ops::Deref for CrateName { type Target = str; - fn deref(&self) -> &Self::Target { + fn deref(&self) -> &str { + &*self.0 + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct CrateDisplayName(CrateName); + +impl From for CrateDisplayName { + fn from(inner: CrateName) -> CrateDisplayName { + CrateDisplayName(inner) + } +} + +impl fmt::Display for CrateDisplayName { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl ops::Deref for CrateDisplayName { + type Target = str; + fn deref(&self) -> &str { &*self.0 } } -- cgit v1.2.3 From 3b1a648539487c08bc613b6fd6e573b0e0e38948 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 20 Oct 2020 17:04:38 +0200 Subject: More type safety around names --- crates/base_db/src/fixture.rs | 4 ++-- crates/base_db/src/input.rs | 27 ++++++++++++++++++++------- crates/base_db/src/lib.rs | 4 ++-- crates/hir/src/code_model.rs | 4 ++-- crates/ide/src/inlay_hints.rs | 5 ++--- crates/project_model/src/lib.rs | 8 +++++--- 6 files changed, 33 insertions(+), 19 deletions(-) diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs index 72f1fd667..66e6443cb 100644 --- a/crates/base_db/src/fixture.rs +++ b/crates/base_db/src/fixture.rs @@ -158,7 +158,7 @@ impl ChangeFixture { let crate_id = crate_graph.add_crate_root( file_id, meta.edition, - Some(crate_name.clone()), + Some(crate_name.clone().into()), meta.cfg, meta.env, Default::default(), @@ -187,7 +187,7 @@ impl ChangeFixture { crate_graph.add_crate_root( crate_root, Edition::Edition2018, - Some(CrateName::new("test").unwrap()), + Some(CrateName::new("test").unwrap().into()), default_cfg, Env::default(), Default::default(), diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs index 02c7348ff..87f0a0ce5 100644 --- a/crates/base_db/src/input.rs +++ b/crates/base_db/src/input.rs @@ -108,24 +108,37 @@ impl ops::Deref for CrateName { } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct CrateDisplayName(CrateName); +pub struct CrateDisplayName { + // The name we use to display various paths (with `_`). + crate_name: CrateName, + // The name as specified in Cargo.toml (with `-`). + canonical_name: String, +} impl From for CrateDisplayName { - fn from(inner: CrateName) -> CrateDisplayName { - CrateDisplayName(inner) + fn from(crate_name: CrateName) -> CrateDisplayName { + let canonical_name = crate_name.to_string(); + CrateDisplayName { crate_name, canonical_name } } } impl fmt::Display for CrateDisplayName { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.0) + write!(f, "{}", self.crate_name) } } impl ops::Deref for CrateDisplayName { type Target = str; fn deref(&self) -> &str { - &*self.0 + &*self.crate_name + } +} + +impl CrateDisplayName { + pub fn from_canonical_name(canonical_name: String) -> CrateDisplayName { + let crate_name = CrateName::normalize_dashes(&canonical_name); + CrateDisplayName { crate_name, canonical_name } } } @@ -155,7 +168,7 @@ pub struct CrateData { /// /// For purposes of analysis, crates are anonymous (only names in /// `Dependency` matters), this name should only be used for UI. - pub display_name: Option, + pub display_name: Option, pub cfg_options: CfgOptions, pub env: Env, pub dependencies: Vec, @@ -184,7 +197,7 @@ impl CrateGraph { &mut self, file_id: FileId, edition: Edition, - display_name: Option, + display_name: Option, cfg_options: CfgOptions, env: Env, proc_macro: Vec<(SmolStr, Arc)>, diff --git a/crates/base_db/src/lib.rs b/crates/base_db/src/lib.rs index e38aa7257..0804202d6 100644 --- a/crates/base_db/src/lib.rs +++ b/crates/base_db/src/lib.rs @@ -13,8 +13,8 @@ pub use crate::{ cancellation::Canceled, change::Change, input::{ - CrateData, CrateGraph, CrateId, CrateName, Dependency, Edition, Env, FileId, ProcMacroId, - SourceRoot, SourceRootId, + CrateData, CrateDisplayName, CrateGraph, CrateId, CrateName, Dependency, Edition, Env, + FileId, ProcMacroId, SourceRoot, SourceRootId, }, }; pub use salsa; diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 64f3fbe31..7f169ccd2 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -2,7 +2,7 @@ use std::{iter, sync::Arc}; use arrayvec::ArrayVec; -use base_db::{CrateId, CrateName, Edition, FileId}; +use base_db::{CrateDisplayName, CrateId, Edition, FileId}; use either::Either; use hir_def::find_path::PrefixKind; use hir_def::{ @@ -103,7 +103,7 @@ impl Crate { db.crate_graph()[self.id].edition } - pub fn display_name(self, db: &dyn HirDatabase) -> Option { + pub fn display_name(self, db: &dyn HirDatabase) -> Option { db.crate_graph()[self.id].display_name.clone() } diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index f5f366354..56b985e80 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -1,15 +1,14 @@ use assists::utils::FamousDefs; +use either::Either; use hir::{known, HirDisplay, Semantics}; use ide_db::RootDatabase; use stdx::to_lower_snake_case; use syntax::{ - ast::{self, ArgListOwner, AstNode}, + ast::{self, ArgListOwner, AstNode, NameOwner}, match_ast, Direction, NodeOrToken, SmolStr, SyntaxKind, TextRange, T, }; use crate::FileId; -use ast::NameOwner; -use either::Either; #[derive(Clone, Debug, PartialEq, Eq)] pub struct InlayHintsConfig { diff --git a/crates/project_model/src/lib.rs b/crates/project_model/src/lib.rs index ea95d1c77..5db41bc16 100644 --- a/crates/project_model/src/lib.rs +++ b/crates/project_model/src/lib.rs @@ -13,7 +13,7 @@ use std::{ }; use anyhow::{bail, Context, Result}; -use base_db::{CrateGraph, CrateId, CrateName, Edition, Env, FileId}; +use base_db::{CrateDisplayName, CrateGraph, CrateId, CrateName, Edition, Env, FileId}; use cfg::CfgOptions; use paths::{AbsPath, AbsPathBuf}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -408,10 +408,12 @@ impl ProjectWorkspace { .map(|it| proc_macro_client.by_dylib_path(&it)) .unwrap_or_default(); + let display_name = + CrateDisplayName::from_canonical_name(cargo[pkg].name.clone()); let crate_id = crate_graph.add_crate_root( file_id, edition, - Some(CrateName::normalize_dashes(&cargo[pkg].name)), + Some(display_name), cfg_options, env, proc_macro.clone(), @@ -556,7 +558,7 @@ fn sysroot_to_crate_graph( let crate_id = crate_graph.add_crate_root( file_id, Edition::Edition2018, - Some(name), + Some(name.into()), cfg_options.clone(), env, proc_macro, -- cgit v1.2.3