From 5d66bfe16343141132c8a7aefa727b209ec3e66a Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 13 Jun 2020 19:05:31 +0200 Subject: Shorten *all* qualified paths when adding use --- crates/ra_assists/src/assist_context.rs | 2 +- .../handlers/replace_qualified_name_with_use.rs | 201 ++++++++++++++++++++- 2 files changed, 192 insertions(+), 11 deletions(-) diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index edd8255f4..ee614de72 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -252,7 +252,7 @@ impl AssistBuilder { pub(crate) fn rewrite(&mut self, rewriter: SyntaxRewriter) { let node = rewriter.rewrite_root().unwrap(); let new = rewriter.rewrite(&node); - algo::diff(&node, &new).into_text_edit(&mut self.edit) + algo::diff(&node, &new).into_text_edit(&mut self.edit); } // FIXME: kill this API diff --git a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs index 0197a8cf0..6cbf8309b 100644 --- a/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/ra_assists/src/handlers/replace_qualified_name_with_use.rs @@ -1,7 +1,11 @@ -use hir; -use ra_syntax::{ast, AstNode, SmolStr, TextRange}; +use hir::{self, ModPath}; +use ra_syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode}; -use crate::{utils::insert_use_statement, AssistContext, AssistId, Assists}; +use crate::{ + utils::{find_insert_use_container, insert_use_statement}, + AssistContext, AssistId, Assists, +}; +use either::Either; // Assist: replace_qualified_name_with_use // @@ -39,16 +43,28 @@ pub(crate) fn replace_qualified_name_with_use( target, |builder| { let path_to_import = hir_path.mod_path().clone(); + let container = match find_insert_use_container(path.syntax(), ctx) { + Some(c) => c, + None => return, + }; insert_use_statement(path.syntax(), &path_to_import, ctx, builder.text_edit_builder()); - if let Some(last) = path.segment() { - // Here we are assuming the assist will provide a correct use statement - // so we can delete the path qualifier - builder.delete(TextRange::new( - path.syntax().text_range().start(), - last.syntax().text_range().start(), - )); + // 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()) { + Some(p) => p, + None => return, + }; + let mut rewriter = SyntaxRewriter::default(); + match container { + Either::Left(l) => { + shorten_paths(&mut rewriter, l.syntax().clone(), hir_path.mod_path()); + } + Either::Right(r) => { + shorten_paths(&mut rewriter, r.syntax().clone(), hir_path.mod_path()); + } } + builder.rewrite(rewriter); }, ) } @@ -73,6 +89,59 @@ fn collect_hir_path_segments(path: &hir::Path) -> Option> { Some(ps) } +/// Adds replacements to `re` that shorten `path` in all descendants of `node`. +fn shorten_paths(re: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { + for child in node.children() { + match_ast! { + match child { + // Don't modify `use` items, as this can break the `use` item when injecting a new + // import into the use tree. + ast::UseItem(_it) => continue, + // Don't descend into submodules, they don't have the same `use` items in scope. + ast::Module(_it) => continue, + + ast::Path(p) => { + match maybe_replace_path(re, &p, path) { + Some(()) => {}, + None => shorten_paths(re, p.syntax().clone(), path), + } + }, + _ => shorten_paths(re, child, path), + } + } + } +} + +fn maybe_replace_path( + re: &mut SyntaxRewriter<'static>, + p: &ast::Path, + path: &ModPath, +) -> Option<()> { + let hir_path = hir::Path::from_ast(p.clone())?; + + if hir_path.mod_path() != path { + return None; + } + + // Replace path with its last "plain" segment. + let mut mod_path = hir_path.mod_path().clone(); + let last = mod_path.segments.len() - 1; + mod_path.segments.swap(0, last); + mod_path.segments.truncate(1); + mod_path.kind = hir::PathKind::Plain; + + let mut new_path = crate::ast_transform::path_to_ast(mod_path); + + let type_args = p.segment().and_then(|s| s.type_arg_list()); + if let Some(type_args) = type_args { + let last_segment = new_path.segment().unwrap(); + new_path = new_path.with_segment(last_segment.with_type_args(type_args)); + } + + re.replace(p.syntax(), new_path.syntax()); + Some(()) +} + #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; @@ -459,6 +528,118 @@ use std::fmt::Debug; fn main() { Debug +} + ", + ); + } + + #[test] + fn replaces_all_affected_paths() { + check_assist( + replace_qualified_name_with_use, + " +fn main() { + std::fmt::Debug<|>; + let x: std::fmt::Debug = std::fmt::Debug; +} + ", + " +use std::fmt::Debug; + +fn main() { + Debug; + let x: Debug = Debug; +} + ", + ); + } + + #[test] + fn replaces_all_affected_paths_mod() { + check_assist( + replace_qualified_name_with_use, + " +mod m { + fn f() { + std::fmt::Debug<|>; + let x: std::fmt::Debug = std::fmt::Debug; + } + fn g() { + std::fmt::Debug; + } +} + +fn f() { + std::fmt::Debug; +} + ", + " +mod m { + use std::fmt::Debug; + + fn f() { + Debug; + let x: Debug = Debug; + } + fn g() { + Debug; + } +} + +fn f() { + std::fmt::Debug; +} + ", + ); + } + + #[test] + fn does_not_replace_in_submodules() { + check_assist( + replace_qualified_name_with_use, + " +fn main() { + std::fmt::Debug<|>; +} + +mod sub { + fn f() { + std::fmt::Debug; + } +} + ", + " +use std::fmt::Debug; + +fn main() { + Debug; +} + +mod sub { + fn f() { + std::fmt::Debug; + } +} + ", + ); + } + + #[test] + fn does_not_replace_in_use() { + check_assist( + replace_qualified_name_with_use, + " +use std::fmt::Display; + +fn main() { + std::fmt<|>; +} + ", + " +use std::fmt::{self, Display}; + +fn main() { + fmt; } ", ); -- cgit v1.2.3