Skip to content

Commit 8f1f66b

Browse files
ShoyuVanillaShoyuVanilla
authored
Merge pull request #21635 from tascord/import-cfg-fixes
fix: Better import placement + merging
2 parents d8e0e96 + 75a8d56 commit 8f1f66b

3 files changed

Lines changed: 194 additions & 16 deletions

File tree

‎crates/ide-db/src/imports/insert_use.rs‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ impl ImportScope {
9494
.item_list()
9595
.map(ImportScopeKind::Module)
9696
.map(|kind| ImportScope { kind, required_cfgs });
97-
} else if let Some(has_attrs) = ast::AnyHasAttrs::cast(syntax) {
97+
} else if let Some(has_attrs) = ast::AnyHasAttrs::cast(syntax.clone()) {
9898
if block.is_none()
9999
&& let Some(b) = ast::BlockExpr::cast(has_attrs.syntax().clone())
100100
&& let Some(b) = sema.original_ast_node(b)
@@ -105,11 +105,34 @@ impl ImportScope {
105105
.attrs()
106106
.any(|attr| attr.as_simple_call().is_some_and(|(ident, _)| ident == "cfg"))
107107
{
108-
if let Some(b) = block {
109-
return Some(ImportScope {
110-
kind: ImportScopeKind::Block(b),
111-
required_cfgs,
108+
if let Some(b) = block.clone() {
109+
let current_cfgs = has_attrs.attrs().filter(|attr| {
110+
attr.as_simple_call().is_some_and(|(ident, _)| ident == "cfg")
112111
});
112+
113+
let total_cfgs: Vec<_> =
114+
required_cfgs.iter().cloned().chain(current_cfgs).collect();
115+
116+
let parent = syntax.parent();
117+
let mut can_merge = false;
118+
if let Some(parent) = parent {
119+
can_merge = parent.children().filter_map(ast::Use::cast).any(|u| {
120+
let u_attrs = u.attrs().filter(|attr| {
121+
attr.as_simple_call().is_some_and(|(ident, _)| ident == "cfg")
122+
});
123+
crate::imports::merge_imports::eq_attrs(
124+
u_attrs,
125+
total_cfgs.iter().cloned(),
126+
)
127+
});
128+
}
129+
130+
if !can_merge {
131+
return Some(ImportScope {
132+
kind: ImportScopeKind::Block(b),
133+
required_cfgs,
134+
});
135+
}
113136
}
114137
required_cfgs.extend(has_attrs.attrs().filter(|attr| {
115138
attr.as_simple_call().is_some_and(|(ident, _)| ident == "cfg")
@@ -546,7 +569,9 @@ fn insert_use_(scope: &ImportScope, use_item: ast::Use, group_imports: bool) {
546569
// skip the curly brace
547570
.skip(l_curly.is_some() as usize)
548571
.take_while(|child| match child {
549-
NodeOrToken::Node(node) => is_inner_attribute(node.clone()),
572+
NodeOrToken::Node(node) => {
573+
is_inner_attribute(node.clone()) && ast::Item::cast(node.clone()).is_none()
574+
}
550575
NodeOrToken::Token(token) => {
551576
[SyntaxKind::WHITESPACE, SyntaxKind::COMMENT, SyntaxKind::SHEBANG]
552577
.contains(&token.kind())
@@ -667,7 +692,9 @@ fn insert_use_with_editor_(
667692
// skip the curly brace
668693
.skip(l_curly.is_some() as usize)
669694
.take_while(|child| match child {
670-
NodeOrToken::Node(node) => is_inner_attribute(node.clone()),
695+
NodeOrToken::Node(node) => {
696+
is_inner_attribute(node.clone()) && ast::Item::cast(node.clone()).is_none()
697+
}
671698
NodeOrToken::Token(token) => {
672699
[SyntaxKind::WHITESPACE, SyntaxKind::COMMENT, SyntaxKind::SHEBANG]
673700
.contains(&token.kind())

‎crates/ide-db/src/imports/insert_use/tests.rs‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,3 +1438,156 @@ fn check_guess(#[rust_analyzer::rust_fixture] ra_fixture: &str, expected: Import
14381438
let file = ImportScope { kind: ImportScopeKind::File(syntax), required_cfgs: vec![] };
14391439
assert_eq!(super::guess_granularity_from_scope(&file), expected);
14401440
}
1441+
1442+
#[test]
1443+
fn insert_with_existing_imports_and_cfg_module() {
1444+
check(
1445+
"std::fmt",
1446+
r#"
1447+
use foo::bar;
1448+
1449+
#[cfg(target_arch = "x86_64")]
1450+
pub mod api;
1451+
"#,
1452+
r#"
1453+
use std::fmt;
1454+
1455+
use foo::bar;
1456+
1457+
#[cfg(target_arch = "x86_64")]
1458+
pub mod api;
1459+
"#,
1460+
ImportGranularity::Crate,
1461+
);
1462+
}
1463+
1464+
#[test]
1465+
fn insert_before_cfg_module() {
1466+
check(
1467+
"std::fmt",
1468+
r#"
1469+
#[cfg(target_arch = "x86_64")]
1470+
pub mod api;
1471+
"#,
1472+
r#"
1473+
use std::fmt;
1474+
1475+
#[cfg(target_arch = "x86_64")]
1476+
pub mod api;
1477+
"#,
1478+
ImportGranularity::Crate,
1479+
);
1480+
}
1481+
1482+
fn check_merge(ra_fixture0: &str, ra_fixture1: &str, last: &str, mb: MergeBehavior) {
1483+
let use0 = ast::SourceFile::parse(ra_fixture0, span::Edition::CURRENT)
1484+
.tree()
1485+
.syntax()
1486+
.descendants()
1487+
.find_map(ast::Use::cast)
1488+
.unwrap();
1489+
1490+
let use1 = ast::SourceFile::parse(ra_fixture1, span::Edition::CURRENT)
1491+
.tree()
1492+
.syntax()
1493+
.descendants()
1494+
.find_map(ast::Use::cast)
1495+
.unwrap();
1496+
1497+
let result = try_merge_imports(&use0, &use1, mb);
1498+
assert_eq!(result.map(|u| u.to_string().trim().to_owned()), Some(last.trim().to_owned()));
1499+
}
1500+
1501+
#[test]
1502+
fn merge_gated_imports() {
1503+
check_merge(
1504+
r#"#[cfg(test)] use foo::bar;"#,
1505+
r#"#[cfg(test)] use foo::baz;"#,
1506+
r#"#[cfg(test)] use foo::{bar, baz};"#,
1507+
MergeBehavior::Crate,
1508+
);
1509+
}
1510+
1511+
#[test]
1512+
fn merge_gated_imports_with_different_values() {
1513+
let use0 = ast::SourceFile::parse(r#"#[cfg(a)] use foo::bar;"#, span::Edition::CURRENT)
1514+
.tree()
1515+
.syntax()
1516+
.descendants()
1517+
.find_map(ast::Use::cast)
1518+
.unwrap();
1519+
1520+
let use1 = ast::SourceFile::parse(r#"#[cfg(b)] use foo::baz;"#, span::Edition::CURRENT)
1521+
.tree()
1522+
.syntax()
1523+
.descendants()
1524+
.find_map(ast::Use::cast)
1525+
.unwrap();
1526+
1527+
let result = try_merge_imports(&use0, &use1, MergeBehavior::Crate);
1528+
assert_eq!(result, None);
1529+
}
1530+
1531+
#[test]
1532+
fn merge_gated_imports_different_order() {
1533+
check_merge(
1534+
r#"#[cfg(a)] #[cfg(b)] use foo::bar;"#,
1535+
r#"#[cfg(b)] #[cfg(a)] use foo::baz;"#,
1536+
r#"#[cfg(a)] #[cfg(b)] use foo::{bar, baz};"#,
1537+
MergeBehavior::Crate,
1538+
);
1539+
}
1540+
1541+
#[test]
1542+
fn merge_into_existing_cfg_import() {
1543+
check(
1544+
r#"foo::Foo"#,
1545+
r#"
1546+
#[cfg(target_os = "windows")]
1547+
use bar::Baz;
1548+
1549+
#[cfg(target_os = "windows")]
1550+
fn buzz() {
1551+
Foo$0;
1552+
}
1553+
"#,
1554+
r#"
1555+
#[cfg(target_os = "windows")]
1556+
use bar::Baz;
1557+
#[cfg(target_os = "windows")]
1558+
use foo::Foo;
1559+
1560+
#[cfg(target_os = "windows")]
1561+
fn buzz() {
1562+
Foo;
1563+
}
1564+
"#,
1565+
ImportGranularity::Crate,
1566+
);
1567+
}
1568+
1569+
#[test]
1570+
fn reproduce_user_issue_missing_semicolon() {
1571+
check(
1572+
"std::fmt",
1573+
r#"
1574+
use {
1575+
foo
1576+
}
1577+
1578+
#[cfg(target_arch = "x86_64")]
1579+
pub mod api;
1580+
"#,
1581+
r#"
1582+
use std::fmt;
1583+
1584+
use {
1585+
foo
1586+
}
1587+
1588+
#[cfg(target_arch = "x86_64")]
1589+
pub mod api;
1590+
"#,
1591+
ImportGranularity::Crate,
1592+
);
1593+
}

‎crates/ide-db/src/imports/merge_imports.rs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::cmp::Ordering;
44
use itertools::{EitherOrBoth, Itertools};
55
use parser::T;
66
use syntax::{
7-
Direction, SyntaxElement, algo,
7+
Direction, SyntaxElement, ToSmolStr, algo,
88
ast::{
99
self, AstNode, HasAttrs, HasName, HasVisibility, PathSegmentKind, edit_in_place::Removable,
1010
make,
@@ -691,14 +691,12 @@ pub fn eq_attrs(
691691
attrs0: impl Iterator<Item = ast::Attr>,
692692
attrs1: impl Iterator<Item = ast::Attr>,
693693
) -> bool {
694-
// FIXME order of attributes should not matter
695-
let attrs0 = attrs0
696-
.flat_map(|attr| attr.syntax().descendants_with_tokens())
697-
.flat_map(|it| it.into_token());
698-
let attrs1 = attrs1
699-
.flat_map(|attr| attr.syntax().descendants_with_tokens())
700-
.flat_map(|it| it.into_token());
701-
stdx::iter_eq_by(attrs0, attrs1, |tok, tok2| tok.text() == tok2.text())
694+
let mut attrs0: Vec<_> = attrs0.map(|attr| attr.syntax().text().to_smolstr()).collect();
695+
let mut attrs1: Vec<_> = attrs1.map(|attr| attr.syntax().text().to_smolstr()).collect();
696+
attrs0.sort_unstable();
697+
attrs1.sort_unstable();
698+
699+
attrs0 == attrs1
702700
}
703701

704702
fn path_is_self(path: &ast::Path) -> bool {

0 commit comments

Comments
 (0)