Make "Replace qualified name with use" replace *all* mentions of the path #4878
Conversation
matklad
left a comment
matklad
left a comment
There was a problem hiding this comment.
Overall LGTM, but I think we shouldn't be using hir::Path here. To constcut hir::Path you need to know hygine info, and that's not something IDE should now about (hopefully :) )
I think just threading ast::Path everywhere should work. You'll need compare_paths(lhs: &ast::Path, rhs: &ast::Path) -> bool function, but it shoudn't be hard to do (and I think already is implemented in merge_use_items intention?)
| )); | ||
| // Now that we've brought the name into scope, re-qualify all paths that could be | ||
| // affected (that is, all paths inside the node we added the `use` to). | ||
| let hir_path = match hir::Path::from_ast(path.clone()) { |
There was a problem hiding this comment.
It seems like it should either be ModPath, or maybe even ast::Path?
| utils::{find_insert_use_container, insert_use_statement}, | ||
| AssistContext, AssistId, Assists, | ||
| }; | ||
| use either::Either; |
There was a problem hiding this comment.
Should be with other extern crates.
| None => return, | ||
| }; | ||
| let mut rewriter = SyntaxRewriter::default(); | ||
| match container { |
There was a problem hiding this comment.
I belive there's a combinator to map Either<A, B> to T which you can use to avoid duplicate arms.
| } | ||
|
|
||
| /// Adds replacements to `re` that shorten `path` in all descendants of `node`. | ||
| fn shorten_paths(re: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { |
There was a problem hiding this comment.
| fn shorten_paths(re: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { | |
| fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { |
We generely try to use verbose names.
| p: &ast::Path, | ||
| path: &ModPath, | ||
| ) -> Option<()> { | ||
| let hir_path = hir::Path::from_ast(p.clone())?; |
There was a problem hiding this comment.
hir::Path::from_ast should really die in flames. It's a broken API, because it can't take hygiene into account. There's only a couple of calls to this function from IDE, and we probably should find ways to remove them.
| fn replaces_all_affected_paths() { | ||
| check_assist( | ||
| replace_qualified_name_with_use, | ||
| " |
There was a problem hiding this comment.
We generally use raw literals here, even if that's not required, because they get syntax higlighting.
|
bors r+ |
This comment has been minimized.
This comment has been minimized.
|
Does that test assists? |
|
Ah, sorry, that was #4849. Not sure how I ended up on the wrong PR. |
3 participants
Fixes #4836