Skip to content

Commit 12c600c

Browse files
committed
new lint absolute_symbol_paths
1 parent 9fa4089 commit 12c600c

34 files changed

+347
-138
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4620,6 +4620,7 @@ Released 2018-09-13
46204620

46214621
<!-- lint disable no-unused-definitions -->
46224622
<!-- begin autogenerated links to lint list -->
4623+
[`absolute_symbol_paths`]: https://rust-lang.github.io/rust-clippy/master/index.html#absolute_symbol_paths
46234624
[`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
46244625
[`alloc_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#alloc_instead_of_core
46254626
[`allow_attributes`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes

clippy_dev/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ pub fn clippy_project_root() -> PathBuf {
5252
for path in current_dir.ancestors() {
5353
let result = std::fs::read_to_string(path.join("Cargo.toml"));
5454
if let Err(err) = &result {
55-
if err.kind() == std::io::ErrorKind::NotFound {
55+
if err.kind() == io::ErrorKind::NotFound {
5656
continue;
5757
}
5858
}
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::is_from_proc_macro;
3+
use rustc_hir::intravisit::{walk_qpath, Visitor};
4+
use rustc_hir::{HirId, QPath};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::hir::nested_filter::OnlyBodies;
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::Span;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Checks for usage of symbols through absolute paths, like `std::env::current_dir`.
13+
///
14+
/// ### Why is this bad?
15+
/// Many codebases have their own style when it comes to symbol importing, but one that is
16+
/// seldom used is using absolute paths *everywhere*. This is generally considered unidiomatic,
17+
/// and you should add a `use` statement.
18+
///
19+
/// Note: One exception to this is code from macro expansion - this does not lint such cases, as
20+
/// using absolute paths is the proper way of referencing symbols in one.
21+
///
22+
/// ### Example
23+
/// ```rust
24+
/// let x = std::f64::consts::PI;
25+
/// ```
26+
/// Use any of the below instead, or anything else:
27+
/// ```rust
28+
/// # use std::f64;
29+
/// # use std::f64::consts;
30+
/// # use std::f64::consts::PI;
31+
/// let x = f64::consts::PI;
32+
/// let x = consts::PI;
33+
/// let x = PI;
34+
/// use std::f64::consts as f64_consts;
35+
/// let x = f64_consts::PI;
36+
/// ```
37+
#[clippy::version = "1.72.0"]
38+
pub ABSOLUTE_SYMBOL_PATHS,
39+
style,
40+
"checks for usage of a symbol without a `use` statement"
41+
}
42+
declare_lint_pass!(AbsoluteSymbolPaths => [ABSOLUTE_SYMBOL_PATHS]);
43+
44+
impl LateLintPass<'_> for AbsoluteSymbolPaths {
45+
fn check_crate(&mut self, cx: &LateContext<'_>) {
46+
cx.tcx.hir().visit_all_item_likes_in_crate(&mut V { cx });
47+
}
48+
}
49+
50+
struct V<'a, 'tcx> {
51+
cx: &'a LateContext<'tcx>,
52+
}
53+
54+
impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
55+
type NestedFilter = OnlyBodies;
56+
57+
fn nested_visit_map(&mut self) -> Self::Map {
58+
self.cx.tcx.hir()
59+
}
60+
61+
fn visit_qpath(&mut self, qpath: &'tcx QPath<'tcx>, hir_id: HirId, span: Span) {
62+
let Self { cx } = *self;
63+
64+
if !span.from_expansion()
65+
&& let QPath::Resolved(_, path) = qpath
66+
&& let Some(def_id) = path.res.opt_def_id()
67+
{
68+
let def_path = cx.tcx.def_path_str(def_id);
69+
let segments = def_path.split("::");
70+
71+
// FIXME: I only allowed 3 segment paths because there's tons of them in clippy :D
72+
// To my humble reviewer: thoughts?
73+
if path.segments.len() >= 4 // A 2 segment path seems okay, like `std::println!`
74+
&& segments.eq(path.segments.iter().map(|segment| segment.ident.name.as_str()))
75+
&& !is_from_proc_macro(cx, qpath)
76+
{
77+
span_lint(
78+
cx,
79+
ABSOLUTE_SYMBOL_PATHS,
80+
span,
81+
"consider referring to this symbol by adding a `use` statement for consistent formatting",
82+
);
83+
}
84+
}
85+
86+
walk_qpath(self, qpath, hir_id);
87+
}
88+
}

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
3737
crate::utils::internal_lints::produce_ice::PRODUCE_ICE_INFO,
3838
#[cfg(feature = "internal")]
3939
crate::utils::internal_lints::unnecessary_def_path::UNNECESSARY_DEF_PATH_INFO,
40+
crate::absolute_symbol_paths::ABSOLUTE_SYMBOL_PATHS_INFO,
4041
crate::allow_attributes::ALLOW_ATTRIBUTES_INFO,
4142
crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
4243
crate::approx_const::APPROX_CONSTANT_INFO,

clippy_lints/src/lib.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@
1010
#![feature(stmt_expr_attributes)]
1111
#![recursion_limit = "512"]
1212
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
13-
#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)]
13+
#![allow(
14+
clippy::absolute_symbol_paths,
15+
clippy::missing_docs_in_private_items,
16+
clippy::must_use_candidate
17+
)]
1418
#![warn(trivial_casts, trivial_numeric_casts)]
1519
// warn on lints, that are included in `rust-lang/rust`s bootstrap
1620
#![warn(rust_2018_idioms, unused_lifetimes)]
@@ -65,6 +69,7 @@ mod declared_lints;
6569
mod renamed_lints;
6670

