Unnamed repository; edit this file 'description' to name the repository.
| -rw-r--r-- | crates/hir_expand/src/lib.rs | 55 | ||||
| -rw-r--r-- | crates/ide/src/display/navigation_target.rs | 7 | ||||
| -rw-r--r-- | crates/ide/src/rename.rs | 9 | ||||
| -rw-r--r-- | crates/ide/src/runnables.rs | 5 | ||||
| -rw-r--r-- | crates/ide_db/src/rename.rs | 52 |
5 files changed, 62 insertions, 66 deletions
diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 736f482924..1452ab08d9 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<ast::Macro>, AstId<ast::Fn>> { 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) } @@ -464,15 +465,10 @@ impl InFile<SyntaxNode> { } 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 }; - } - - log::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<FileRange> { + match original_range_opt(db, self) { + Some(range) => { + let original_file = range.file_id.original_file(db); + if range.file_id != original_file.into() { + log::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 84a2f63637..b8c14dc97b 100644 --- a/crates/ide/src/display/navigation_target.rs +++ b/crates/ide/src/display/navigation_target.rs @@ -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) diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index 9d452f40ba..a4297a2fec 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -1869,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#" @@ -1878,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 2cf801751c..42f6ec5d9c 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_db/src/rename.rs b/crates/ide_db/src/rename.rs index ef8b31ba5f..a6f7c09af8 100644 --- a/crates/ide_db/src/rename.rs +++ b/crates/ide_db/src/rename.rs @@ -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<RootDatabase>) -> Option<FileRange> { - // 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<D>(def: D, sema: &Semantics<RootDatabase>) -> Option<FileRange> 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) } } } |