From 829338a64016cd50fc766649d53cf5ea0b27284f Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 1 Apr 2024 23:38:18 +0200 Subject: [PATCH 1/5] add enum variant field names to make the code clearer --- compiler/rustc_resolve/src/check_unused.rs | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index f6004fed8284e..fdec48fe26a63 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -268,9 +268,9 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { enum UnusedSpanResult { Used, - FlatUnused(Span, Span), - NestedFullUnused(Vec, Span), - NestedPartialUnused(Vec, Vec), + FlatUnused { span: Span, remove: Span }, + NestedFullUnused { spans: Vec, remove: Span }, + NestedPartialUnused { spans: Vec, remove: Vec }, } fn calc_unused_spans( @@ -288,14 +288,14 @@ fn calc_unused_spans( match use_tree.kind { ast::UseTreeKind::Simple(..) | ast::UseTreeKind::Glob => { if unused_import.unused.contains(&use_tree_id) { - UnusedSpanResult::FlatUnused(use_tree.span, full_span) + UnusedSpanResult::FlatUnused { span: use_tree.span, remove: full_span } } else { UnusedSpanResult::Used } } ast::UseTreeKind::Nested(ref nested) => { if nested.is_empty() { - return UnusedSpanResult::FlatUnused(use_tree.span, full_span); + return UnusedSpanResult::FlatUnused { span: use_tree.span, remove: full_span }; } let mut unused_spans = Vec::new(); @@ -308,15 +308,15 @@ fn calc_unused_spans( all_nested_unused = false; None } - UnusedSpanResult::FlatUnused(span, remove) => { + UnusedSpanResult::FlatUnused { span, remove } => { unused_spans.push(span); Some(remove) } - UnusedSpanResult::NestedFullUnused(mut spans, remove) => { + UnusedSpanResult::NestedFullUnused { mut spans, remove } => { unused_spans.append(&mut spans); Some(remove) } - UnusedSpanResult::NestedPartialUnused(mut spans, mut to_remove_extra) => { + UnusedSpanResult::NestedPartialUnused { mut spans, remove: mut to_remove_extra } => { all_nested_unused = false; unused_spans.append(&mut spans); to_remove.append(&mut to_remove_extra); @@ -349,9 +349,9 @@ fn calc_unused_spans( if unused_spans.is_empty() { UnusedSpanResult::Used } else if all_nested_unused { - UnusedSpanResult::NestedFullUnused(unused_spans, full_span) + UnusedSpanResult::NestedFullUnused { spans: unused_spans, remove: full_span } } else { - UnusedSpanResult::NestedPartialUnused(unused_spans, to_remove) + UnusedSpanResult::NestedPartialUnused { spans: unused_spans, remove: to_remove } } } } @@ -417,15 +417,15 @@ impl Resolver<'_, '_> { let mut fixes = Vec::new(); let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) { UnusedSpanResult::Used => continue, - UnusedSpanResult::FlatUnused(span, remove) => { + UnusedSpanResult::FlatUnused { span, remove } => { fixes.push((remove, String::new())); vec![span] } - UnusedSpanResult::NestedFullUnused(spans, remove) => { + UnusedSpanResult::NestedFullUnused { spans, remove } => { fixes.push((remove, String::new())); spans } - UnusedSpanResult::NestedPartialUnused(spans, remove) => { + UnusedSpanResult::NestedPartialUnused { spans, remove } => { for fix in &remove { fixes.push((*fix, String::new())); } From 2c26072d8cdca85e813b423f65632911f4781ff5 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 1 Apr 2024 23:42:33 +0200 Subject: [PATCH 2/5] remove redundant flat vs nested distinction to simplify enum --- compiler/rustc_resolve/src/check_unused.rs | 29 ++++++++-------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index fdec48fe26a63..613709b0cb687 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -268,9 +268,8 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { enum UnusedSpanResult { Used, - FlatUnused { span: Span, remove: Span }, - NestedFullUnused { spans: Vec, remove: Span }, - NestedPartialUnused { spans: Vec, remove: Vec }, + Unused { spans: Vec, remove: Span }, + PartialUnused { spans: Vec, remove: Vec }, } fn calc_unused_spans( @@ -288,14 +287,14 @@ fn calc_unused_spans( match use_tree.kind { ast::UseTreeKind::Simple(..) | ast::UseTreeKind::Glob => { if unused_import.unused.contains(&use_tree_id) { - UnusedSpanResult::FlatUnused { span: use_tree.span, remove: full_span } + UnusedSpanResult::Unused { spans: vec![use_tree.span], remove: full_span } } else { UnusedSpanResult::Used } } ast::UseTreeKind::Nested(ref nested) => { if nested.is_empty() { - return UnusedSpanResult::FlatUnused { span: use_tree.span, remove: full_span }; + return UnusedSpanResult::Unused { spans: vec![use_tree.span], remove: full_span }; } let mut unused_spans = Vec::new(); @@ -308,15 +307,11 @@ fn calc_unused_spans( all_nested_unused = false; None } - UnusedSpanResult::FlatUnused { span, remove } => { - unused_spans.push(span); - Some(remove) - } - UnusedSpanResult::NestedFullUnused { mut spans, remove } => { + UnusedSpanResult::Unused { mut spans, remove } => { unused_spans.append(&mut spans); Some(remove) } - UnusedSpanResult::NestedPartialUnused { mut spans, remove: mut to_remove_extra } => { + UnusedSpanResult::PartialUnused { mut spans, remove: mut to_remove_extra } => { all_nested_unused = false; unused_spans.append(&mut spans); to_remove.append(&mut to_remove_extra); @@ -349,9 +344,9 @@ fn calc_unused_spans( if unused_spans.is_empty() { UnusedSpanResult::Used } else if all_nested_unused { - UnusedSpanResult::NestedFullUnused { spans: unused_spans, remove: full_span } + UnusedSpanResult::Unused { spans: unused_spans, remove: full_span } } else { - UnusedSpanResult::NestedPartialUnused { spans: unused_spans, remove: to_remove } + UnusedSpanResult::PartialUnused { spans: unused_spans, remove: to_remove } } } } @@ -417,15 +412,11 @@ impl Resolver<'_, '_> { let mut fixes = Vec::new(); let spans = match calc_unused_spans(unused, &unused.use_tree, unused.use_tree_id) { UnusedSpanResult::Used => continue, - UnusedSpanResult::FlatUnused { span, remove } => { - fixes.push((remove, String::new())); - vec![span] - } - UnusedSpanResult::NestedFullUnused { spans, remove } => { + UnusedSpanResult::Unused { spans, remove } => { fixes.push((remove, String::new())); spans } - UnusedSpanResult::NestedPartialUnused { spans, remove } => { + UnusedSpanResult::PartialUnused { spans, remove } => { for fix in &remove { fixes.push((*fix, String::new())); } From 2ec337c19336845b371dcd6bc1369e6b34124afd Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 1 Apr 2024 23:53:17 +0200 Subject: [PATCH 3/5] turn all_nested_unused into used_childs --- compiler/rustc_resolve/src/check_unused.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 613709b0cb687..e054847fe5e77 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -299,12 +299,12 @@ fn calc_unused_spans( let mut unused_spans = Vec::new(); let mut to_remove = Vec::new(); - let mut all_nested_unused = true; + let mut used_childs = 0; let mut previous_unused = false; for (pos, (use_tree, use_tree_id)) in nested.iter().enumerate() { let remove = match calc_unused_spans(unused_import, use_tree, *use_tree_id) { UnusedSpanResult::Used => { - all_nested_unused = false; + used_childs += 1; None } UnusedSpanResult::Unused { mut spans, remove } => { @@ -312,7 +312,7 @@ fn calc_unused_spans( Some(remove) } UnusedSpanResult::PartialUnused { mut spans, remove: mut to_remove_extra } => { - all_nested_unused = false; + used_childs += 1; unused_spans.append(&mut spans); to_remove.append(&mut to_remove_extra); None @@ -321,7 +321,7 @@ fn calc_unused_spans( if let Some(remove) = remove { let remove_span = if nested.len() == 1 { remove - } else if pos == nested.len() - 1 || !all_nested_unused { + } else if pos == nested.len() - 1 || used_childs > 0 { // Delete everything from the end of the last import, to delete the // previous comma nested[pos - 1].0.span.shrink_to_hi().to(use_tree.span) @@ -343,7 +343,7 @@ fn calc_unused_spans( } if unused_spans.is_empty() { UnusedSpanResult::Used - } else if all_nested_unused { + } else if used_childs == 0 { UnusedSpanResult::Unused { spans: unused_spans, remove: full_span } } else { UnusedSpanResult::PartialUnused { spans: unused_spans, remove: to_remove } From 13f76235b333f4b9c750be45839b488d7f5c8203 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Tue, 2 Apr 2024 00:26:10 +0200 Subject: [PATCH 4/5] store the span of the nested part of the use tree in the ast --- compiler/rustc_ast/src/ast.rs | 2 +- compiler/rustc_ast/src/mut_visit.rs | 2 +- compiler/rustc_ast/src/visit.rs | 4 ++-- compiler/rustc_ast_lowering/src/item.rs | 6 +++--- compiler/rustc_ast_pretty/src/pprust/state/item.rs | 4 ++-- compiler/rustc_builtin_macros/src/assert/context.rs | 11 +++++++---- compiler/rustc_expand/src/expand.rs | 4 ++-- compiler/rustc_lint/src/unused.rs | 4 ++-- compiler/rustc_parse/src/parser/item.rs | 8 ++++++-- compiler/rustc_resolve/src/build_reduced_graph.rs | 2 +- compiler/rustc_resolve/src/check_unused.rs | 6 +++--- compiler/rustc_resolve/src/late.rs | 6 +++--- .../clippy_lints/src/single_component_path_imports.rs | 8 ++++---- .../clippy_lints/src/unnecessary_self_imports.rs | 4 ++-- .../clippy_lints/src/unsafe_removed_from_name.rs | 4 ++-- src/tools/clippy/clippy_utils/src/ast_utils.rs | 2 +- src/tools/rustfmt/src/imports.rs | 4 +++- 17 files changed, 45 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 5b708cf4e1a55..37a13aed24d65 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2703,7 +2703,7 @@ pub enum UseTreeKind { /// `use prefix` or `use prefix as rename` Simple(Option), /// `use prefix::{...}` - Nested(ThinVec<(UseTree, NodeId)>), + Nested { items: ThinVec<(UseTree, NodeId)>, span: Span }, /// `use prefix::*` Glob, } diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index da57def263df5..e633c0743a701 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -436,7 +436,7 @@ pub fn noop_visit_use_tree(use_tree: &mut UseTree, vis: &mut T) { vis.visit_path(prefix); match kind { UseTreeKind::Simple(rename) => visit_opt(rename, |rename| vis.visit_ident(rename)), - UseTreeKind::Nested(items) => { + UseTreeKind::Nested { items, .. } => { for (tree, id) in items { vis.visit_use_tree(tree); vis.visit_id(id); diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 9e9ae52962d87..30a09d4163c94 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -488,8 +488,8 @@ pub fn walk_use_tree<'a, V: Visitor<'a>>( visit_opt!(visitor, visit_ident, rename); } UseTreeKind::Glob => {} - UseTreeKind::Nested(ref use_trees) => { - for &(ref nested_tree, nested_id) in use_trees { + UseTreeKind::Nested { ref items, .. } => { + for &(ref nested_tree, nested_id) in items { try_visit!(visitor.visit_use_tree(nested_tree, nested_id, true)); } } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index abfea6078f21c..21a1671e39809 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -134,8 +134,8 @@ impl<'hir> LoweringContext<'_, 'hir> { fn lower_item_id_use_tree(&mut self, tree: &UseTree, vec: &mut SmallVec<[hir::ItemId; 1]>) { match &tree.kind { - UseTreeKind::Nested(nested_vec) => { - for &(ref nested, id) in nested_vec { + UseTreeKind::Nested { items, .. } => { + for &(ref nested, id) in items { vec.push(hir::ItemId { owner_id: hir::OwnerId { def_id: self.local_def_id(id) }, }); @@ -517,7 +517,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let path = self.lower_use_path(res, &path, ParamMode::Explicit); hir::ItemKind::Use(path, hir::UseKind::Glob) } - UseTreeKind::Nested(ref trees) => { + UseTreeKind::Nested { items: ref trees, .. } => { // Nested imports are desugared into simple imports. // So, if we start with // diff --git a/compiler/rustc_ast_pretty/src/pprust/state/item.rs b/compiler/rustc_ast_pretty/src/pprust/state/item.rs index 13f27c1c95c2e..ff24874bbcf21 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/item.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/item.rs @@ -712,7 +712,7 @@ impl<'a> State<'a> { } self.word("*"); } - ast::UseTreeKind::Nested(items) => { + ast::UseTreeKind::Nested { items, .. } => { if !tree.prefix.segments.is_empty() { self.print_path(&tree.prefix, false, 0); self.word("::"); @@ -731,7 +731,7 @@ impl<'a> State<'a> { self.print_use_tree(&use_tree.0); if !is_last { self.word(","); - if let ast::UseTreeKind::Nested(_) = use_tree.0.kind { + if let ast::UseTreeKind::Nested { .. } = use_tree.0.kind { self.hardbreak(); } else { self.space(); diff --git a/compiler/rustc_builtin_macros/src/assert/context.rs b/compiler/rustc_builtin_macros/src/assert/context.rs index 92efeab08ebea..085ea3458bbfa 100644 --- a/compiler/rustc_builtin_macros/src/assert/context.rs +++ b/compiler/rustc_builtin_macros/src/assert/context.rs @@ -120,10 +120,13 @@ impl<'cx, 'a> Context<'cx, 'a> { thin_vec![self.cx.attr_nested_word(sym::allow, sym::unused_imports, self.span)], ItemKind::Use(UseTree { prefix: self.cx.path(self.span, self.cx.std_path(&[sym::asserting])), - kind: UseTreeKind::Nested(thin_vec![ - nested_tree(self, sym::TryCaptureGeneric), - nested_tree(self, sym::TryCapturePrintable), - ]), + kind: UseTreeKind::Nested { + items: thin_vec![ + nested_tree(self, sym::TryCaptureGeneric), + nested_tree(self, sym::TryCapturePrintable), + ], + span: self.span, + }, span: self.span, }), ), diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 6029caa965c0b..6de3a392e7857 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1190,8 +1190,8 @@ impl InvocationCollectorNode for P { match &ut.kind { ast::UseTreeKind::Glob => {} ast::UseTreeKind::Simple(_) => idents.push(ut.ident()), - ast::UseTreeKind::Nested(nested) => { - for (ut, _) in nested { + ast::UseTreeKind::Nested { items, .. } => { + for (ut, _) in items { collect_use_tree_leaves(ut, idents); } } diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 503caa35358ab..e8f9d373289ce 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1499,7 +1499,7 @@ declare_lint_pass!(UnusedImportBraces => [UNUSED_IMPORT_BRACES]); impl UnusedImportBraces { fn check_use_tree(&self, cx: &EarlyContext<'_>, use_tree: &ast::UseTree, item: &ast::Item) { - if let ast::UseTreeKind::Nested(ref items) = use_tree.kind { + if let ast::UseTreeKind::Nested { ref items, .. } = use_tree.kind { // Recursively check nested UseTrees for (tree, _) in items { self.check_use_tree(cx, tree, item); @@ -1520,7 +1520,7 @@ impl UnusedImportBraces { rename.unwrap_or(orig_ident).name } ast::UseTreeKind::Glob => Symbol::intern("*"), - ast::UseTreeKind::Nested(_) => return, + ast::UseTreeKind::Nested { .. } => return, }; cx.emit_span_lint( diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index d54eb8dc4c9eb..7332cc8622063 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -336,7 +336,7 @@ impl<'a> Parser<'a> { UseTreeKind::Glob => { e.note("the wildcard token must be last on the path"); } - UseTreeKind::Nested(..) => { + UseTreeKind::Nested { .. } => { e.note("glob-like brace syntax must be last on the path"); } _ => (), @@ -1056,7 +1056,11 @@ impl<'a> Parser<'a> { Ok(if self.eat(&token::BinOp(token::Star)) { UseTreeKind::Glob } else { - UseTreeKind::Nested(self.parse_use_tree_list()?) + let lo = self.token.span; + UseTreeKind::Nested { + items: self.parse_use_tree_list()?, + span: lo.to(self.prev_token.span), + } }) } diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 375f20dd809f2..e484fdd45177c 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -565,7 +565,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> { self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis); } - ast::UseTreeKind::Nested(ref items) => { + ast::UseTreeKind::Nested { ref items, .. } => { // Ensure there is at most one `self` in the list let self_spans = items .iter() diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index e054847fe5e77..950a0f9ff652d 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -128,7 +128,7 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> { self.unused_import(self.base_id).add(id); } } - ast::UseTreeKind::Nested(ref items) => self.check_imports_as_underscore(items), + ast::UseTreeKind::Nested { ref items, .. } => self.check_imports_as_underscore(items), _ => {} } } @@ -254,7 +254,7 @@ impl<'a, 'b, 'tcx> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'tcx> { return; } - if let ast::UseTreeKind::Nested(ref items) = use_tree.kind { + if let ast::UseTreeKind::Nested { ref items, .. } = use_tree.kind { if items.is_empty() { self.unused_import(self.base_id).add(id); } @@ -292,7 +292,7 @@ fn calc_unused_spans( UnusedSpanResult::Used } } - ast::UseTreeKind::Nested(ref nested) => { + ast::UseTreeKind::Nested { items: ref nested, .. } => { if nested.is_empty() { return UnusedSpanResult::Unused { spans: vec![use_tree.span], remove: full_span }; } diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 49b4a6efd3c3e..37b259dc5393b 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2332,8 +2332,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { None => {} } } - } else if let UseTreeKind::Nested(use_trees) = &use_tree.kind { - for (use_tree, _) in use_trees { + } else if let UseTreeKind::Nested { items, .. } = &use_tree.kind { + for (use_tree, _) in items { self.future_proof_import(use_tree); } } @@ -2510,7 +2510,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { ItemKind::Use(ref use_tree) => { let maybe_exported = match use_tree.kind { UseTreeKind::Simple(_) | UseTreeKind::Glob => MaybeExported::Ok(item.id), - UseTreeKind::Nested(_) => MaybeExported::NestedUse(&item.vis), + UseTreeKind::Nested { .. } => MaybeExported::NestedUse(&item.vis), }; self.resolve_doc_links(&item.attrs, maybe_exported); diff --git a/src/tools/clippy/clippy_lints/src/single_component_path_imports.rs b/src/tools/clippy/clippy_lints/src/single_component_path_imports.rs index 18fbbdb407911..acf44a9bb5ab4 100644 --- a/src/tools/clippy/clippy_lints/src/single_component_path_imports.rs +++ b/src/tools/clippy/clippy_lints/src/single_component_path_imports.rs @@ -201,8 +201,8 @@ impl SingleComponentPathImports { if segments.is_empty() { // keep track of `use {some_module, some_other_module};` usages - if let UseTreeKind::Nested(trees) = &use_tree.kind { - for tree in trees { + if let UseTreeKind::Nested { items, .. } = &use_tree.kind { + for tree in items { let segments = &tree.0.prefix.segments; if segments.len() == 1 { if let UseTreeKind::Simple(None) = tree.0.kind { @@ -229,8 +229,8 @@ impl SingleComponentPathImports { } // nested case such as `use self::{module1::Struct1, module2::Struct2}` - if let UseTreeKind::Nested(trees) = &use_tree.kind { - for tree in trees { + if let UseTreeKind::Nested { items, .. } = &use_tree.kind { + for tree in items { let segments = &tree.0.prefix.segments; if !segments.is_empty() { imports_reused_with_self.push(segments[0].ident.name); diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_self_imports.rs b/src/tools/clippy/clippy_lints/src/unnecessary_self_imports.rs index ddee06b59cae1..528a1dfcfc10f 100644 --- a/src/tools/clippy/clippy_lints/src/unnecessary_self_imports.rs +++ b/src/tools/clippy/clippy_lints/src/unnecessary_self_imports.rs @@ -36,8 +36,8 @@ declare_lint_pass!(UnnecessarySelfImports => [UNNECESSARY_SELF_IMPORTS]); impl EarlyLintPass for UnnecessarySelfImports { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { if let ItemKind::Use(use_tree) = &item.kind - && let UseTreeKind::Nested(nodes) = &use_tree.kind - && let [(self_tree, _)] = &**nodes + && let UseTreeKind::Nested { items, .. } = &use_tree.kind + && let [(self_tree, _)] = &**items && let [self_seg] = &*self_tree.prefix.segments && self_seg.ident.name == kw::SelfLower && let Some(last_segment) = use_tree.prefix.segments.last() diff --git a/src/tools/clippy/clippy_lints/src/unsafe_removed_from_name.rs b/src/tools/clippy/clippy_lints/src/unsafe_removed_from_name.rs index 51b3ea93b6dc9..309eaedac8d29 100644 --- a/src/tools/clippy/clippy_lints/src/unsafe_removed_from_name.rs +++ b/src/tools/clippy/clippy_lints/src/unsafe_removed_from_name.rs @@ -49,8 +49,8 @@ fn check_use_tree(use_tree: &UseTree, cx: &EarlyContext<'_>, span: Span) { unsafe_to_safe_check(old_name, new_name, cx, span); }, UseTreeKind::Simple(None) | UseTreeKind::Glob => {}, - UseTreeKind::Nested(ref nested_use_tree) => { - for (use_tree, _) in nested_use_tree { + UseTreeKind::Nested { ref items, .. } => { + for (use_tree, _) in items { check_use_tree(use_tree, cx, span); } }, diff --git a/src/tools/clippy/clippy_utils/src/ast_utils.rs b/src/tools/clippy/clippy_utils/src/ast_utils.rs index f594a40ff59ad..36d54bb41fa4d 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils.rs @@ -637,7 +637,7 @@ pub fn eq_use_tree_kind(l: &UseTreeKind, r: &UseTreeKind) -> bool { match (l, r) { (Glob, Glob) => true, (Simple(l), Simple(r)) => both(l, r, |l, r| eq_id(*l, *r)), - (Nested(l), Nested(r)) => over(l, r, |(l, _), (r, _)| eq_use_tree(l, r)), + (Nested { items: l, .. }, Nested { items: r, .. }) => over(l, r, |(l, _), (r, _)| eq_use_tree(l, r)), _ => false, } } diff --git a/src/tools/rustfmt/src/imports.rs b/src/tools/rustfmt/src/imports.rs index 09f6e75233880..2da746295fc96 100644 --- a/src/tools/rustfmt/src/imports.rs +++ b/src/tools/rustfmt/src/imports.rs @@ -458,7 +458,9 @@ impl UseTree { version, }); } - UseTreeKind::Nested(ref list) => { + UseTreeKind::Nested { + items: ref list, .. + } => { // Extract comments between nested use items. // This needs to be done before sorting use items. let items = itemize_list( From 2d3a9a5847dccb2623c956e5aa175493fa68aa2a Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sun, 14 Apr 2024 18:40:49 +0200 Subject: [PATCH 5/5] remove braces when fixing a nested use tree into a single use --- compiler/rustc_resolve/src/check_unused.rs | 27 +++++++++++++- tests/ui/suggestions/unused-imports.fixed | 35 ++++++++++++++++++ tests/ui/suggestions/unused-imports.rs | 42 ++++++++++++++++++++++ tests/ui/suggestions/unused-imports.stderr | 32 +++++++++++++++++ 4 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 tests/ui/suggestions/unused-imports.fixed create mode 100644 tests/ui/suggestions/unused-imports.rs create mode 100644 tests/ui/suggestions/unused-imports.stderr diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 950a0f9ff652d..5fe68085d6537 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -292,7 +292,7 @@ fn calc_unused_spans( UnusedSpanResult::Used } } - ast::UseTreeKind::Nested { items: ref nested, .. } => { + ast::UseTreeKind::Nested { items: ref nested, span: tree_span } => { if nested.is_empty() { return UnusedSpanResult::Unused { spans: vec![use_tree.span], remove: full_span }; } @@ -300,6 +300,7 @@ fn calc_unused_spans( let mut unused_spans = Vec::new(); let mut to_remove = Vec::new(); let mut used_childs = 0; + let mut contains_self = false; let mut previous_unused = false; for (pos, (use_tree, use_tree_id)) in nested.iter().enumerate() { let remove = match calc_unused_spans(unused_import, use_tree, *use_tree_id) { @@ -339,6 +340,8 @@ fn calc_unused_spans( to_remove.push(remove_span); } } + contains_self |= use_tree.prefix == kw::SelfLower + && matches!(use_tree.kind, ast::UseTreeKind::Simple(None)); previous_unused = remove.is_some(); } if unused_spans.is_empty() { @@ -346,6 +349,28 @@ fn calc_unused_spans( } else if used_childs == 0 { UnusedSpanResult::Unused { spans: unused_spans, remove: full_span } } else { + // If there is only one remaining child that is used, the braces around the use + // tree are not needed anymore. In that case, we determine the span of the left + // brace and the right brace, and tell rustfix to remove them as well. + // + // This means that `use a::{B, C};` will be turned into `use a::B;` rather than + // `use a::{B};`, removing a rustfmt roundtrip. + // + // Note that we cannot remove the braces if the only item inside the use tree is + // `self`: `use foo::{self};` is valid Rust syntax, while `use foo::self;` errors + // out. We also cannot turn `use foo::{self}` into `use foo`, as the former doesn't + // import types with the same name as the module. + if used_childs == 1 && !contains_self { + // Left brace, from the start of the nested group to the first item. + to_remove.push( + tree_span.shrink_to_lo().to(nested.first().unwrap().0.span.shrink_to_lo()), + ); + // Right brace, from the end of the last item to the end of the nested group. + to_remove.push( + nested.last().unwrap().0.span.shrink_to_hi().to(tree_span.shrink_to_hi()), + ); + } + UnusedSpanResult::PartialUnused { spans: unused_spans, remove: to_remove } } } diff --git a/tests/ui/suggestions/unused-imports.fixed b/tests/ui/suggestions/unused-imports.fixed new file mode 100644 index 0000000000000..57dd091c0436d --- /dev/null +++ b/tests/ui/suggestions/unused-imports.fixed @@ -0,0 +1,35 @@ +//@ run-rustfix +//@ check-pass + +#![warn(unused_imports)] + +pub mod nested { + pub struct A; + pub struct B; + pub struct C; + pub struct D; + pub mod even_more { + pub struct E; + pub struct F; + pub struct G; + } + pub mod another { + pub struct H; + pub struct I; + } +} + +use nested::B; +//~^ WARN unused import + +use nested::even_more::F; +//~^^^^^^^ WARN unused import + +// Note that the following fix should result in `::{self}`, not `::self`. The latter is invalid +// Rust syntax, so the braces should not be removed. +use nested::another::{self}; +//~^ WARN unused import + +fn main() { + let _ = (B, F, another::I); +} diff --git a/tests/ui/suggestions/unused-imports.rs b/tests/ui/suggestions/unused-imports.rs new file mode 100644 index 0000000000000..5f9dd243bdd2a --- /dev/null +++ b/tests/ui/suggestions/unused-imports.rs @@ -0,0 +1,42 @@ +//@ run-rustfix +//@ check-pass + +#![warn(unused_imports)] + +pub mod nested { + pub struct A; + pub struct B; + pub struct C; + pub struct D; + pub mod even_more { + pub struct E; + pub struct F; + pub struct G; + } + pub mod another { + pub struct H; + pub struct I; + } +} + +use nested::{A, B, C}; +//~^ WARN unused import + +use nested::{ + D, + even_more::{ + E, + F, + G, + }, + }; +//~^^^^^^^ WARN unused import + +// Note that the following fix should result in `::{self}`, not `::self`. The latter is invalid +// Rust syntax, so the braces should not be removed. +use nested::another::{self, I}; +//~^ WARN unused import + +fn main() { + let _ = (B, F, another::I); +} diff --git a/tests/ui/suggestions/unused-imports.stderr b/tests/ui/suggestions/unused-imports.stderr new file mode 100644 index 0000000000000..bf112608da7ed --- /dev/null +++ b/tests/ui/suggestions/unused-imports.stderr @@ -0,0 +1,32 @@ +warning: unused imports: `A`, `C` + --> $DIR/unused-imports.rs:22:14 + | +LL | use nested::{A, B, C}; + | ^ ^ + | +note: the lint level is defined here + --> $DIR/unused-imports.rs:4:9 + | +LL | #![warn(unused_imports)] + | ^^^^^^^^^^^^^^ + +warning: unused imports: `D`, `E`, `G` + --> $DIR/unused-imports.rs:26:5 + | +LL | D, + | ^ +LL | even_more::{ +LL | E, + | ^ +LL | F, +LL | G, + | ^ + +warning: unused import: `I` + --> $DIR/unused-imports.rs:37:29 + | +LL | use nested::another::{self, I}; + | ^ + +warning: 3 warnings emitted +