From 919a1d7b278ada6063c948df7e63d3ef735af343 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 9 Jan 2021 16:59:00 +0100 Subject: Refactor rename name checking --- crates/ide/src/hover.rs | 7 +- crates/ide/src/references/rename.rs | 156 ++++++++++++++++++++++++------------ 2 files changed, 104 insertions(+), 59 deletions(-) (limited to 'crates/ide') diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs index 8cb4a51d8..88d215129 100644 --- a/crates/ide/src/hover.rs +++ b/crates/ide/src/hover.rs @@ -175,12 +175,7 @@ fn show_implementations_action(db: &RootDatabase, def: Definition) -> Option it.target_ty(db).as_adt(), _ => None, }?; - match adt { - Adt::Struct(it) => it.try_to_nav(db), - Adt::Union(it) => it.try_to_nav(db), - Adt::Enum(it) => it.try_to_nav(db), - } - .map(to_action) + adt.try_to_nav(db).map(to_action) } fn runnable_action( diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs index 53d79333c..dd322631b 100644 --- a/crates/ide/src/references/rename.rs +++ b/crates/ide/src/references/rename.rs @@ -20,10 +20,11 @@ use test_utils::mark; use text_edit::TextEdit; use crate::{ - references::find_all_refs, FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind, + FilePosition, FileSystemEdit, RangeInfo, Reference, ReferenceKind, ReferenceSearchResult, SourceChange, SourceFileEdit, TextRange, TextSize, }; +type RenameResult = Result; #[derive(Debug)] pub struct RenameError(pub(crate) String); @@ -38,7 +39,7 @@ impl Error for RenameError {} pub(crate) fn prepare_rename( db: &RootDatabase, position: FilePosition, -) -> Result, RenameError> { +) -> RenameResult> { let sema = Semantics::new(db); let source_file = sema.parse(position.file_id); let syntax = source_file.syntax(); @@ -49,10 +50,7 @@ pub(crate) fn prepare_rename( { rename_self_to_param(&sema, position, self_token, "dummy") } else { - let range = match find_all_refs(&sema, position, None) { - Some(RangeInfo { range, .. }) => range, - None => return Err(RenameError("No references found at position".to_string())), - }; + let RangeInfo { range, .. } = find_all_refs(&sema, position)?; Ok(RangeInfo::new(range, SourceChange::from(vec![]))) } .map(|info| RangeInfo::new(info.range, ())) @@ -62,7 +60,7 @@ pub(crate) fn rename( db: &RootDatabase, position: FilePosition, new_name: &str, -) -> Result, RenameError> { +) -> RenameResult> { let sema = Semantics::new(db); rename_with_semantics(&sema, position, new_name) } @@ -71,42 +69,18 @@ pub(crate) fn rename_with_semantics( sema: &Semantics, position: FilePosition, new_name: &str, -) -> Result, RenameError> { - let is_lifetime_name = match lex_single_syntax_kind(new_name) { - Some(res) => match res { - (SyntaxKind::IDENT, _) => false, - (SyntaxKind::UNDERSCORE, _) => false, - (SyntaxKind::SELF_KW, _) => return rename_to_self(&sema, position), - (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => true, - (SyntaxKind::LIFETIME_IDENT, _) => { - return Err(RenameError(format!( - "Invalid name `{0}`: Cannot rename lifetime to {0}", - new_name - ))) - } - (_, Some(syntax_error)) => { - return Err(RenameError(format!("Invalid name `{}`: {}", new_name, syntax_error))) - } - (_, None) => { - return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))) - } - }, - None => return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))), - }; - +) -> RenameResult> { let source_file = sema.parse(position.file_id); let syntax = source_file.syntax(); - // this is here to prevent lifetime renames from happening on modules and self - if is_lifetime_name { - rename_reference(&sema, position, new_name, is_lifetime_name) - } else if let Some(module) = find_module_at_offset(&sema, position, syntax) { + + if let Some(module) = find_module_at_offset(&sema, position, syntax) { rename_mod(&sema, position, module, new_name) } else if let Some(self_token) = syntax.token_at_offset(position.offset).find(|t| t.kind() == SyntaxKind::SELF_KW) { rename_self_to_param(&sema, position, self_token, new_name) } else { - rename_reference(&sema, position, new_name, is_lifetime_name) + rename_reference(&sema, position, new_name) } } @@ -127,6 +101,36 @@ pub(crate) fn will_rename_file( Some(change) } +#[derive(PartialEq)] +enum IdentifierKind { + Ident, + Lifetime, + ToSelf, + Underscore, +} + +fn check_identifier(new_name: &str) -> RenameResult { + match lex_single_syntax_kind(new_name) { + Some(res) => match res { + (SyntaxKind::IDENT, _) => Ok(IdentifierKind::Ident), + (SyntaxKind::UNDERSCORE, _) => Ok(IdentifierKind::Underscore), + (SyntaxKind::SELF_KW, _) => Ok(IdentifierKind::ToSelf), + (SyntaxKind::LIFETIME_IDENT, _) if new_name != "'static" && new_name != "'_" => { + Ok(IdentifierKind::Lifetime) + } + (SyntaxKind::LIFETIME_IDENT, _) => { + Err(format!("Invalid name `{0}`: Cannot rename lifetime to {0}", new_name)) + } + (_, Some(syntax_error)) => { + Err(format!("Invalid name `{}`: {}", new_name, syntax_error)) + } + (_, None) => Err(format!("Invalid name `{}`: not an identifier", new_name)), + }, + None => Err(format!("Invalid name `{}`: not an identifier", new_name)), + } + .map_err(RenameError) +} + fn find_module_at_offset( sema: &Semantics, position: FilePosition, @@ -155,6 +159,14 @@ fn find_module_at_offset( Some(module) } +fn find_all_refs( + sema: &Semantics, + position: FilePosition, +) -> RenameResult> { + crate::references::find_all_refs(sema, position, None) + .ok_or_else(|| RenameError("No references found at position".to_string())) +} + fn source_edit_from_reference( sema: &Semantics, reference: Reference, @@ -223,7 +235,13 @@ fn rename_mod( position: FilePosition, module: Module, new_name: &str, -) -> Result, RenameError> { +) -> RenameResult> { + if IdentifierKind::Ident != check_identifier(new_name)? { + return Err(RenameError(format!( + "Invalid name `{0}`: cannot rename module to {0}", + new_name + ))); + } let mut source_file_edits = Vec::new(); let mut file_system_edits = Vec::new(); @@ -254,8 +272,7 @@ fn rename_mod( source_file_edits.push(edit); } - let RangeInfo { range, info: refs } = find_all_refs(sema, position, None) - .ok_or_else(|| RenameError("No references found at position".to_string()))?; + let RangeInfo { range, info: refs } = find_all_refs(sema, position)?; let ref_edits = refs .references .into_iter() @@ -310,8 +327,7 @@ fn rename_to_self( return Err(RenameError("Parameter type differs from impl block type".to_string())); } - let RangeInfo { range, info: refs } = find_all_refs(sema, position, None) - .ok_or_else(|| RenameError("No reference found at position".to_string()))?; + let RangeInfo { range, info: refs } = find_all_refs(sema, position)?; let (param_ref, usages): (Vec, Vec) = refs .into_iter() @@ -367,6 +383,17 @@ fn rename_self_to_param( self_token: SyntaxToken, new_name: &str, ) -> Result, RenameError> { + let ident_kind = check_identifier(new_name)?; + match ident_kind { + IdentifierKind::Lifetime => { + return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))) + } + IdentifierKind::ToSelf => { + // no-op + return Ok(RangeInfo::new(self_token.text_range(), SourceChange::default())); + } + _ => (), + } let source_file = sema.parse(position.file_id); let syn = source_file.syntax(); @@ -395,6 +422,12 @@ fn rename_self_to_param( } } + if edits.len() > 1 && ident_kind == IdentifierKind::Underscore { + return Err(RenameError(format!( + "Cannot rename reference to `_` as it is being referenced multiple times", + ))); + } + let range = ast::SelfParam::cast(self_token.parent()) .map_or(self_token.text_range(), |p| p.syntax().text_range()); @@ -405,24 +438,36 @@ fn rename_reference( sema: &Semantics, position: FilePosition, new_name: &str, - is_lifetime_name: bool, ) -> Result, RenameError> { - let RangeInfo { range, info: refs } = match find_all_refs(sema, position, None) { - Some(range_info) => range_info, - None => return Err(RenameError("No references found at position".to_string())), - }; + let ident_kind = check_identifier(new_name)?; + let RangeInfo { range, info: refs } = find_all_refs(sema, position)?; - match (refs.declaration.kind == ReferenceKind::Lifetime, is_lifetime_name) { - (true, false) => { + match (ident_kind, &refs.declaration.kind) { + (IdentifierKind::ToSelf, ReferenceKind::Lifetime) + | (IdentifierKind::Underscore, ReferenceKind::Lifetime) + | (IdentifierKind::Ident, ReferenceKind::Lifetime) => { return Err(RenameError(format!( "Invalid name `{}`: not a lifetime identifier", new_name ))) } - (false, true) => { + (IdentifierKind::Lifetime, ReferenceKind::Lifetime) => (), + (IdentifierKind::Lifetime, _) => { return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))) } - _ => (), + (IdentifierKind::ToSelf, ReferenceKind::SelfKw) => { + //no-op + return Ok(RangeInfo::new(range, SourceChange::default())); + } + (IdentifierKind::ToSelf, _) => { + return rename_to_self(sema, position); + } + (IdentifierKind::Underscore, _) if !refs.references.is_empty() => { + return Err(RenameError(format!( + "Cannot rename reference to `_` as it is being referenced multiple times", + ))) + } + (IdentifierKind::Ident, _) | (IdentifierKind::Underscore, _) => (), } let edit = refs @@ -430,10 +475,6 @@ fn rename_reference( .map(|reference| source_edit_from_reference(sema, reference, new_name)) .collect::>(); - if edit.is_empty() { - return Err(RenameError("No references found at position".to_string())); - } - Ok(RangeInfo::new(range, SourceChange::from(edit))) } @@ -546,6 +587,15 @@ mod tests { ); } + #[test] + fn test_rename_to_underscore_invalid() { + check( + "_", + r#"fn main(foo$0: ()) {foo;}"#, + "error: Cannot rename reference to `_` as it is being referenced multiple times", + ); + } + #[test] fn test_rename_for_local() { check( -- cgit v1.2.3