From 5b0c91d11844ee3975476cd2c7bf42af480d7781 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 3 Oct 2021 13:52:46 +0200 Subject: [PATCH] fix: fix insert_use incorrectly merging glob imports --- crates/ide_db/src/helpers/insert_use/tests.rs | 14 +++++++++- crates/ide_db/src/helpers/merge_imports.rs | 28 +++++++++---------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/crates/ide_db/src/helpers/insert_use/tests.rs b/crates/ide_db/src/helpers/insert_use/tests.rs index 339360ba4a4..f3b9c7130f4 100644 --- a/crates/ide_db/src/helpers/insert_use/tests.rs +++ b/crates/ide_db/src/helpers/insert_use/tests.rs @@ -612,6 +612,7 @@ fn merge_mod_into_glob() { #[test] fn merge_self_glob() { + cov_mark::check!(merge_self_glob); check_with_config( "self", r"use self::*;", @@ -627,6 +628,17 @@ fn merge_self_glob() { // FIXME: have it emit `use {self, *}`? } +#[test] +fn merge_glob() { + check_crate( + "syntax::SyntaxKind", + r" +use syntax::{SyntaxKind::*};", + r" +use syntax::{SyntaxKind::{*, self}};", + ) +} + #[test] fn merge_glob_nested() { check_crate( @@ -931,5 +943,5 @@ fn check_merge_only_fail(ra_fixture0: &str, ra_fixture1: &str, mb: MergeBehavior fn check_guess(ra_fixture: &str, expected: ImportGranularityGuess) { let syntax = ast::SourceFile::parse(ra_fixture).tree().syntax().clone(); let file = super::ImportScope::from(syntax).unwrap(); - assert_eq!(file.guess_granularity_from_scope(), expected); + assert_eq!(super::guess_granularity_from_scope(&file), expected); } diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs index 8189d6e53f7..6a7a9b8f61c 100644 --- a/crates/ide_db/src/helpers/merge_imports.rs +++ b/crates/ide_db/src/helpers/merge_imports.rs @@ -19,7 +19,6 @@ pub enum MergeBehavior { } impl MergeBehavior { - #[inline] fn is_tree_allowed(&self, tree: &ast::UseTree) -> bool { match self { MergeBehavior::Crate => true, @@ -114,7 +113,7 @@ fn recursive_merge( let rhs_path = rhs_path?; let (lhs_prefix, rhs_prefix) = common_prefix(&lhs_path, &rhs_path)?; if lhs_prefix == lhs_path && rhs_prefix == rhs_path { - let tree_is_self = |tree: ast::UseTree| { + let tree_is_self = |tree: &ast::UseTree| { tree.path().as_ref().map(path_is_self).unwrap_or(false) }; // Check if only one of the two trees has a tree list, and @@ -123,7 +122,7 @@ fn recursive_merge( // the list is already included in the other one via `self`. let tree_contains_self = |tree: &ast::UseTree| { tree.use_tree_list() - .map(|tree_list| tree_list.use_trees().any(tree_is_self)) + .map(|tree_list| tree_list.use_trees().any(|it| tree_is_self(&it))) .unwrap_or(false) }; match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) { @@ -141,17 +140,18 @@ fn recursive_merge( // 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() { - *lhs_t = make::use_tree( - make::path_unqualified(make::path_segment_self()), - None, - None, - false, - ); - use_trees.insert(idx, make::use_tree_glob()); - continue; - } - - if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() { + if tree_is_self(lhs_t) || tree_is_self(&rhs_t) { + cov_mark::hit!(merge_self_glob); + *lhs_t = make::use_tree( + make::path_unqualified(make::path_segment_self()), + None, + None, + false, + ); + use_trees.insert(idx, make::use_tree_glob()); + continue; + } + } else if lhs_t.use_tree_list().is_none() && rhs_t.use_tree_list().is_none() { continue; } }