Skip to content

New lint [absolute_paths] #11003

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 -->
21 changes: 21 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)


2 changes: 1 addition & 1 deletion clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
100 changes: 100 additions & 0 deletions clippy_lints/src/absolute_paths.rs
Original file line number Diff line number Diff line change
@@ -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",
);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 9 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/redundant_static_lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 10 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/mir/possible_origin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)) |
Expand Down
3 changes: 2 additions & 1 deletion clippy_utils/src/usage.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
}
Expand Down
28 changes: 28 additions & 0 deletions tests/ui-toml/absolute_paths/absolute_paths.allow_crates.stderr
Original file line number Diff line number Diff line change
@@ -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

70 changes: 70 additions & 0 deletions tests/ui-toml/absolute_paths/absolute_paths.disallow_crates.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading