about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNicholas Nethercote <n.nethercote@gmail.com>2025-05-30 01:37:48 +1000
committerNicholas Nethercote <n.nethercote@gmail.com>2025-06-03 08:23:21 +1000
commit1b2b7e629155f7bc11290147ff042425c618de6a (patch)
tree16b378c14d9e801d6511da029ca3675ac1db1688
parentabf5d4c33436a0ecb85b9756649979476971612b (diff)
downloadrust-1b2b7e629155f7bc11290147ff042425c618de6a.tar.gz
rust-1b2b7e629155f7bc11290147ff042425c618de6a.zip
Overhaul `UsePath`.
`UsePath` contains a `SmallVec<[Res; 3]>`. This holds up to three `Res`
results, one per namespace (type, value, or macro). `lower_import_res`
takes a `PerNS<Option<Res<NodeId>>>` result and lowers it into the
`SmallVec`. This is pretty weird. The input `PerNS` makes it clear which
`Res` belongs to which namespace, but the `SmallVec` throws that
information away.

And code that operates on the `SmallVec` tends to use iteration (or even
just grabbing the first entry!) without knowing which namespace the
`Res` belongs to. Even weirder! Also, `SmallVec` is an overly flexible
type to use here, because it can contain any number of elements (even
though it's optimized for 3 in this case).

This commit changes `UsePath` so it also contains a
`PerNS<Option<Res<HirId>>>`. This type preserves more information and is
more self-documenting. The commit also changes a lot of the use sites to
access the result for a particular namespace. E.g. if you're looking up
a trait, it will be in the `Res` for the type namespace if it's present;
it's silly to look in the `Res` for the value namespace or macro
namespace. Overall I find the new code much easier to understand.

However, some use sites still iterate. These now use `present_items`
because that filters out the `None` results.

Also, `redundant_pub_crate.rs` gets a bigger change. A
`UseKind:ListStem` item gets no `Res` results, which means the old `all`
call in `is_not_macro_export` would succeed (because `all` succeeds on
an empty iterator) and the `ListStem` would be ignored. This is what we
want, but was more by luck than design. The new code detects `ListStem`
explicitly. The commit generalizes the name of that function
accordingly.

Finally, the commit also removes the `use_path` arena, because
`PerNS<Option<Res>>` impls `Copy` (unlike `SmallVec`) and it can be
allocated in the arena shared by all `Copy` types.
-rw-r--r--clippy_lints/src/disallowed_types.rs4
-rw-r--r--clippy_lints/src/legacy_numeric_constants.rs4
-rw-r--r--clippy_lints/src/macro_use.rs5
-rw-r--r--clippy_lints/src/min_ident_chars.rs5
-rw-r--r--clippy_lints/src/missing_enforced_import_rename.rs3
-rw-r--r--clippy_lints/src/redundant_pub_crate.rs17
-rw-r--r--clippy_lints/src/unused_trait_names.rs2
-rw-r--r--clippy_lints/src/wildcard_imports.rs4
-rw-r--r--clippy_utils/src/paths.rs11
9 files changed, 29 insertions, 26 deletions
diff --git a/clippy_lints/src/disallowed_types.rs b/clippy_lints/src/disallowed_types.rs
index d0b2f0c8407..821bb25d2ce 100644
--- a/clippy_lints/src/disallowed_types.rs
+++ b/clippy_lints/src/disallowed_types.rs
@@ -106,8 +106,8 @@ impl_lint_pass!(DisallowedTypes => [DISALLOWED_TYPES]);
 impl<'tcx> LateLintPass<'tcx> for DisallowedTypes {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
         if let ItemKind::Use(path, UseKind::Single(_)) = &item.kind {
-            for res in &path.res {
-                self.check_res_emit(cx, res, item.span);
+            if let Some(res) = path.res.type_ns {
+                self.check_res_emit(cx, &res, item.span);
             }
         }
     }
diff --git a/clippy_lints/src/legacy_numeric_constants.rs b/clippy_lints/src/legacy_numeric_constants.rs
index 01b49403cac..b3c63f022d3 100644
--- a/clippy_lints/src/legacy_numeric_constants.rs
+++ b/clippy_lints/src/legacy_numeric_constants.rs
@@ -51,7 +51,9 @@ impl<'tcx> LateLintPass<'tcx> for LegacyNumericConstants {
         // so lint on the `use` statement directly.
         if let ItemKind::Use(path, kind @ (UseKind::Single(_) | UseKind::Glob)) = item.kind
             && !item.span.in_external_macro(cx.sess().source_map())
-            && let Some(def_id) = path.res[0].opt_def_id()
+            // use `present_items` because it could be in either type_ns or value_ns
+            && let Some(res) = path.res.present_items().next()
+            && let Some(def_id) = res.opt_def_id()
             && self.msrv.meets(cx, msrvs::NUMERIC_ASSOCIATED_CONSTANTS)
         {
             let module = if is_integer_module(cx, def_id) {
diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs
index 98ad1f6a160..c1a26c5a9c7 100644
--- a/clippy_lints/src/macro_use.rs
+++ b/clippy_lints/src/macro_use.rs
@@ -100,10 +100,7 @@ impl LateLintPass<'_> for MacroUseImports {
             && let hir_id = item.hir_id()
             && let attrs = cx.tcx.hir_attrs(hir_id)
             && let Some(mac_attr) = attrs.iter().find(|attr| attr.has_name(sym::macro_use))
-            && let Some(id) = path.res.iter().find_map(|res| match res {
-                Res::Def(DefKind::Mod, id) => Some(id),
-                _ => None,
-            })
+            && let Some(Res::Def(DefKind::Mod, id)) = path.res.type_ns
             && !id.is_local()
         {
             for kid in cx.tcx.module_children(id) {
diff --git a/clippy_lints/src/min_ident_chars.rs b/clippy_lints/src/min_ident_chars.rs
index 00ea9bba0d1..99f01c8001a 100644
--- a/clippy_lints/src/min_ident_chars.rs
+++ b/clippy_lints/src/min_ident_chars.rs
@@ -131,8 +131,9 @@ impl Visitor<'_> for IdentVisitor<'_, '_> {
             // If however the identifier is different, this means it is an alias (`use foo::bar as baz`). In
             // this case, we need to emit the warning for `baz`.
             if let Some(imported_item_path) = usenode
-                && let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.first()
-                && cx.tcx.item_name(*imported_item_defid).as_str() == str
+                // use `present_items` because it could be in any of type_ns, value_ns, macro_ns
+                && let Some(Res::Def(_, imported_item_defid)) = imported_item_path.res.value_ns
+                && cx.tcx.item_name(imported_item_defid).as_str() == str
             {
                 return;
             }
diff --git a/clippy_lints/src/missing_enforced_import_rename.rs b/clippy_lints/src/missing_enforced_import_rename.rs
index a1e621cc9f6..eeea6dfd5f4 100644
--- a/clippy_lints/src/missing_enforced_import_rename.rs
+++ b/clippy_lints/src/missing_enforced_import_rename.rs
@@ -72,7 +72,8 @@ impl_lint_pass!(ImportRename => [MISSING_ENFORCED_IMPORT_RENAMES]);
 impl LateLintPass<'_> for ImportRename {
     fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
         if let ItemKind::Use(path, UseKind::Single(_)) = &item.kind {
-            for &res in &path.res {
+            // use `present_items` because it could be in any of type_ns, value_ns, macro_ns
+            for res in path.res.present_items() {
                 if let Res::Def(_, id) = res
                     && let Some(name) = self.renames.get(&id)
                     // Remove semicolon since it is not present for nested imports
diff --git a/clippy_lints/src/redundant_pub_crate.rs b/clippy_lints/src/redundant_pub_crate.rs
index 743bdb945ef..3828aff4164 100644
--- a/clippy_lints/src/redundant_pub_crate.rs
+++ b/clippy_lints/src/redundant_pub_crate.rs
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::HasSession;
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
-use rustc_hir::{Item, ItemKind};
+use rustc_hir::{Item, ItemKind, UseKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
 use rustc_session::impl_lint_pass;
@@ -49,7 +49,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
         if cx.tcx.visibility(item.owner_id.def_id) == ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id())
             && !cx.effective_visibilities.is_exported(item.owner_id.def_id)
             && self.is_exported.last() == Some(&false)
-            && !is_macro_export(item)
+            && !is_ignorable_export(item)
             && !item.span.in_external_macro(cx.sess().source_map())
         {
             let span = item
@@ -86,13 +86,12 @@ impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
     }
 }
 
-fn is_macro_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
-    if let ItemKind::Use(path, _) = item.kind {
-        if path
-            .res
-            .iter()
-            .all(|res| matches!(res, Res::Def(DefKind::Macro(MacroKind::Bang), _)))
-        {
+// We ignore macro exports. And `ListStem` uses, which aren't interesting.
+fn is_ignorable_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
+    if let ItemKind::Use(path, kind) = item.kind {
+        let ignore = matches!(path.res.macro_ns, Some(Res::Def(DefKind::Macro(MacroKind::Bang), _)))
+            || kind == UseKind::ListStem;
+        if ignore {
             return true;
         }
     } else if let ItemKind::Macro(..) = item.kind {
diff --git a/clippy_lints/src/unused_trait_names.rs b/clippy_lints/src/unused_trait_names.rs
index 14ac65cf4df..610cec7b8c8 100644
--- a/clippy_lints/src/unused_trait_names.rs
+++ b/clippy_lints/src/unused_trait_names.rs
@@ -64,7 +64,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedTraitNames {
             // Ignore imports that already use Underscore
             && ident.name != kw::Underscore
             // Only check traits
-            && let Some(Res::Def(DefKind::Trait, _)) = path.res.first()
+            && let Some(Res::Def(DefKind::Trait, _)) = path.res.type_ns
             && cx.tcx.maybe_unused_trait_imports(()).contains(&item.owner_id.def_id)
             // Only check this import if it is visible to its module only (no pub, pub(crate), ...)
             && let module = cx.tcx.parent_module_from_def_id(item.owner_id.def_id)
diff --git a/clippy_lints/src/wildcard_imports.rs b/clippy_lints/src/wildcard_imports.rs
index 45a5dbabeb4..467811c586b 100644
--- a/clippy_lints/src/wildcard_imports.rs
+++ b/clippy_lints/src/wildcard_imports.rs
@@ -169,8 +169,8 @@ impl LateLintPass<'_> for WildcardImports {
                 format!("{import_source_snippet}::{imports_string}")
             };
 
-            // Glob imports always have a single resolution.
-            let (lint, message) = if let Res::Def(DefKind::Enum, _) = use_path.res[0] {
+            // Glob imports always have a single resolution. Enums are in the value namespace.
+            let (lint, message) = if let Some(Res::Def(DefKind::Enum, _)) = use_path.res.value_ns {
                 (ENUM_GLOB_USE, "usage of wildcard import for enum variants")
             } else {
                 (WILDCARD_IMPORTS, "usage of wildcard import")
diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
index 9d7f3086b05..f37a609497e 100644
--- a/clippy_utils/src/paths.rs
+++ b/clippy_utils/src/paths.rs
@@ -306,10 +306,13 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
             let item = tcx.hir_item(item_id);
             if let ItemKind::Use(path, UseKind::Single(ident)) = item.kind {
                 if ident.name == name {
-                    path.res
-                        .iter()
-                        .find(|res| ns.matches(res.ns()))
-                        .and_then(Res::opt_def_id)
+                    let opt_def_id = |ns: Option<Res>| ns.and_then(|res| res.opt_def_id());
+                    match ns {
+                        PathNS::Type => opt_def_id(path.res.type_ns),
+                        PathNS::Value => opt_def_id(path.res.value_ns),
+                        PathNS::Macro => opt_def_id(path.res.macro_ns),
+                        PathNS::Arbitrary => unreachable!(),
+                    }
                 } else {
                     None
                 }