Properly handle shorthands in destructure patterns when renaming#6552
Properly handle shorthands in destructure patterns when renaming#6552bors[bot] merged 4 commits intorust-lang:masterfrom
Conversation
| } | ||
|
|
||
| #[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.
| TextRange::new(reference.file_range.range.end(), reference.file_range.range.end()) | ||
| } | ||
| ReferenceKind::RecordFieldExprOrPat => { | ||
| mark::hit!(test_rename_field_expr_pat); |
|
bors r+ |
| } | ||
| } | ||
|
|
||
| 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 :-)
3 participants
Fixes #6548 and #6551.