From b4c608896c1f5f37ebacc00de76da979c0439700 Mon Sep 17 00:00:00 2001 From: iDawer Date: Fri, 1 Apr 2022 19:12:50 +0500 Subject: [PATCH 1/2] fix: splitting path of a glob import wrongly adds `self` `ast::UseTree::split_prefix` handles globs now. Removed an extra branch for globs in `ide_db::imports::merge_imports::recursive_merge` (superseeded by split_prefix). --- .../ide_assists/src/handlers/merge_imports.rs | 16 +++++++++++- crates/ide_db/src/imports/insert_use/tests.rs | 9 +++---- crates/ide_db/src/imports/merge_imports.rs | 26 ++----------------- crates/syntax/src/ast/edit_in_place.rs | 23 +++++++++++++--- 4 files changed, 41 insertions(+), 33 deletions(-) diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs index e35c962a3f3..705ae666176 100644 --- a/crates/ide_assists/src/handlers/merge_imports.rs +++ b/crates/ide_assists/src/handlers/merge_imports.rs @@ -330,7 +330,7 @@ use std$0::{fmt::*}; use std::{fmt::{self, Display}}; ", r" -use std::{fmt::{self, *, Display}}; +use std::{fmt::{*, self, Display}}; ", ) } @@ -440,4 +440,18 @@ use std::$0 fn main() {}", ); } + + #[test] + fn split_glob() { + check_assist( + merge_imports, + r" +use foo::$0*; +use foo::bar::Baz; +", + r" +use foo::{*, bar::Baz}; +", + ); + } } diff --git a/crates/ide_db/src/imports/insert_use/tests.rs b/crates/ide_db/src/imports/insert_use/tests.rs index 4219358a07f..e4b9651e5e8 100644 --- a/crates/ide_db/src/imports/insert_use/tests.rs +++ b/crates/ide_db/src/imports/insert_use/tests.rs @@ -656,7 +656,7 @@ fn merge_mod_into_glob() { check_with_config( "token::TokenKind", r"use token::TokenKind::*;", - r"use token::TokenKind::{self, *};", + r"use token::TokenKind::{*, self};", &InsertUseConfig { granularity: ImportGranularity::Crate, enforce_granularity: true, @@ -670,11 +670,10 @@ fn merge_mod_into_glob() { #[test] fn merge_self_glob() { - cov_mark::check!(merge_self_glob); check_with_config( "self", r"use self::*;", - r"use self::{self, *};", + r"use self::{*, self};", &InsertUseConfig { granularity: ImportGranularity::Crate, enforce_granularity: true, @@ -693,7 +692,7 @@ fn merge_glob() { r" use syntax::{SyntaxKind::*};", r" -use syntax::{SyntaxKind::{self, *}};", +use syntax::{SyntaxKind::{*, self}};", ) } @@ -702,7 +701,7 @@ fn merge_glob_nested() { check_crate( "foo::bar::quux::Fez", r"use foo::bar::{Baz, quux::*};", - r"use foo::bar::{Baz, quux::{self::*, Fez}};", + r"use foo::bar::{Baz, quux::{*, Fez}};", ) } diff --git a/crates/ide_db/src/imports/merge_imports.rs b/crates/ide_db/src/imports/merge_imports.rs index 9a9097ba5e2..d4e85db16e3 100644 --- a/crates/ide_db/src/imports/merge_imports.rs +++ b/crates/ide_db/src/imports/merge_imports.rs @@ -3,7 +3,7 @@ use std::cmp::Ordering; use itertools::{EitherOrBoth, Itertools}; use syntax::{ - ast::{self, make, AstNode, HasAttrs, HasVisibility, PathSegmentKind}, + ast::{self, AstNode, HasAttrs, HasVisibility, PathSegmentKind}, ted, }; @@ -129,29 +129,7 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) _ => (), } - // Glob imports aren't part of the use-tree lists so we need - // to special handle them here as well this special handling - // is only required for when we merge a module import into a - // glob import of said module see the `merge_self_glob` or - // `merge_mod_into_glob` tests. - if lhs_t.star_token().is_some() || rhs_t.star_token().is_some() { - if tree_is_self(lhs_t) || tree_is_self(&rhs_t) { - cov_mark::hit!(merge_self_glob); - let self_tree = make::use_tree( - make::path_unqualified(make::path_segment_self()), - None, - None, - false, - ) - .clone_for_update(); - ted::replace(lhs_t.syntax(), self_tree.syntax()); - *lhs_t = self_tree; - let glob = make::use_tree_glob().clone_for_update(); - use_trees.insert(idx, glob.clone()); - lhs.get_or_create_use_tree_list().add_use_tree(glob); - continue; - } - } else if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() { + if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() { continue; } } diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index bf4f9d6d18b..296c6bfec01 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -302,16 +302,33 @@ impl ast::UseTree { /// Splits off the given prefix, making it the path component of the use tree, /// appending the rest of the path to all UseTreeList items. + /// + /// # Examples + /// + /// `prefix$0::suffix` -> `prefix::{suffix}` + /// + /// `prefix$0` -> `prefix::{self}` + /// + /// `prefix$0::*` -> `prefix::{*}` pub fn split_prefix(&self, prefix: &ast::Path) { debug_assert_eq!(self.path(), Some(prefix.top_path())); let path = self.path().unwrap(); if &path == prefix && self.use_tree_list().is_none() { - let self_suffix = make::path_unqualified(make::path_segment_self()).clone_for_update(); - ted::replace(path.syntax(), self_suffix.syntax()); + if self.star_token().is_some() { + // path$0::* -> * + self.coloncolon_token().map(ted::remove); + ted::remove(prefix.syntax()); + } else { + // path$0 -> self + let self_suffix = + make::path_unqualified(make::path_segment_self()).clone_for_update(); + ted::replace(path.syntax(), self_suffix.syntax()); + } } else if split_path_prefix(prefix).is_none() { return; } - + // At this point, prefix path is detached; _self_ use tree has suffix path. + // Next, transoform 'suffix' use tree into 'prefix::{suffix}' let subtree = self.clone_subtree().clone_for_update(); ted::remove_all_iter(self.syntax().children_with_tokens()); ted::insert(Position::first_child_of(self.syntax()), prefix.syntax()); From c8c21aabff0a7dc86c84646c587e2b400bc01d30 Mon Sep 17 00:00:00 2001 From: iDawer Date: Sat, 2 Apr 2022 14:18:42 +0500 Subject: [PATCH 2/2] fix: `merge_imports::recursive_merge` exiting early --- crates/ide_db/src/imports/merge_imports.rs | 2 +- crates/syntax/src/ast/edit_in_place.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/ide_db/src/imports/merge_imports.rs b/crates/ide_db/src/imports/merge_imports.rs index d4e85db16e3..c7d9034f74d 100644 --- a/crates/ide_db/src/imports/merge_imports.rs +++ b/crates/ide_db/src/imports/merge_imports.rs @@ -129,7 +129,7 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) _ => (), } - if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() { + if lhs_t.is_simple_path() && rhs_t.is_simple_path() { continue; } } diff --git a/crates/syntax/src/ast/edit_in_place.rs b/crates/syntax/src/ast/edit_in_place.rs index 296c6bfec01..19edead30a6 100644 --- a/crates/syntax/src/ast/edit_in_place.rs +++ b/crates/syntax/src/ast/edit_in_place.rs @@ -328,7 +328,7 @@ impl ast::UseTree { return; } // At this point, prefix path is detached; _self_ use tree has suffix path. - // Next, transoform 'suffix' use tree into 'prefix::{suffix}' + // Next, transform 'suffix' use tree into 'prefix::{suffix}' let subtree = self.clone_subtree().clone_for_update(); ted::remove_all_iter(self.syntax().children_with_tokens()); ted::insert(Position::first_child_of(self.syntax()), prefix.syntax());