diff --git a/Cargo.lock b/Cargo.lock index 83e7c0eebbd8..996864291835 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -754,6 +754,26 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b23360e99b8717f20aaa4598f5a6541efbe30630039fbc7706cf954a87947ae" +[[package]] +name = "kqueue" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "058a107a784f8be94c7d35c1300f4facced2e93d2fbe5b1452b44e905ddca4a9" +dependencies = [ + "kqueue-sys", + "libc", +] + +[[package]] +name = "kqueue-sys" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8367585489f01bc55dd27404dcf56b95e6da061a256a666ab23be9ba96a2e587" +dependencies = [ + "bitflags", + "libc", +] + [[package]] name = "la-arena" version = "0.2.1" @@ -934,15 +954,16 @@ dependencies = [ [[package]] name = "notify" -version = "5.0.0-pre.10" +version = "5.0.0-pre.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51f18203a26893ca1d3526cf58084025d5639f91c44f8b70ab3b724f60e819a0" +checksum = "20a629259bb2c87a884bb76f6086c8637919de6d074754341c12e5dd3aed6326" dependencies = [ "bitflags", "crossbeam-channel", "filetime", "fsevent-sys", "inotify", + "kqueue", "libc", "mio", "walkdir", diff --git a/crates/base_db/src/input.rs b/crates/base_db/src/input.rs index 3abe570336cf..671039e46bcd 100644 --- a/crates/base_db/src/input.rs +++ b/crates/base_db/src/input.rs @@ -247,12 +247,17 @@ impl CrateGraph { to: CrateId, ) -> Result<(), CyclicDependenciesError> { let _p = profile::span("add_dep"); - if self.dfs_find(from, to, &mut FxHashSet::default()) { - return Err(CyclicDependenciesError { - from: (from, self[from].display_name.clone()), - to: (to, self[to].display_name.clone()), - }); + + // Check if adding a dep from `from` to `to` creates a cycle. To figure + // that out, look for a path in the *opposite* direction, from `to` to + // `from`. + if let Some(path) = self.find_path(&mut FxHashSet::default(), to, from) { + let path = path.into_iter().map(|it| (it, self[it].display_name.clone())).collect(); + let err = CyclicDependenciesError { path }; + assert!(err.from().0 == from && err.to().0 == to); + return Err(err); } + self.arena.get_mut(&from).unwrap().add_dep(name, to); Ok(()) } @@ -361,22 +366,29 @@ impl CrateGraph { start } - fn dfs_find(&self, target: CrateId, from: CrateId, visited: &mut FxHashSet) -> bool { + fn find_path( + &self, + visited: &mut FxHashSet, + from: CrateId, + to: CrateId, + ) -> Option> { if !visited.insert(from) { - return false; + return None; } - if target == from { - return true; + if from == to { + return Some(vec![to]); } for dep in &self[from].dependencies { let crate_id = dep.crate_id; - if self.dfs_find(target, crate_id, visited) { - return true; + if let Some(mut path) = self.find_path(visited, crate_id, to) { + path.push(from); + return Some(path); } } - false + + None } // Work around for https://github.com/rust-analyzer/rust-analyzer/issues/6038. @@ -481,8 +493,16 @@ impl std::error::Error for ParseEditionError {} #[derive(Debug)] pub struct CyclicDependenciesError { - from: (CrateId, Option), - to: (CrateId, Option), + path: Vec<(CrateId, Option)>, +} + +impl CyclicDependenciesError { + fn from(&self) -> &(CrateId, Option) { + self.path.first().unwrap() + } + fn to(&self) -> &(CrateId, Option) { + self.path.last().unwrap() + } } impl fmt::Display for CyclicDependenciesError { @@ -491,7 +511,14 @@ impl fmt::Display for CyclicDependenciesError { Some(it) => format!("{}({:?})", it, id), None => format!("{:?}", id), }; - write!(f, "cyclic deps: {} -> {}", render(&self.from), render(&self.to)) + let path = self.path.iter().rev().map(render).collect::>().join(" -> "); + write!( + f, + "cyclic deps: {} -> {}, alternative path: {}", + render(&self.from()), + render(&self.to()), + path + ) } } diff --git a/crates/hir/src/semantics/source_to_def.rs b/crates/hir/src/semantics/source_to_def.rs index 93b78a1a16c7..773eab79315c 100644 --- a/crates/hir/src/semantics/source_to_def.rs +++ b/crates/hir/src/semantics/source_to_def.rs @@ -95,7 +95,7 @@ use hir_def::{ GenericDefId, ImplId, LifetimeParamId, ModuleId, StaticId, StructId, TraitId, TypeAliasId, TypeParamId, UnionId, VariantId, }; -use hir_expand::{name::AsName, AstId, MacroCallId, MacroDefId, MacroDefKind}; +use hir_expand::{name::AsName, AstId, HirFileId, MacroCallId, MacroDefId, MacroDefKind}; use rustc_hash::FxHashMap; use smallvec::SmallVec; use stdx::impl_from; @@ -106,7 +106,7 @@ use syntax::{ use crate::{db::HirDatabase, InFile}; -pub(super) type SourceToDefCache = FxHashMap; +pub(super) type SourceToDefCache = FxHashMap<(ChildContainer, HirFileId), DynMap>; pub(super) struct SourceToDefCtx<'a, 'b> { pub(super) db: &'b dyn HirDatabase, @@ -131,13 +131,8 @@ impl SourceToDefCtx<'_, '_> { pub(super) fn module_to_def(&mut self, src: InFile) -> Option { let _p = profile::span("module_to_def"); - let parent_declaration = src - .as_ref() - .map(|it| it.syntax()) - .cloned() - .ancestors_with_macros(self.db.upcast()) - .skip(1) - .find_map(|it| { + let parent_declaration = + src.syntax().cloned().ancestors_with_macros(self.db.upcast()).skip(1).find_map(|it| { let m = ast::Module::cast(it.value.clone())?; Some(it.with_value(m)) }); @@ -217,7 +212,7 @@ impl SourceToDefCtx<'_, '_> { &mut self, src: InFile, ) -> Option<(DefWithBodyId, PatId)> { - let container = self.find_pat_or_label_container(src.as_ref().map(|it| it.syntax()))?; + let container = self.find_pat_or_label_container(src.syntax())?; let (_body, source_map) = self.db.body_with_source_map(container); let src = src.map(ast::Pat::from); let pat_id = source_map.node_pat(src.as_ref())?; @@ -227,7 +222,7 @@ impl SourceToDefCtx<'_, '_> { &mut self, src: InFile, ) -> Option<(DefWithBodyId, PatId)> { - let container = self.find_pat_or_label_container(src.as_ref().map(|it| it.syntax()))?; + let container = self.find_pat_or_label_container(src.syntax())?; let (_body, source_map) = self.db.body_with_source_map(container); let pat_id = source_map.node_self_param(src.as_ref())?; Some((container, pat_id)) @@ -236,7 +231,7 @@ impl SourceToDefCtx<'_, '_> { &mut self, src: InFile, ) -> Option<(DefWithBodyId, LabelId)> { - let container = self.find_pat_or_label_container(src.as_ref().map(|it| it.syntax()))?; + let container = self.find_pat_or_label_container(src.syntax())?; let (_body, source_map) = self.db.body_with_source_map(container); let label_id = source_map.node_label(src.as_ref())?; Some((container, label_id)) @@ -257,18 +252,19 @@ impl SourceToDefCtx<'_, '_> { fn dyn_map(&mut self, src: InFile<&Ast>) -> Option<&DynMap> { let container = self.find_container(src.map(|it| it.syntax()))?; + Some(self.cache_for(container, src.file_id)) + } + + fn cache_for(&mut self, container: ChildContainer, file_id: HirFileId) -> &DynMap { let db = self.db; - let dyn_map = - &*self.cache.entry(container).or_insert_with(|| container.child_by_source(db)); - Some(dyn_map) + self.cache + .entry((container, file_id)) + .or_insert_with(|| container.child_by_source(db, file_id)) } pub(super) fn type_param_to_def(&mut self, src: InFile) -> Option { - let container: ChildContainer = - self.find_generic_param_container(src.as_ref().map(|it| it.syntax()))?.into(); - let db = self.db; - let dyn_map = - &*self.cache.entry(container).or_insert_with(|| container.child_by_source(db)); + let container: ChildContainer = self.find_generic_param_container(src.syntax())?.into(); + let dyn_map = self.cache_for(container, src.file_id); dyn_map[keys::TYPE_PARAM].get(&src).copied() } @@ -276,11 +272,8 @@ impl SourceToDefCtx<'_, '_> { &mut self, src: InFile, ) -> Option { - let container: ChildContainer = - self.find_generic_param_container(src.as_ref().map(|it| it.syntax()))?.into(); - let db = self.db; - let dyn_map = - &*self.cache.entry(container).or_insert_with(|| container.child_by_source(db)); + let container: ChildContainer = self.find_generic_param_container(src.syntax())?.into(); + let dyn_map = self.cache_for(container, src.file_id); dyn_map[keys::LIFETIME_PARAM].get(&src).copied() } @@ -288,11 +281,8 @@ impl SourceToDefCtx<'_, '_> { &mut self, src: InFile, ) -> Option { - let container: ChildContainer = - self.find_generic_param_container(src.as_ref().map(|it| it.syntax()))?.into(); - let db = self.db; - let dyn_map = - &*self.cache.entry(container).or_insert_with(|| container.child_by_source(db)); + let container: ChildContainer = self.find_generic_param_container(src.syntax())?.into(); + let dyn_map = self.cache_for(container, src.file_id); dyn_map[keys::CONST_PARAM].get(&src).copied() } @@ -430,17 +420,17 @@ impl_from! { } impl ChildContainer { - fn child_by_source(self, db: &dyn HirDatabase) -> DynMap { + fn child_by_source(self, db: &dyn HirDatabase, file_id: HirFileId) -> DynMap { let db = db.upcast(); match self { - ChildContainer::DefWithBodyId(it) => it.child_by_source(db), - ChildContainer::ModuleId(it) => it.child_by_source(db), - ChildContainer::TraitId(it) => it.child_by_source(db), - ChildContainer::ImplId(it) => it.child_by_source(db), - ChildContainer::EnumId(it) => it.child_by_source(db), - ChildContainer::VariantId(it) => it.child_by_source(db), + ChildContainer::DefWithBodyId(it) => it.child_by_source(db, file_id), + ChildContainer::ModuleId(it) => it.child_by_source(db, file_id), + ChildContainer::TraitId(it) => it.child_by_source(db, file_id), + ChildContainer::ImplId(it) => it.child_by_source(db, file_id), + ChildContainer::EnumId(it) => it.child_by_source(db, file_id), + ChildContainer::VariantId(it) => it.child_by_source(db, file_id), ChildContainer::TypeAliasId(_) => DynMap::default(), - ChildContainer::GenericDefId(it) => it.child_by_source(db), + ChildContainer::GenericDefId(it) => it.child_by_source(db, file_id), } } } diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index cb6fd69bc600..b6ee5968fa2f 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -129,7 +129,7 @@ impl SourceAnalyzer { .and_then(|adjusts| adjusts.last().map(|adjust| adjust.target.clone())); let ty = infer[expr_id].clone(); let mk_ty = |ty| Type::new_with_resolver(db, &self.resolver, ty); - mk_ty(ty.clone()).zip(Some(coerced.and_then(mk_ty))) + mk_ty(ty).zip(Some(coerced.and_then(mk_ty))) } pub(crate) fn type_of_pat( @@ -145,7 +145,7 @@ impl SourceAnalyzer { .and_then(|adjusts| adjusts.last().map(|adjust| adjust.target.clone())); let ty = infer[pat_id].clone(); let mk_ty = |ty| Type::new_with_resolver(db, &self.resolver, ty); - mk_ty(ty.clone()).zip(Some(coerced.and_then(mk_ty))) + mk_ty(ty).zip(Some(coerced.and_then(mk_ty))) } pub(crate) fn type_of_self( diff --git a/crates/hir_def/src/child_by_source.rs b/crates/hir_def/src/child_by_source.rs index f22383e221d9..1a2b47cd030c 100644 --- a/crates/hir_def/src/child_by_source.rs +++ b/crates/hir_def/src/child_by_source.rs @@ -5,6 +5,7 @@ //! node for a *child*, and get its hir. use either::Either; +use hir_expand::HirFileId; use crate::{ db::DefDatabase, @@ -17,30 +18,39 @@ use crate::{ }; pub trait ChildBySource { - fn child_by_source(&self, db: &dyn DefDatabase) -> DynMap { + fn child_by_source(&self, db: &dyn DefDatabase, file_id: HirFileId) -> DynMap { let mut res = DynMap::default(); - self.child_by_source_to(db, &mut res); + self.child_by_source_to(db, &mut res, file_id); res } - fn child_by_source_to(&self, db: &dyn DefDatabase, map: &mut DynMap); + fn child_by_source_to(&self, db: &dyn DefDatabase, map: &mut DynMap, file_id: HirFileId); } impl ChildBySource for TraitId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { let data = db.trait_data(*self); for (_name, item) in data.items.iter() { match *item { AssocItemId::FunctionId(func) => { - let src = func.lookup(db).source(db); - res[keys::FUNCTION].insert(src, func) + let loc = func.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + res[keys::FUNCTION].insert(src, func) + } } AssocItemId::ConstId(konst) => { - let src = konst.lookup(db).source(db); - res[keys::CONST].insert(src, konst) + let loc = konst.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + res[keys::CONST].insert(src, konst) + } } AssocItemId::TypeAliasId(ty) => { - let src = ty.lookup(db).source(db); - res[keys::TYPE_ALIAS].insert(src, ty) + let loc = ty.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + res[keys::TYPE_ALIAS].insert(src, ty) + } } } } @@ -48,21 +58,30 @@ impl ChildBySource for TraitId { } impl ChildBySource for ImplId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { let data = db.impl_data(*self); for &item in data.items.iter() { match item { AssocItemId::FunctionId(func) => { - let src = func.lookup(db).source(db); - res[keys::FUNCTION].insert(src, func) + let loc = func.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + res[keys::FUNCTION].insert(src, func) + } } AssocItemId::ConstId(konst) => { - let src = konst.lookup(db).source(db); - res[keys::CONST].insert(src, konst) + let loc = konst.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + res[keys::CONST].insert(src, konst) + } } AssocItemId::TypeAliasId(ty) => { - let src = ty.lookup(db).source(db); - res[keys::TYPE_ALIAS].insert(src, ty) + let loc = ty.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + res[keys::TYPE_ALIAS].insert(src, ty) + } } } } @@ -70,84 +89,117 @@ impl ChildBySource for ImplId { } impl ChildBySource for ModuleId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { let def_map = self.def_map(db); let module_data = &def_map[self.local_id]; - module_data.scope.child_by_source_to(db, res); + module_data.scope.child_by_source_to(db, res, file_id); } } impl ChildBySource for ItemScope { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { - self.declarations().for_each(|item| add_module_def(db, res, item)); + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { + self.declarations().for_each(|item| add_module_def(db, file_id, res, item)); self.unnamed_consts().for_each(|konst| { let src = konst.lookup(db).source(db); res[keys::CONST].insert(src, konst); }); - self.impls().for_each(|imp| add_impl(db, res, imp)); + self.impls().for_each(|imp| add_impl(db, file_id, res, imp)); self.attr_macro_invocs().for_each(|(ast_id, call_id)| { let item = ast_id.with_value(ast_id.to_node(db.upcast())); res[keys::ATTR_MACRO].insert(item, call_id); }); - fn add_module_def(db: &dyn DefDatabase, map: &mut DynMap, item: ModuleDefId) { + fn add_module_def( + db: &dyn DefDatabase, + file_id: HirFileId, + map: &mut DynMap, + item: ModuleDefId, + ) { match item { ModuleDefId::FunctionId(func) => { - let src = func.lookup(db).source(db); - map[keys::FUNCTION].insert(src, func) + let loc = func.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::FUNCTION].insert(src, func) + } } ModuleDefId::ConstId(konst) => { - let src = konst.lookup(db).source(db); - map[keys::CONST].insert(src, konst) + let loc = konst.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::CONST].insert(src, konst) + } } ModuleDefId::StaticId(statik) => { - let src = statik.lookup(db).source(db); - map[keys::STATIC].insert(src, statik) + let loc = statik.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::STATIC].insert(src, statik) + } } ModuleDefId::TypeAliasId(ty) => { - let src = ty.lookup(db).source(db); - map[keys::TYPE_ALIAS].insert(src, ty) + let loc = ty.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::TYPE_ALIAS].insert(src, ty) + } } ModuleDefId::TraitId(trait_) => { - let src = trait_.lookup(db).source(db); - map[keys::TRAIT].insert(src, trait_) + let loc = trait_.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::TRAIT].insert(src, trait_) + } } ModuleDefId::AdtId(adt) => match adt { AdtId::StructId(strukt) => { - let src = strukt.lookup(db).source(db); - map[keys::STRUCT].insert(src, strukt) + let loc = strukt.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::STRUCT].insert(src, strukt) + } } AdtId::UnionId(union_) => { - let src = union_.lookup(db).source(db); - map[keys::UNION].insert(src, union_) + let loc = union_.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::UNION].insert(src, union_) + } } AdtId::EnumId(enum_) => { - let src = enum_.lookup(db).source(db); - map[keys::ENUM].insert(src, enum_) + let loc = enum_.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::ENUM].insert(src, enum_) + } } }, _ => (), } } - fn add_impl(db: &dyn DefDatabase, map: &mut DynMap, imp: ImplId) { - let src = imp.lookup(db).source(db); - map[keys::IMPL].insert(src, imp) + fn add_impl(db: &dyn DefDatabase, file_id: HirFileId, map: &mut DynMap, imp: ImplId) { + let loc = imp.lookup(db); + if loc.id.file_id() == file_id { + let src = loc.source(db); + map[keys::IMPL].insert(src, imp) + } } } } impl ChildBySource for VariantId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, _: HirFileId) { let arena_map = self.child_source(db); let arena_map = arena_map.as_ref(); + let parent = *self; for (local_id, source) in arena_map.value.iter() { - let id = FieldId { parent: *self, local_id }; - match source { + let id = FieldId { parent, local_id }; + match source.clone() { Either::Left(source) => { - res[keys::TUPLE_FIELD].insert(arena_map.with_value(source.clone()), id) + res[keys::TUPLE_FIELD].insert(arena_map.with_value(source), id) } Either::Right(source) => { - res[keys::RECORD_FIELD].insert(arena_map.with_value(source.clone()), id) + res[keys::RECORD_FIELD].insert(arena_map.with_value(source), id) } } } @@ -155,7 +207,7 @@ impl ChildBySource for VariantId { } impl ChildBySource for EnumId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, _: HirFileId) { let arena_map = self.child_source(db); let arena_map = arena_map.as_ref(); for (local_id, source) in arena_map.value.iter() { @@ -166,12 +218,12 @@ impl ChildBySource for EnumId { } impl ChildBySource for DefWithBodyId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, file_id: HirFileId) { let body = db.body(*self); for (_, def_map) in body.blocks(db) { // All block expressions are merged into the same map, because they logically all add // inner items to the containing `DefWithBodyId`. - def_map[def_map.root()].scope.child_by_source_to(db, res); + def_map[def_map.root()].scope.child_by_source_to(db, res, file_id); } } } diff --git a/crates/hir_def/src/generics.rs b/crates/hir_def/src/generics.rs index 40104833db30..5425ea8beaf5 100644 --- a/crates/hir_def/src/generics.rs +++ b/crates/hir_def/src/generics.rs @@ -7,7 +7,7 @@ use base_db::FileId; use either::Either; use hir_expand::{ name::{name, AsName, Name}, - InFile, + HirFileId, InFile, }; use la_arena::{Arena, ArenaMap}; use syntax::ast::{self, GenericParamsOwner, NameOwner, TypeBoundsOwner}; @@ -438,7 +438,7 @@ impl HasChildSource for GenericDefId { } impl ChildBySource for GenericDefId { - fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap) { + fn child_by_source_to(&self, db: &dyn DefDatabase, res: &mut DynMap, _: HirFileId) { let (_, sm) = GenericParams::new(db, *self); let sm = sm.as_ref(); diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index dc6058dba590..73fefbaee620 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -272,7 +272,7 @@ fn macro_arg(db: &dyn AstDatabase, id: MacroCallId) -> Option Option { let loc = db.lookup_intern_macro(id); let arg = loc.kind.arg(db)?; - let arg = process_macro_input(db, arg, id); + let arg = process_macro_input(&loc.kind, arg); if matches!(loc.kind, MacroCallKind::FnLike { .. }) { let first = arg.first_child_or_token().map_or(T![.], |it| it.kind()); let last = arg.last_child_or_token().map_or(T![.], |it| it.kind()); @@ -336,35 +336,17 @@ fn macro_def(db: &dyn AstDatabase, id: MacroDefId) -> Option> } fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult>> { - macro_expand_with_arg(db, id, None) -} - -fn macro_expand_error(db: &dyn AstDatabase, macro_call: MacroCallId) -> Option { - db.macro_expand(macro_call).err -} - -fn macro_expand_with_arg( - db: &dyn AstDatabase, - id: MacroCallId, - arg: Option>, -) -> ExpandResult>> { let _p = profile::span("macro_expand"); let loc: MacroCallLoc = db.lookup_intern_macro(id); if let Some(eager) = &loc.eager { - if arg.is_some() { - return ExpandResult::str_err( - "speculative macro expansion not implemented for eager macro".to_owned(), - ); - } else { - return ExpandResult { - value: Some(eager.arg_or_expansion.clone()), - // FIXME: There could be errors here! - err: None, - }; - } + return ExpandResult { + value: Some(eager.arg_or_expansion.clone()), + // FIXME: There could be errors here! + err: None, + }; } - let macro_arg = match arg.or_else(|| db.macro_arg(id)) { + let macro_arg = match db.macro_arg(id) { Some(it) => it, None => return ExpandResult::str_err("Fail to args in to tt::TokenTree".into()), }; @@ -388,6 +370,10 @@ fn macro_expand_with_arg( ExpandResult { value: Some(Arc::new(tt)), err } } +fn macro_expand_error(db: &dyn AstDatabase, macro_call: MacroCallId) -> Option { + db.macro_expand(macro_call).err +} + fn expand_proc_macro( db: &dyn AstDatabase, id: MacroCallId, diff --git a/crates/hir_expand/src/input.rs b/crates/hir_expand/src/input.rs index bc3ecc59301a..55935ed3dd70 100644 --- a/crates/hir_expand/src/input.rs +++ b/crates/hir_expand/src/input.rs @@ -6,19 +6,12 @@ use syntax::{ }; use crate::{ - db::AstDatabase, name::{name, AsName}, - MacroCallId, MacroCallKind, MacroCallLoc, + MacroCallKind, }; -pub(crate) fn process_macro_input( - db: &dyn AstDatabase, - node: SyntaxNode, - id: MacroCallId, -) -> SyntaxNode { - let loc: MacroCallLoc = db.lookup_intern_macro(id); - - match loc.kind { +pub(crate) fn process_macro_input(macro_call_kind: &MacroCallKind, node: SyntaxNode) -> SyntaxNode { + match macro_call_kind { MacroCallKind::FnLike { .. } => node, MacroCallKind::Derive { derive_attr_index, .. } => { let item = match ast::Item::cast(node.clone()) { @@ -26,7 +19,7 @@ pub(crate) fn process_macro_input( None => return node, }; - remove_derives_up_to(item, derive_attr_index as usize).syntax().clone() + remove_derives_up_to(item, *derive_attr_index as usize).syntax().clone() } MacroCallKind::Attr { invoc_attr_index, .. } => { let item = match ast::Item::cast(node.clone()) { @@ -34,7 +27,7 @@ pub(crate) fn process_macro_input( None => return node, }; - remove_attr_invoc(item, invoc_attr_index as usize).syntax().clone() + remove_attr_invoc(item, *invoc_attr_index as usize).syntax().clone() } } } diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 461be80807cd..6c8438398755 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -22,8 +22,7 @@ use either::Either; pub use mbe::{ExpandError, ExpandResult}; pub use parser::FragmentKind; -use std::hash::Hash; -use std::sync::Arc; +use std::{hash::Hash, sync::Arc}; use base_db::{impl_intern_key, salsa, CrateId, FileId, FileRange}; use syntax::{ @@ -32,11 +31,13 @@ use syntax::{ Direction, SyntaxNode, SyntaxToken, TextRange, TextSize, }; -use crate::ast_id_map::FileAstId; -use crate::builtin_attr::BuiltinAttrExpander; -use crate::builtin_derive::BuiltinDeriveExpander; -use crate::builtin_macro::{BuiltinFnLikeExpander, EagerExpander}; -use crate::proc_macro::ProcMacroExpander; +use crate::{ + ast_id_map::FileAstId, + builtin_attr::BuiltinAttrExpander, + builtin_derive::BuiltinDeriveExpander, + builtin_macro::{BuiltinFnLikeExpander, EagerExpander}, + proc_macro::ProcMacroExpander, +}; #[cfg(test)] mod test_db; @@ -210,12 +211,12 @@ impl MacroDefId { pub fn ast_id(&self) -> Either, AstId> { let id = match &self.kind { - MacroDefKind::Declarative(id) => id, - MacroDefKind::BuiltIn(_, id) => id, - MacroDefKind::BuiltInAttr(_, id) => id, - MacroDefKind::BuiltInDerive(_, id) => id, - MacroDefKind::BuiltInEager(_, id) => id, MacroDefKind::ProcMacro(.., id) => return Either::Right(*id), + MacroDefKind::Declarative(id) + | MacroDefKind::BuiltIn(_, id) + | MacroDefKind::BuiltInAttr(_, id) + | MacroDefKind::BuiltInDerive(_, id) + | MacroDefKind::BuiltInEager(_, id) => id, }; Either::Left(*id) } @@ -376,7 +377,7 @@ impl ExpansionInfo { db::TokenExpander::MacroRules { def_site_token_map, .. } | db::TokenExpander::MacroDef { def_site_token_map, .. }, Some(tt), - ) => (def_site_token_map, tt.as_ref().map(|tt| tt.syntax().clone())), + ) => (def_site_token_map, tt.syntax().cloned()), _ => panic!("`Origin::Def` used with non-`macro_rules!` macro"), }, }; @@ -464,15 +465,10 @@ impl InFile { } impl<'a> InFile<&'a SyntaxNode> { + /// Falls back to the macro call range if the node cannot be mapped up fully. pub fn original_file_range(self, db: &dyn db::AstDatabase) -> FileRange { - if let Some(range) = original_range_opt(db, self) { - let original_file = range.file_id.original_file(db); - if range.file_id == original_file.into() { - return FileRange { file_id: original_file, range: range.value }; - } - - tracing::error!("Fail to mapping up more for {:?}", range); - return FileRange { file_id: range.file_id.original_file(db), range: range.value }; + if let Some(res) = self.original_file_range_opt(db) { + return res; } // Fall back to whole macro call. @@ -483,8 +479,27 @@ impl<'a> InFile<&'a SyntaxNode> { let orig_file = node.file_id.original_file(db); assert_eq!(node.file_id, orig_file.into()); + FileRange { file_id: orig_file, range: node.value.text_range() } } + + /// Attempts to map the syntax node back up its macro calls. + pub fn original_file_range_opt(self, db: &dyn db::AstDatabase) -> Option { + match original_range_opt(db, self) { + Some(range) => { + let original_file = range.file_id.original_file(db); + if range.file_id != original_file.into() { + tracing::error!("Failed mapping up more for {:?}", range); + } + Some(FileRange { file_id: original_file, range: range.value }) + } + _ if !self.file_id.is_macro() => Some(FileRange { + file_id: self.file_id.original_file(db), + range: self.value.text_range(), + }), + _ => None, + } + } } fn original_range_opt( diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs index e94533041784..4a0c0fcca9c2 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -92,7 +92,7 @@ impl NavigationTarget { pub(crate) fn from_module_to_decl(db: &RootDatabase, module: hir::Module) -> NavigationTarget { let name = module.name(db).map(|it| it.to_string().into()).unwrap_or_default(); if let Some(src) = module.declaration_source(db) { - let node = src.as_ref().map(|it| it.syntax()); + let node = src.syntax(); let full_range = node.original_file_range(db); let focus_range = src .value @@ -143,8 +143,11 @@ impl NavigationTarget { kind: SymbolKind, ) -> NavigationTarget { let name = node.value.name().map(|it| it.text().into()).unwrap_or_else(|| "_".into()); - let focus_range = - node.value.name().map(|it| node.with_value(it.syntax()).original_file_range(db).range); + let focus_range = node + .value + .name() + .and_then(|it| node.with_value(it.syntax()).original_file_range_opt(db)) + .map(|it| it.range); let frange = node.map(|it| it.syntax()).original_file_range(db); NavigationTarget::from_syntax(frange.file_id, name, focus_range, frange.range, kind) @@ -298,7 +301,7 @@ impl TryToNav for hir::Impl { let frange = if let Some(item) = &derive_attr { item.syntax().original_file_range(db) } else { - src.as_ref().map(|it| it.syntax()).original_file_range(db) + src.syntax().original_file_range(db) }; let focus_range = if derive_attr.is_some() { None diff --git a/crates/ide/src/highlight_related.rs b/crates/ide/src/highlight_related.rs index b4715b10a065..bd0d261ba999 100644 --- a/crates/ide/src/highlight_related.rs +++ b/crates/ide/src/highlight_related.rs @@ -31,8 +31,10 @@ pub struct HighlightRelatedConfig { // Highlights constructs related to the thing under the cursor: // - if on an identifier, highlights all references to that identifier in the current file // - if on an `async` or `await token, highlights all yield points for that async context -// - if on a `return` token, `?` character or `->` return type arrow, highlights all exit points for that context +// - if on a `return` or `fn` keyword, `?` character or `->` return type arrow, highlights all exit points for that context // - if on a `break`, `loop`, `while` or `for` token, highlights all break points for that loop or block context +// +// Note: `?` and `->` do not currently trigger this behavior in the VSCode editor. pub(crate) fn highlight_related( sema: &Semantics, config: HighlightRelatedConfig, @@ -42,24 +44,22 @@ pub(crate) fn highlight_related( let syntax = sema.parse(position.file_id).syntax().clone(); let token = pick_best_token(syntax.token_at_offset(position.offset), |kind| match kind { - T![?] => 2, // prefer `?` when the cursor is sandwiched like `await$0?` - T![await] - | T![async] - | T![return] - | T![break] - | T![loop] - | T![for] - | T![while] - | T![->] => 1, + T![?] => 3, // prefer `?` when the cursor is sandwiched like in `await$0?` + T![->] => 2, + kind if kind.is_keyword() => 1, _ => 0, })?; match token.kind() { - T![return] | T![?] | T![->] if config.exit_points => highlight_exit_points(sema, token), + T![?] if config.exit_points && token.parent().and_then(ast::TryExpr::cast).is_some() => { + highlight_exit_points(sema, token) + } + T![fn] | T![return] | T![->] if config.exit_points => highlight_exit_points(sema, token), T![await] | T![async] if config.yield_points => highlight_yield_points(token), - T![break] | T![loop] | T![for] | T![while] if config.break_points => { + T![for] if config.break_points && token.parent().and_then(ast::ForExpr::cast).is_some() => { highlight_break_points(token) } + T![break] | T![loop] | T![while] if config.break_points => highlight_break_points(token), _ if config.references => highlight_references(sema, &syntax, position), _ => None, } @@ -474,6 +474,25 @@ fn foo() ->$0 u32 { ); } + #[test] + fn test_hl_exit_points3() { + check( + r#" +fn$0 foo() -> u32 { + if true { + return 0; + // ^^^^^^ + } + + 0?; + // ^ + 0xDEAD_BEEF + // ^^^^^^^^^^^ +} +"#, + ); + } + #[test] fn test_hl_prefer_ref_over_tail_exit() { check( diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index f11eaaa546da..09e6156dd5d3 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -117,8 +117,6 @@ pub(crate) fn hover( let node = token.parent()?; let definition = match_ast! { match node { - // We don't use NameClass::referenced_or_defined here as we do not want to resolve - // field pattern shorthands to their definition. ast::Name(name) => NameClass::classify(&sema, &name).map(|class| match class { NameClass::Definition(it) | NameClass::ConstReference(it) => it, NameClass::PatFieldShorthand { local_def, field_ref: _ } => Definition::Local(local_def), @@ -139,6 +137,7 @@ pub(crate) fn hover( NameClass::defined, ), _ => { + // intra-doc links if ast::Comment::cast(token.clone()).is_some() { cov_mark::hit!(no_highlight_on_comment_hover); let (attributes, def) = doc_attributes(&sema, &node)?; @@ -153,9 +152,12 @@ pub(crate) fn hover( Either::Left(it) => Definition::ModuleDef(it), Either::Right(it) => Definition::Macro(it), }) + // attributes, require special machinery as they are mere ident tokens } else if let Some(attr) = token.ancestors().find_map(ast::Attr::cast) { + // lints if let res@Some(_) = try_hover_for_lint(&attr, &token) { return res; + // derives } else { range_override = Some(token.text_range()); try_resolve_derive_input_at(&sema, &attr, &token).map(Definition::Macro) @@ -276,7 +278,7 @@ fn hover_type_info( "```text\nType: {:>apad$}\nCoerced to: {:>opad$}\n```\n", uncoerced = original, coerced = adjusted, - // 6 base padding for static text prefix of each line + // 6 base padding for difference of length of the two text prefixes apad = 6 + adjusted.len().max(original.len()), opad = original.len(), ) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index d0a49d1f37a9..a4297a2fec55 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -274,6 +274,7 @@ mod tests { use super::{RangeInfo, RenameError}; + #[track_caller] fn check(new_name: &str, ra_fixture_before: &str, ra_fixture_after: &str) { let ra_fixture_after = &trim_indent(ra_fixture_after); let (analysis, position) = fixture::position(ra_fixture_before); @@ -1332,9 +1333,71 @@ fn foo(foo: Foo) { struct Foo { baz: i32 } fn foo(foo: Foo) { - let Foo { ref baz @ qux } = foo; + let Foo { baz: ref baz @ qux } = foo; let _ = qux; } +"#, + ); + check( + "baz", + r#" +struct Foo { i$0: i32 } + +fn foo(foo: Foo) { + let Foo { i: ref baz } = foo; + let _ = qux; +} +"#, + r#" +struct Foo { baz: i32 } + +fn foo(foo: Foo) { + let Foo { ref baz } = foo; + let _ = qux; +} +"#, + ); + } + + #[test] + fn test_struct_local_pat_into_shorthand() { + cov_mark::check!(test_rename_local_put_init_shorthand_pat); + check( + "field", + r#" +struct Foo { field: i32 } + +fn foo(foo: Foo) { + let Foo { field: qux$0 } = foo; + let _ = qux; +} +"#, + r#" +struct Foo { field: i32 } + +fn foo(foo: Foo) { + let Foo { field } = foo; + let _ = field; +} +"#, + ); + check( + "field", + r#" +struct Foo { field: i32 } + +fn foo(foo: Foo) { + let Foo { field: x @ qux$0 } = foo; + let _ = qux; +} +"#, + r#" +struct Foo { field: i32 } + +fn foo(foo: Foo) { + let Foo { field: x @ field } = foo; + let _ = field; +} "#, ); } @@ -1390,7 +1453,7 @@ struct Foo { i: i32 } -fn foo(Foo { i }: foo) -> i32 { +fn foo(Foo { i }: Foo) -> i32 { i$0 } "#, @@ -1399,7 +1462,7 @@ struct Foo { i: i32 } -fn foo(Foo { i: bar }: foo) -> i32 { +fn foo(Foo { i: bar }: Foo) -> i32 { bar } "#, @@ -1408,6 +1471,7 @@ fn foo(Foo { i: bar }: foo) -> i32 { #[test] fn test_struct_field_complex_ident_pat() { + cov_mark::check!(rename_record_pat_field_name_split); check( "baz", r#" @@ -1805,8 +1869,7 @@ fn f() { <()>::BAR$0; }"#, } #[test] - fn macros_are_broken_lol() { - cov_mark::check!(macros_are_broken_lol); + fn defs_from_macros_arent_renamed() { check( "lol", r#" @@ -1814,11 +1877,7 @@ macro_rules! m { () => { fn f() {} } } m!(); fn main() { f$0() } "#, - r#" -macro_rules! m { () => { fn f() {} } } -lol -fn main() { lol() } -"#, + "error: No identifier available to rename", ) } } diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 2cf801751ca7..42f6ec5d9cf0 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -1446,7 +1446,6 @@ gen2!(); 0, ), full_range: 228..236, - focus_range: 228..236, name: "foo_test2", kind: Function, }, @@ -1467,7 +1466,6 @@ gen2!(); 0, ), full_range: 218..225, - focus_range: 218..225, name: "foo_test", kind: Function, }, @@ -1533,7 +1531,6 @@ foo!(); 0, ), full_range: 210..217, - focus_range: 210..217, name: "foo0", kind: Function, }, @@ -1554,7 +1551,6 @@ foo!(); 0, ), full_range: 210..217, - focus_range: 210..217, name: "foo1", kind: Function, }, @@ -1575,7 +1571,6 @@ foo!(); 0, ), full_range: 210..217, - focus_range: 210..217, name: "foo2", kind: Function, }, diff --git a/crates/ide_assists/src/handlers/destructure_tuple_binding.rs b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs new file mode 100644 index 000000000000..a421f5775467 --- /dev/null +++ b/crates/ide_assists/src/handlers/destructure_tuple_binding.rs @@ -0,0 +1,2148 @@ +use ide_db::{ + assists::{AssistId, AssistKind}, + defs::Definition, + search::{FileReference, SearchScope, UsageSearchResult}, +}; +use itertools::Itertools; +use syntax::{ + ast::{self, AstNode, FieldExpr, IdentPat, MethodCallExpr, NameOwner}, + TextRange, +}; + +use crate::assist_context::{AssistBuilder, AssistContext, Assists}; + +// Assist: destructure_tuple_binding +// +// Destructures a tuple binding in place. +// +// ``` +// fn main() { +// let $0t = (1,2); +// let v = t.0; +// } +// ``` +// -> +// ``` +// fn main() { +// let ($0_0, _1) = (1,2); +// let v = _0; +// } +// ``` +pub(crate) fn destructure_tuple_binding(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, false) +} + +// And when `with_sub_pattern` enabled (currently disabled): +// Assist: destructure_tuple_binding_in_sub_pattern +// +// Destructures tuple items in sub-pattern (after `@`). +// +// ``` +// fn main() { +// let $0t = (1,2); +// let v = t.0; +// } +// ``` +// -> +// ``` +// fn main() { +// let t @ ($0_0, _1) = (1,2); +// let v = _0; +// } +// ``` +pub(crate) fn destructure_tuple_binding_impl( + acc: &mut Assists, + ctx: &AssistContext, + with_sub_pattern: bool, +) -> Option<()> { + let ident_pat = ctx.find_node_at_offset::()?; + let data = collect_data(ident_pat, ctx)?; + + if with_sub_pattern { + acc.add( + AssistId("destructure_tuple_binding_in_sub_pattern", AssistKind::RefactorRewrite), + "Destructure tuple in sub-pattern", + data.range, + |builder| { + edit_tuple_assignment(ctx, builder, &data, true); + edit_tuple_usages(&data, builder, ctx, true); + }, + ); + } + + acc.add( + AssistId("destructure_tuple_binding", AssistKind::RefactorRewrite), + if with_sub_pattern { "Destructure tuple in place" } else { "Destructure tuple" }, + data.range, + |builder| { + edit_tuple_assignment(ctx, builder, &data, false); + edit_tuple_usages(&data, builder, ctx, false); + }, + ); + + Some(()) +} + +fn collect_data(ident_pat: IdentPat, ctx: &AssistContext) -> Option { + if ident_pat.at_token().is_some() { + // Cannot destructure pattern with sub-pattern: + // Only IdentPat can have sub-pattern, + // but not TuplePat (`(a,b)`). + cov_mark::hit!(destructure_tuple_subpattern); + return None; + } + + let ty = ctx.sema.type_of_pat(&ident_pat.clone().into())?.adjusted(); + let ref_type = if ty.is_mutable_reference() { + Some(RefType::Mutable) + } else if ty.is_reference() { + Some(RefType::ReadOnly) + } else { + None + }; + // might be reference + let ty = ty.strip_references(); + // must be tuple + let field_types = ty.tuple_fields(ctx.db()); + if field_types.is_empty() { + cov_mark::hit!(destructure_tuple_no_tuple); + return None; + } + + let name = ident_pat.name()?.to_string(); + let range = ident_pat.syntax().text_range(); + + let usages = ctx.sema.to_def(&ident_pat).map(|def| { + Definition::Local(def) + .usages(&ctx.sema) + .in_scope(SearchScope::single_file(ctx.frange.file_id)) + .all() + }); + + let field_names = (0..field_types.len()) + .map(|i| generate_name(ctx, i, &name, &ident_pat, &usages)) + .collect_vec(); + + Some(TupleData { ident_pat, range, ref_type, field_names, usages }) +} + +fn generate_name( + _ctx: &AssistContext, + index: usize, + _tuple_name: &str, + _ident_pat: &IdentPat, + _usages: &Option, +) -> String { + // FIXME: detect if name already used + format!("_{}", index) +} + +enum RefType { + ReadOnly, + Mutable, +} +struct TupleData { + ident_pat: IdentPat, + // name: String, + range: TextRange, + ref_type: Option, + field_names: Vec, + // field_types: Vec, + usages: Option, +} +fn edit_tuple_assignment( + ctx: &AssistContext, + builder: &mut AssistBuilder, + data: &TupleData, + in_sub_pattern: bool, +) { + let tuple_pat = { + let original = &data.ident_pat; + let is_ref = original.ref_token().is_some(); + let is_mut = original.mut_token().is_some(); + let fields = data.field_names.iter().map(|name| { + ast::Pat::from(ast::make::ident_pat(is_ref, is_mut, ast::make::name(name))) + }); + ast::make::tuple_pat(fields) + }; + + let add_cursor = |text: &str| { + // place cursor on first tuple item + let first_tuple = &data.field_names[0]; + text.replacen(first_tuple, &format!("$0{}", first_tuple), 1) + }; + + // with sub_pattern: keep original tuple and add subpattern: `tup @ (_0, _1)` + if in_sub_pattern { + let text = format!(" @ {}", tuple_pat.to_string()); + match ctx.config.snippet_cap { + Some(cap) => { + let snip = add_cursor(&text); + builder.insert_snippet(cap, data.range.end(), snip); + } + None => builder.insert(data.range.end(), text), + }; + } else { + let text = tuple_pat.to_string(); + match ctx.config.snippet_cap { + Some(cap) => { + let snip = add_cursor(&text); + builder.replace_snippet(cap, data.range, snip); + } + None => builder.replace(data.range, text), + }; + } +} + +fn edit_tuple_usages( + data: &TupleData, + builder: &mut AssistBuilder, + ctx: &AssistContext, + in_sub_pattern: bool, +) { + if let Some(usages) = data.usages.as_ref() { + for (file_id, refs) in usages.iter() { + builder.edit_file(*file_id); + + for r in refs { + edit_tuple_usage(ctx, builder, r, data, in_sub_pattern); + } + } + } +} +fn edit_tuple_usage( + ctx: &AssistContext, + builder: &mut AssistBuilder, + usage: &FileReference, + data: &TupleData, + in_sub_pattern: bool, +) { + match detect_tuple_index(usage, data) { + Some(index) => edit_tuple_field_usage(ctx, builder, data, index), + None => { + if in_sub_pattern { + cov_mark::hit!(destructure_tuple_call_with_subpattern); + return; + } + + // no index access -> make invalid -> requires handling by user + // -> put usage in block comment + // + // Note: For macro invocations this might result in still valid code: + // When a macro accepts the tuple as argument, as well as no arguments at all, + // uncommenting the tuple still leaves the macro call working (see `tests::in_macro_call::empty_macro`). + // But this is an unlikely case. Usually the resulting macro call will become erroneous. + builder.insert(usage.range.start(), "/*"); + builder.insert(usage.range.end(), "*/"); + } + } +} + +fn edit_tuple_field_usage( + ctx: &AssistContext, + builder: &mut AssistBuilder, + data: &TupleData, + index: TupleIndex, +) { + let field_name = &data.field_names[index.index]; + + if data.ref_type.is_some() { + let ref_data = handle_ref_field_usage(ctx, &index.field_expr); + builder.replace(ref_data.range, ref_data.format(field_name)); + } else { + builder.replace(index.range, field_name); + } +} +struct TupleIndex { + index: usize, + range: TextRange, + field_expr: FieldExpr, +} +fn detect_tuple_index(usage: &FileReference, data: &TupleData) -> Option { + // usage is IDENT + // IDENT + // NAME_REF + // PATH_SEGMENT + // PATH + // PATH_EXPR + // PAREN_EXRP* + // FIELD_EXPR + + let node = usage + .name + .syntax() + .ancestors() + .skip_while(|s| !ast::PathExpr::can_cast(s.kind())) + .skip(1) // PATH_EXPR + .find(|s| !ast::ParenExpr::can_cast(s.kind()))?; // skip parentheses + + if let Some(field_expr) = ast::FieldExpr::cast(node) { + let idx = field_expr.name_ref()?.as_tuple_field()?; + if idx < data.field_names.len() { + // special case: in macro call -> range of `field_expr` in applied macro, NOT range in actual file! + if field_expr.syntax().ancestors().any(|a| ast::MacroStmts::can_cast(a.kind())) { + cov_mark::hit!(destructure_tuple_macro_call); + + // issue: cannot differentiate between tuple index passed into macro or tuple index as result of macro: + // ```rust + // macro_rules! m { + // ($t1:expr, $t2:expr) => { $t1; $t2.0 } + // } + // let t = (1,2); + // m!(t.0, t) + // ``` + // -> 2 tuple index usages detected! + // + // -> only handle `t` + return None; + } + + Some(TupleIndex { index: idx, range: field_expr.syntax().text_range(), field_expr }) + } else { + // tuple index out of range + None + } + } else { + None + } +} + +struct RefData { + range: TextRange, + needs_deref: bool, + needs_parentheses: bool, +} +impl RefData { + fn format(&self, field_name: &str) -> String { + match (self.needs_deref, self.needs_parentheses) { + (true, true) => format!("(*{})", field_name), + (true, false) => format!("*{}", field_name), + (false, true) => format!("({})", field_name), + (false, false) => field_name.to_string(), + } + } +} +fn handle_ref_field_usage(ctx: &AssistContext, field_expr: &FieldExpr) -> RefData { + let s = field_expr.syntax(); + let mut ref_data = + RefData { range: s.text_range(), needs_deref: true, needs_parentheses: true }; + + let parent = match s.parent().map(ast::Expr::cast) { + Some(Some(parent)) => parent, + Some(None) => { + ref_data.needs_parentheses = false; + return ref_data; + } + None => return ref_data, + }; + + match parent { + ast::Expr::ParenExpr(it) => { + // already parens in place -> don't replace + ref_data.needs_parentheses = false; + // there might be a ref outside: `&(t.0)` -> can be removed + if let Some(it) = it.syntax().parent().and_then(ast::RefExpr::cast) { + ref_data.needs_deref = false; + ref_data.range = it.syntax().text_range(); + } + } + ast::Expr::RefExpr(it) => { + // `&*` -> cancel each other out + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + // might be surrounded by parens -> can be removed too + match it.syntax().parent().and_then(ast::ParenExpr::cast) { + Some(parent) => ref_data.range = parent.syntax().text_range(), + None => ref_data.range = it.syntax().text_range(), + }; + } + // higher precedence than deref `*` + // https://doc.rust-lang.org/reference/expressions.html#expression-precedence + // -> requires parentheses + ast::Expr::PathExpr(_it) => {} + ast::Expr::MethodCallExpr(it) => { + // `field_expr` is `self_param` (otherwise it would be in `ArgList`) + + // test if there's already auto-ref in place (`value` -> `&value`) + // -> no method accepting `self`, but `&self` -> no need for deref + // + // other combinations (`&value` -> `value`, `&&value` -> `&value`, `&value` -> `&&value`) might or might not be able to auto-ref/deref, + // but there might be trait implementations an added `&` might resolve to + // -> ONLY handle auto-ref from `value` to `&value` + fn is_auto_ref(ctx: &AssistContext, call_expr: &MethodCallExpr) -> bool { + fn impl_(ctx: &AssistContext, call_expr: &MethodCallExpr) -> Option { + let rec = call_expr.receiver()?; + let rec_ty = ctx.sema.type_of_expr(&rec)?.adjusted(); + // input must be actual value + if rec_ty.is_reference() { + return Some(false); + } + + // doesn't resolve trait impl + let f = ctx.sema.resolve_method_call(call_expr)?; + let self_param = f.self_param(ctx.db())?; + // self must be ref + match self_param.access(ctx.db()) { + hir::Access::Shared | hir::Access::Exclusive => Some(true), + hir::Access::Owned => Some(false), + } + } + impl_(ctx, call_expr).unwrap_or(false) + } + + if is_auto_ref(ctx, &it) { + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + } + } + ast::Expr::FieldExpr(_it) => { + // `t.0.my_field` + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + } + ast::Expr::IndexExpr(_it) => { + // `t.0[1]` + ref_data.needs_deref = false; + ref_data.needs_parentheses = false; + } + ast::Expr::TryExpr(_it) => { + // `t.0?` + // requires deref and parens: `(*_0)` + } + // lower precedence than deref `*` -> no parens + _ => { + ref_data.needs_parentheses = false; + } + }; + + ref_data +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::tests::{check_assist, check_assist_not_applicable}; + + // Tests for direct tuple destructure: + // `let $0t = (1,2);` -> `let (_0, _1) = (1,2);` + + fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, false) + } + + #[test] + fn dont_trigger_on_unit() { + cov_mark::check!(destructure_tuple_no_tuple); + check_assist_not_applicable( + assist, + r#" +fn main() { +let $0v = (); +} + "#, + ) + } + #[test] + fn dont_trigger_on_number() { + cov_mark::check!(destructure_tuple_no_tuple); + check_assist_not_applicable( + assist, + r#" +fn main() { +let $0v = 32; +} + "#, + ) + } + + #[test] + fn destructure_3_tuple() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2,3); +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); +} + "#, + ) + } + #[test] + fn destructure_2_tuple() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2); +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); +} + "#, + ) + } + #[test] + fn replace_indices() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2,3); + let v1 = tup.0; + let v2 = tup.1; + let v3 = tup.2; +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); + let v1 = _0; + let v2 = _1; + let v3 = _2; +} + "#, + ) + } + + #[test] + fn replace_usage_in_parentheses() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2,3); + let a = (tup).1; + let b = ((tup)).1; +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); + let a = _1; + let b = _1; +} + "#, + ) + } + + #[test] + fn handle_function_call() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2); + let v = tup.into(); +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + let v = /*tup*/.into(); +} + "#, + ) + } + + #[test] + fn handle_invalid_index() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2); + let v = tup.3; +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + let v = /*tup*/.3; +} + "#, + ) + } + + #[test] + fn dont_replace_variable_with_same_name_as_tuple() { + check_assist( + assist, + r#" +fn main() { + let tup = (1,2); + let v = tup.1; + let $0tup = (1,2,3); + let v = tup.1; + let tup = (1,2,3); + let v = tup.1; +} + "#, + r#" +fn main() { + let tup = (1,2); + let v = tup.1; + let ($0_0, _1, _2) = (1,2,3); + let v = _1; + let tup = (1,2,3); + let v = tup.1; +} + "#, + ) + } + + #[test] + fn keep_function_call_in_tuple_item() { + check_assist( + assist, + r#" +fn main() { + let $0t = ("3.14", 0); + let pi: f32 = t.0.parse().unwrap_or(0.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = ("3.14", 0); + let pi: f32 = _0.parse().unwrap_or(0.0); +} + "#, + ) + } + + #[test] + fn keep_type() { + check_assist( + assist, + r#" +fn main() { + let $0t: (usize, i32) = (1,2); +} + "#, + r#" +fn main() { + let ($0_0, _1): (usize, i32) = (1,2); +} + "#, + ) + } + + #[test] + fn destructure_reference() { + check_assist( + assist, + r#" +fn main() { + let t = (1,2); + let $0t = &t; + let v = t.0; +} + "#, + r#" +fn main() { + let t = (1,2); + let ($0_0, _1) = &t; + let v = *_0; +} + "#, + ) + } + + #[test] + fn destructure_multiple_reference() { + check_assist( + assist, + r#" +fn main() { + let t = (1,2); + let $0t = &&t; + let v = t.0; +} + "#, + r#" +fn main() { + let t = (1,2); + let ($0_0, _1) = &&t; + let v = *_0; +} + "#, + ) + } + + #[test] + fn keep_reference() { + check_assist( + assist, + r#" +fn foo(t: &(usize, usize)) -> usize { + match t { + &$0t => t.0 + } +} + "#, + r#" +fn foo(t: &(usize, usize)) -> usize { + match t { + &($0_0, _1) => _0 + } +} + "#, + ) + } + + #[test] + fn with_ref() { + check_assist( + assist, + r#" +fn main() { + let ref $0t = (1,2); + let v = t.0; +} + "#, + r#" +fn main() { + let (ref $0_0, ref _1) = (1,2); + let v = *_0; +} + "#, + ) + } + + #[test] + fn with_mut() { + check_assist( + assist, + r#" +fn main() { + let mut $0t = (1,2); + t.0 = 42; + let v = t.0; +} + "#, + r#" +fn main() { + let (mut $0_0, mut _1) = (1,2); + _0 = 42; + let v = _0; +} + "#, + ) + } + + #[test] + fn with_ref_mut() { + check_assist( + assist, + r#" +fn main() { + let ref mut $0t = (1,2); + t.0 = 42; + let v = t.0; +} + "#, + r#" +fn main() { + let (ref mut $0_0, ref mut _1) = (1,2); + *_0 = 42; + let v = *_0; +} + "#, + ) + } + + #[test] + fn dont_trigger_for_non_tuple_reference() { + check_assist_not_applicable( + assist, + r#" +fn main() { + let v = 42; + let $0v = &42; +} + "#, + ) + } + + #[test] + fn dont_trigger_on_static_tuple() { + check_assist_not_applicable( + assist, + r#" +static $0TUP: (usize, usize) = (1,2); + "#, + ) + } + + #[test] + fn dont_trigger_on_wildcard() { + check_assist_not_applicable( + assist, + r#" +fn main() { + let $0_ = (1,2); +} + "#, + ) + } + + #[test] + fn dont_trigger_in_struct() { + check_assist_not_applicable( + assist, + r#" +struct S { + $0tup: (usize, usize), +} + "#, + ) + } + + #[test] + fn dont_trigger_in_struct_creation() { + check_assist_not_applicable( + assist, + r#" +struct S { + tup: (usize, usize), +} +fn main() { + let s = S { + $0tup: (1,2), + }; +} + "#, + ) + } + + #[test] + fn dont_trigger_on_tuple_struct() { + check_assist_not_applicable( + assist, + r#" +struct S(usize, usize); +fn main() { + let $0s = S(1,2); +} + "#, + ) + } + + #[test] + fn dont_trigger_when_subpattern_exists() { + // sub-pattern is only allowed with IdentPat (name), not other patterns (like TuplePat) + cov_mark::check!(destructure_tuple_subpattern); + check_assist_not_applicable( + assist, + r#" +fn sum(t: (usize, usize)) -> usize { + match t { + $0t @ (1..=3,1..=3) => t.0 + t.1, + _ => 0, + } +} + "#, + ) + } + + #[test] + fn in_subpattern() { + check_assist( + assist, + r#" +fn main() { + let t1 @ (_, $0t2) = (1, (2,3)); + let v = t1.0 + t2.0 + t2.1; +} + "#, + r#" +fn main() { + let t1 @ (_, ($0_0, _1)) = (1, (2,3)); + let v = t1.0 + _0 + _1; +} + "#, + ) + } + + #[test] + fn in_nested_tuple() { + check_assist( + assist, + r#" +fn main() { + let ($0tup, v) = ((1,2),3); +} + "#, + r#" +fn main() { + let (($0_0, _1), v) = ((1,2),3); +} + "#, + ) + } + + #[test] + fn in_closure() { + check_assist( + assist, + r#" +fn main() { + let $0tup = (1,2,3); + let f = |v| v + tup.1; +} + "#, + r#" +fn main() { + let ($0_0, _1, _2) = (1,2,3); + let f = |v| v + _1; +} + "#, + ) + } + + #[test] + fn in_closure_args() { + check_assist( + assist, + r#" +fn main() { + let f = |$0t| t.0 + t.1; + let v = f((1,2)); +} + "#, + r#" +fn main() { + let f = |($0_0, _1)| _0 + _1; + let v = f((1,2)); +} + "#, + ) + } + + #[test] + fn in_function_args() { + check_assist( + assist, + r#" +fn f($0t: (usize, usize)) { + let v = t.0; +} + "#, + r#" +fn f(($0_0, _1): (usize, usize)) { + let v = _0; +} + "#, + ) + } + + #[test] + fn in_if_let() { + check_assist( + assist, + r#" +fn f(t: (usize, usize)) { + if let $0t = t { + let v = t.0; + } +} + "#, + r#" +fn f(t: (usize, usize)) { + if let ($0_0, _1) = t { + let v = _0; + } +} + "#, + ) + } + #[test] + fn in_if_let_option() { + check_assist( + assist, + r#" +//- minicore: option +fn f(o: Option<(usize, usize)>) { + if let Some($0t) = o { + let v = t.0; + } +} + "#, + r#" +fn f(o: Option<(usize, usize)>) { + if let Some(($0_0, _1)) = o { + let v = _0; + } +} + "#, + ) + } + + #[test] + fn in_match() { + check_assist( + assist, + r#" +fn main() { + match (1,2) { + $0t => t.1, + }; +} + "#, + r#" +fn main() { + match (1,2) { + ($0_0, _1) => _1, + }; +} + "#, + ) + } + #[test] + fn in_match_option() { + check_assist( + assist, + r#" +//- minicore: option +fn main() { + match Some((1,2)) { + Some($0t) => t.1, + _ => 0, + }; +} + "#, + r#" +fn main() { + match Some((1,2)) { + Some(($0_0, _1)) => _1, + _ => 0, + }; +} + "#, + ) + } + #[test] + fn in_match_reference_option() { + check_assist( + assist, + r#" +//- minicore: option +fn main() { + let t = (1,2); + match Some(&t) { + Some($0t) => t.1, + _ => 0, + }; +} + "#, + r#" +fn main() { + let t = (1,2); + match Some(&t) { + Some(($0_0, _1)) => *_1, + _ => 0, + }; +} + "#, + ) + } + + #[test] + fn in_for() { + check_assist( + assist, + r#" +//- minicore: iterators +fn main() { + for $0t in core::iter::repeat((1,2)) { + let v = t.1; + } +} + "#, + r#" +fn main() { + for ($0_0, _1) in core::iter::repeat((1,2)) { + let v = _1; + } +} + "#, + ) + } + #[test] + fn in_for_nested() { + check_assist( + assist, + r#" +//- minicore: iterators +fn main() { + for (a, $0b) in core::iter::repeat((1,(2,3))) { + let v = b.1; + } +} + "#, + r#" +fn main() { + for (a, ($0_0, _1)) in core::iter::repeat((1,(2,3))) { + let v = _1; + } +} + "#, + ) + } + + #[test] + fn not_applicable_on_tuple_usage() { + //Improvement: might be reasonable to allow & implement + check_assist_not_applicable( + assist, + r#" +fn main() { + let t = (1,2); + let v = $0t.0; +} + "#, + ) + } + + #[test] + fn replace_all() { + check_assist( + assist, + r#" +fn main() { + let $0t = (1,2); + let v = t.1; + let s = (t.0 + t.1) / 2; + let f = |v| v + t.0; + let r = f(t.1); + let e = t == (9,0); + let m = + match t { + (_,2) if t.0 > 2 => 1, + _ => 0, + }; +} + "#, + r#" +fn main() { + let ($0_0, _1) = (1,2); + let v = _1; + let s = (_0 + _1) / 2; + let f = |v| v + _0; + let r = f(_1); + let e = /*t*/ == (9,0); + let m = + match /*t*/ { + (_,2) if _0 > 2 => 1, + _ => 0, + }; +} + "#, + ) + } + + #[test] + fn non_trivial_tuple_assignment() { + check_assist( + assist, + r#" +fn main { + let $0t = + if 1 > 2 { + (1,2) + } else { + (5,6) + }; + let v1 = t.0; + let v2 = + if t.0 > t.1 { + t.0 - t.1 + } else { + t.1 - t.0 + }; +} + "#, + r#" +fn main { + let ($0_0, _1) = + if 1 > 2 { + (1,2) + } else { + (5,6) + }; + let v1 = _0; + let v2 = + if _0 > _1 { + _0 - _1 + } else { + _1 - _0 + }; +} + "#, + ) + } + + mod assist { + use super::*; + use crate::tests::check_assist_by_label; + + fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, true) + } + fn in_place_assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, false) + } + + pub(crate) fn check_in_place_assist(ra_fixture_before: &str, ra_fixture_after: &str) { + check_assist_by_label( + in_place_assist, + ra_fixture_before, + ra_fixture_after, + // "Destructure tuple in place", + "Destructure tuple", + ); + } + + pub(crate) fn check_sub_pattern_assist(ra_fixture_before: &str, ra_fixture_after: &str) { + check_assist_by_label( + assist, + ra_fixture_before, + ra_fixture_after, + "Destructure tuple in sub-pattern", + ); + } + + pub(crate) fn check_both_assists( + ra_fixture_before: &str, + ra_fixture_after_in_place: &str, + ra_fixture_after_in_sub_pattern: &str, + ) { + check_in_place_assist(ra_fixture_before, ra_fixture_after_in_place); + check_sub_pattern_assist(ra_fixture_before, ra_fixture_after_in_sub_pattern); + } + } + + /// Tests for destructure of tuple in sub-pattern: + /// `let $0t = (1,2);` -> `let t @ (_0, _1) = (1,2);` + mod sub_pattern { + use super::assist::*; + use super::*; + use crate::tests::check_assist_by_label; + + #[test] + fn destructure_in_sub_pattern() { + check_sub_pattern_assist( + r#" +#![feature(bindings_after_at)] + +fn main() { + let $0t = (1,2); +} + "#, + r#" +#![feature(bindings_after_at)] + +fn main() { + let t @ ($0_0, _1) = (1,2); +} + "#, + ) + } + + #[test] + fn trigger_both_destructure_tuple_assists() { + fn assist(acc: &mut Assists, ctx: &AssistContext) -> Option<()> { + destructure_tuple_binding_impl(acc, ctx, true) + } + let text = r#" +fn main() { + let $0t = (1,2); +} + "#; + check_assist_by_label( + assist, + text, + r#" +fn main() { + let ($0_0, _1) = (1,2); +} + "#, + "Destructure tuple in place", + ); + check_assist_by_label( + assist, + text, + r#" +fn main() { + let t @ ($0_0, _1) = (1,2); +} + "#, + "Destructure tuple in sub-pattern", + ); + } + + #[test] + fn replace_indices() { + check_sub_pattern_assist( + r#" +fn main() { + let $0t = (1,2); + let v1 = t.0; + let v2 = t.1; +} + "#, + r#" +fn main() { + let t @ ($0_0, _1) = (1,2); + let v1 = _0; + let v2 = _1; +} + "#, + ) + } + + #[test] + fn keep_function_call() { + cov_mark::check!(destructure_tuple_call_with_subpattern); + check_sub_pattern_assist( + r#" +fn main() { + let $0t = (1,2); + let v = t.into(); +} + "#, + r#" +fn main() { + let t @ ($0_0, _1) = (1,2); + let v = t.into(); +} + "#, + ) + } + + #[test] + fn keep_type() { + check_sub_pattern_assist( + r#" +fn main() { + let $0t: (usize, i32) = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let t @ ($0_0, _1): (usize, i32) = (1,2); + let v = _1; + let f = t.into(); +} + "#, + ) + } + + #[test] + fn in_function_args() { + check_sub_pattern_assist( + r#" +fn f($0t: (usize, usize)) { + let v = t.0; + let f = t.into(); +} + "#, + r#" +fn f(t @ ($0_0, _1): (usize, usize)) { + let v = _0; + let f = t.into(); +} + "#, + ) + } + + #[test] + fn with_ref() { + check_sub_pattern_assist( + r#" +fn main() { + let ref $0t = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let ref t @ (ref $0_0, ref _1) = (1,2); + let v = *_1; + let f = t.into(); +} + "#, + ) + } + #[test] + fn with_mut() { + check_sub_pattern_assist( + r#" +fn main() { + let mut $0t = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let mut t @ (mut $0_0, mut _1) = (1,2); + let v = _1; + let f = t.into(); +} + "#, + ) + } + #[test] + fn with_ref_mut() { + check_sub_pattern_assist( + r#" +fn main() { + let ref mut $0t = (1,2); + let v = t.1; + let f = t.into(); +} + "#, + r#" +fn main() { + let ref mut t @ (ref mut $0_0, ref mut _1) = (1,2); + let v = *_1; + let f = t.into(); +} + "#, + ) + } + } + + /// Tests for tuple usage in macro call: + /// `println!("{}", t.0)` + mod in_macro_call { + use super::assist::*; + + #[test] + fn detect_macro_call() { + cov_mark::check!(destructure_tuple_macro_call); + check_in_place_assist( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t.0); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/.0); +} + "#, + ) + } + + #[test] + fn tuple_usage() { + check_both_assists( + // leading `"foo"` to ensure `$e` doesn't start at position `0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t); +} + "#, + ) + } + + #[test] + fn tuple_function_usage() { + check_both_assists( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t.into()); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/.into()); +} + "#, + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t.into()); +} + "#, + ) + } + + #[test] + fn tuple_index_usage() { + check_both_assists( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!(t.0); +} + "#, + // FIXME: replace `t.0` with `_0` (cannot detect range of tuple index in macro call) + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/.0); +} + "#, + // FIXME: replace `t.0` with `_0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t.0); +} + "#, + ) + } + + #[test] + fn tuple_in_parentheses_index_usage() { + check_both_assists( + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let $0t = (1,2); + m!((t).0); +} + "#, + // FIXME: replace `(t).0` with `_0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!((/*t*/).0); +} + "#, + // FIXME: replace `(t).0` with `_0` + r#" +macro_rules! m { + ($e:expr) => { "foo"; $e }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!((t).0); +} + "#, + ) + } + + #[test] + fn empty_macro() { + check_in_place_assist( + r#" +macro_rules! m { + () => { "foo" }; + ($e:expr) => { $e; "foo" }; +} + +fn main() { + let $0t = (1,2); + m!(t); +} + "#, + // FIXME: macro allows no arg -> is valid. But assist should result in invalid code + r#" +macro_rules! m { + () => { "foo" }; + ($e:expr) => { $e; "foo" }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/); +} + "#, + ) + } + + #[test] + fn tuple_index_in_macro() { + check_both_assists( + r#" +macro_rules! m { + ($t:expr, $i:expr) => { $t.0 + $i }; +} + +fn main() { + let $0t = (1,2); + m!(t, t.0); +} + "#, + // FIXME: replace `t.0` in macro call (not IN macro) with `_0` + r#" +macro_rules! m { + ($t:expr, $i:expr) => { $t.0 + $i }; +} + +fn main() { + let ($0_0, _1) = (1,2); + m!(/*t*/, /*t*/.0); +} + "#, + // FIXME: replace `t.0` in macro call with `_0` + r#" +macro_rules! m { + ($t:expr, $i:expr) => { $t.0 + $i }; +} + +fn main() { + let t @ ($0_0, _1) = (1,2); + m!(t, t.0); +} + "#, + ) + } + } + + mod refs { + use super::assist::*; + + #[test] + fn no_ref() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: i32 = t.0; +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: i32 = *_0; +} + "#, + ) + } + #[test] + fn no_ref_with_parens() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: i32 = (t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: i32 = (*_0); +} + "#, + ) + } + #[test] + fn with_ref() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: &i32 = &t.0; +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: &i32 = _0; +} + "#, + ) + } + #[test] + fn with_ref_in_parens_ref() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: &i32 = &(t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: &i32 = _0; +} + "#, + ) + } + #[test] + fn with_ref_in_ref_parens() { + check_in_place_assist( + r#" +fn main() { + let $0t = &(1,2); + let v: &i32 = (&t.0); +} + "#, + r#" +fn main() { + let ($0_0, _1) = &(1,2); + let v: &i32 = _0; +} + "#, + ) + } + + #[test] + fn deref_and_parentheses() { + // Operator/Expressions with higher precedence than deref (`*`): + // https://doc.rust-lang.org/reference/expressions.html#expression-precedence + // * Path + // * Method call + // * Field expression + // * Function calls, array indexing + // * `?` + check_in_place_assist( + r#" +//- minicore: option +fn f1(v: i32) {} +fn f2(v: &i32) {} +trait T { + fn do_stuff(self) {} +} +impl T for i32 { + fn do_stuff(self) {} +} +impl T for &i32 { + fn do_stuff(self) {} +} +struct S4 { + value: i32, +} + +fn foo() -> Option<()> { + let $0t = &(0, (1,"1"), Some(2), [3;3], S4 { value: 4 }, &5); + let v: i32 = t.0; // deref, no parens + let v: &i32 = &t.0; // no deref, no parens, remove `&` + f1(t.0); // deref, no parens + f2(&t.0); // `&*` -> cancel out -> no deref, no parens + // https://github.com/rust-analyzer/rust-analyzer/issues/1109#issuecomment-658868639 + // let v: i32 = t.1.0; // no deref, no parens + let v: i32 = t.4.value; // no deref, no parens + t.0.do_stuff(); // deref, parens + let v: i32 = t.2?; // deref, parens + let v: i32 = t.3[0]; // no deref, no parens + (t.0).do_stuff(); // deref, no additional parens + let v: i32 = *t.5; // deref (-> 2), no parens + + None +} + "#, + r#" +fn f1(v: i32) {} +fn f2(v: &i32) {} +trait T { + fn do_stuff(self) {} +} +impl T for i32 { + fn do_stuff(self) {} +} +impl T for &i32 { + fn do_stuff(self) {} +} +struct S4 { + value: i32, +} + +fn foo() -> Option<()> { + let ($0_0, _1, _2, _3, _4, _5) = &(0, (1,"1"), Some(2), [3;3], S4 { value: 4 }, &5); + let v: i32 = *_0; // deref, no parens + let v: &i32 = _0; // no deref, no parens, remove `&` + f1(*_0); // deref, no parens + f2(_0); // `&*` -> cancel out -> no deref, no parens + // https://github.com/rust-analyzer/rust-analyzer/issues/1109#issuecomment-658868639 + // let v: i32 = t.1.0; // no deref, no parens + let v: i32 = _4.value; // no deref, no parens + (*_0).do_stuff(); // deref, parens + let v: i32 = (*_2)?; // deref, parens + let v: i32 = _3[0]; // no deref, no parens + (*_0).do_stuff(); // deref, no additional parens + let v: i32 = **_5; // deref (-> 2), no parens + + None +} + "#, + ) + } + + // --------- + // auto-ref/deref + + #[test] + fn self_auto_ref_doesnt_need_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(&self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(&self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = _0.f(); +} + "#, + ) + } + + #[test] + fn self_owned_requires_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn f(self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = (*_0).f(); +} + "#, + ) + } + + #[test] + fn self_auto_ref_in_trait_call_doesnt_require_deref() { + check_in_place_assist( + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for &S { + fn f(self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + // FIXME: doesn't need deref * parens. But `ctx.sema.resolve_method_call` doesn't resolve trait implementations + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for &S { + fn f(self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = (*_0).f(); +} + "#, + ) + } + #[test] + fn no_auto_deref_because_of_owned_and_ref_trait_impl() { + check_in_place_assist( + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for S { + fn f(self) {} +} +impl T for &S { + fn f(self) {} +} + +fn main() { + let $0t = &(S,2); + let s = t.0.f(); +} + "#, + r#" +trait T { + fn f(self); +} +#[derive(Clone, Copy)] +struct S; +impl T for S { + fn f(self) {} +} +impl T for &S { + fn f(self) {} +} + +fn main() { + let ($0_0, _1) = &(S,2); + let s = (*_0).f(); +} + "#, + ) + } + + #[test] + fn no_outer_parens_when_ref_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let $0t = &(S,&S); + let v = (&t.0).do_stuff(); +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let ($0_0, _1) = &(S,&S); + let v = _0.do_stuff(); +} + "#, + ) + } + + #[test] + fn auto_ref_deref() { + check_in_place_assist( + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let $0t = &(S,&S); + let v = (&t.0).do_stuff(); // no deref, remove parens + // `t.0` gets auto-refed -> no deref needed -> no parens + let v = t.0.do_stuff(); // no deref, no parens + let v = &t.0.do_stuff(); // `&` is for result -> no deref, no parens + // deref: `_1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` + let v = t.1.do_stuff(); // deref, parens +} + "#, + r#" +#[derive(Clone, Copy)] +struct S; +impl S { + fn do_stuff(&self) -> i32 { 42 } +} +fn main() { + let ($0_0, _1) = &(S,&S); + let v = _0.do_stuff(); // no deref, remove parens + // `t.0` gets auto-refed -> no deref needed -> no parens + let v = _0.do_stuff(); // no deref, no parens + let v = &_0.do_stuff(); // `&` is for result -> no deref, no parens + // deref: `_1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` + let v = (*_1).do_stuff(); // deref, parens +} + "#, + ) + } + + #[test] + fn mutable() { + check_in_place_assist( + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let $0t = &mut (1,2); + let v = t.0; + t.0 = 42; + f_owned(t.0); + f(&t.0); + f_mut(&mut t.0); +} + "#, + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let ($0_0, _1) = &mut (1,2); + let v = *_0; + *_0 = 42; + f_owned(*_0); + f(_0); + f_mut(_0); +} + "#, + ) + } + + #[test] + fn with_ref_keyword() { + check_in_place_assist( + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} + +fn main() { + let ref $0t = (1,2); + let v = t.0; + f_owned(t.0); + f(&t.0); +} + "#, + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} + +fn main() { + let (ref $0_0, ref _1) = (1,2); + let v = *_0; + f_owned(*_0); + f(_0); +} + "#, + ) + } + #[test] + fn with_ref_mut_keywords() { + check_in_place_assist( + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let ref mut $0t = (1,2); + let v = t.0; + t.0 = 42; + f_owned(t.0); + f(&t.0); + f_mut(&mut t.0); +} + "#, + r#" +fn f_owned(v: i32) {} +fn f(v: &i32) {} +fn f_mut(v: &mut i32) { *v = 42; } + +fn main() { + let (ref mut $0_0, ref mut _1) = (1,2); + let v = *_0; + *_0 = 42; + f_owned(*_0); + f(_0); + f_mut(_0); +} + "#, + ) + } + } +} diff --git a/crates/ide_assists/src/handlers/generate_enum_is_method.rs b/crates/ide_assists/src/handlers/generate_enum_is_method.rs index 24939f2622ad..94216f2dff91 100644 --- a/crates/ide_assists/src/handlers/generate_enum_is_method.rs +++ b/crates/ide_assists/src/handlers/generate_enum_is_method.rs @@ -28,6 +28,8 @@ use crate::{ // // impl Version { // /// Returns `true` if the version is [`Minor`]. +// /// +// /// [`Minor`]: Version::Minor // fn is_minor(&self) -> bool { // matches!(self, Self::Minor) // } @@ -43,7 +45,8 @@ pub(crate) fn generate_enum_is_method(acc: &mut Assists, ctx: &AssistContext) -> ast::StructKind::Unit => "", }; - let enum_lowercase_name = to_lower_snake_case(&parent_enum.name()?.to_string()); + let enum_name = parent_enum.name()?; + let enum_lowercase_name = to_lower_snake_case(&enum_name.to_string()).replace('_', " "); let fn_name = format!("is_{}", &to_lower_snake_case(&variant_name.text())); // Return early if we've found an existing new fn @@ -57,11 +60,18 @@ pub(crate) fn generate_enum_is_method(acc: &mut Assists, ctx: &AssistContext) -> |builder| { let vis = parent_enum.visibility().map_or(String::new(), |v| format!("{} ", v)); let method = format!( - " /// Returns `true` if the {} is [`{}`]. + " /// Returns `true` if the {} is [`{variant}`]. + /// + /// [`{variant}`]: {}::{variant} {}fn {}(&self) -> bool {{ - matches!(self, Self::{}{}) + matches!(self, Self::{variant}{}) }}", - enum_lowercase_name, variant_name, vis, fn_name, variant_name, pattern_suffix, + enum_lowercase_name, + enum_name, + vis, + fn_name, + pattern_suffix, + variant = variant_name ); add_method_to_adt(builder, &parent_enum, impl_def, &method); @@ -93,6 +103,8 @@ enum Variant { impl Variant { /// Returns `true` if the variant is [`Minor`]. + /// + /// [`Minor`]: Variant::Minor fn is_minor(&self) -> bool { matches!(self, Self::Minor) } @@ -137,6 +149,8 @@ enum Variant { impl Variant { /// Returns `true` if the variant is [`Minor`]. + /// + /// [`Minor`]: Variant::Minor fn is_minor(&self) -> bool { matches!(self, Self::Minor(..)) } @@ -162,6 +176,8 @@ enum Variant { impl Variant { /// Returns `true` if the variant is [`Minor`]. + /// + /// [`Minor`]: Variant::Minor fn is_minor(&self) -> bool { matches!(self, Self::Minor { .. }) } @@ -179,6 +195,8 @@ enum Variant { Undefined } impl Variant { /// Returns `true` if the variant is [`Undefined`]. + /// + /// [`Undefined`]: Variant::Undefined fn is_undefined(&self) -> bool { matches!(self, Self::Undefined) } @@ -204,6 +222,8 @@ pub(crate) enum Variant { impl Variant { /// Returns `true` if the variant is [`Minor`]. + /// + /// [`Minor`]: Variant::Minor pub(crate) fn is_minor(&self) -> bool { matches!(self, Self::Minor) } @@ -224,6 +244,8 @@ enum Variant { impl Variant { /// Returns `true` if the variant is [`Minor`]. + /// + /// [`Minor`]: Variant::Minor fn is_minor(&self) -> bool { matches!(self, Self::Minor) } @@ -236,14 +258,45 @@ impl Variant { impl Variant { /// Returns `true` if the variant is [`Minor`]. + /// + /// [`Minor`]: Variant::Minor fn is_minor(&self) -> bool { matches!(self, Self::Minor) } /// Returns `true` if the variant is [`Major`]. + /// + /// [`Major`]: Variant::Major fn is_major(&self) -> bool { matches!(self, Self::Major) } +}"#, + ); + } + + #[test] + fn test_generate_enum_is_variant_names() { + check_assist( + generate_enum_is_method, + r#" +enum GeneratorState { + Yielded, + Complete$0, + Major, +}"#, + r#"enum GeneratorState { + Yielded, + Complete, + Major, +} + +impl GeneratorState { + /// Returns `true` if the generator state is [`Complete`]. + /// + /// [`Complete`]: GeneratorState::Complete + fn is_complete(&self) -> bool { + matches!(self, Self::Complete) + } }"#, ); } diff --git a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs index d29a312eba59..d6319ea5c13f 100644 --- a/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs +++ b/crates/ide_assists/src/handlers/replace_derive_with_manual_impl.rs @@ -302,6 +302,71 @@ impl core::fmt::Debug for Foo { } } } +"#, + ) + } + + #[test] + fn add_custom_impl_debug_tuple_enum() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: fmt +#[derive(Debu$0g)] +enum Foo { + Bar(usize, usize), + Baz, +} +"#, + r#" +enum Foo { + Bar(usize, usize), + Baz, +} + +impl core::fmt::Debug for Foo { + $0fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Bar(arg0, arg1) => f.debug_tuple("Bar").field(arg0).field(arg1).finish(), + Self::Baz => write!(f, "Baz"), + } + } +} +"#, + ) + } + #[test] + fn add_custom_impl_debug_record_enum() { + check_assist( + replace_derive_with_manual_impl, + r#" +//- minicore: fmt +#[derive(Debu$0g)] +enum Foo { + Bar { + baz: usize, + qux: usize, + }, + Baz, +} +"#, + r#" +enum Foo { + Bar { + baz: usize, + qux: usize, + }, + Baz, +} + +impl core::fmt::Debug for Foo { + $0fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Bar { baz, qux } => f.debug_struct("Bar").field("baz", baz).field("qux", qux).finish(), + Self::Baz => write!(f, "Baz"), + } + } +} "#, ) } diff --git a/crates/ide_assists/src/lib.rs b/crates/ide_assists/src/lib.rs index 98a9085a51ed..7c074a4f6321 100644 --- a/crates/ide_assists/src/lib.rs +++ b/crates/ide_assists/src/lib.rs @@ -62,6 +62,7 @@ mod handlers { mod convert_iter_for_each_to_for; mod convert_tuple_struct_to_named_struct; mod convert_to_guarded_return; + mod destructure_tuple_binding; mod expand_glob_import; mod extract_function; mod extract_struct_from_enum_variant; @@ -134,6 +135,7 @@ mod handlers { convert_iter_for_each_to_for::convert_iter_for_each_to_for, convert_to_guarded_return::convert_to_guarded_return, convert_tuple_struct_to_named_struct::convert_tuple_struct_to_named_struct, + destructure_tuple_binding::destructure_tuple_binding, expand_glob_import::expand_glob_import, extract_struct_from_enum_variant::extract_struct_from_enum_variant, extract_type_alias::extract_type_alias, diff --git a/crates/ide_assists/src/tests/generated.rs b/crates/ide_assists/src/tests/generated.rs index 853c41f78f43..21daaf46c4c5 100644 --- a/crates/ide_assists/src/tests/generated.rs +++ b/crates/ide_assists/src/tests/generated.rs @@ -367,6 +367,25 @@ impl Point { ) } +#[test] +fn doctest_destructure_tuple_binding() { + check_doc_test( + "destructure_tuple_binding", + r#####" +fn main() { + let $0t = (1,2); + let v = t.0; +} +"#####, + r#####" +fn main() { + let ($0_0, _1) = (1,2); + let v = _0; +} +"#####, + ) +} + #[test] fn doctest_expand_glob_import() { check_doc_test( @@ -723,6 +742,8 @@ enum Version { impl Version { /// Returns `true` if the version is [`Minor`]. + /// + /// [`Minor`]: Version::Minor fn is_minor(&self) -> bool { matches!(self, Self::Minor) } diff --git a/crates/ide_assists/src/utils.rs b/crates/ide_assists/src/utils.rs index 718fbeda8d3a..2431f5c4e77c 100644 --- a/crates/ide_assists/src/utils.rs +++ b/crates/ide_assists/src/utils.rs @@ -464,7 +464,7 @@ pub(crate) fn add_method_to_adt( } pub fn useless_type_special_case(field_name: &str, field_ty: &String) -> Option<(String, String)> { - if field_ty.to_string() == "String" { + if field_ty == "String" { cov_mark::hit!(useless_type_special_case); return Some(("&str".to_string(), format!("self.{}.as_str()", field_name))); } diff --git a/crates/ide_assists/src/utils/gen_trait_fn_body.rs b/crates/ide_assists/src/utils/gen_trait_fn_body.rs index 5a8914b3316e..b9c7da71b5c0 100644 --- a/crates/ide_assists/src/utils/gen_trait_fn_body.rs +++ b/crates/ide_assists/src/utils/gen_trait_fn_body.rs @@ -149,16 +149,90 @@ fn gen_debug_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { let mut arms = vec![]; for variant in list.variants() { let name = variant.name()?; - let variant_name = - make::path_pat(make::ext::path_from_idents(["Self", &format!("{}", name)])?); - + let variant_name = make::ext::path_from_idents(["Self", &format!("{}", name)])?; let target = make::expr_path(make::ext::ident_path("f").into()); - let fmt_string = make::expr_literal(&(format!("\"{}\"", name))).into(); - let args = make::arg_list(vec![target, fmt_string]); - let macro_name = make::expr_path(make::ext::ident_path("write")); - let macro_call = make::expr_macro_call(macro_name, args); - arms.push(make::match_arm(Some(variant_name.into()), None, macro_call.into())); + match variant.field_list() { + Some(ast::FieldList::RecordFieldList(list)) => { + // => f.debug_struct(name) + let target = make::expr_path(make::ext::ident_path("f")); + let method = make::name_ref("debug_struct"); + let struct_name = format!("\"{}\"", name); + let args = make::arg_list(Some(make::expr_literal(&struct_name).into())); + let mut expr = make::expr_method_call(target, method, args); + + let mut pats = vec![]; + for field in list.fields() { + let field_name = field.name()?; + + // create a field pattern for use in `MyStruct { fields.. }` + let pat = make::ident_pat(false, false, field_name.clone()); + pats.push(pat.into()); + + // => .field("field_name", field) + let method_name = make::name_ref("field"); + let name = make::expr_literal(&(format!("\"{}\"", field_name))).into(); + let path = &format!("{}", field_name); + let path = make::expr_path(make::ext::ident_path(path)); + let args = make::arg_list(vec![name, path]); + expr = make::expr_method_call(expr, method_name, args); + } + + // => .finish() + let method = make::name_ref("finish"); + let expr = make::expr_method_call(expr, method, make::arg_list(None)); + + // => MyStruct { fields.. } => f.debug_struct("MyStruct")...finish(), + let pat = make::record_pat(variant_name.clone(), pats.into_iter()); + arms.push(make::match_arm(Some(pat.into()), None, expr)); + } + Some(ast::FieldList::TupleFieldList(list)) => { + // => f.debug_tuple(name) + let target = make::expr_path(make::ext::ident_path("f")); + let method = make::name_ref("debug_tuple"); + let struct_name = format!("\"{}\"", name); + let args = make::arg_list(Some(make::expr_literal(&struct_name).into())); + let mut expr = make::expr_method_call(target, method, args); + + let mut pats = vec![]; + for (i, _) in list.fields().enumerate() { + let name = format!("arg{}", i); + + // create a field pattern for use in `MyStruct(fields..)` + let field_name = make::name(&name); + let pat = make::ident_pat(false, false, field_name.clone()); + pats.push(pat.into()); + + // => .field(field) + let method_name = make::name_ref("field"); + let field_path = &format!("{}", name); + let field_path = make::expr_path(make::ext::ident_path(field_path)); + let args = make::arg_list(vec![field_path]); + expr = make::expr_method_call(expr, method_name, args); + } + + // => .finish() + let method = make::name_ref("finish"); + let expr = make::expr_method_call(expr, method, make::arg_list(None)); + + // => MyStruct (fields..) => f.debug_tuple("MyStruct")...finish(), + let pat = make::tuple_struct_pat(variant_name.clone(), pats.into_iter()); + arms.push(make::match_arm(Some(pat.into()), None, expr)); + } + None => { + let fmt_string = make::expr_literal(&(format!("\"{}\"", name))).into(); + let args = make::arg_list([target, fmt_string]); + let macro_name = make::expr_path(make::ext::ident_path("write")); + let macro_call = make::expr_macro_call(macro_name, args); + + let variant_name = make::path_pat(variant_name); + arms.push(make::match_arm( + Some(variant_name.into()), + None, + macro_call.into(), + )); + } + } } let match_target = make::expr_path(make::ext::ident_path("self")); @@ -190,7 +264,7 @@ fn gen_debug_impl(adt: &ast::Adt, func: &ast::Fn) -> Option<()> { let f_path = make::expr_path(make::ext::ident_path("self")); let f_path = make::expr_ref(f_path, false); let f_path = make::expr_field(f_path, &format!("{}", name)).into(); - let args = make::arg_list(vec![f_name, f_path]); + let args = make::arg_list([f_name, f_path]); expr = make::expr_method_call(expr, make::name_ref("field"), args); } expr diff --git a/crates/ide_db/src/helpers/import_assets.rs b/crates/ide_db/src/helpers/import_assets.rs index 3433398ebab4..a672c1091740 100644 --- a/crates/ide_db/src/helpers/import_assets.rs +++ b/crates/ide_db/src/helpers/import_assets.rs @@ -120,11 +120,11 @@ impl ImportAssets { } pub fn for_ident_pat(pat: &ast::IdentPat, sema: &Semantics) -> Option { - let name = pat.name()?; - let candidate_node = pat.syntax().clone(); if !pat.is_simple_ident() { return None; } + let name = pat.name()?; + let candidate_node = pat.syntax().clone(); Some(Self { import_candidate: ImportCandidate::for_name(sema, &name)?, module_with_candidate: sema.scope(&candidate_node).module()?, diff --git a/crates/ide_db/src/rename.rs b/crates/ide_db/src/rename.rs index 4ca3260877c6..a6f7c09af8be 100644 --- a/crates/ide_db/src/rename.rs +++ b/crates/ide_db/src/rename.rs @@ -30,7 +30,7 @@ use syntax::{ ast::{self, NameOwner}, lex_single_syntax_kind, AstNode, SyntaxKind, TextRange, T, }; -use text_edit::TextEdit; +use text_edit::{TextEdit, TextEditBuilder}; use crate::{ defs::Definition, @@ -81,14 +81,6 @@ impl Definition { /// `Definition`. Note that some definitions, like buitin types, can't be /// renamed. pub fn range_for_rename(self, sema: &Semantics) -> Option { - // FIXME: the `original_file_range` calls here are wrong -- they never fail, - // and _fall back_ to the entirety of the macro call. Such fall back is - // incorrect for renames. The safe behavior would be to return an error for - // such cases. The correct behavior would be to return an auxiliary list of - // "can't rename these occurrences in macros" items, and then show some kind - // of a dialog to the user. See: - cov_mark::hit!(macros_are_broken_lol); - let res = match self { Definition::Macro(mac) => { let src = mac.source(sema.db)?; @@ -96,38 +88,35 @@ impl Definition { Either::Left(it) => it.name()?, Either::Right(it) => it.name()?, }; - src.with_value(name.syntax()).original_file_range(sema.db) + src.with_value(name.syntax()).original_file_range_opt(sema.db) } Definition::Field(field) => { let src = field.source(sema.db)?; - match &src.value { FieldSource::Named(record_field) => { let name = record_field.name()?; - src.with_value(name.syntax()).original_file_range(sema.db) - } - FieldSource::Pos(_) => { - return None; + src.with_value(name.syntax()).original_file_range_opt(sema.db) } + FieldSource::Pos(_) => None, } } Definition::ModuleDef(module_def) => match module_def { hir::ModuleDef::Module(module) => { let src = module.declaration_source(sema.db)?; let name = src.value.name()?; - src.with_value(name.syntax()).original_file_range(sema.db) + src.with_value(name.syntax()).original_file_range_opt(sema.db) } - hir::ModuleDef::Function(it) => name_range(it, sema)?, + hir::ModuleDef::Function(it) => name_range(it, sema), hir::ModuleDef::Adt(adt) => match adt { - hir::Adt::Struct(it) => name_range(it, sema)?, - hir::Adt::Union(it) => name_range(it, sema)?, - hir::Adt::Enum(it) => name_range(it, sema)?, + hir::Adt::Struct(it) => name_range(it, sema), + hir::Adt::Union(it) => name_range(it, sema), + hir::Adt::Enum(it) => name_range(it, sema), }, - hir::ModuleDef::Variant(it) => name_range(it, sema)?, - hir::ModuleDef::Const(it) => name_range(it, sema)?, - hir::ModuleDef::Static(it) => name_range(it, sema)?, - hir::ModuleDef::Trait(it) => name_range(it, sema)?, - hir::ModuleDef::TypeAlias(it) => name_range(it, sema)?, + hir::ModuleDef::Variant(it) => name_range(it, sema), + hir::ModuleDef::Const(it) => name_range(it, sema), + hir::ModuleDef::Static(it) => name_range(it, sema), + hir::ModuleDef::Trait(it) => name_range(it, sema), + hir::ModuleDef::TypeAlias(it) => name_range(it, sema), hir::ModuleDef::BuiltinType(_) => return None, }, Definition::SelfType(_) => return None, @@ -137,7 +126,7 @@ impl Definition { Either::Left(bind_pat) => bind_pat.name()?, Either::Right(_) => return None, }; - src.with_value(name.syntax()).original_file_range(sema.db) + src.with_value(name.syntax()).original_file_range_opt(sema.db) } Definition::GenericParam(generic_param) => match generic_param { hir::GenericParam::TypeParam(type_param) => { @@ -146,22 +135,22 @@ impl Definition { Either::Left(type_param) => type_param.name()?, Either::Right(_trait) => return None, }; - src.with_value(name.syntax()).original_file_range(sema.db) + src.with_value(name.syntax()).original_file_range_opt(sema.db) } hir::GenericParam::LifetimeParam(lifetime_param) => { let src = lifetime_param.source(sema.db)?; let lifetime = src.value.lifetime()?; - src.with_value(lifetime.syntax()).original_file_range(sema.db) + src.with_value(lifetime.syntax()).original_file_range_opt(sema.db) } - hir::GenericParam::ConstParam(it) => name_range(it, sema)?, + hir::GenericParam::ConstParam(it) => name_range(it, sema), }, Definition::Label(label) => { let src = label.source(sema.db); let lifetime = src.value.lifetime()?; - src.with_value(lifetime.syntax()).original_file_range(sema.db) + src.with_value(lifetime.syntax()).original_file_range_opt(sema.db) } }; - return Some(res); + return res; fn name_range(def: D, sema: &Semantics) -> Option where @@ -170,8 +159,7 @@ impl Definition { { let src = def.source(sema.db)?; let name = src.value.name()?; - let res = src.with_value(name.syntax()).original_file_range(sema.db); - Some(res) + src.with_value(name.syntax()).original_file_range_opt(sema.db) } } } @@ -303,108 +291,134 @@ pub fn source_edit_from_references( ) -> TextEdit { let mut edit = TextEdit::builder(); for reference in references { - let (range, replacement) = match &reference.name { + let has_emitted_edit = match &reference.name { // if the ranges differ then the node is inside a macro call, we can't really attempt // to make special rewrites like shorthand syntax and such, so just rename the node in // the macro input ast::NameLike::NameRef(name_ref) if name_ref.syntax().text_range() == reference.range => { - source_edit_from_name_ref(name_ref, new_name, def) + source_edit_from_name_ref(&mut edit, name_ref, new_name, def) } ast::NameLike::Name(name) if name.syntax().text_range() == reference.range => { - source_edit_from_name(name, new_name) + source_edit_from_name(&mut edit, name, new_name) } - _ => None, + _ => false, + }; + if !has_emitted_edit { + edit.replace(reference.range, new_name.to_string()); } - .unwrap_or_else(|| (reference.range, new_name.to_string())); - edit.replace(range, replacement); } + edit.finish() } -fn source_edit_from_name(name: &ast::Name, new_name: &str) -> Option<(TextRange, String)> { +fn source_edit_from_name(edit: &mut TextEditBuilder, name: &ast::Name, new_name: &str) -> bool { if let Some(_) = ast::RecordPatField::for_field_name(name) { - // FIXME: instead of splitting the shorthand, recursively trigger a rename of the - // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547 if let Some(ident_pat) = name.syntax().parent().and_then(ast::IdentPat::cast) { - return Some(( - TextRange::empty(ident_pat.syntax().text_range().start()), - [new_name, ": "].concat(), - )); + cov_mark::hit!(rename_record_pat_field_name_split); + // Foo { ref mut field } -> Foo { new_name: ref mut field } + // ^ insert `new_name: ` + + // FIXME: instead of splitting the shorthand, recursively trigger a rename of the + // other name https://github.com/rust-analyzer/rust-analyzer/issues/6547 + edit.insert(ident_pat.syntax().text_range().start(), format!("{}: ", new_name)); + return true; } } - None + + false } fn source_edit_from_name_ref( + edit: &mut TextEditBuilder, name_ref: &ast::NameRef, new_name: &str, def: Definition, -) -> Option<(TextRange, String)> { +) -> bool { if let Some(record_field) = ast::RecordExprField::for_name_ref(name_ref) { let rcf_name_ref = record_field.name_ref(); let rcf_expr = record_field.expr(); - match (rcf_name_ref, rcf_expr.and_then(|it| it.name_ref())) { + match &(rcf_name_ref, rcf_expr.and_then(|it| it.name_ref())) { // field: init-expr, check if we can use a field init shorthand (Some(field_name), Some(init)) => { - if field_name == *name_ref { + if field_name == name_ref { if init.text() == new_name { cov_mark::hit!(test_rename_field_put_init_shorthand); + // Foo { field: local } -> Foo { local } + // ^^^^^^^ delete this + // same names, we can use a shorthand here instead. // we do not want to erase attributes hence this range start let s = field_name.syntax().text_range().start(); - let e = record_field.syntax().text_range().end(); - return Some((TextRange::new(s, e), new_name.to_owned())); + let e = init.syntax().text_range().start(); + edit.delete(TextRange::new(s, e)); + return true; } - } else if init == *name_ref { + } else if init == name_ref { if field_name.text() == new_name { cov_mark::hit!(test_rename_local_put_init_shorthand); + // Foo { field: local } -> Foo { field } + // ^^^^^^^ delete this + // same names, we can use a shorthand here instead. // we do not want to erase attributes hence this range start - let s = field_name.syntax().text_range().start(); - let e = record_field.syntax().text_range().end(); - return Some((TextRange::new(s, e), new_name.to_owned())); + let s = field_name.syntax().text_range().end(); + let e = init.syntax().text_range().end(); + edit.delete(TextRange::new(s, e)); + return true; } } - None } // init shorthand (None, Some(_)) if matches!(def, Definition::Field(_)) => { cov_mark::hit!(test_rename_field_in_field_shorthand); - let s = name_ref.syntax().text_range().start(); - Some((TextRange::empty(s), format!("{}: ", new_name))) + // Foo { field } -> Foo { new_name: field } + // ^ insert `new_name: ` + let offset = name_ref.syntax().text_range().start(); + edit.insert(offset, format!("{}: ", new_name)); + return true; } (None, Some(_)) if matches!(def, Definition::Local(_)) => { cov_mark::hit!(test_rename_local_in_field_shorthand); - let s = name_ref.syntax().text_range().end(); - Some((TextRange::empty(s), format!(": {}", new_name))) + // Foo { field } -> Foo { field: new_name } + // ^ insert `: new_name` + let offset = name_ref.syntax().text_range().end(); + edit.insert(offset, format!(": {}", new_name)); + return true; } - _ => None, + _ => (), } } else if let Some(record_field) = ast::RecordPatField::for_field_name_ref(name_ref) { let rcf_name_ref = record_field.name_ref(); let rcf_pat = record_field.pat(); match (rcf_name_ref, rcf_pat) { // field: rename - (Some(field_name), Some(ast::Pat::IdentPat(pat))) if field_name == *name_ref => { + (Some(field_name), Some(ast::Pat::IdentPat(pat))) + if field_name == *name_ref && pat.at_token().is_none() => + { // field name is being renamed - if pat.name().map_or(false, |it| it.text() == new_name) { - cov_mark::hit!(test_rename_field_put_init_shorthand_pat); - // same names, we can use a shorthand here instead/ - // we do not want to erase attributes hence this range start - let s = field_name.syntax().text_range().start(); - let e = record_field.syntax().text_range().end(); - Some((TextRange::new(s, e), pat.to_string())) - } else { - None + if let Some(name) = pat.name() { + if name.text() == new_name { + cov_mark::hit!(test_rename_field_put_init_shorthand_pat); + // Foo { field: ref mut local } -> Foo { ref mut field } + // ^^^^^^^ delete this + // ^^^^^ replace this with `field` + + // same names, we can use a shorthand here instead/ + // we do not want to erase attributes hence this range start + let s = field_name.syntax().text_range().start(); + let e = pat.syntax().text_range().start(); + edit.delete(TextRange::new(s, e)); + edit.replace(name.syntax().text_range(), new_name.to_string()); + return true; + } } } - _ => None, + _ => (), } - } else { - None } + false } fn source_edit_from_def( @@ -412,32 +426,52 @@ fn source_edit_from_def( def: Definition, new_name: &str, ) -> Result<(FileId, TextEdit)> { - let frange = def + let FileRange { file_id, range } = def .range_for_rename(sema) .ok_or_else(|| format_err!("No identifier available to rename"))?; - let mut replacement_text = String::new(); - let mut repl_range = frange.range; + let mut edit = TextEdit::builder(); if let Definition::Local(local) = def { if let Either::Left(pat) = local.source(sema.db).value { - if matches!( - pat.syntax().parent().and_then(ast::RecordPatField::cast), - Some(pat_field) if pat_field.name_ref().is_none() - ) { - replacement_text.push_str(": "); - replacement_text.push_str(new_name); - repl_range = TextRange::new( - pat.syntax().text_range().end(), - pat.syntax().text_range().end(), - ); + // special cases required for renaming fields/locals in Record patterns + if let Some(pat_field) = pat.syntax().parent().and_then(ast::RecordPatField::cast) { + let name_range = pat.name().unwrap().syntax().text_range(); + if let Some(name_ref) = pat_field.name_ref() { + if new_name == name_ref.text() && pat.at_token().is_none() { + // Foo { field: ref mut local } -> Foo { ref mut field } + // ^^^^^^ delete this + // ^^^^^ replace this with `field` + cov_mark::hit!(test_rename_local_put_init_shorthand_pat); + edit.delete( + name_ref + .syntax() + .text_range() + .cover_offset(pat.syntax().text_range().start()), + ); + edit.replace(name_range, name_ref.text().to_string()); + } else { + // Foo { field: ref mut local @ local 2} -> Foo { field: ref mut new_name @ local2 } + // Foo { field: ref mut local } -> Foo { field: ref mut new_name } + // ^^^^^ replace this with `new_name` + edit.replace(name_range, new_name.to_string()); + } + } else { + // Foo { ref mut field } -> Foo { field: ref mut new_name } + // ^ insert `field: ` + // ^^^^^ replace this with `new_name` + edit.insert( + pat.syntax().text_range().start(), + format!("{}: ", pat_field.field_name().unwrap()), + ); + edit.replace(name_range, new_name.to_string()); + } } } } - if replacement_text.is_empty() { - replacement_text.push_str(new_name); + if edit.is_empty() { + edit.replace(range, new_name.to_string()); } - let edit = TextEdit::replace(repl_range, replacement_text); - Ok((frange.file_id, edit)) + Ok((file_id, edit.finish())) } #[derive(Copy, Clone, Debug, PartialEq)] diff --git a/crates/proc_macro_api/src/version.rs b/crates/proc_macro_api/src/version.rs index fa2b60fcb9c4..4c065c93c44a 100644 --- a/crates/proc_macro_api/src/version.rs +++ b/crates/proc_macro_api/src/version.rs @@ -14,8 +14,8 @@ use snap::read::FrameDecoder as SnapDecoder; pub struct RustCInfo { pub version: (usize, usize, usize), pub channel: String, - pub commit: String, - pub date: String, + pub commit: Option, + pub date: Option, } /// Read rustc dylib information @@ -38,18 +38,24 @@ pub fn read_dylib_info(dylib_path: &AbsPath) -> io::Result { let version = version_parts.next().ok_or_else(|| err!("no version"))?; let channel = version_parts.next().unwrap_or_default().to_string(); - let commit = items.next().ok_or_else(|| err!("no commit info"))?; - // remove ( - if commit.len() == 0 { - return Err(err!("commit format error")); - } - let commit = commit[1..].to_string(); - let date = items.next().ok_or_else(|| err!("no date info"))?; - // remove ) - if date.len() == 0 { - return Err(err!("date format error")); - } - let date = date[0..date.len() - 2].to_string(); + let commit = match items.next() { + Some(commit) => { + match commit.len() { + 0 => None, + _ => Some(commit[1..].to_string() /* remove ( */), + } + } + None => None, + }; + let date = match items.next() { + Some(date) => { + match date.len() { + 0 => None, + _ => Some(date[0..date.len() - 2].to_string() /* remove ) */), + } + } + None => None, + }; let version_numbers = version .split('.') diff --git a/crates/text_edit/src/lib.rs b/crates/text_edit/src/lib.rs index 685039c4bec3..a43ffe202ffc 100644 --- a/crates/text_edit/src/lib.rs +++ b/crates/text_edit/src/lib.rs @@ -159,6 +159,9 @@ impl<'a> IntoIterator for &'a TextEdit { } impl TextEditBuilder { + pub fn is_empty(&self) -> bool { + self.indels.is_empty() + } pub fn replace(&mut self, range: TextRange, replace_with: String) { self.indel(Indel::replace(range, replace_with)) } diff --git a/crates/vfs-notify/Cargo.toml b/crates/vfs-notify/Cargo.toml index e4723dd93144..ecd589c58275 100644 --- a/crates/vfs-notify/Cargo.toml +++ b/crates/vfs-notify/Cargo.toml @@ -14,7 +14,7 @@ rustc-hash = "1.0" jod-thread = "0.1.0" walkdir = "2.3.1" crossbeam-channel = "0.5.0" -notify = "=5.0.0-pre.10" # check that it builds on NetBSD before upgrading +notify = "=5.0.0-pre.12" vfs = { path = "../vfs", version = "0.0.0" } paths = { path = "../paths", version = "0.0.0" } diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs index b16a79b3ee0c..637438bd66c3 100644 --- a/crates/vfs-notify/src/lib.rs +++ b/crates/vfs-notify/src/lib.rs @@ -83,10 +83,9 @@ impl NotifyActor { self.watcher = None; if !config.watch.is_empty() { let (watcher_sender, watcher_receiver) = unbounded(); - let watcher = - log_notify_error(RecommendedWatcher::new_immediate(move |event| { - watcher_sender.send(event).unwrap() - })); + let watcher = log_notify_error(RecommendedWatcher::new(move |event| { + watcher_sender.send(event).unwrap() + })); self.watcher = watcher.map(|it| (it, watcher_receiver)); } @@ -215,7 +214,7 @@ impl NotifyActor { fn watch(&mut self, path: AbsPathBuf) { if let Some((watcher, _)) = &mut self.watcher { - log_notify_error(watcher.watch(&path, RecursiveMode::NonRecursive)); + log_notify_error(watcher.watch(path.as_ref(), RecursiveMode::NonRecursive)); } } fn send(&mut self, msg: loader::Message) {