6771
// begin lints modules, do not remove this comment, it’s used in `update_lints`
72+
mod absolute_symbol_paths;
6873
mod allow_attributes;
6974
mod almost_complete_range;
7075
mod approx_const;
@@ -1063,6 +1068,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10631068
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
10641069
})
10651070
});
1071+
store.register_late_pass(|_| Box::new(absolute_symbol_paths::AbsoluteSymbolPaths));
10661072
// add lints here, do not remove this comment, it's used in `new_lint`
10671073
}
10681074

clippy_lints/src/lifetimes.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use rustc_hir::{
1515
PredicateOrigin, TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WherePredicate,
1616
};
1717
use rustc_lint::{LateContext, LateLintPass, LintContext};
18+
use rustc_middle::hir::map::Map;
1819
use rustc_middle::hir::nested_filter as middle_nested_filter;
1920
use rustc_middle::lint::in_external_macro;
2021
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -620,7 +621,7 @@ impl<'cx, 'tcx, F> Visitor<'tcx> for LifetimeChecker<'cx, 'tcx, F>
620621
where
621622
F: NestedFilter<'tcx>,
622623
{
623-
type Map = rustc_middle::hir::map::Map<'tcx>;
624+
type Map = Map<'tcx>;
624625
type NestedFilter = F;
625626

626627
// for lifetimes as parameters of generics

clippy_lints/src/redundant_static_lifetimes.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use rustc_ast::ast::{ConstItem, Item, ItemKind, StaticItem, Ty, TyKind};
55
use rustc_errors::Applicability;
66
use rustc_lint::{EarlyContext, EarlyLintPass};
77
use rustc_session::{declare_tool_lint, impl_lint_pass};
8+
use rustc_span::symbol::kw;
89

910
declare_clippy_lint! {
1011
/// ### What it does
@@ -64,7 +65,7 @@ impl RedundantStaticLifetimes {
6465
if let Some(lifetime) = *optional_lifetime {
6566
match borrow_type.ty.kind {
6667
TyKind::Path(..) | TyKind::Slice(..) | TyKind::Array(..) | TyKind::Tup(..) => {
67-
if lifetime.ident.name == rustc_span::symbol::kw::StaticLifetime {
68+
if lifetime.ident.name == kw::StaticLifetime {
6869
let snip = snippet(cx, borrow_type.ty.span, "<type>");
6970
let sugg = format!("&{}{snip}", borrow_type.mutbl.prefix_str());
7071
span_lint_and_then(

clippy_utils/src/check_proc_macro.rs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
304304
},
305305
AttrKind::DocComment(_kind @ CommentKind::Line, ..) => {
306306
if matches!(attr.style, AttrStyle::Outer) {
307-
(Pat::Str("///"), Pat::Str(""))
307+
(Pat::Str("///*"), Pat::Str(""))
308308
} else {
309309
(Pat::Str("//!"), Pat::Str(""))
310310
}
@@ -354,7 +354,7 @@ pub trait WithSearchPat<'cx> {
354354
fn span(&self) -> Span;
355355
}
356356
macro_rules! impl_with_search_pat {
357-
($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))?) => {
357+
($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))? && $span_method:ident$($par:tt)?) => {
358358
impl<'cx> WithSearchPat<'cx> for $ty<'cx> {
359359
type Context = $cx<'cx>;
360360
#[allow(unused_variables)]
@@ -363,18 +363,19 @@ macro_rules! impl_with_search_pat {
363363
$fn($($tcx,)? self)
364364
}
365365
fn span(&self) -> Span {
366-
self.span
366+
self.$span_method$($par)?
367367
}
368368
}
369369
};
370370
}
371-
impl_with_search_pat!(LateContext: Expr with expr_search_pat(tcx));
372-
impl_with_search_pat!(LateContext: Item with item_search_pat);
373-
impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat);
374-
impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat);
375-
impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat);
376-
impl_with_search_pat!(LateContext: Variant with variant_search_pat);
377-
impl_with_search_pat!(LateContext: Ty with ty_search_pat);
371+
impl_with_search_pat!(LateContext: Expr with expr_search_pat(tcx) && span);
372+
impl_with_search_pat!(LateContext: Item with item_search_pat && span);
373+
impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat && span);
374+
impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat && span);
375+
impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat && span);
376+
impl_with_search_pat!(LateContext: Variant with variant_search_pat && span);
377+
impl_with_search_pat!(LateContext: Ty with ty_search_pat && span);
378+
impl_with_search_pat!(LateContext: QPath with qpath_search_pat && span());
378379

