Properly handle shorthands in destructure patterns when renaming #6552
Conversation
crates/ide/src/references/rename.rs
Outdated
| } | ||
|
|
||
| #[test] | ||
| fn test_struct_field_destructure_into_shorthand() { |
There was a problem hiding this comment.
Hm, should we also check for destructuring in arguments too fn foo()?
Nit: we can also put all rename tests into one snippet to slightly reduce the test count, diff size, etc.
There was a problem hiding this comment.
Regarding the nit you mean as in merging most of the before_fixtures of the difference cases to reuse them behind a static? Ah nevermind you mean merging the test functions and test multiple things per test.
And yes, testing for parameter destructuring also seems like a good idea
There was a problem hiding this comment.
Hm I don't think we can put the tests together though, as they require slightly different fixtures and/or different <|> cursor positions. Edit: Actually two can be put together as their expected fixture is the same.
There was a problem hiding this comment.
Oh, never mind then, I din't think it's worth supporting the multiple caret tests for this case.
| @@ -126,6 +126,7 @@ fn source_edit_from_reference( | |||
| TextRange::new(reference.file_range.range.end(), reference.file_range.range.end()) | |||
| } | |||
| ReferenceKind::RecordFieldExprOrPat => { | |||
| mark::hit!(test_rename_field_expr_pat); | |||
|
bors r+ |
| @@ -22,6 +22,18 @@ impl ast::Expr { | |||
| _ => false, | |||
| } | |||
| } | |||
|
|
|||
| pub fn name_ref(&self) -> Option<ast::NameRef> { | |||
There was a problem hiding this comment.
The same thing here -- this methong is non-primitives -- it's a convenience function which reaches deep into the node. We should have such functions, but not as inherent methods on AST nodes.
There was a problem hiding this comment.
the same -- see my recent comment on another PR :-)
Fixes #6548 and #6551.