aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRĂ¼diger Herrmann <[email protected]>2020-08-24 15:37:45 +0100
committerRĂ¼diger Herrmann <[email protected]>2020-10-10 12:46:10 +0100
commite90931a2047e6e38f173e2e2f2a24ad648e92e9d (patch)
tree0ef92bcd580446c4ea44902f0f959d547a698943
parent05261f5aeb1dcf01c5dfcf3e01826e57b1249ca9 (diff)
Show reason for failed rename refactoring
Return an error with a meaningful message for requests to `textDocument/rename` if the operation cannot be performed. Pass errors raised by rename handling code to the LSP runtime. As a consequence, the VS Code client shows and logs the request as if a server-side programming error occured. Resolves https://github.com/rust-analyzer/rust-analyzer/issues/3981
-rw-r--r--crates/ide/src/lib.rs6
-rw-r--r--crates/ide/src/references.rs1
-rw-r--r--crates/ide/src/references/rename.rs175
-rw-r--r--crates/rust-analyzer/src/handlers.rs17
4 files changed, 138 insertions, 61 deletions
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index 57f3581b6..d54c06b14 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -77,7 +77,9 @@ pub use crate::{
77 hover::{HoverAction, HoverConfig, HoverGotoTypeData, HoverResult}, 77 hover::{HoverAction, HoverConfig, HoverGotoTypeData, HoverResult},
78 inlay_hints::{InlayHint, InlayHintsConfig, InlayKind}, 78 inlay_hints::{InlayHint, InlayHintsConfig, InlayKind},
79 markup::Markup, 79 markup::Markup,
80 references::{Declaration, Reference, ReferenceAccess, ReferenceKind, ReferenceSearchResult}, 80 references::{
81 Declaration, Reference, ReferenceAccess, ReferenceKind, ReferenceSearchResult, RenameError,
82 },
81 runnables::{Runnable, RunnableKind, TestId}, 83 runnables::{Runnable, RunnableKind, TestId},
82 syntax_highlighting::{ 84 syntax_highlighting::{
83 Highlight, HighlightModifier, HighlightModifiers, HighlightTag, HighlightedRange, 85 Highlight, HighlightModifier, HighlightModifiers, HighlightTag, HighlightedRange,
@@ -490,7 +492,7 @@ impl Analysis {
490 &self, 492 &self,
491 position: FilePosition, 493 position: FilePosition,
492 new_name: &str, 494 new_name: &str,
493 ) -> Cancelable<Option<RangeInfo<SourceChange>>> { 495 ) -> Cancelable<Result<RangeInfo<SourceChange>, RenameError>> {
494 self.with_db(|db| references::rename(db, position, new_name)) 496 self.with_db(|db| references::rename(db, position, new_name))
495 } 497 }
496 498
diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs
index 9315f7354..f65a05ea3 100644
--- a/crates/ide/src/references.rs
+++ b/crates/ide/src/references.rs
@@ -26,6 +26,7 @@ use syntax::{
26use crate::{display::TryToNav, FilePosition, FileRange, NavigationTarget, RangeInfo}; 26use crate::{display::TryToNav, FilePosition, FileRange, NavigationTarget, RangeInfo};
27 27
28pub(crate) use self::rename::rename; 28pub(crate) use self::rename::rename;
29pub use self::rename::RenameError;
29 30
30pub use ide_db::search::{Reference, ReferenceAccess, ReferenceKind}; 31pub use ide_db::search::{Reference, ReferenceAccess, ReferenceKind};
31 32
diff --git a/crates/ide/src/references/rename.rs b/crates/ide/src/references/rename.rs
index 8cbe1ae5a..f3b5cfc8c 100644
--- a/crates/ide/src/references/rename.rs
+++ b/crates/ide/src/references/rename.rs
@@ -6,11 +6,16 @@ use ide_db::{
6 defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass}, 6 defs::{classify_name, classify_name_ref, Definition, NameClass, NameRefClass},
7 RootDatabase, 7 RootDatabase,
8}; 8};
9use std::convert::TryInto; 9
10use std::{
11 convert::TryInto,
12 error::Error,
13 fmt::{self, Display},
14};
10use syntax::{ 15use syntax::{
11 algo::find_node_at_offset, 16 algo::find_node_at_offset,
12 ast::{self, NameOwner}, 17 ast::{self, NameOwner},
13 lex_single_valid_syntax_kind, match_ast, AstNode, SyntaxKind, SyntaxNode, SyntaxToken, 18 lex_single_syntax_kind, match_ast, AstNode, SyntaxKind, SyntaxNode, SyntaxToken,
14}; 19};
15use test_utils::mark; 20use test_utils::mark;
16use text_edit::TextEdit; 21use text_edit::TextEdit;
@@ -20,17 +25,37 @@ use crate::{
20 SourceChange, SourceFileEdit, TextRange, TextSize, 25 SourceChange, SourceFileEdit, TextRange, TextSize,
21}; 26};
22 27
28#[derive(Debug)]
29pub struct RenameError(pub(crate) String);
30
31impl fmt::Display for RenameError {
32 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
33 Display::fmt(&self.0, f)
34 }
35}
36
37impl Error for RenameError {}
38
23pub(crate) fn rename( 39pub(crate) fn rename(
24 db: &RootDatabase, 40 db: &RootDatabase,
25 position: FilePosition, 41 position: FilePosition,
26 new_name: &str, 42 new_name: &str,
27) -> Option<RangeInfo<SourceChange>> { 43) -> Result<RangeInfo<SourceChange>, RenameError> {
28 let sema = Semantics::new(db); 44 let sema = Semantics::new(db);
29 45
30 match lex_single_valid_syntax_kind(new_name)? { 46 match lex_single_syntax_kind(new_name) {
31 SyntaxKind::IDENT | SyntaxKind::UNDERSCORE => (), 47 Some(res) => match res {
32 SyntaxKind::SELF_KW => return rename_to_self(&sema, position), 48 (SyntaxKind::IDENT, _) => (),
33 _ => return None, 49 (SyntaxKind::UNDERSCORE, _) => (),
50 (SyntaxKind::SELF_KW, _) => return rename_to_self(&sema, position),
51 (_, Some(syntax_error)) => {
52 return Err(RenameError(format!("Invalid name `{}`: {}", new_name, syntax_error)))
53 }
54 (_, None) => {
55 return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name)))
56 }
57 },
58 None => return Err(RenameError(format!("Invalid name `{}`: not an identifier", new_name))),
34 } 59 }
35 60
36 let source_file = sema.parse(position.file_id); 61 let source_file = sema.parse(position.file_id);
@@ -103,7 +128,7 @@ fn rename_mod(
103 position: FilePosition, 128 position: FilePosition,
104 module: Module, 129 module: Module,
105 new_name: &str, 130 new_name: &str,
106) -> Option<RangeInfo<SourceChange>> { 131) -> Result<RangeInfo<SourceChange>, RenameError> {
107 let mut source_file_edits = Vec::new(); 132 let mut source_file_edits = Vec::new();
108 let mut file_system_edits = Vec::new(); 133 let mut file_system_edits = Vec::new();
109 134
@@ -125,7 +150,7 @@ fn rename_mod(
125 150
126 if let Some(src) = module.declaration_source(sema.db) { 151 if let Some(src) = module.declaration_source(sema.db) {
127 let file_id = src.file_id.original_file(sema.db); 152 let file_id = src.file_id.original_file(sema.db);
128 let name = src.value.name()?; 153 let name = src.value.name().unwrap();
129 let edit = SourceFileEdit { 154 let edit = SourceFileEdit {
130 file_id, 155 file_id,
131 edit: TextEdit::replace(name.syntax().text_range(), new_name.into()), 156 edit: TextEdit::replace(name.syntax().text_range(), new_name.into()),
@@ -133,35 +158,40 @@ fn rename_mod(
133 source_file_edits.push(edit); 158 source_file_edits.push(edit);
134 } 159 }
135 160
136 let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)?; 161 let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
162 .ok_or_else(|| RenameError("No references found at position".to_string()))?;
137 let ref_edits = refs 163 let ref_edits = refs
138 .references 164 .references
139 .into_iter() 165 .into_iter()
140 .map(|reference| source_edit_from_reference(reference, new_name)); 166 .map(|reference| source_edit_from_reference(reference, new_name));
141 source_file_edits.extend(ref_edits); 167 source_file_edits.extend(ref_edits);
142 168
143 Some(RangeInfo::new(range, SourceChange::from_edits(source_file_edits, file_system_edits))) 169 Ok(RangeInfo::new(range, SourceChange::from_edits(source_file_edits, file_system_edits)))
144} 170}
145 171
146fn rename_to_self( 172fn rename_to_self(
147 sema: &Semantics<RootDatabase>, 173 sema: &Semantics<RootDatabase>,
148 position: FilePosition, 174 position: FilePosition,
149) -> Option<RangeInfo<SourceChange>> { 175) -> Result<RangeInfo<SourceChange>, RenameError> {
150 let source_file = sema.parse(position.file_id); 176 let source_file = sema.parse(position.file_id);
151 let syn = source_file.syntax(); 177 let syn = source_file.syntax();
152 178
153 let fn_def = find_node_at_offset::<ast::Fn>(syn, position.offset)?; 179 let fn_def = find_node_at_offset::<ast::Fn>(syn, position.offset)
154 let params = fn_def.param_list()?; 180 .ok_or_else(|| RenameError("No surrounding method declaration found".to_string()))?;
181 let params =
182 fn_def.param_list().ok_or_else(|| RenameError("Method has no parameters".to_string()))?;
155 if params.self_param().is_some() { 183 if params.self_param().is_some() {
156 return None; // method already has self param 184 return Err(RenameError("Method already has a self parameter".to_string()));
157 } 185 }
158 let first_param = params.params().next()?; 186 let first_param =
187 params.params().next().ok_or_else(|| RenameError("Method has no parameters".into()))?;
159 let mutable = match first_param.ty() { 188 let mutable = match first_param.ty() {
160 Some(ast::Type::RefType(rt)) => rt.mut_token().is_some(), 189 Some(ast::Type::RefType(rt)) => rt.mut_token().is_some(),
161 _ => return None, // not renaming other types 190 _ => return Err(RenameError("Not renaming other types".to_string())),
162 }; 191 };
163 192
164 let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)?; 193 let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)
194 .ok_or_else(|| RenameError("No reference found at position".to_string()))?;
165 195
166 let param_range = first_param.syntax().text_range(); 196 let param_range = first_param.syntax().text_range();
167 let (param_ref, usages): (Vec<Reference>, Vec<Reference>) = refs 197 let (param_ref, usages): (Vec<Reference>, Vec<Reference>) = refs
@@ -169,7 +199,7 @@ fn rename_to_self(
169 .partition(|reference| param_range.intersect(reference.file_range.range).is_some()); 199 .partition(|reference| param_range.intersect(reference.file_range.range).is_some());
170 200
171 if param_ref.is_empty() { 201 if param_ref.is_empty() {
172 return None; 202 return Err(RenameError("Parameter to rename not found".to_string()));
173 } 203 }
174 204
175 let mut edits = usages 205 let mut edits = usages
@@ -185,7 +215,7 @@ fn rename_to_self(
185 ), 215 ),
186 }); 216 });
187 217
188 Some(RangeInfo::new(range, SourceChange::from(edits))) 218 Ok(RangeInfo::new(range, SourceChange::from(edits)))
189} 219}
190 220
191fn text_edit_from_self_param( 221fn text_edit_from_self_param(
@@ -216,12 +246,13 @@ fn rename_self_to_param(
216 position: FilePosition, 246 position: FilePosition,
217 self_token: SyntaxToken, 247 self_token: SyntaxToken,
218 new_name: &str, 248 new_name: &str,
219) -> Option<RangeInfo<SourceChange>> { 249) -> Result<RangeInfo<SourceChange>, RenameError> {
220 let source_file = sema.parse(position.file_id); 250 let source_file = sema.parse(position.file_id);
221 let syn = source_file.syntax(); 251 let syn = source_file.syntax();
222 252
223 let text = sema.db.file_text(position.file_id); 253 let text = sema.db.file_text(position.file_id);
224 let fn_def = find_node_at_offset::<ast::Fn>(syn, position.offset)?; 254 let fn_def = find_node_at_offset::<ast::Fn>(syn, position.offset)
255 .ok_or_else(|| RenameError("No surrounding method declaration found".to_string()))?;
225 let search_range = fn_def.syntax().text_range(); 256 let search_range = fn_def.syntax().text_range();
226 257
227 let mut edits: Vec<SourceFileEdit> = vec![]; 258 let mut edits: Vec<SourceFileEdit> = vec![];
@@ -235,7 +266,8 @@ fn rename_self_to_param(
235 syn.token_at_offset(offset).find(|t| t.kind() == SyntaxKind::SELF_KW) 266 syn.token_at_offset(offset).find(|t| t.kind() == SyntaxKind::SELF_KW)
236 { 267 {
237 let edit = if let Some(ref self_param) = ast::SelfParam::cast(usage.parent()) { 268 let edit = if let Some(ref self_param) = ast::SelfParam::cast(usage.parent()) {
238 text_edit_from_self_param(syn, self_param, new_name)? 269 text_edit_from_self_param(syn, self_param, new_name)
270 .ok_or_else(|| RenameError("No target type found".to_string()))?
239 } else { 271 } else {
240 TextEdit::replace(usage.text_range(), String::from(new_name)) 272 TextEdit::replace(usage.text_range(), String::from(new_name))
241 }; 273 };
@@ -246,15 +278,18 @@ fn rename_self_to_param(
246 let range = ast::SelfParam::cast(self_token.parent()) 278 let range = ast::SelfParam::cast(self_token.parent())
247 .map_or(self_token.text_range(), |p| p.syntax().text_range()); 279 .map_or(self_token.text_range(), |p| p.syntax().text_range());
248 280
249 Some(RangeInfo::new(range, SourceChange::from(edits))) 281 Ok(RangeInfo::new(range, SourceChange::from(edits)))
250} 282}
251 283
252fn rename_reference( 284fn rename_reference(
253 sema: &Semantics<RootDatabase>, 285 sema: &Semantics<RootDatabase>,
254 position: FilePosition, 286 position: FilePosition,
255 new_name: &str, 287 new_name: &str,
256) -> Option<RangeInfo<SourceChange>> { 288) -> Result<RangeInfo<SourceChange>, RenameError> {
257 let RangeInfo { range, info: refs } = find_all_refs(sema, position, None)?; 289 let RangeInfo { range, info: refs } = match find_all_refs(sema, position, None) {
290 Some(range_info) => range_info,
291 None => return Err(RenameError("No references found at position".to_string())),
292 };
258 293
259 let edit = refs 294 let edit = refs
260 .into_iter() 295 .into_iter()
@@ -262,10 +297,10 @@ fn rename_reference(
262 .collect::<Vec<_>>(); 297 .collect::<Vec<_>>();
263 298
264 if edit.is_empty() { 299 if edit.is_empty() {
265 return None; 300 return Err(RenameError("No references found at position".to_string()));
266 } 301 }
267 302
268 Some(RangeInfo::new(range, SourceChange::from(edit))) 303 Ok(RangeInfo::new(range, SourceChange::from(edit)))
269} 304}
270 305
271#[cfg(test)] 306#[cfg(test)]
@@ -280,25 +315,45 @@ mod tests {
280 fn check(new_name: &str, ra_fixture_before: &str, ra_fixture_after: &str) { 315 fn check(new_name: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
281 let ra_fixture_after = &trim_indent(ra_fixture_after); 316 let ra_fixture_after = &trim_indent(ra_fixture_after);
282 let (analysis, position) = fixture::position(ra_fixture_before); 317 let (analysis, position) = fixture::position(ra_fixture_before);
283 let source_change = analysis.rename(position, new_name).unwrap(); 318 let rename_result = analysis
284 let mut text_edit_builder = TextEdit::builder(); 319 .rename(position, new_name)
285 let mut file_id: Option<FileId> = None; 320 .unwrap_or_else(|err| panic!("Rename to '{}' was cancelled: {}", new_name, err));
286 if let Some(change) = source_change { 321 match rename_result {
287 for edit in change.info.source_file_edits { 322 Ok(source_change) => {
288 file_id = Some(edit.file_id); 323 let mut text_edit_builder = TextEdit::builder();
289 for indel in edit.edit.into_iter() { 324 let mut file_id: Option<FileId> = None;
290 text_edit_builder.replace(indel.delete, indel.insert); 325 for edit in source_change.info.source_file_edits {
326 file_id = Some(edit.file_id);
327 for indel in edit.edit.into_iter() {
328 text_edit_builder.replace(indel.delete, indel.insert);
329 }
291 } 330 }
331 let mut result = analysis.file_text(file_id.unwrap()).unwrap().to_string();
332 text_edit_builder.finish().apply(&mut result);
333 assert_eq_text!(ra_fixture_after, &*result);
292 } 334 }
293 } 335 Err(err) => {
294 let mut result = analysis.file_text(file_id.unwrap()).unwrap().to_string(); 336 if ra_fixture_after.starts_with("error:") {
295 text_edit_builder.finish().apply(&mut result); 337 let error_message = ra_fixture_after
296 assert_eq_text!(ra_fixture_after, &*result); 338 .chars()
339 .into_iter()
340 .skip("error:".len())
341 .collect::<String>();
342 assert_eq!(error_message.trim(), err.to_string());
343 return;
344 } else {
345 panic!("Rename to '{}' failed unexpectedly: {}", new_name, err)
346 }
347 }
348 };
297 } 349 }
298 350
299 fn check_expect(new_name: &str, ra_fixture: &str, expect: Expect) { 351 fn check_expect(new_name: &str, ra_fixture: &str, expect: Expect) {
300 let (analysis, position) = fixture::position(ra_fixture); 352 let (analysis, position) = fixture::position(ra_fixture);
301 let source_change = analysis.rename(position, new_name).unwrap().unwrap(); 353 let source_change = analysis
354 .rename(position, new_name)
355 .unwrap()
356 .expect("Expect returned RangeInfo to be Some, but was None");
302 expect.assert_debug_eq(&source_change) 357 expect.assert_debug_eq(&source_change)
303 } 358 }
304 359
@@ -313,11 +368,30 @@ mod tests {
313 } 368 }
314 369
315 #[test] 370 #[test]
316 fn test_rename_to_invalid_identifier() { 371 fn test_rename_to_invalid_identifier1() {
317 let (analysis, position) = fixture::position(r#"fn main() { let i<|> = 1; }"#); 372 check(
318 let new_name = "invalid!"; 373 "invalid!",
319 let source_change = analysis.rename(position, new_name).unwrap(); 374 r#"fn main() { let i<|> = 1; }"#,
320 assert!(source_change.is_none()); 375 "error: Invalid name `invalid!`: not an identifier",
376 );
377 }
378
379 #[test]
380 fn test_rename_to_invalid_identifier2() {
381 check(
382 "multiple tokens",
383 r#"fn main() { let i<|> = 1; }"#,
384 "error: Invalid name `multiple tokens`: not an identifier",
385 );
386 }
387
388 #[test]
389 fn test_rename_to_invalid_identifier3() {
390 check(
391 "let",
392 r#"fn main() { let i<|> = 1; }"#,
393 "error: Invalid name `let`: not an identifier",
394 );
321 } 395 }
322 396
323 #[test] 397 #[test]
@@ -350,6 +424,15 @@ fn main() {
350 } 424 }
351 425
352 #[test] 426 #[test]
427 fn test_rename_unresolved_reference() {
428 check(
429 "new_name",
430 r#"fn main() { let _ = unresolved_ref<|>; }"#,
431 "error: No references found at position",
432 );
433 }
434
435 #[test]
353 fn test_rename_for_macro_args() { 436 fn test_rename_for_macro_args() {
354 check( 437 check(
355 "b", 438 "b",
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 468655f9c..4e3340b0d 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -646,14 +646,9 @@ pub(crate) fn handle_prepare_rename(
646 let _p = profile::span("handle_prepare_rename"); 646 let _p = profile::span("handle_prepare_rename");
647 let position = from_proto::file_position(&snap, params)?; 647 let position = from_proto::file_position(&snap, params)?;
648 648
649 let optional_change = snap.analysis.rename(position, "dummy")?; 649 let change = snap.analysis.rename(position, "dummy")??;
650 let range = match optional_change {
651 None => return Ok(None),
652 Some(it) => it.range,
653 };
654
655 let line_index = snap.analysis.file_line_index(position.file_id)?; 650 let line_index = snap.analysis.file_line_index(position.file_id)?;
656 let range = to_proto::range(&line_index, range); 651 let range = to_proto::range(&line_index, change.range);
657 Ok(Some(PrepareRenameResponse::Range(range))) 652 Ok(Some(PrepareRenameResponse::Range(range)))
658} 653}
659 654
@@ -672,12 +667,8 @@ pub(crate) fn handle_rename(
672 .into()); 667 .into());
673 } 668 }
674 669
675 let optional_change = snap.analysis.rename(position, &*params.new_name)?; 670 let change = snap.analysis.rename(position, &*params.new_name)??;
676 let source_change = match optional_change { 671 let workspace_edit = to_proto::workspace_edit(&snap, change.info)?;
677 None => return Ok(None),
678 Some(it) => it.info,
679 };
680 let workspace_edit = to_proto::workspace_edit(&snap, source_change)?;
681 Ok(Some(workspace_edit)) 672 Ok(Some(workspace_edit))
682} 673}
683 674