From df2eb3c7cbe1a67fc870a4ee5dde6569a6b7deab Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 14 Feb 2022 18:46:16 -0500 Subject: [PATCH 1/3] fix: Don't drop tree when the other has `self` --- .../ide_assists/src/handlers/merge_imports.rs | 69 +++++++++++++++++++ crates/ide_db/src/helpers/merge_imports.rs | 5 +- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs index 94400be7a5bd..cf30897ab42c 100644 --- a/crates/ide_assists/src/handlers/merge_imports.rs +++ b/crates/ide_assists/src/handlers/merge_imports.rs @@ -252,6 +252,75 @@ use std::{fmt::{Display, Debug}}; ); } + #[test] + fn test_merge_with_nested_self_item() { + check_assist( + merge_imports, + r" +use std$0::{fmt::{Write, Display}}; +use std::{fmt::{self, Debug}}; +", + r" +use std::{fmt::{Write, Display, self, Debug}}; +", + ); + } + + #[test] + fn test_merge_with_nested_self_item2() { + check_assist( + merge_imports, + r" +use std$0::{fmt::{self, Debug}}; +use std::{fmt::{Write, Display}}; +", + r" +use std::{fmt::{self, Debug, Write, Display}}; +", + ); + } + + #[test] + fn test_merge_self_with_nested_self_item() { + check_assist( + merge_imports, + r" +use std::{fmt$0::{self, Debug}, fmt::{Write, Display}}; +", + r" +use std::{fmt::{self, Debug, Write, Display}}; +", + ); + } + + #[test] + fn test_merge_nested_self_and_empty() { + check_assist( + merge_imports, + r" +use foo::$0{bar::{self}}; +use foo::{bar}; +", + r" +use foo::{bar::{self}}; +", + ) + } + + #[test] + fn test_merge_nested_empty_and_self() { + check_assist( + merge_imports, + r" +use foo::$0{bar}; +use foo::{bar::{self}}; +", + r" +use foo::{bar::{self}}; +", + ) + } + #[test] fn test_merge_single_wildcard_diff_prefixes() { check_assist( diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs index 8581a8343613..614784b46745 100644 --- a/crates/ide_db/src/helpers/merge_imports.rs +++ b/crates/ide_db/src/helpers/merge_imports.rs @@ -115,11 +115,10 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) let tree_contains_self = |tree: &ast::UseTree| { tree.use_tree_list() .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)) { - (true, false) => continue, - (false, true) => { + (Some(true), None) => continue, + (None, Some(true)) => { ted::replace(lhs_t.syntax(), rhs_t.syntax()); *lhs_t = rhs_t; continue; From 86c1251afb1476137ce81b86bf1fa229eecdb5fc Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 14 Feb 2022 19:41:49 -0500 Subject: [PATCH 2/3] fix: Don't drop glob with nested self --- crates/ide_assists/src/handlers/merge_imports.rs | 12 ++++++++++++ crates/ide_db/src/helpers/merge_imports.rs | 3 +++ 2 files changed, 15 insertions(+) diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs index cf30897ab42c..2ded8c980372 100644 --- a/crates/ide_assists/src/handlers/merge_imports.rs +++ b/crates/ide_assists/src/handlers/merge_imports.rs @@ -321,6 +321,18 @@ use foo::{bar::{self}}; ) } + #[test] + fn test_merge_nested_list_self_and_glob() { + check_assist( + merge_imports, + r" +use std$0::{fmt::*}; +use std::{fmt::{self, Display}}; +", + r"use std::{fmt::{self, *, Display}};", + ) + } + #[test] fn test_merge_single_wildcard_diff_prefixes() { check_assist( diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs index 614784b46745..f113dccdea9b 100644 --- a/crates/ide_db/src/helpers/merge_imports.rs +++ b/crates/ide_db/src/helpers/merge_imports.rs @@ -115,6 +115,9 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) let tree_contains_self = |tree: &ast::UseTree| { tree.use_tree_list() .map(|tree_list| tree_list.use_trees().any(|it| tree_is_self(&it))) + // Glob imports aren't part of the use-tree lists, + // so they need to be handled explicitly + .or_else(|| tree.star_token().is_some().then(|| false)) }; match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) { (Some(true), None) => continue, From a1a23d343a10bd5124fb7d01d058566cb4748af2 Mon Sep 17 00:00:00 2001 From: DropDemBits Date: Mon, 14 Feb 2022 20:41:01 -0500 Subject: [PATCH 3/3] Apply review fixes --- crates/ide_assists/src/handlers/merge_imports.rs | 4 +++- crates/ide_db/src/helpers/merge_imports.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/ide_assists/src/handlers/merge_imports.rs b/crates/ide_assists/src/handlers/merge_imports.rs index 2ded8c980372..68aa741face2 100644 --- a/crates/ide_assists/src/handlers/merge_imports.rs +++ b/crates/ide_assists/src/handlers/merge_imports.rs @@ -329,7 +329,9 @@ use foo::{bar::{self}}; use std$0::{fmt::*}; use std::{fmt::{self, Display}}; ", - r"use std::{fmt::{self, *, Display}};", + r" +use std::{fmt::{self, *, Display}}; +", ) } diff --git a/crates/ide_db/src/helpers/merge_imports.rs b/crates/ide_db/src/helpers/merge_imports.rs index f113dccdea9b..dfaf578cb158 100644 --- a/crates/ide_db/src/helpers/merge_imports.rs +++ b/crates/ide_db/src/helpers/merge_imports.rs @@ -117,7 +117,7 @@ fn recursive_merge(lhs: &ast::UseTree, rhs: &ast::UseTree, merge: MergeBehavior) .map(|tree_list| tree_list.use_trees().any(|it| tree_is_self(&it))) // Glob imports aren't part of the use-tree lists, // so they need to be handled explicitly - .or_else(|| tree.star_token().is_some().then(|| false)) + .or_else(|| tree.star_token().map(|_| false)) }; match (tree_contains_self(lhs_t), tree_contains_self(&rhs_t)) { (Some(true), None) => continue,