379380
impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) {
380381
type Context = LateContext<'cx>;

clippy_utils/src/mir/possible_origin.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleOriginVisitor<'a, 'tcx> {
4444
let lhs = place.local;
4545
match rvalue {
4646
// Only consider `&mut`, which can modify origin place
47-
mir::Rvalue::Ref(_, rustc_middle::mir::BorrowKind::Mut { .. }, borrowed) |
47+
mir::Rvalue::Ref(_, mir::BorrowKind::Mut { .. }, borrowed) |
4848
// _2: &mut _;
4949
// _3 = move _2
5050
mir::Rvalue::Use(mir::Operand::Move(borrowed)) |

tests/ui/absolute_symbol_paths.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
//@aux-build:proc_macros.rs
2+
#![allow(clippy::no_effect, unused)]
3+
#![warn(clippy::absolute_symbol_paths)]
4+
#![feature(decl_macro)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
pub mod a {
10+
pub mod b {
11+
pub mod c {
12+
pub struct C;
13+
14+
impl C {
15+
pub const ZERO: u32 = 0;
16+
}
17+
}
18+
19+
pub struct B;
20+
}
21+
22+
pub struct A;
23+
}
24+
25+
fn main() {
26+
f32::max(1.0, 2.0);
27+
std::f32::MAX;
28+
core::f32::MAX;
29+
a::b::c::C;
30+
a::A;
31+
a::b::B;
32+
a::b::c::C::ZERO;
33+
fn b() -> a::b::B {
34+
todo!()
35+
}
36+
// Does not lint
37+
use a::b;
38+
use a::b::c::C;
39+
b::c::C;
40+
fn a() -> a::A {
41+
todo!()
42+
}
43+
use a::b::c;
44+
45+
fn c() -> c::C {
46+
todo!()
47+
}
48+
fn d() -> Result<(), ()> {
49+
todo!()
50+
}
51+
external! {
52+
a::b::c::C::ZERO;
53+
}
54+
with_span! {
55+
span
56+
a::b::c::C::ZERO;
57+
}
58+
macro_rules! local_crate {
59+
() => {
60+
a::b::c::C::ZERO;
61+
};
62+
}
63+
macro local_crate_2_0() {
64+
a::b::c::C::ZERO;
65+
}
66+
local_crate!();
67+
local_crate_2_0!();
68+
// Macros are unfortunately tricky, as these are defined in `std::macros` but we have to access
69+
// them from `std::println!`, both due to privacy and because they're not macros 2.0
70+
std::println!("a");
71+
let x = 1;
72+
std::ptr::addr_of!(x);
73+
}

0 commit comments

Comments
 (0)