6139: Make find_path_prefixed configurable r=matklad a=Veykril

This makes `find_path_prefixed` more configurable allowing one to choose whether it always returns absolute paths, self-prefixed paths or to ignore local imports when building the path. 

The config names are just thrown in here, taking better names if they exist :)

This should fix #6131 as well?

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
This commit is contained in:
bors[bot] 2020-10-06 11:43:08 +00:00 committed by GitHub
commit af0e54a566
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 188 additions and 119 deletions

View File

@ -4,6 +4,8 @@
//! module, and we use to statically check that we only produce snippet //! module, and we use to statically check that we only produce snippet
//! assists if we are allowed to. //! assists if we are allowed to.
use hir::PrefixKind;
use crate::{utils::MergeBehaviour, AssistKind}; use crate::{utils::MergeBehaviour, AssistKind};
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
@ -37,10 +39,11 @@ impl Default for AssistConfig {
#[derive(Clone, Copy, Debug, PartialEq, Eq)] #[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct InsertUseConfig { pub struct InsertUseConfig {
pub merge: Option<MergeBehaviour>, pub merge: Option<MergeBehaviour>,
pub prefix_kind: PrefixKind,
} }
impl Default for InsertUseConfig { impl Default for InsertUseConfig {
fn default() -> Self { fn default() -> Self {
InsertUseConfig { merge: Some(MergeBehaviour::Full) } InsertUseConfig { merge: Some(MergeBehaviour::Full), prefix_kind: PrefixKind::Plain }
} }
} }

View File

@ -191,12 +191,16 @@ impl AutoImportAssets {
_ => Some(candidate), _ => Some(candidate),
}) })
.filter_map(|candidate| match candidate { .filter_map(|candidate| match candidate {
Either::Left(module_def) => { Either::Left(module_def) => self.module_with_name_to_import.find_use_path_prefixed(
self.module_with_name_to_import.find_use_path_prefixed(db, module_def) db,
} module_def,
Either::Right(macro_def) => { ctx.config.insert_use.prefix_kind,
self.module_with_name_to_import.find_use_path_prefixed(db, macro_def) ),
} Either::Right(macro_def) => self.module_with_name_to_import.find_use_path_prefixed(
db,
macro_def,
ctx.config.insert_use.prefix_kind,
),
}) })
.filter(|use_path| !use_path.segments.is_empty()) .filter(|use_path| !use_path.segments.is_empty())
.take(20) .take(20)

View File

@ -4,6 +4,7 @@ use std::{iter, sync::Arc};
use arrayvec::ArrayVec; use arrayvec::ArrayVec;
use base_db::{CrateId, Edition, FileId}; use base_db::{CrateId, Edition, FileId};
use either::Either; use either::Either;
use hir_def::find_path::PrefixKind;
use hir_def::{ use hir_def::{
adt::ReprKind, adt::ReprKind,
adt::StructKind, adt::StructKind,
@ -390,8 +391,9 @@ impl Module {
self, self,
db: &dyn DefDatabase, db: &dyn DefDatabase,
item: impl Into<ItemInNs>, item: impl Into<ItemInNs>,
prefix_kind: PrefixKind,
) -> Option<ModPath> { ) -> Option<ModPath> {
hir_def::find_path::find_path_prefixed(db, item.into(), self.into()) hir_def::find_path::find_path_prefixed(db, item.into(), self.into(), prefix_kind)
} }
} }

View File

