Skip to content

Commit 5b0c91d

Browse files
committed
fix: fix insert_use incorrectly merging glob imports
1 parent e684235 commit 5b0c91d

File tree

2 files changed

+27
-15
lines changed

2 files changed

+27
-15
lines changed

crates/ide_db/src/helpers/insert_use/tests.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ fn merge_mod_into_glob() {
612612

613613
#[test]
614614
fn merge_self_glob() {
615+
cov_mark::check!(merge_self_glob);
615616
check_with_config(
616617
"self",
617618
r"use self::*;",
@@ -627,6 +628,17 @@ fn merge_self_glob() {
627628
// FIXME: have it emit `use {self, *}`?
628629
}
629630

631+
#[test]
632+
fn merge_glob() {
633+
check_crate(
634+
"syntax::SyntaxKind",
635+
r"
636+
use syntax::{SyntaxKind::*};",
637+
r"
638+
use syntax::{SyntaxKind::{*, self}};",
639+
)
640+
}
641+
630642
#[test]
631643
fn merge_glob_nested() {
632644
check_crate(
@@ -931,5 +943,5 @@ fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior
931943
fn check_guess(ra_fixture: &str, expected: ImportGranularityGuess) {
932944
let syntax = ast::SourceFile::parse(ra_fixture).tree().syntax().clone();
933945
let file = super::ImportScope::from(syntax).unwrap();
934-
assert_eq!(file.guess_granularity_from_scope(), expected);
946+
assert_eq!(super::guess_granularity_from_scope(&file), expected);
935947
}

crates/ide_db/src/helpers/merge_imports.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ pub enum MergeBehavior {
1919
}
2020

2121
impl MergeBehavior {
22-
#[inline]
2322
fn is_tree_allowed(&self, tree: &ast::UseTree) -> bool {
2423
match self {
2524
MergeBehavior::Crate => true,
@@ -114,7 +113,7 @@ fn recursive_merge(
114113
let rhs_path = rhs_path?;
115114
let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?;
116115
if lhs_prefix == lhs_path && rhs_prefix == rhs_path {
117-
let tree_is_self = |tree: ast::UseTree| {
116+
let tree_is_self = |tree: &ast::UseTree| {
118117
tree.path().as_ref().map(path_is_self).unwrap_or(false)
119118
};
120119
// Check if only one of the two trees has a tree list, and
@@ -123,7 +122,7 @@ fn recursive_merge(
123122
// the list is already included in the other one via `self`.
124123
let tree_contains_self = |tree: &ast::UseTree| {
125124
tree.use_tree_list()
126-
.map(|tree_list| tree_list.use_trees().any(tree_is_self))
125+
.map(|tree_list| tree_list.use_trees().any(|it| tree_is_self(&it)))
127126
.unwrap_or(false)
128127
};
129128
match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) {
@@ -141,17 +140,18 @@ fn recursive_merge(
141140
// glob import of said module see the `merge_self_glob` or
142141
// `merge_mod_into_glob` tests.
143142
if lhs_t.star_token().is_some() || rhs_t.star_token().is_some() {
144-
*lhs_t = make::use_tree(
145-
make::path_unqualified(make::path_segment_self()),
146-
None,
147-
None,
148-
false,
149-
);
150-
use_trees.insert(idx, make::use_tree_glob());
151-
continue;
152-
}
153-
154-
if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() {
143+
if tree_is_self(lhs_t) || tree_is_self(&rhs_t) {
144+
cov_mark::hit!(merge_self_glob);
145+
*lhs_t = make::use_tree(
146+
make::path_unqualified(make::path_segment_self()),
147+
None,
148+
None,
149+
false,
150+
);
151+
use_trees.insert(idx, make::use_tree_glob());
152+
continue;
153+
}
154+
} else if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() {
155155
continue;
156156
}
157157
}

0 commit comments

Comments
 (0)