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 --- .../handlers/replace_qualified_name_with_use.rs | 201 ++++++++++++++++++++- 1 file changed, 191 insertions(+), 10 deletions(-) (limited to 'crates/ra_assists/src/handlers') 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 From 71c002e58947a7e19d2ba4bc41336618a087824b Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 15 Jun 2020 22:39:26 +0200 Subject: It's fookin' raw --- .../handlers/replace_qualified_name_with_use.rs | 96 +++++++++++----------- 1 file changed, 48 insertions(+), 48 deletions(-) (limited to 'crates/ra_assists/src/handlers') 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 6cbf8309b..301ae8497 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 @@ -152,10 +152,10 @@ mod tests { fn test_replace_add_use_no_anchor() { check_assist( replace_qualified_name_with_use, - " + r" std::fmt::Debug<|> ", - " + r" use std::fmt::Debug; Debug @@ -166,13 +166,13 @@ Debug fn test_replace_add_use_no_anchor_with_item_below() { check_assist( replace_qualified_name_with_use, - " + r" std::fmt::Debug<|> fn main() { } ", - " + r" use std::fmt::Debug; Debug @@ -187,13 +187,13 @@ fn main() { fn test_replace_add_use_no_anchor_with_item_above() { check_assist( replace_qualified_name_with_use, - " + r" fn main() { } std::fmt::Debug<|> ", - " + r" use std::fmt::Debug; fn main() { @@ -208,10 +208,10 @@ Debug fn test_replace_add_use_no_anchor_2seg() { check_assist( replace_qualified_name_with_use, - " + r" std::fmt<|>::Debug ", - " + r" use std::fmt; fmt::Debug @@ -223,13 +223,13 @@ fmt::Debug fn test_replace_add_use() { check_assist( replace_qualified_name_with_use, - " + r" use stdx; impl std::fmt::Debug<|> for Foo { } ", - " + r" use stdx; use std::fmt::Debug; @@ -243,11 +243,11 @@ impl Debug for Foo { fn test_replace_file_use_other_anchor() { check_assist( replace_qualified_name_with_use, - " + r" impl std::fmt::Debug<|> for Foo { } ", - " + r" use std::fmt::Debug; impl Debug for Foo { @@ -260,11 +260,11 @@ impl Debug for Foo { fn test_replace_add_use_other_anchor_indent() { check_assist( replace_qualified_name_with_use, - " + r" impl std::fmt::Debug<|> for Foo { } ", - " + r" use std::fmt::Debug; impl Debug for Foo { @@ -277,13 +277,13 @@ impl Debug for Foo { fn test_replace_split_different() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt; impl std::io<|> for Foo { } ", - " + r" use std::{io, fmt}; impl io for Foo { @@ -296,13 +296,13 @@ impl io for Foo { fn test_replace_split_self_for_use() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt; impl std::fmt::Debug<|> for Foo { } ", - " + r" use std::fmt::{self, Debug, }; impl Debug for Foo { @@ -315,13 +315,13 @@ impl Debug for Foo { fn test_replace_split_self_for_target() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::Debug; impl std::fmt<|> for Foo { } ", - " + r" use std::fmt::{self, Debug}; impl fmt for Foo { @@ -334,13 +334,13 @@ impl fmt for Foo { fn test_replace_add_to_nested_self_nested() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::{Debug, nested::{Display}}; impl std::fmt::nested<|> for Foo { } ", - " + r" use std::fmt::{Debug, nested::{Display, self}}; impl nested for Foo { @@ -353,13 +353,13 @@ impl nested for Foo { fn test_replace_add_to_nested_self_already_included() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::{Debug, nested::{self, Display}}; impl std::fmt::nested<|> for Foo { } ", - " + r" use std::fmt::{Debug, nested::{self, Display}}; impl nested for Foo { @@ -372,13 +372,13 @@ impl nested for Foo { fn test_replace_add_to_nested_nested() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::{Debug, nested::{Display}}; impl std::fmt::nested::Debug<|> for Foo { } ", - " + r" use std::fmt::{Debug, nested::{Display, Debug}}; impl Debug for Foo { @@ -391,13 +391,13 @@ impl Debug for Foo { fn test_replace_split_common_target_longer() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::Debug; impl std::fmt::nested::Display<|> for Foo { } ", - " + r" use std::fmt::{nested::Display, Debug}; impl Display for Foo { @@ -410,13 +410,13 @@ impl Display for Foo { fn test_replace_split_common_use_longer() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::nested::Debug; impl std::fmt::Display<|> for Foo { } ", - " + r" use std::fmt::{Display, nested::Debug}; impl Display for Foo { @@ -429,7 +429,7 @@ impl Display for Foo { fn test_replace_use_nested_import() { check_assist( replace_qualified_name_with_use, - " + r" use crate::{ ty::{Substs, Ty}, AssocItem, @@ -437,7 +437,7 @@ use crate::{ fn foo() { crate::ty::lower<|>::trait_env() } ", - " + r" use crate::{ ty::{Substs, Ty, lower}, AssocItem, @@ -452,13 +452,13 @@ fn foo() { lower::trait_env() } fn test_replace_alias() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt as foo; impl foo::Debug<|> for Foo { } ", - " + r" use std::fmt as foo; impl Debug for Foo { @@ -471,7 +471,7 @@ impl Debug for Foo { fn test_replace_not_applicable_one_segment() { check_assist_not_applicable( replace_qualified_name_with_use, - " + r" impl foo<|> for Foo { } ", @@ -482,7 +482,7 @@ impl foo<|> for Foo { fn test_replace_not_applicable_in_use() { check_assist_not_applicable( replace_qualified_name_with_use, - " + r" use std::fmt<|>; ", ); @@ -492,14 +492,14 @@ use std::fmt<|>; fn test_replace_add_use_no_anchor_in_mod_mod() { check_assist( replace_qualified_name_with_use, - " + r" mod foo { mod bar { std::fmt::Debug<|> } } ", - " + r" mod foo { mod bar { use std::fmt::Debug; @@ -515,14 +515,14 @@ mod foo { fn inserts_imports_after_inner_attributes() { check_assist( replace_qualified_name_with_use, - " + r" #![allow(dead_code)] fn main() { std::fmt::Debug<|> } ", - " + r" #![allow(dead_code)] use std::fmt::Debug; @@ -537,13 +537,13 @@ fn main() { fn replaces_all_affected_paths() { check_assist( replace_qualified_name_with_use, - " + r" fn main() { std::fmt::Debug<|>; let x: std::fmt::Debug = std::fmt::Debug; } ", - " + r" use std::fmt::Debug; fn main() { @@ -558,7 +558,7 @@ fn main() { fn replaces_all_affected_paths_mod() { check_assist( replace_qualified_name_with_use, - " + r" mod m { fn f() { std::fmt::Debug<|>; @@ -573,7 +573,7 @@ fn f() { std::fmt::Debug; } ", - " + r" mod m { use std::fmt::Debug; @@ -597,7 +597,7 @@ fn f() { fn does_not_replace_in_submodules() { check_assist( replace_qualified_name_with_use, - " + r" fn main() { std::fmt::Debug<|>; } @@ -608,7 +608,7 @@ mod sub { } } ", - " + r" use std::fmt::Debug; fn main() { @@ -628,14 +628,14 @@ mod sub { fn does_not_replace_in_use() { check_assist( replace_qualified_name_with_use, - " + r" use std::fmt::Display; fn main() { std::fmt<|>; } ", - " + r" use std::fmt::{self, Display}; fn main() { -- cgit v1.2.3 From aaaa68b56cc8520f09649acef22ae0eb26ee9678 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 15 Jun 2020 22:59:49 +0200 Subject: Simplify --- .../src/handlers/replace_qualified_name_with_use.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) (limited to 'crates/ra_assists/src/handlers') 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 301ae8497..f2286c9f6 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 @@ -5,7 +5,6 @@ use crate::{ utils::{find_insert_use_container, insert_use_statement}, AssistContext, AssistId, Assists, }; -use either::Either; // Assist: replace_qualified_name_with_use // @@ -56,14 +55,8 @@ pub(crate) fn replace_qualified_name_with_use( 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()); - } - } + let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); + shorten_paths(&mut rewriter, syntax, hir_path.mod_path()); builder.rewrite(rewriter); }, ) @@ -90,7 +83,7 @@ fn collect_hir_path_segments(path: &hir::Path) -> Option> { } /// Adds replacements to `re` that shorten `path` in all descendants of `node`. -fn shorten_paths(re: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { +fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { for child in node.children() { match_ast! { match child { @@ -101,12 +94,12 @@ fn shorten_paths(re: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModP ast::Module(_it) => continue, ast::Path(p) => { - match maybe_replace_path(re, &p, path) { + match maybe_replace_path(rewriter, &p, path) { Some(()) => {}, - None => shorten_paths(re, p.syntax().clone(), path), + None => shorten_paths(rewriter, p.syntax().clone(), path), } }, - _ => shorten_paths(re, child, path), + _ => shorten_paths(rewriter, child, path), } } } -- cgit v1.2.3 From 4295a004ed2fbc1046163acb5c100d4be7e4912b Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 16 Jun 2020 00:49:59 +0200 Subject: Operate only on AST paths instead of HIR --- .../handlers/replace_qualified_name_with_use.rs | 66 ++++++++++++---------- 1 file changed, 36 insertions(+), 30 deletions(-) (limited to 'crates/ra_assists/src/handlers') 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 f2286c9f6..b4784c333 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,4 +1,4 @@ -use hir::{self, ModPath}; +use hir; use ra_syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode}; use crate::{ @@ -50,13 +50,9 @@ pub(crate) fn replace_qualified_name_with_use( // 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(); let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); - shorten_paths(&mut rewriter, syntax, hir_path.mod_path()); + shorten_paths(&mut rewriter, syntax, path); builder.rewrite(rewriter); }, ) @@ -83,7 +79,7 @@ fn collect_hir_path_segments(path: &hir::Path) -> Option> { } /// Adds replacements to `re` that shorten `path` in all descendants of `node`. -fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: &ModPath) { +fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) { for child in node.children() { match_ast! { match child { @@ -94,47 +90,57 @@ fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Module(_it) => continue, ast::Path(p) => { - match maybe_replace_path(rewriter, &p, path) { + match maybe_replace_path(rewriter, p.clone(), path.clone()) { Some(()) => {}, - None => shorten_paths(rewriter, p.syntax().clone(), path), + None => shorten_paths(rewriter, p.syntax().clone(), path.clone()), } }, - _ => shorten_paths(rewriter, child, path), + _ => shorten_paths(rewriter, child, path.clone()), } } } } fn maybe_replace_path( - re: &mut SyntaxRewriter<'static>, - p: &ast::Path, - path: &ModPath, + rewriter: &mut SyntaxRewriter<'static>, + path: ast::Path, + target: ast::Path, ) -> Option<()> { - let hir_path = hir::Path::from_ast(p.clone())?; - - if hir_path.mod_path() != path { + if !path_eq(path.clone(), target.clone()) { 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)); + // Shorten `path`, leaving only its last segment. + if let Some(parent) = path.qualifier() { + rewriter.delete(parent.syntax()); + } + if let Some(double_colon) = path.coloncolon_token() { + rewriter.delete(&double_colon); } - re.replace(p.syntax(), new_path.syntax()); Some(()) } +fn path_eq(lhs: ast::Path, rhs: ast::Path) -> bool { + let mut lhs_curr = lhs; + let mut rhs_curr = rhs; + loop { + match (lhs_curr.segment(), rhs_curr.segment()) { + (Some(lhs), Some(rhs)) if lhs.syntax().text() == rhs.syntax().text() => (), + _ => return false, + } + + match (lhs_curr.qualifier(), rhs_curr.qualifier()) { + (Some(lhs), Some(rhs)) => { + lhs_curr = lhs; + rhs_curr = rhs; + } + (None, None) => return true, + _ => return false, + } + } +} + #[cfg(test)] mod tests { use crate::tests::{check_assist, check_assist_not_applicable}; -- cgit v1.2.3