@ -48,6 +48,7 @@ pub use hir_def::{
body::scope::ExprScopes, body::scope::ExprScopes,
builtin_type::BuiltinType, builtin_type::BuiltinType,
docs::Documentation, docs::Documentation,
find_path::PrefixKind,
item_scope::ItemInNs, item_scope::ItemInNs,
nameres::ModuleSource, nameres::ModuleSource,
path::ModPath, path::ModPath,

View File

@ -19,12 +19,17 @@ use crate::{
/// *from where* you're referring to the item, hence the `from` parameter. /// *from where* you're referring to the item, hence the `from` parameter.
pub fn find_path(db: &dyn DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> { pub fn find_path(db: &dyn DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
let _p = profile::span("find_path"); let _p = profile::span("find_path");
find_path_inner(db, item, from, MAX_PATH_LEN, Prefixed::Not) find_path_inner(db, item, from, MAX_PATH_LEN, None)
} }
pub fn find_path_prefixed(db: &dyn DefDatabase, item: ItemInNs, from: ModuleId) -> Option<ModPath> { pub fn find_path_prefixed(
let _p = profile::span("find_path_absolute"); db: &dyn DefDatabase,
find_path_inner(db, item, from, MAX_PATH_LEN, Prefixed::Plain) item: ItemInNs,
from: ModuleId,
prefix_kind: PrefixKind,
) -> Option<ModPath> {
let _p = profile::span("find_path_prefixed");
find_path_inner(db, item, from, MAX_PATH_LEN, Some(prefix_kind))
} }
const MAX_PATH_LEN: usize = 15; const MAX_PATH_LEN: usize = 15;
@ -42,58 +47,52 @@ impl ModPath {
} }
} }
fn check_crate_self_super( fn check_self_super(def_map: &CrateDefMap, item: ItemInNs, from: ModuleId) -> Option<ModPath> {
def_map: &CrateDefMap, if item == ItemInNs::Types(from.into()) {
item: ItemInNs,
from: ModuleId,
) -> Option<ModPath> {
// - if the item is the crate root, return `crate`
if item
== ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
krate: from.krate,
local_id: def_map.root,
}))
{
Some(ModPath::from_segments(PathKind::Crate, Vec::new()))
} else if item == ItemInNs::Types(from.into()) {
// - if the item is the module we're in, use `self` // - if the item is the module we're in, use `self`
Some(ModPath::from_segments(PathKind::Super(0), Vec::new())) Some(ModPath::from_segments(PathKind::Super(0), Vec::new()))
} else { } else if let Some(parent_id) = def_map.modules[from.local_id].parent {
if let Some(parent_id) = def_map.modules[from.local_id].parent { // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly)
// - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly) if item
if item == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
== ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { krate: from.krate,
krate: from.krate, local_id: parent_id,
local_id: parent_id, }))
})) {
{ Some(ModPath::from_segments(PathKind::Super(1), Vec::new()))
return Some(ModPath::from_segments(PathKind::Super(1), Vec::new())); } else {
} None
} }
} else {
None None
} }
} }
#[derive(Copy, Clone, PartialEq, Eq)] #[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum Prefixed { pub enum PrefixKind {
Not, /// Causes paths to always start with either `self`, `super`, `crate` or a crate-name.
/// This is the same as plain, just that paths will start with `self` iprepended f the path
/// starts with an identifier that is not a crate.
BySelf, BySelf,
/// Causes paths to ignore imports in the local module.
Plain, Plain,
/// Causes paths to start with `crate` where applicable, effectively forcing paths to be absolute.
ByCrate,
} }
impl Prefixed { impl PrefixKind {
#[inline] #[inline]
fn prefix(self) -> Option<PathKind> { fn prefix(self) -> PathKind {
match self { match self {
Prefixed::Not => None, PrefixKind::BySelf => PathKind::Super(0),
Prefixed::BySelf => Some(PathKind::Super(0)), PrefixKind::Plain => PathKind::Plain,
Prefixed::Plain => Some(PathKind::Plain), PrefixKind::ByCrate => PathKind::Crate,
} }
} }
#[inline] #[inline]
fn prefixed(self) -> bool { fn is_absolute(&self) -> bool {
self != Prefixed::Not self == &PrefixKind::ByCrate
} }
} }
@ -102,7 +101,7 @@ fn find_path_inner(
item: ItemInNs, item: ItemInNs,
from: ModuleId, from: ModuleId,
max_len: usize, max_len: usize,
prefixed: Prefixed, prefixed: Option<PrefixKind>,
) -> Option<ModPath> { ) -> Option<ModPath> {
if max_len == 0 { if max_len == 0 {
return None; return None;
@ -115,13 +114,25 @@ fn find_path_inner(
let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope; let from_scope: &crate::item_scope::ItemScope = &def_map.modules[from.local_id].scope;
let scope_name = let scope_name =
if let Some((name, _)) = from_scope.name_of(item) { Some(name.clone()) } else { None }; if let Some((name, _)) = from_scope.name_of(item) { Some(name.clone()) } else { None };
if !prefixed.prefixed() && scope_name.is_some() { if prefixed.is_none() && scope_name.is_some() {
return scope_name return scope_name
.map(|scope_name| ModPath::from_segments(PathKind::Plain, vec![scope_name])); .map(|scope_name| ModPath::from_segments(PathKind::Plain, vec![scope_name]));
} }
if let modpath @ Some(_) = check_crate_self_super(&def_map, item, from) { // - if the item is the crate root, return `crate`
return modpath; if item
== ItemInNs::Types(ModuleDefId::ModuleId(ModuleId {
krate: from.krate,
local_id: def_map.root,
}))
{
return Some(ModPath::from_segments(PathKind::Crate, Vec::new()));
}
if prefixed.filter(PrefixKind::is_absolute).is_none() {
if let modpath @ Some(_) = check_self_super(&def_map, item, from) {
return modpath;
}
} }
// - if the item is the crate root of a dependency crate, return the name from the extern prelude // - if the item is the crate root of a dependency crate, return the name from the extern prelude
@ -226,7 +237,7 @@ fn find_path_inner(
} }
} }
if let Some(prefix) = prefixed.prefix() { if let Some(prefix) = prefixed.map(PrefixKind::prefix) {
best_path.or_else(|| { best_path.or_else(|| {
scope_name.map(|scope_name| ModPath::from_segments(prefix, vec![scope_name])) scope_name.map(|scope_name| ModPath::from_segments(prefix, vec![scope_name]))
}) })
@ -355,7 +366,7 @@ mod tests {
/// `code` needs to contain a cursor marker; checks that `find_path` for the /// `code` needs to contain a cursor marker; checks that `find_path` for the
/// item the `path` refers to returns that same path when called from the /// item the `path` refers to returns that same path when called from the
/// module the cursor is in. /// module the cursor is in.
fn check_found_path_(ra_fixture: &str, path: &str, absolute: bool) { fn check_found_path_(ra_fixture: &str, path: &str, prefix_kind: Option<PrefixKind>) {
let (db, pos) = TestDB::with_position(ra_fixture); let (db, pos) = TestDB::with_position(ra_fixture);
let module = db.module_for_file(pos.file_id); let module = db.module_for_file(pos.file_id);
let parsed_path_file = syntax::SourceFile::parse(&format!("use {};", path)); let parsed_path_file = syntax::SourceFile::parse(&format!("use {};", path));
@ -375,20 +386,22 @@ mod tests {
.take_types() .take_types()
.unwrap(); .unwrap();
let found_path = if absolute { find_path_prefixed } else { find_path }( let found_path =
&db, find_path_inner(&db, ItemInNs::Types(resolved), module, MAX_PATH_LEN, prefix_kind);
ItemInNs::Types(resolved), assert_eq!(found_path, Some(mod_path), "{:?}", prefix_kind);
module,
);
assert_eq!(found_path, Some(mod_path), "absolute {}", absolute);
} }
fn check_found_path(ra_fixture: &str, path: &str) { fn check_found_path(
check_found_path_(ra_fixture, path, false); ra_fixture: &str,
} unprefixed: &str,
prefixed: &str,
fn check_found_path_abs(ra_fixture: &str, path: &str) { absolute: &str,
check_found_path_(ra_fixture, path, true); self_prefixed: &str,
) {
check_found_path_(ra_fixture, unprefixed, None);
check_found_path_(ra_fixture, prefixed, Some(PrefixKind::Plain));
check_found_path_(ra_fixture, absolute, Some(PrefixKind::ByCrate));
check_found_path_(ra_fixture, self_prefixed, Some(PrefixKind::BySelf));
} }
#[test] #[test]
@ -398,8 +411,7 @@ mod tests {
struct S; struct S;
<|> <|>
"#; "#;
check_found_path(code, "S"); check_found_path(code, "S", "S", "crate::S", "self::S");
check_found_path_abs(code, "S");
} }
#[test] #[test]
@ -409,8 +421,7 @@ mod tests {
enum E { A } enum E { A }
<|> <|>
"#; "#;
check_found_path(code, "E::A"); check_found_path(code, "E::A", "E::A", "E::A", "E::A");
check_found_path_abs(code, "E::A");
} }
#[test] #[test]
@ -422,8 +433,7 @@ mod tests {
} }
<|> <|>
"#; "#;
check_found_path(code, "foo::S"); check_found_path(code, "foo::S", "foo::S", "crate::foo::S", "self::foo::S");
check_found_path_abs(code, "foo::S");
} }
#[test] #[test]
@ -437,8 +447,7 @@ mod tests {
//- /foo/bar.rs //- /foo/bar.rs
<|> <|>
"#; "#;
check_found_path(code, "super::S"); check_found_path(code, "super::S", "super::S", "crate::foo::S", "super::S");
check_found_path_abs(code, "super::S");
} }
#[test] #[test]
@ -449,8 +458,7 @@ mod tests {
//- /foo.rs //- /foo.rs
<|> <|>
"#; "#;
check_found_path(code, "self"); check_found_path(code, "self", "self", "crate::foo", "self");
check_found_path_abs(code, "self");
} }
#[test] #[test]
@ -461,8 +469,7 @@ mod tests {
//- /foo.rs //- /foo.rs
<|> <|>
"#; "#;
check_found_path(code, "crate"); check_found_path(code, "crate", "crate", "crate", "crate");
check_found_path_abs(code, "crate");
} }
#[test] #[test]
@ -474,8 +481,7 @@ mod tests {
//- /foo.rs //- /foo.rs
<|> <|>
"#; "#;
check_found_path(code, "crate::S"); check_found_path(code, "crate::S", "crate::S", "crate::S", "crate::S");
check_found_path_abs(code, "crate::S");
} }
#[test] #[test]
@ -486,8 +492,7 @@ mod tests {
//- /std.rs crate:std //- /std.rs crate:std
pub struct S; pub struct S;
"#; "#;
check_found_path(code, "std::S"); check_found_path(code, "std::S", "std::S", "std::S", "std::S");
check_found_path_abs(code, "std::S");
} }
#[test] #[test]
@ -499,8 +504,13 @@ mod tests {
//- /std.rs crate:std //- /std.rs crate:std
pub struct S; pub struct S;
"#; "#;
check_found_path(code, "std_renamed::S"); check_found_path(
check_found_path_abs(code, "std_renamed::S"); code,
"std_renamed::S",
"std_renamed::S",
"std_renamed::S",
"std_renamed::S",
);
} }
#[test] #[test]
@ -520,8 +530,13 @@ mod tests {
} }
} }
"#; "#;
check_found_path(code, "ast::ModuleItem"); check_found_path(
check_found_path_abs(code, "syntax::ast::ModuleItem"); code,
"ast::ModuleItem",
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
);
let code = r#" let code = r#"
//- /main.rs crate:main deps:syntax //- /main.rs crate:main deps:syntax
@ -535,8 +550,13 @@ mod tests {
} }
} }
"#; "#;
check_found_path(code, "syntax::ast::ModuleItem"); check_found_path(
check_found_path_abs(code, "syntax::ast::ModuleItem"); code,
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
"syntax::ast::ModuleItem",
);
} }
#[test] #[test]
@ -549,8 +569,7 @@ mod tests {
} }
<|> <|>
"#; "#;
check_found_path(code, "bar::S"); check_found_path(code, "bar::S", "bar::S", "crate::bar::S", "self::bar::S");
check_found_path_abs(code, "bar::S");
} }
#[test] #[test]
@ -563,8 +582,7 @@ mod tests {
} }
<|> <|>
"#; "#;
check_found_path(code, "bar::U"); check_found_path(code, "bar::U", "bar::U", "crate::bar::U", "self::bar::U");
check_found_path_abs(code, "bar::U");
} }
#[test] #[test]
@ -577,8 +595,7 @@ mod tests {
//- /core.rs crate:core //- /core.rs crate:core
pub struct S; pub struct S;
"#; "#;
check_found_path(code, "std::S"); check_found_path(code, "std::S", "std::S", "std::S", "std::S");
check_found_path_abs(code, "std::S");
} }
#[test] #[test]
@ -591,8 +608,7 @@ mod tests {
#[prelude_import] #[prelude_import]
pub use prelude::*; pub use prelude::*;
"#; "#;
check_found_path(code, "S"); check_found_path(code, "S", "S", "S", "S");
check_found_path_abs(code, "S");
} }
#[test] #[test]
@ -608,10 +624,8 @@ mod tests {
#[prelude_import] #[prelude_import]
pub use prelude::*; pub use prelude::*;
"#; "#;
check_found_path(code, "None"); check_found_path(code, "None", "None", "None", "None");
check_found_path(code, "Some"); check_found_path(code, "Some", "Some", "Some", "Some");
check_found_path_abs(code, "None");
check_found_path_abs(code, "Some");
} }
#[test] #[test]
@ -627,8 +641,7 @@ mod tests {
//- /baz.rs //- /baz.rs
pub use crate::foo::bar::S; pub use crate::foo::bar::S;
"#; "#;
check_found_path(code, "baz::S"); check_found_path(code, "baz::S", "baz::S", "crate::baz::S", "self::baz::S");
check_found_path_abs(code, "baz::S");
} }
#[test] #[test]
@ -642,8 +655,7 @@ mod tests {
<|> <|>
"#; "#;
// crate::S would be shorter, but using private imports seems wrong // crate::S would be shorter, but using private imports seems wrong
check_found_path(code, "crate::bar::S"); check_found_path(code, "crate::bar::S", "crate::bar::S", "crate::bar::S", "crate::bar::S");
check_found_path_abs(code, "crate::bar::S");
} }
#[test] #[test]
@ -661,8 +673,7 @@ mod tests {
//- /baz.rs //- /baz.rs
pub use super::foo; pub use super::foo;
"#; "#;
check_found_path(code, "crate::foo::S"); check_found_path(code, "crate::foo::S", "crate::foo::S", "crate::foo::S", "crate::foo::S");
check_found_path_abs(code, "crate::foo::S");
} }
#[test] #[test]
@ -682,8 +693,13 @@ mod tests {
pub struct Arc; pub struct Arc;
} }
"#; "#;
check_found_path(code, "std::sync::Arc"); check_found_path(
check_found_path_abs(code, "std::sync::Arc"); code,
"std::sync::Arc",
"std::sync::Arc",
"std::sync::Arc",
"std::sync::Arc",
);
} }
#[test] #[test]
@ -707,8 +723,13 @@ mod tests {
pub struct Error; pub struct Error;
} }
"#; "#;
check_found_path(code, "core::fmt::Error"); check_found_path(
check_found_path_abs(code, "core::fmt::Error"); code,
"core::fmt::Error",
"core::fmt::Error",
"core::fmt::Error",
"core::fmt::Error",
);
} }
#[test] #[test]
@ -731,8 +752,13 @@ mod tests {
pub struct Arc; pub struct Arc;
} }
"#; "#;
check_found_path(code, "alloc::sync::Arc"); check_found_path(
check_found_path_abs(code, "alloc::sync::Arc"); code,
"alloc::sync::Arc",
"alloc::sync::Arc",
"alloc::sync::Arc",
"alloc::sync::Arc",
);
} }
#[test] #[test]
@ -749,8 +775,13 @@ mod tests {
//- /zzz.rs crate:megaalloc //- /zzz.rs crate:megaalloc
pub struct Arc; pub struct Arc;
"#; "#;
check_found_path(code, "megaalloc::Arc"); check_found_path(
check_found_path_abs(code, "megaalloc::Arc"); code,
"megaalloc::Arc",
"megaalloc::Arc",
"megaalloc::Arc",
"megaalloc::Arc",
);
} }
#[test] #[test]
@ -763,9 +794,7 @@ mod tests {
pub use u8; pub use u8;
} }
"#; "#;
check_found_path(code, "u8"); check_found_path(code, "u8", "u8", "u8", "u8");
check_found_path(code, "u16"); check_found_path(code, "u16", "u16", "u16", "u16");
check_found_path_abs(code, "u8");
check_found_path_abs(code, "u16");
} }
} }

