From c1925df7fc91a171925ecb59b9f1895ee59ce72f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 3 Sep 2020 14:22:22 +0200 Subject: Replace insert_use_statement with the new insert_use --- crates/assists/src/handlers/auto_import.rs | 19 ++++--- .../handlers/extract_struct_from_enum_variant.rs | 20 ++++--- .../handlers/replace_qualified_name_with_use.rs | 65 +++++++++++----------- 3 files changed, 58 insertions(+), 46 deletions(-) (limited to 'crates/assists/src/handlers') diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs index c4770f336..f3b4c5708 100644 --- a/crates/assists/src/handlers/auto_import.rs +++ b/crates/assists/src/handlers/auto_import.rs @@ -13,8 +13,11 @@ use syntax::{ }; use crate::{ - utils::insert_use_statement, AssistContext, AssistId, AssistKind, Assists, GroupLabel, + utils::{insert_use, MergeBehaviour}, + AssistContext, AssistId, AssistKind, Assists, GroupLabel, }; +use ast::make; +use insert_use::find_insert_use_container; // Assist: auto_import // @@ -44,6 +47,8 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range; let group = auto_import_assets.get_import_group_message(); + let container = find_insert_use_container(&auto_import_assets.syntax_under_caret, ctx)?; + let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); for import in proposed_imports { acc.add_group( &group, @@ -51,12 +56,12 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> format!("Import `{}`", &import), range, |builder| { - insert_use_statement( - &auto_import_assets.syntax_under_caret, - &import.to_string(), - ctx, - builder.text_edit_builder(), + let new_syntax = insert_use( + &syntax, + make::path_from_text(&import.to_string()), + Some(MergeBehaviour::Full), ); + builder.replace(syntax.text_range(), new_syntax.to_string()) }, ); } @@ -358,7 +363,7 @@ mod tests { } ", r" - use PubMod::{PubStruct2, PubStruct1}; + use PubMod::{PubStruct1, PubStruct2}; struct Test { test: PubStruct2, diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index 8ac20210a..d9c50f25f 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -10,9 +10,12 @@ use syntax::{ }; use crate::{ - assist_context::AssistBuilder, utils::insert_use_statement, AssistContext, AssistId, - AssistKind, Assists, + assist_context::AssistBuilder, + utils::{insert_use, MergeBehaviour}, + AssistContext, AssistId, AssistKind, Assists, }; +use ast::make; +use insert_use::find_insert_use_container; // Assist: extract_struct_from_enum_variant // @@ -107,12 +110,15 @@ fn insert_import( if let Some(mut mod_path) = mod_path { mod_path.segments.pop(); mod_path.segments.push(variant_hir_name.clone()); - insert_use_statement( - path.syntax(), - &mod_path.to_string(), - ctx, - builder.text_edit_builder(), + let container = find_insert_use_container(path.syntax(), ctx)?; + let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); + + let new_syntax = insert_use( + &syntax, + make::path_from_text(&mod_path.to_string()), + Some(MergeBehaviour::Full), ); + builder.replace(syntax.text_range(), new_syntax.to_string()) } Some(()) } diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs index 470e5f8ff..56e85125d 100644 --- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs @@ -2,9 +2,10 @@ use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRang use test_utils::mark; use crate::{ - utils::{find_insert_use_container, insert_use_statement}, + utils::{find_insert_use_container, insert_use, MergeBehaviour}, AssistContext, AssistId, AssistKind, Assists, }; +use ast::make; // Assist: replace_qualified_name_with_use // @@ -32,7 +33,7 @@ pub(crate) fn replace_qualified_name_with_use( mark::hit!(dont_import_trivial_paths); return None; } - let path_to_import = path.to_string().clone(); + let path_to_import = path.to_string(); let path_to_import = match path.segment()?.generic_arg_list() { Some(generic_args) => { let generic_args_start = @@ -43,28 +44,24 @@ pub(crate) fn replace_qualified_name_with_use( }; let target = path.syntax().text_range(); + let container = find_insert_use_container(path.syntax(), ctx)?; + let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); acc.add( AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite), "Replace qualified path with use", target, |builder| { - let container = match find_insert_use_container(path.syntax(), ctx) { - Some(c) => c, - None => return, - }; - insert_use_statement( - path.syntax(), - &path_to_import.to_string(), - ctx, - builder.text_edit_builder(), - ); - // 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 mut rewriter = SyntaxRewriter::default(); - let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); - shorten_paths(&mut rewriter, syntax, path); - builder.rewrite(rewriter); + shorten_paths(&mut rewriter, syntax.clone(), path); + let rewritten_syntax = rewriter.rewrite(&syntax); + let new_syntax = insert_use( + &rewritten_syntax, + make::path_from_text(path_to_import), + Some(MergeBehaviour::Full), + ); + builder.replace(syntax.text_range(), new_syntax.to_string()) }, ) } @@ -220,9 +217,10 @@ impl std::fmt::Debug<|> for Foo { } ", r" -use stdx; use std::fmt::Debug; +use stdx; + impl Debug for Foo { } ", @@ -274,7 +272,7 @@ impl std::io<|> for Foo { } ", r" -use std::{io, fmt}; +use std::{fmt, io}; impl io for Foo { } @@ -293,7 +291,7 @@ impl std::fmt::Debug<|> for Foo { } ", r" -use std::fmt::{self, Debug, }; +use std::fmt::{self, Debug}; impl Debug for Foo { } @@ -312,7 +310,7 @@ impl std::fmt<|> for Foo { } ", r" -use std::fmt::{self, Debug}; +use std::fmt::{Debug, self}; impl fmt for Foo { } @@ -330,8 +328,9 @@ use std::fmt::{Debug, nested::{Display}}; impl std::fmt::nested<|> for Foo { } ", + // FIXME(veykril): should be nested::{self, Display} here r" -use std::fmt::{Debug, nested::{Display, self}}; +use std::fmt::{Debug, nested::{Display}, nested}; impl nested for Foo { } @@ -349,8 +348,9 @@ use std::fmt::{Debug, nested::{self, Display}}; impl std::fmt::nested<|> for Foo { } ", + // FIXME(veykril): self is being pulled out for some reason now r" -use std::fmt::{Debug, nested::{self, Display}}; +use std::fmt::{Debug, nested::{Display}, nested}; impl nested for Foo { } @@ -369,7 +369,7 @@ impl std::fmt::nested::Debug<|> for Foo { } ", r" -use std::fmt::{Debug, nested::{Display, Debug}}; +use std::fmt::{Debug, nested::{Display}, nested::Debug}; impl Debug for Foo { } @@ -388,7 +388,7 @@ impl std::fmt::nested::Display<|> for Foo { } ", r" -use std::fmt::{nested::Display, Debug}; +use std::fmt::{Debug, nested::Display}; impl Display for Foo { } @@ -407,7 +407,7 @@ impl std::fmt::Display<|> for Foo { } ", r" -use std::fmt::{Display, nested::Debug}; +use std::fmt::{nested::Debug, Display}; impl Display for Foo { } @@ -427,11 +427,12 @@ use crate::{ fn foo() { crate::ty::lower<|>::trait_env() } ", + // FIXME(veykril): formatting broke here r" use crate::{ - ty::{Substs, Ty, lower}, + ty::{Substs, Ty}, AssocItem, -}; +ty::lower}; fn foo() { lower::trait_env() } ", @@ -451,6 +452,8 @@ impl foo::Debug<|> for Foo { r" use std::fmt as foo; +use foo::Debug; + impl Debug for Foo { } ", @@ -627,7 +630,7 @@ fn main() { } ", r" -use std::fmt::{self, Display}; +use std::fmt::{Display, self}; fn main() { fmt; @@ -647,9 +650,8 @@ impl std::io<|> for Foo { } ", r" -use std::io; - pub use std::fmt; +use std::io; impl io for Foo { } @@ -668,9 +670,8 @@ impl std::io<|> for Foo { } ", r" -use std::io; - pub(crate) use std::fmt; +use std::io; impl io for Foo { } -- cgit v1.2.3 From 98e2f674e9e736720d1cd0a5b8c24e1fb10f64a1 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 3 Sep 2020 14:38:34 +0200 Subject: Fix inserting imports in front of inner attributes --- crates/assists/src/handlers/replace_qualified_name_with_use.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'crates/assists/src/handlers') diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs index 56e85125d..597bc268c 100644 --- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs @@ -348,9 +348,9 @@ use std::fmt::{Debug, nested::{self, Display}}; impl std::fmt::nested<|> for Foo { } ", - // FIXME(veykril): self is being pulled out for some reason now + // FIXME(veykril): nested is duplicated now r" -use std::fmt::{Debug, nested::{Display}, nested}; +use std::fmt::{Debug, nested::{self, Display}, nested}; impl nested for Foo { } @@ -518,6 +518,7 @@ fn main() { ", r" #![allow(dead_code)] + use std::fmt::Debug; fn main() { -- cgit v1.2.3 From 7de2a30f4080d0e6d8790882238bed1cb6ccbc21 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 3 Sep 2020 16:23:33 +0200 Subject: Fix import insertion breaking nested modules --- crates/assists/src/handlers/auto_import.rs | 11 ++++++----- .../src/handlers/extract_struct_from_enum_variant.rs | 8 ++++---- .../src/handlers/replace_qualified_name_with_use.rs | 20 +++++++++++--------- 3 files changed, 21 insertions(+), 18 deletions(-) (limited to 'crates/assists/src/handlers') diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs index f3b4c5708..66e819154 100644 --- a/crates/assists/src/handlers/auto_import.rs +++ b/crates/assists/src/handlers/auto_import.rs @@ -1,11 +1,13 @@ use std::collections::BTreeSet; +use ast::make; use either::Either; use hir::{ AsAssocItem, AssocItemContainer, ModPath, Module, ModuleDef, PathResolution, Semantics, Trait, Type, }; use ide_db::{imports_locator, RootDatabase}; +use insert_use::ImportScope; use rustc_hash::FxHashSet; use syntax::{ ast::{self, AstNode}, @@ -16,8 +18,6 @@ use crate::{ utils::{insert_use, MergeBehaviour}, AssistContext, AssistId, AssistKind, Assists, GroupLabel, }; -use ast::make; -use insert_use::find_insert_use_container; // Assist: auto_import // @@ -47,8 +47,9 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> let range = ctx.sema.original_range(&auto_import_assets.syntax_under_caret).range; let group = auto_import_assets.get_import_group_message(); - let container = find_insert_use_container(&auto_import_assets.syntax_under_caret, ctx)?; - let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); + let scope = + ImportScope::find_insert_use_container(&auto_import_assets.syntax_under_caret, ctx)?; + let syntax = scope.as_syntax_node(); for import in proposed_imports { acc.add_group( &group, @@ -57,7 +58,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> range, |builder| { let new_syntax = insert_use( - &syntax, + &scope, make::path_from_text(&import.to_string()), Some(MergeBehaviour::Full), ); diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index d9c50f25f..a50a57f1a 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -15,7 +15,7 @@ use crate::{ AssistContext, AssistId, AssistKind, Assists, }; use ast::make; -use insert_use::find_insert_use_container; +use insert_use::ImportScope; // Assist: extract_struct_from_enum_variant // @@ -110,11 +110,11 @@ fn insert_import( if let Some(mut mod_path) = mod_path { mod_path.segments.pop(); mod_path.segments.push(variant_hir_name.clone()); - let container = find_insert_use_container(path.syntax(), ctx)?; - let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); + let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?; + let syntax = scope.as_syntax_node(); let new_syntax = insert_use( - &syntax, + &scope, make::path_from_text(&mod_path.to_string()), Some(MergeBehaviour::Full), ); diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs index 597bc268c..85c70d16b 100644 --- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs +++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs @@ -2,7 +2,7 @@ use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRang use test_utils::mark; use crate::{ - utils::{find_insert_use_container, insert_use, MergeBehaviour}, + utils::{insert_use, ImportScope, MergeBehaviour}, AssistContext, AssistId, AssistKind, Assists, }; use ast::make; @@ -44,8 +44,8 @@ pub(crate) fn replace_qualified_name_with_use( }; let target = path.syntax().text_range(); - let container = find_insert_use_container(path.syntax(), ctx)?; - let syntax = container.either(|l| l.syntax().clone(), |r| r.syntax().clone()); + let scope = ImportScope::find_insert_use_container(path.syntax(), ctx)?; + let syntax = scope.as_syntax_node(); acc.add( AssistId("replace_qualified_name_with_use", AssistKind::RefactorRewrite), "Replace qualified path with use", @@ -56,12 +56,14 @@ pub(crate) fn replace_qualified_name_with_use( let mut rewriter = SyntaxRewriter::default(); shorten_paths(&mut rewriter, syntax.clone(), path); let rewritten_syntax = rewriter.rewrite(&syntax); - let new_syntax = insert_use( - &rewritten_syntax, - make::path_from_text(path_to_import), - Some(MergeBehaviour::Full), - ); - builder.replace(syntax.text_range(), new_syntax.to_string()) + if let Some(ref import_scope) = ImportScope::from(rewritten_syntax) { + let new_syntax = insert_use( + import_scope, + make::path_from_text(path_to_import), + Some(MergeBehaviour::Full), + ); + builder.replace(syntax.text_range(), new_syntax.to_string()) + } }, ) } -- cgit v1.2.3 From d29b69cbe61d20556c55e4f6a53da0784df8b6d0 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 3 Sep 2020 18:32:29 +0200 Subject: Disable insert_import in extract_struct_from_enum_variant until its fixed --- crates/assists/src/handlers/extract_struct_from_enum_variant.rs | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'crates/assists/src/handlers') diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index a50a57f1a..eb812c1c9 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -118,6 +118,7 @@ fn insert_import( make::path_from_text(&mod_path.to_string()), Some(MergeBehaviour::Full), ); + // FIXME: this will currently panic as multiple imports will have overlapping text ranges builder.replace(syntax.text_range(), new_syntax.to_string()) } Some(()) @@ -191,6 +192,7 @@ fn update_reference( list_range.end().checked_sub(TextSize::from(1))?, ); builder.edit_file(reference.file_range.file_id); + /* FIXME: this most likely requires AST-based editing, see `insert_import` if !visited_modules_set.contains(&module) { if insert_import(ctx, builder, &path_expr, &module, enum_module_def, variant_hir_name) .is_some() @@ -198,6 +200,7 @@ fn update_reference( visited_modules_set.insert(module); } } + */ builder.replace(inside_list_range, format!("{}{}", segment, list)); Some(()) } @@ -256,6 +259,7 @@ pub enum A { One(One) }"#, } #[test] + #[ignore] // FIXME: this currently panics if `insert_import` is used fn test_extract_struct_with_complex_imports() { check_assist( extract_struct_from_enum_variant, -- cgit v1.2.3 From 82f61e6629f709d7f347fd801ef5c31f476ff29e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 3 Sep 2020 18:44:39 +0200 Subject: Add extra insert_use test for pub(crate) re-export handling --- crates/assists/src/handlers/extract_struct_from_enum_variant.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'crates/assists/src/handlers') diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs index eb812c1c9..80c62d8bb 100644 --- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs +++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs @@ -97,6 +97,7 @@ fn existing_struct_def(db: &RootDatabase, variant_name: &str, variant: &EnumVari .any(|(name, _)| name.to_string() == variant_name.to_string()) } +#[allow(dead_code)] fn insert_import( ctx: &AssistContext, builder: &mut AssistBuilder, @@ -174,9 +175,9 @@ fn update_reference( builder: &mut AssistBuilder, reference: Reference, source_file: &SourceFile, - enum_module_def: &ModuleDef, - variant_hir_name: &Name, - visited_modules_set: &mut FxHashSet, + _enum_module_def: &ModuleDef, + _variant_hir_name: &Name, + _visited_modules_set: &mut FxHashSet, ) -> Option<()> { let path_expr: ast::PathExpr = find_node_at_offset::( source_file.syntax(), @@ -185,7 +186,7 @@ fn update_reference( let call = path_expr.syntax().parent().and_then(ast::CallExpr::cast)?; let list = call.arg_list()?; let segment = path_expr.path()?.segment()?; - let module = ctx.sema.scope(&path_expr.syntax()).module()?; + let _module = ctx.sema.scope(&path_expr.syntax()).module()?; let list_range = list.syntax().text_range(); let inside_list_range = TextRange::new( list_range.start().checked_add(TextSize::from(1))?, -- cgit v1.2.3