Auto merge of #11003 - Centri3:absolute_path, r=Alexendoo,blyxyas

New lint [`absolute_paths`]

Closes #10568

Maybe we should make the max segments allowed a configuration option? I see quite a bit of 3-segment paths in clippy, and while I think only really `<mod/type>::<item>` or `<item>` should be (usually) used but anything above may be too widespread 😕

PS, despite this being "max segments allowed" it only lints if it's absolute, as is the point of the lint, e.g., `std::io::ErrorKind::etc` is linted but `io::ErrorKind::NotFound::etc` isn't

changelog: New lint [`absolute_paths`]

CHANGELOG.md

@@ -4680,6 +4680,7 @@ Released 2018-09-13
 
 <!-- lint disable no-unused-definitions -->
 <!-- begin autogenerated links to lint list -->
+[`absolute_paths`]: https://rust-lang.github.io/rust-clippy/master/index.html#absolute_paths
 [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
 [`alloc_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#alloc_instead_of_core
 [`allow_attributes`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes
@@ -5467,4 +5468,6 @@ Released 2018-09-13
 [`accept-comment-above-statement`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-statement
 [`accept-comment-above-attributes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-attributes
 [`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
+[`absolute-paths-max-segments`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-max-segments
+[`absolute-paths-allowed-crates`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-allowed-crates
 <!-- end autogenerated links to configuration documentation -->

book/src/lint_configuration.md

@@ -730,3 +730,24 @@ Whether to allow `r#""#` when `r""` can be used
 * [`unnecessary_raw_string_hashes`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_raw_string_hashes)
 
 
+## `absolute-paths-max-segments`
+The maximum number of segments a path can have before being linted, anything above this will
+be linted.
+
+**Default Value:** `2` (`u64`)
+
+---
+**Affected lints:**
+* [`absolute_paths`](https://rust-lang.github.io/rust-clippy/master/index.html#absolute_paths)
+
+
+## `absolute-paths-allowed-crates`
+Which crates to allow absolute paths from
+
+**Default Value:** `{}` (`rustc_data_structures::fx::FxHashSet<String>`)
+
+---
+**Affected lints:**
+* [`absolute_paths`](https://rust-lang.github.io/rust-clippy/master/index.html#absolute_paths)
+
+

clippy_dev/src/lib.rs

@@ -51,7 +51,7 @@ pub fn clippy_project_root() -> PathBuf {
     for path in current_dir.ancestors() {
         let result = std::fs::read_to_string(path.join("Cargo.toml"));
         if let Err(err) = &result {
-            if err.kind() == std::io::ErrorKind::NotFound {
+            if err.kind() == io::ErrorKind::NotFound {
                 continue;
             }
         }

clippy_lints/src/absolute_paths.rs

@@ -0,0 +1,100 @@
+use clippy_utils::diagnostics::span_lint;
+use clippy_utils::source::snippet_opt;
+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::{declare_tool_lint, impl_lint_pass};
+use rustc_span::symbol::kw;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of items through absolute paths, like `std::env::current_dir`.
+    ///
+    /// ### Why is this bad?
+    /// Many codebases have their own style when it comes to importing, but one that is seldom used
+    /// is using absolute paths *everywhere*. This is generally considered unidiomatic, and you
+    /// should add a `use` statement.
+    ///
+    /// The default maximum segments (2) is pretty strict, you may want to increase this in
+    /// `clippy.toml`.
+    ///
+    /// 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.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let x = std::f64::consts::PI;
+    /// ```
+    /// Use any of the below instead, or anything else:
+    /// ```rust
+    /// use std::f64;
+    /// use std::f64::consts;
+    /// use std::f64::consts::PI;
+    /// let x = f64::consts::PI;
+    /// let x = consts::PI;
+    /// let x = PI;
+    /// use std::f64::consts as f64_consts;
+    /// let x = f64_consts::PI;
+    /// ```
+    #[clippy::version = "1.73.0"]
+    pub ABSOLUTE_PATHS,
+    restriction,
+    "checks for usage of an item without a `use` statement"
+}
+impl_lint_pass!(AbsolutePaths => [ABSOLUTE_PATHS]);
+
+pub struct AbsolutePaths {
+    pub absolute_paths_max_segments: u64,
+    pub absolute_paths_allowed_crates: FxHashSet<String>,
+}
+
+impl LateLintPass<'_> 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;
+
+        if !path.span.from_expansion()
+            && let Some(node) = cx.tcx.hir().find(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()
+        {
+            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",
+                );
+            }
+        }
+    }
+}

clippy_lints/src/declared_lints.rs

@@ -37,6 +37,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::utils::internal_lints::produce_ice::PRODUCE_ICE_INFO,
     #[cfg(feature = "internal")]
     crate::utils::internal_lints::unnecessary_def_path::UNNECESSARY_DEF_PATH_INFO,
+    crate::absolute_paths::ABSOLUTE_PATHS_INFO,
     crate::allow_attributes::ALLOW_ATTRIBUTES_INFO,
     crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
     crate::approx_const::APPROX_CONSTANT_INFO,

clippy_lints/src/lib.rs

@@ -65,6 +65,7 @@ mod declared_lints;
 mod renamed_lints;
 
 // begin lints modules, do not remove this comment, it’s used in `update_lints`
+mod absolute_paths;
 mod allow_attributes;
 mod almost_complete_range;
 mod approx_const;
@@ -1082,6 +1083,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(manual_float_methods::ManualFloatMethods));
     store.register_late_pass(|_| Box::new(four_forward_slashes::FourForwardSlashes));
     store.register_late_pass(|_| Box::new(error_impl_error::ErrorImplError));
+    let absolute_paths_max_segments = conf.absolute_paths_max_segments;
+    let absolute_paths_allowed_crates = conf.absolute_paths_allowed_crates.clone();
+    store.register_late_pass(move |_| {
+        Box::new(absolute_paths::AbsolutePaths {
+            absolute_paths_max_segments,
+            absolute_paths_allowed_crates: absolute_paths_allowed_crates.clone(),
+        })
+    });
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/lifetimes.rs

@@ -15,6 +15,7 @@ use rustc_hir::{
     PredicateOrigin, TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WherePredicate,
 };
 use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::hir::map::Map;
 use rustc_middle::hir::nested_filter as middle_nested_filter;
 use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -620,7 +621,7 @@ impl<'cx, 'tcx, F> Visitor<'tcx> for LifetimeChecker<'cx, 'tcx, F>
 where
     F: NestedFilter<'tcx>,
 {
-    type Map = rustc_middle::hir::map::Map<'tcx>;
+    type Map = Map<'tcx>;
     type NestedFilter = F;
 
     // for lifetimes as parameters of generics

clippy_lints/src/redundant_static_lifetimes.rs

@@ -5,6 +5,7 @@ use rustc_ast::ast::{ConstItem, Item, ItemKind, StaticItem, Ty, TyKind};
 use rustc_errors::Applicability;
 use rustc_lint::{EarlyContext, EarlyLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::symbol::kw;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -64,7 +65,7 @@ impl RedundantStaticLifetimes {
                 if let Some(lifetime) = *optional_lifetime {
                     match borrow_type.ty.kind {
                         TyKind::Path(..) | TyKind::Slice(..) | TyKind::Array(..) | TyKind::Tup(..) => {
-                            if lifetime.ident.name == rustc_span::symbol::kw::StaticLifetime {
+                            if lifetime.ident.name == kw::StaticLifetime {
                                 let snip = snippet(cx, borrow_type.ty.span, "<type>");
                                 let sugg = format!("&{}{snip}", borrow_type.mutbl.prefix_str());
                                 span_lint_and_then(

clippy_lints/src/utils/conf.rs

@@ -551,6 +551,16 @@ define_Conf! {
     ///
     /// Whether to allow `r#""#` when `r""` can be used
     (allow_one_hash_in_raw_strings: bool = false),
+    /// Lint: ABSOLUTE_PATHS.
+    ///
+    /// The maximum number of segments a path can have before being linted, anything above this will
+    /// be linted.
+    (absolute_paths_max_segments: u64 = 2),
+    /// Lint: ABSOLUTE_PATHS.
+    ///
+    /// Which crates to allow absolute paths from
+    (absolute_paths_allowed_crates: rustc_data_structures::fx::FxHashSet<String> =
+        rustc_data_structures::fx::FxHashSet::default()),
 }
 
 /// Search for the configuration file.

clippy_utils/src/mir/possible_origin.rs

@@ -44,7 +44,7 @@ impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleOriginVisitor<'a, 'tcx> {
         let lhs = place.local;
         match rvalue {
             // Only consider `&mut`, which can modify origin place
-            mir::Rvalue::Ref(_, rustc_middle::mir::BorrowKind::Mut { .. }, borrowed) |
+            mir::Rvalue::Ref(_, mir::BorrowKind::Mut { .. }, borrowed) |
             // _2: &mut _;
             // _3 = move _2
             mir::Rvalue::Use(mir::Operand::Move(borrowed))  |

clippy_utils/src/usage.rs

@@ -1,5 +1,6 @@
 use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend};
 use core::ops::ControlFlow;
+use hir::def::Res;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{Expr, ExprKind, HirId, HirIdSet, Node};
 use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
@@ -127,7 +128,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for BindingUsageFinder<'a, 'tcx> {
     }
 
     fn visit_path(&mut self, path: &hir::Path<'tcx>, _: hir::HirId) {
-        if let hir::def::Res::Local(id) = path.res {
+        if let Res::Local(id) = path.res {
             if self.binding_ids.contains(&id) {
                 self.usage_found = true;
             }

tests/ui-toml/absolute_paths/absolute_paths.allow_crates.stderr

@@ -0,0 +1,28 @@
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:40:5
+   |
+LL |     std::f32::MAX;
+   |     ^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::absolute-paths` implied by `-D warnings`
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:41:5
+   |
+LL |     core::f32::MAX;
+   |     ^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:42:5
+   |
+LL |     ::core::f32::MAX;
+   |     ^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:58:5
+   |
+LL |     ::std::f32::MAX;
+   |     ^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+

tests/ui-toml/absolute_paths/absolute_paths.disallow_crates.stderr

@@ -0,0 +1,70 @@
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:40:5
+   |
+LL |     std::f32::MAX;
+   |     ^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::absolute-paths` implied by `-D warnings`
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:41:5
+   |
+LL |     core::f32::MAX;
+   |     ^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:42:5
+   |
+LL |     ::core::f32::MAX;
+   |     ^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:43:5
+   |
+LL |     crate::a::b::c::C;
+   |     ^^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:44:5
+   |
+LL |     crate::a::b::c::d::e::f::F;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:45:5
+   |
+LL |     crate::a::A;
+   |     ^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:46:5
+   |
+LL |     crate::a::b::B;
+   |     ^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:47:5
+   |
+LL |     crate::a::b::c::C::ZERO;
+   |     ^^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:48:5
+   |
+LL |     helper::b::c::d::e::f();
+   |     ^^^^^^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:49:5
+   |
+LL |     ::helper::b::c::d::e::f();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^
+
+error: consider bringing this path into scope with the `use` keyword
+  --> $DIR/absolute_paths.rs:58:5
+   |
+LL |     ::std::f32::MAX;
+   |     ^^^^^^^^^^^^^^^
+
+error: aborting due to 11 previous errors
+

tests/ui-toml/absolute_paths/absolute_paths.rs

@@ -0,0 +1,97 @@
+//@aux-build:../../ui/auxiliary/proc_macros.rs:proc-macro
+//@aux-build:helper.rs
+//@revisions: allow_crates disallow_crates
+//@[allow_crates] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/absolute_paths/allow_crates
+//@[disallow_crates] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/absolute_paths/disallow_crates
+#![allow(clippy::no_effect, unused)]
+#![warn(clippy::absolute_paths)]
+#![feature(decl_macro)]
+
+extern crate helper;
+#[macro_use]
+extern crate proc_macros;
+
+pub mod a {
+    pub mod b {
+        pub mod c {
+            pub struct C;
+
+            impl C {
+                pub const ZERO: u32 = 0;
+            }
+
+            pub mod d {
+                pub mod e {
+                    pub mod f {
+                        pub struct F;
+                    }
+                }
+            }
+        }
+
+        pub struct B;
+    }
+
+    pub struct A;
+}
+
+fn main() {
+    f32::max(1.0, 2.0);
+    std::f32::MAX;
+    core::f32::MAX;
+    ::core::f32::MAX;
+    crate::a::b::c::C;
+    crate::a::b::c::d::e::f::F;
+    crate::a::A;
+    crate::a::b::B;
+    crate::a::b::c::C::ZERO;
+    helper::b::c::d::e::f();
+    ::helper::b::c::d::e::f();
+    fn b() -> a::b::B {
+        todo!()
+    }
+    std::println!("a");
+    let x = 1;
+    std::ptr::addr_of!(x);
+    // Test we handle max segments with `PathRoot` properly; this has 4 segments but we should say it
+    // has 3
+    ::std::f32::MAX;
+    // Do not lint due to the above
+    ::helper::a();
+    // Do not lint
+    helper::a();
+    use crate::a::b::c::C;
+    use a::b;
+    use std::f32::MAX;
+    a::b::c::d::e::f::F;
+    b::c::C;
+    fn a() -> a::A {
+        todo!()
+    }
+    use a::b::c;
+
+    fn c() -> c::C {
+        todo!()
+    }
+    fn d() -> Result<(), ()> {
+        todo!()
+    }
+    external! {
+        crate::a::b::c::C::ZERO;
+    }
+    // For some reason, `path.span.from_expansion()` takes care of this for us
+    with_span! {
+        span
+        crate::a::b::c::C::ZERO;
+    }
+    macro_rules! local_crate {
+        () => {
+            crate::a::b::c::C::ZERO;
+        };
+    }
+    macro local_crate_2_0() {
+        crate::a::b::c::C::ZERO;
+    }
+    local_crate!();
+    local_crate_2_0!();
+}

tests/ui-toml/absolute_paths/allow_crates/clippy.toml

@@ -0,0 +1,2 @@
+absolute-paths-max-segments = 2
+absolute-paths-allowed-crates = ["crate", "helper"]

tests/ui-toml/absolute_paths/auxiliary/helper.rs

@@ -0,0 +1,11 @@
+pub fn a() {}
+
+pub mod b {
+    pub mod c {
+        pub mod d {
+            pub mod e {
+                pub fn f() {}
+            }
+        }
+    }
+}

tests/ui-toml/absolute_paths/disallow_crates/clippy.toml

@@ -0,0 +1 @@
+absolute-paths-max-segments = 2

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -1,4 +1,6 @@
 error: error reading Clippy's configuration file: unknown field `foobar`, expected one of
+           absolute-paths-allowed-crates
+           absolute-paths-max-segments
            accept-comment-above-attributes
            accept-comment-above-statement
            allow-dbg-in-tests
@@ -68,6 +70,8 @@ LL | foobar = 42
    | ^^^^^^
 
 error: error reading Clippy's configuration file: unknown field `barfoo`, expected one of
+           absolute-paths-allowed-crates
+           absolute-paths-max-segments
            accept-comment-above-attributes
            accept-comment-above-statement
            allow-dbg-in-tests