View File

@ -10,6 +10,7 @@
use std::{ffi::OsString, path::PathBuf}; use std::{ffi::OsString, path::PathBuf};
use flycheck::FlycheckConfig; use flycheck::FlycheckConfig;
use hir::PrefixKind;
use ide::{ use ide::{
AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, InlayHintsConfig, AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, InlayHintsConfig,
MergeBehaviour, MergeBehaviour,
@ -289,6 +290,11 @@ impl Config {
MergeBehaviourDef::Full => Some(MergeBehaviour::Full), MergeBehaviourDef::Full => Some(MergeBehaviour::Full),
MergeBehaviourDef::Last => Some(MergeBehaviour::Last), MergeBehaviourDef::Last => Some(MergeBehaviour::Last),
}; };
self.assist.insert_use.prefix_kind = match data.assist_importPrefix {
ImportPrefixDef::Plain => PrefixKind::Plain,
ImportPrefixDef::ByCrate => PrefixKind::ByCrate,
ImportPrefixDef::BySelf => PrefixKind::BySelf,
};
self.call_info_full = data.callInfo_full; self.call_info_full = data.callInfo_full;
@ -403,13 +409,21 @@ enum ManifestOrProjectJson {
} }
#[derive(Deserialize)] #[derive(Deserialize)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "snake_case")]
enum MergeBehaviourDef { enum MergeBehaviourDef {
None, None,
Full, Full,
Last, Last,
} }
#[derive(Deserialize)]
#[serde(rename_all = "snake_case")]
enum ImportPrefixDef {
Plain,
BySelf,
ByCrate,
}
macro_rules! config_data { macro_rules! config_data {
(struct $name:ident { $($field:ident: $ty:ty = $default:expr,)*}) => { (struct $name:ident { $($field:ident: $ty:ty = $default:expr,)*}) => {
#[allow(non_snake_case)] #[allow(non_snake_case)]
@ -434,6 +448,7 @@ macro_rules! config_data {
config_data! { config_data! {
struct ConfigData { struct ConfigData {
assist_importMergeBehaviour: MergeBehaviourDef = MergeBehaviourDef::None, assist_importMergeBehaviour: MergeBehaviourDef = MergeBehaviourDef::None,
assist_importPrefix: ImportPrefixDef = ImportPrefixDef::Plain,
callInfo_full: bool = true, callInfo_full: bool = true,

View File

@ -652,6 +652,21 @@
"default": "full", "default": "full",
"description": "The strategy to use when inserting new imports or merging imports." "description": "The strategy to use when inserting new imports or merging imports."
}, },
"rust-analyzer.assist.importPrefix": {
"type": "string",
"enum": [
"plain",
"by_self",
"by_crate"
],
"enumDescriptions": [
"Insert import paths relative to the current module, using up to one `super` prefix if the parent module contains the requested item.",
"Prefix all import paths with `self` if they don't begin with `self`, `super`, `crate` or a crate name",
"Force import paths to be absolute by always starting them with `crate` or the crate name they refer to."
],
"default": "plain",
"description": "The path structure for newly inserted paths to use."
},
"rust-analyzer.runnables.overrideCargo": { "rust-analyzer.runnables.overrideCargo": {
"type": [ "type": [
"null", "null",
@ -1033,4 +1048,4 @@
] ]
} }
} }
} }