Refactor absolute_paths:

* Check the path length first
* Use `is_from_proc_macro`
* Use symbols instead of strings when checking crate names

Author: Jarcho

clippy_lints/src/absolute_paths.rs

@@ -1,12 +1,13 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::source::snippet_opt;
+use clippy_utils::is_from_proc_macro;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc_hir::{HirId, ItemKind, Node, Path};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::kw;
+use rustc_span::Symbol;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -23,6 +24,14 @@ declare_clippy_lint! {
     /// Note: One exception to this is code from macro expansion - this does not lint such cases, as
     /// using absolute paths is the proper way of referencing items in one.
     ///
+    /// ### Known issues
+    ///
+    /// There are currently a few cases which are not caught by this lint:
+    /// * Macro calls. e.g. `path::to::macro!()`
+    /// * Derive macros. e.g. `#[derive(path::to::macro)]`
+    /// * Attribute macros. e.g. `#[path::to::macro]`
+    /// * Traits in disambiguated paths. e.g. `<T as path::to::trait>::item`
+    ///
     /// ### Example
     /// ```no_run
     /// let x = std::f64::consts::PI;
@@ -47,54 +56,40 @@ impl_lint_pass!(AbsolutePaths => [ABSOLUTE_PATHS]);
 
 pub struct AbsolutePaths {
     pub absolute_paths_max_segments: u64,
-    pub absolute_paths_allowed_crates: FxHashSet<String>,
+    pub absolute_paths_allowed_crates: FxHashSet<Symbol>,
 }
 
-impl LateLintPass<'_> for AbsolutePaths {
+impl<'tcx> LateLintPass<'tcx> for AbsolutePaths {
     // We should only lint `QPath::Resolved`s, but since `Path` is only used in `Resolved` and `UsePath`
     // we don't need to use a visitor or anything as we can just check if the `Node` for `hir_id` isn't
     // a `Use`
-    #[expect(clippy::cast_possible_truncation)]
-    fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, hir_id: HirId) {
-        let Self {
-            absolute_paths_max_segments,
-            absolute_paths_allowed_crates,
-        } = self;
+    fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, hir_id: HirId) {
+        let (first, len) = match path.segments {
+            // Any path starting with `::` must have the second segment refer to an external crate.
+            // Make sure not to include the `::` segment when counting the segments.
+            [root, first, ..] if root.ident.name == kw::PathRoot => (first, path.segments.len() - 1),
+            [first, _, ..]
+                if first.ident.name == kw::Crate
+                    || matches!(first.res, Res::Def(DefKind::Mod, DefId { index, .. }) if index == CRATE_DEF_INDEX) =>
+            {
+                (first, path.segments.len())
+            },
+            _ => return,
+        };
 
-        if !path.span.from_expansion()
+        if len as u64 > self.absolute_paths_max_segments
+            && !path.span.from_expansion()
             && let node = cx.tcx.hir_node(hir_id)
-            && !matches!(node, Node::Item(item) if matches!(item.kind, ItemKind::Use(_, _)))
-            && let [first, rest @ ..] = path.segments
-            // Handle `::std`
-            && let (segment, len) = if first.ident.name == kw::PathRoot {
-                // Indexing is fine as `PathRoot` must be followed by another segment. `len() - 1`
-                // is fine here for the same reason
-                (&rest[0], path.segments.len() - 1)
-            } else {
-                (first, path.segments.len())
-            }
-            && len > *absolute_paths_max_segments as usize
-            && let Some(segment_snippet) = snippet_opt(cx, segment.ident.span)
-            && segment_snippet == segment.ident.as_str()
+            && !matches!(node, Node::Item(item) if matches!(item.kind, ItemKind::Use(..)))
+            && !self.absolute_paths_allowed_crates.contains(&first.ident.name)
+            && !is_from_proc_macro(cx, path)
         {
-            let is_abs_external =
-                matches!(segment.res, Res::Def(DefKind::Mod, DefId { index, .. }) if index == CRATE_DEF_INDEX);
-            let is_abs_crate = segment.ident.name == kw::Crate;
-
-            if is_abs_external && absolute_paths_allowed_crates.contains(segment.ident.name.as_str())
-                || is_abs_crate && absolute_paths_allowed_crates.contains("crate")
-            {
-                return;
-            }
-
-            if is_abs_external || is_abs_crate {
-                span_lint(
-                    cx,
-                    ABSOLUTE_PATHS,
-                    path.span,
-                    "consider bringing this path into scope with the `use` keyword",
-                );
-            }
+            span_lint(
+                cx,
+                ABSOLUTE_PATHS,
+                path.span,
+                "consider bringing this path into scope with the `use` keyword",
+            );
         }
     }
 }

clippy_lints/src/lib.rs

@@ -389,6 +389,7 @@ use clippy_config::{get_configuration_metadata, Conf};
 use clippy_utils::macros::FormatArgsStorage;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_lint::{Lint, LintId};
+use rustc_span::Symbol;
 use std::collections::BTreeMap;
 
 /// Register all pre expansion lints
@@ -1128,7 +1129,10 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| {
         Box::new(absolute_paths::AbsolutePaths {
             absolute_paths_max_segments,
-            absolute_paths_allowed_crates: absolute_paths_allowed_crates.clone(),
+            absolute_paths_allowed_crates: absolute_paths_allowed_crates
+                .iter()
+                .map(|x| Symbol::intern(x))
+                .collect(),
         })
     });
     store.register_late_pass(|_| Box::new(redundant_locals::RedundantLocals));