Skip to content

Commit d30af5e

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #123344 - pietroalbini:pa-unused-imports, r=Nilstrieb
Remove braces when fixing a nested use tree into a single item [Back in 2019](#56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`. This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then. A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`. This PR is best reviewed commit-by-commit.
2 parents c3202af + 2d3a9a5 commit d30af5e

File tree

20 files changed

+194
-60
lines changed

20 files changed

+194
-60
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2729,7 +2729,7 @@ pub enum UseTreeKind {
27292729
/// `use prefix` or `use prefix as rename`
27302730
Simple(Option<Ident>),
27312731
/// `use prefix::{...}`
2732-
Nested(ThinVec<(UseTree, NodeId)>),
2732+
Nested { items: ThinVec<(UseTree, NodeId)>, span: Span },
27332733
/// `use prefix::*`
27342734
Glob,
27352735
}

‎compiler/rustc_ast/src/mut_visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ fn noop_visit_use_tree<T: MutVisitor>(use_tree: &mut UseTree, vis: &mut T) {
441441
vis.visit_path(prefix);
442442
match kind {
443443
UseTreeKind::Simple(rename) => visit_opt(rename, |rename| vis.visit_ident(rename)),
444-
UseTreeKind::Nested(items) => {
444+
UseTreeKind::Nested { items, .. } => {
445445
for (tree, id) in items {
446446
vis.visit_use_tree(tree);
447447
vis.visit_id(id);

‎compiler/rustc_ast/src/visit.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,8 @@ pub fn walk_use_tree<'a, V: Visitor<'a>>(
517517
visit_opt!(visitor, visit_ident, rename);
518518
}
519519
UseTreeKind::Glob => {}
520-
UseTreeKind::Nested(ref use_trees) => {
521-
for &(ref nested_tree, nested_id) in use_trees {
520+
UseTreeKind::Nested { ref items, .. } => {
521+
for &(ref nested_tree, nested_id) in items {
522522
try_visit!(visitor.visit_use_tree(nested_tree, nested_id, true));
523523
}
524524
}

‎compiler/rustc_ast_lowering/src/item.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
135135

136136
fn lower_item_id_use_tree(&mut self, tree: &UseTree, vec: &mut SmallVec<[hir::ItemId; 1]>) {
137137
match &tree.kind {
138-
UseTreeKind::Nested(nested_vec) => {
139-
for &(ref nested, id) in nested_vec {
138+
UseTreeKind::Nested { items, .. } => {
139+
for &(ref nested, id) in items {
140140
vec.push(hir::ItemId {
141141
owner_id: hir::OwnerId { def_id: self.local_def_id(id) },
142142
});
@@ -518,7 +518,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
518518
let path = self.lower_use_path(res, &path, ParamMode::Explicit);
519519
hir::ItemKind::Use(path, hir::UseKind::Glob)
520520
}
521-
UseTreeKind::Nested(ref trees) => {
521+
UseTreeKind::Nested { items: ref trees, .. } => {
522522
// Nested imports are desugared into simple imports.
523523
// So, if we start with
524524
//

‎compiler/rustc_ast_pretty/src/pprust/state/item.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ impl<'a> State<'a> {
715715
}
716716
self.word("*");
717717
}
718-
ast::UseTreeKind::Nested(items) => {
718+
ast::UseTreeKind::Nested { items, .. } => {
719719
if !tree.prefix.segments.is_empty() {
720720
self.print_path(&tree.prefix, false, 0);
721721
self.word("::");
@@ -734,7 +734,7 @@ impl<'a> State<'a> {
734734
self.print_use_tree(&use_tree.0);
735735
if !is_last {
736736
self.word(",");
737-
if let ast::UseTreeKind::Nested(_) = use_tree.0.kind {
737+
if let ast::UseTreeKind::Nested { .. } = use_tree.0.kind {
738738
self.hardbreak();
739739
} else {
740740
self.space();

‎compiler/rustc_builtin_macros/src/assert/context.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,13 @@ impl<'cx, 'a> Context<'cx, 'a> {
120120
thin_vec![self.cx.attr_nested_word(sym::allow, sym::unused_imports, self.span)],
121121
ItemKind::Use(UseTree {
122122
prefix: self.cx.path(self.span, self.cx.std_path(&[sym::asserting])),
123-
kind: UseTreeKind::Nested(thin_vec![
124-
nested_tree(self, sym::TryCaptureGeneric),
125-
nested_tree(self, sym::TryCapturePrintable),
126-
]),
123+
kind: UseTreeKind::Nested {
124+
items: thin_vec![
125+
nested_tree(self, sym::TryCaptureGeneric),
126+
nested_tree(self, sym::TryCapturePrintable),
127+
],
128+
span: self.span,
129+
},
127130
span: self.span,
128131
}),
129132
),

‎compiler/rustc_expand/src/expand.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,8 +1194,8 @@ impl InvocationCollectorNode for P<ast::Item> {
11941194
match &ut.kind {
11951195
ast::UseTreeKind::Glob => {}
11961196
ast::UseTreeKind::Simple(_) => idents.push(ut.ident()),
1197-
ast::UseTreeKind::Nested(nested) => {
1198-
for (ut, _) in nested {
1197+
ast::UseTreeKind::Nested { items, .. } => {
1198+
for (ut, _) in items {
11991199
collect_use_tree_leaves(ut, idents);
12001200
}
12011201
}

‎compiler/rustc_lint/src/unused.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,7 +1501,7 @@ declare_lint_pass!(UnusedImportBraces => [UNUSED_IMPORT_BRACES]);
15011501

15021502
impl UnusedImportBraces {
15031503
fn check_use_tree(&self, cx: &EarlyContext<'_>, use_tree: &ast::UseTree, item: &ast::Item) {
1504-
if let ast::UseTreeKind::Nested(ref items) = use_tree.kind {
1504+
if let ast::UseTreeKind::Nested { ref items, .. } = use_tree.kind {
15051505
// Recursively check nested UseTrees
15061506
for (tree, _) in items {
15071507
self.check_use_tree(cx, tree, item);
@@ -1522,7 +1522,7 @@ impl UnusedImportBraces {
15221522
rename.unwrap_or(orig_ident).name
15231523
}
15241524
ast::UseTreeKind::Glob => Symbol::intern("*"),
1525-
ast::UseTreeKind::Nested(_) => return,
1525+
ast::UseTreeKind::Nested { .. } => return,
15261526
};
15271527

15281528
cx.emit_span_lint(

‎compiler/rustc_parse/src/parser/item.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl<'a> Parser<'a> {
336336
UseTreeKind::Glob => {
337337
e.note("the wildcard token must be last on the path");
338338
}
339-
UseTreeKind::Nested(..) => {
339+
UseTreeKind::Nested { .. } => {
340340
e.note("glob-like brace syntax must be last on the path");
341341
}
342342
_ => (),
@@ -1056,7 +1056,11 @@ impl<'a> Parser<'a> {
10561056
Ok(if self.eat(&token::BinOp(token::Star)) {
10571057
UseTreeKind::Glob
10581058
} else {
1059-
UseTreeKind::Nested(self.parse_use_tree_list()?)
1059+
let lo = self.token.span;
1060+
UseTreeKind::Nested {
1061+
items: self.parse_use_tree_list()?,
1062+
span: lo.to(self.prev_token.span),
1063+
}
10601064
})
10611065
}
10621066

‎compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
560560

561561
self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis);
562562
}
563-
ast::UseTreeKind::Nested(ref items) => {
563+
ast::UseTreeKind::Nested { ref items, .. } => {
564564
// Ensure there is at most one `self` in the list
565565
let self_spans = items
566566
.iter()

‎compiler/rustc_resolve/src/check_unused.rs

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> {
128128
self.unused_import(self.base_id).add(id);
129129
}
130130
}
131-
ast::UseTreeKind::Nested(ref items) => self.check_imports_as_underscore(items),
131+
ast::UseTreeKind::Nested { ref items, .. } => self.check_imports_as_underscore(items),
132132
_ => {}
133133
}
134134
}
@@ -254,7 +254,7 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
254254
return;
255255
}
256256

257-
if let ast::UseTreeKind::Nested(ref items) = use_tree.kind {
257+
if let ast::UseTreeKind::Nested { ref items, .. } = use_tree.kind {
258258
if items.is_empty() {
259259
self.unused_import(self.base_id).add(id);
260260
}
@@ -268,9 +268,8 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> {
268268

269269
enum UnusedSpanResult {
270270
Used,
271-
FlatUnused(Span, Span),
272-
NestedFullUnused(Vec<Span>, Span),
273-
NestedPartialUnused(Vec<Span>, Vec<Span>),
271+
Unused { spans: Vec<Span>, remove: Span },
272+
PartialUnused { spans: Vec<Span>, remove: Vec<Span> },
274273
}
275274

276275
fn calc_unused_spans(
@@ -288,36 +287,33 @@ fn calc_unused_spans(
288287
match use_tree.kind {
289288
ast::UseTreeKind::Simple(..) | ast::UseTreeKind::Glob => {
290289
if unused_import.unused.contains(&use_tree_id) {
291-
UnusedSpanResult::FlatUnused(use_tree.span, full_span)
290+
UnusedSpanResult::Unused { spans: vec![use_tree.span], remove: full_span }
292291
} else {
293292
UnusedSpanResult::Used
294293
}
295294
}
296-
ast::UseTreeKind::Nested(ref nested) => {
295+
ast::UseTreeKind::Nested { items: ref nested, span: tree_span } => {
297296
if nested.is_empty() {
298-
return UnusedSpanResult::FlatUnused(use_tree.span, full_span);
297+
return UnusedSpanResult::Unused { spans: vec![use_tree.span], remove: full_span };
299298
}
300299

301300
let mut unused_spans = Vec::new();
302301
let mut to_remove = Vec::new();
303-
let mut all_nested_unused = true;
302+
let mut used_childs = 0;
303+
let mut contains_self = false;
304304
let mut previous_unused = false;
305305
for (pos, (use_tree, use_tree_id)) in nested.iter().enumerate() {
306306
let remove = match calc_unused_spans(unused_import, use_tree, *use_tree_id) {
307307
UnusedSpanResult::Used => {
308-
all_nested_unused = false;
308+
used_childs += 1;
309309
None
310310
}
311-
UnusedSpanResult::FlatUnused(span, remove) => {
312-
unused_spans.push(span);
313-
Some(remove)
314-
}
315-
UnusedSpanResult::NestedFullUnused(mut spans, remove) => {
311+
UnusedSpanResult::Unused { mut spans, remove } => {
316312
unused_spans.append(&mut spans);
317313
Some(remove)
318314
}
319-
UnusedSpanResult::NestedPartialUnused(mut spans, mut to_remove_extra) => {
320-
all_nested_unused = false;
315+
UnusedSpanResult::PartialUnused { mut spans, remove: mut to_remove_extra } => {
316+
used_childs += 1;
321317
unused_spans.append(&mut spans);
322318
to_remove.append(&mut to_remove_extra);
323319
None
@@ -326,7 +322,7 @@ fn calc_unused_spans(
326322
if let Some(remove) = remove {
327323
let remove_span = if nested.len() == 1 {
328324
remove
329-
} else if pos == nested.len() - 1 || !all_nested_unused {
325+
} else if pos == nested.len() - 1 || used_childs > 0 {
330326
// Delete everything from the end of the last import, to delete the
331327
// previous comma
332328
nested[pos - 1].0.span.shrink_to_hi().to(use_tree.span)
@@ -344,14 +340,38 @@ fn calc_unused_spans(
344340
to_remove.push(remove_span);
345341
}
346342
}
343+
contains_self |= use_tree.prefix == kw::SelfLower
344+
&& matches!(use_tree.kind, ast::UseTreeKind::Simple(None));
347345
previous_unused = remove.is_some();
348346
}
349347
if unused_spans.is_empty() {
350348
UnusedSpanResult::Used
351-
} else if all_nested_unused {
352-
UnusedSpanResult::NestedFullUnused(unused_spans, full_span)
349+
} else if used_childs == 0 {
350+
UnusedSpanResult::Unused { spans: unused_spans, remove: full_span }
353351
} else {
354-
UnusedSpanResult::NestedPartialUnused(unused_spans, to_remove)
352+
// If there is only one remaining child that is used, the braces around the use
353+
// tree are not needed anymore. In that case, we determine the span of the left
354+
// brace and the right brace, and tell rustfix to remove them as well.
355+
//
356+
// This means that `use a::{B, C};` will be turned into `use a::B;` rather than
357+
// `use a::{B};`, removing a rustfmt roundtrip.
358+
//
359+
// Note that we cannot remove the braces if the only item inside the use tree is
360+
// `self`: `use foo::{self};` is valid Rust syntax, while `use foo::self;` errors
361+
// out. We also cannot turn `use foo::{self}` into `use foo`, as the former doesn't
362+
// import types with the same name as the module.
363+
if used_childs == 1 && !contains_self {
364+
// Left brace, from the start of the nested group to the first item.
365+
to_remove.push(
366+
tree_span.shrink_to_lo().to(nested.first().unwrap().0.span.shrink_to_lo()),
367+
);
368+
// Right brace, from the end of the last item to the end of the nested group.
369+
to_remove.push(
370+
nested.last().unwrap().0.span.shrink_to_hi().to(tree_span.shrink_to_hi()),
371+
);
372+
}
373+
374+
UnusedSpanResult::PartialUnused { spans: unused_spans, remove: to_remove }
355375
}
356376
}
357377
}
@@ -417,15 +437,11 @@ impl Resolver<'_, '_> {
417437
let mut fixes = Vec::new();
418438
let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) {
419439
UnusedSpanResult::Used => continue,
420-
UnusedSpanResult::FlatUnused(span, remove) => {
421-
fixes.push((remove, String::new()));
422-
vec![span]
423-
}
424-
UnusedSpanResult::NestedFullUnused(spans, remove) => {
440+
UnusedSpanResult::Unused { spans, remove } => {
425441
fixes.push((remove, String::new()));
426442
spans
427443
}
428-
UnusedSpanResult::NestedPartialUnused(spans, remove) => {
444+
UnusedSpanResult::PartialUnused { spans, remove } => {
429445
for fix in &remove {
430446
fixes.push((*fix, String::new()));
431447
}

‎compiler/rustc_resolve/src/late.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,8 +2347,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
23472347
None => {}
23482348
}
23492349
}
2350-
} else if let UseTreeKind::Nested(use_trees) = &use_tree.kind {
2351-
for (use_tree, _) in use_trees {
2350+
} else if let UseTreeKind::Nested { items, .. } = &use_tree.kind {
2351+
for (use_tree, _) in items {
23522352
self.future_proof_import(use_tree);
23532353
}
23542354
}
@@ -2525,7 +2525,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
25252525
ItemKind::Use(ref use_tree) => {
25262526
let maybe_exported = match use_tree.kind {
25272527
UseTreeKind::Simple(_) | UseTreeKind::Glob => MaybeExported::Ok(item.id),
2528-
UseTreeKind::Nested(_) => MaybeExported::NestedUse(&item.vis),
2528+
UseTreeKind::Nested { .. } => MaybeExported::NestedUse(&item.vis),
25292529
};
25302530
self.resolve_doc_links(&item.attrs, maybe_exported);
25312531

‎src/tools/clippy/clippy_lints/src/single_component_path_imports.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ impl SingleComponentPathImports {
201201

202202
if segments.is_empty() {
203203
// keep track of `use {some_module, some_other_module};` usages
204-
if let UseTreeKind::Nested(trees) = &use_tree.kind {
205-
for tree in trees {
204+
if let UseTreeKind::Nested { items, .. } = &use_tree.kind {
205+
for tree in items {
206206
let segments = &tree.0.prefix.segments;
207207
if segments.len() == 1 {
208208
if let UseTreeKind::Simple(None) = tree.0.kind {
@@ -229,8 +229,8 @@ impl SingleComponentPathImports {
229229
}
230230

231231
// nested case such as `use self::{module1::Struct1, module2::Struct2}`
232-
if let UseTreeKind::Nested(trees) = &use_tree.kind {
233-
for tree in trees {
232+
if let UseTreeKind::Nested { items, .. } = &use_tree.kind {
233+
for tree in items {
234234
let segments = &tree.0.prefix.segments;
235235
if !segments.is_empty() {
236236
imports_reused_with_self.push(segments[0].ident.name);

‎src/tools/clippy/clippy_lints/src/unnecessary_self_imports.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ declare_lint_pass!(UnnecessarySelfImports => [UNNECESSARY_SELF_IMPORTS]);
3636
impl EarlyLintPass for UnnecessarySelfImports {
3737
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
3838
if let ItemKind::Use(use_tree) = &item.kind
39-
&& let UseTreeKind::Nested(nodes) = &use_tree.kind
40-
&& let [(self_tree, _)] = &**nodes
39+
&& let UseTreeKind::Nested { items, .. } = &use_tree.kind
40+
&& let [(self_tree, _)] = &**items
4141
&& let [self_seg] = &*self_tree.prefix.segments
4242
&& self_seg.ident.name == kw::SelfLower
4343
&& let Some(last_segment) = use_tree.prefix.segments.last()

‎src/tools/clippy/clippy_lints/src/unsafe_removed_from_name.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ fn check_use_tree(use_tree: &UseTree, cx: &EarlyContext<'_>, span: Span) {
4949
unsafe_to_safe_check(old_name, new_name, cx, span);
5050
},
5151
UseTreeKind::Simple(None) | UseTreeKind::Glob => {},
52-
UseTreeKind::Nested(ref nested_use_tree) => {
53-
for (use_tree, _) in nested_use_tree {
52+
UseTreeKind::Nested { ref items, .. } => {
53+
for (use_tree, _) in items {
5454
check_use_tree(use_tree, cx, span);
5555
}
5656
},

‎src/tools/clippy/clippy_utils/src/ast_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ pub fn eq_use_tree_kind(l: &UseTreeKind, r: &UseTreeKind) -> bool {
648648
match (l, r) {
649649
(Glob, Glob) => true,
650650
(Simple(l), Simple(r)) => both(l, r, |l, r| eq_id(*l, *r)),
651-
(Nested(l), Nested(r)) => over(l, r, |(l, _), (r, _)| eq_use_tree(l, r)),
651+
(Nested { items: l, .. }, Nested { items: r, .. }) => over(l, r, |(l, _), (r, _)| eq_use_tree(l, r)),
652652
_ => false,
653653
}
654654
}

‎src/tools/rustfmt/src/imports.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,9 @@ impl UseTree {
458458
version,
459459
});
460460
}
461-
UseTreeKind::Nested(ref list) => {
461+
UseTreeKind::Nested {
462+
items: ref list, ..
463+
} => {
462464
// Extract comments between nested use items.
463465
// This needs to be done before sorting use items.
464466
let items = itemize_list(

0 commit comments

Comments
 (0)