Skip to content

Commit

Permalink
new lint absolute_symbol_paths
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 22, 2023
1 parent 9fa4089 commit 69d4cb3
Show file tree
Hide file tree
Showing 57 changed files with 480 additions and 235 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4620,6 +4620,7 @@ Released 2018-09-13

<!-- lint disable no-unused-definitions -->
<!-- begin autogenerated links to lint list -->
[`absolute_symbol_paths`]: https://rust-lang.github.io/rust-clippy/master/index.html#absolute_symbol_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
2 changes: 1 addition & 1 deletion clippy_dev/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,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
88 changes: 88 additions & 0 deletions clippy_lints/src/absolute_symbol_paths.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::is_from_proc_macro;
use rustc_hir::intravisit::{walk_qpath, Visitor};
use rustc_hir::{HirId, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::nested_filter::OnlyBodies;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of symbols through absolute paths, like `std::env::current_dir`.
///
/// ### Why is this bad?
/// Many codebases have their own style when it comes to symbol importing, but one that is
/// seldom used is using absolute paths *everywhere*. This is generally considered unidiomatic,
/// and you should add a `use` statement.
///
/// 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 symbols 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.72.0"]
pub ABSOLUTE_SYMBOL_PATHS,
style,
"checks for usage of a symbol without a `use` statement"
}
declare_lint_pass!(AbsoluteSymbolPaths => [ABSOLUTE_SYMBOL_PATHS]);

impl LateLintPass<'_> for AbsoluteSymbolPaths {
fn check_crate(&mut self, cx: &LateContext<'_>) {
cx.tcx.hir().visit_all_item_likes_in_crate(&mut V { cx });
}
}

struct V<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
}

impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
type NestedFilter = OnlyBodies;

fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}

fn visit_qpath(&mut self, qpath: &'tcx QPath<'tcx>, hir_id: HirId, span: Span) {
let Self { cx } = *self;

if !span.from_expansion()
&& let QPath::Resolved(_, path) = qpath
&& let Some(def_id) = path.res.opt_def_id()
{
let def_path = cx.tcx.def_path_str(def_id);
let segments = def_path.split("::");

// FIXME: I only allowed 3 segment paths because there's tons of them in clippy :D
// To my humble reviewer: thoughts?
if path.segments.len() >= 4 // A 2 segment path seems okay, like `std::println!`
&& segments.eq(path.segments.iter().map(|segment| segment.ident.name.as_str()))
&& !is_from_proc_macro(cx, qpath)
{
span_lint(
cx,
ABSOLUTE_SYMBOL_PATHS,
span,
"consider referring to this symbol by adding a `use` statement for consistent formatting",
);
}
}

walk_qpath(self, qpath, hir_id);
}
}
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_symbol_paths::ABSOLUTE_SYMBOL_PATHS_INFO,
crate::allow_attributes::ALLOW_ATTRIBUTES_INFO,
crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
crate::approx_const::APPROX_CONSTANT_INFO,
Expand Down
8 changes: 7 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
#![feature(stmt_expr_attributes)]
#![recursion_limit = "512"]
#![cfg_attr(feature = "deny-warnings", deny(warnings))]
#![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)]
#![allow(
clippy::absolute_symbol_paths,
clippy::missing_docs_in_private_items,
clippy::must_use_candidate
)]
#![warn(trivial_casts, trivial_numeric_casts)]
// warn on lints, that are included in `rust-lang/rust`s bootstrap
#![warn(rust_2018_idioms, unused_lifetimes)]
Expand Down Expand Up @@ -65,6 +69,7 @@ mod declared_lints;
mod renamed_lints;

// begin lints modules, do not remove this comment, it’s used in `update_lints`
mod absolute_symbol_paths;
mod allow_attributes;
mod almost_complete_range;
mod approx_const;
Expand Down Expand Up @@ -1063,6 +1068,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
})
});
store.register_late_pass(|_| Box::new(absolute_symbol_paths::AbsoluteSymbolPaths));
// 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
21 changes: 11 additions & 10 deletions clippy_utils/src/check_proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
},
AttrKind::DocComment(_kind @ CommentKind::Line, ..) => {
if matches!(attr.style, AttrStyle::Outer) {
(Pat::Str("///"), Pat::Str(""))
(Pat::Str("///*"), Pat::Str(""))
} else {
(Pat::Str("//!"), Pat::Str(""))
}
Expand Down Expand Up @@ -354,7 +354,7 @@ pub trait WithSearchPat<'cx> {
fn span(&self) -> Span;
}
macro_rules! impl_with_search_pat {
($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))?) => {
($cx:ident: $ty:ident with $fn:ident $(($tcx:ident))? && $span_method:ident$($par:tt)?) => {
impl<'cx> WithSearchPat<'cx> for $ty<'cx> {
type Context = $cx<'cx>;
#[allow(unused_variables)]
Expand All @@ -363,18 +363,19 @@ macro_rules! impl_with_search_pat {
$fn($($tcx,)? self)
}
fn span(&self) -> Span {
self.span
self.$span_method$($par)?
}
}
};
}
impl_with_search_pat!(LateContext: Expr with expr_search_pat(tcx));
impl_with_search_pat!(LateContext: Item with item_search_pat);
impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat);
impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat);
impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat);
impl_with_search_pat!(LateContext: Variant with variant_search_pat);
impl_with_search_pat!(LateContext: Ty with ty_search_pat);
impl_with_search_pat!(LateContext: Expr with expr_search_pat(tcx) && span);
impl_with_search_pat!(LateContext: Item with item_search_pat && span);
impl_with_search_pat!(LateContext: TraitItem with trait_item_search_pat && span);
impl_with_search_pat!(LateContext: ImplItem with impl_item_search_pat && span);
impl_with_search_pat!(LateContext: FieldDef with field_def_search_pat && span);
impl_with_search_pat!(LateContext: Variant with variant_search_pat && span);
impl_with_search_pat!(LateContext: Ty with ty_search_pat && span);
impl_with_search_pat!(LateContext: QPath with qpath_search_pat && span());

impl<'cx> WithSearchPat<'cx> for (&FnKind<'cx>, &Body<'cx>, HirId, Span) {
type Context = LateContext<'cx>;
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
1 change: 1 addition & 0 deletions tests/ui-cargo/module_style/fail_mod/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(clippy::absolute_symbol_paths)]
#![warn(clippy::self_named_module_files)]

mod bad;
Expand Down
6 changes: 5 additions & 1 deletion tests/ui-internal/interning_defined_symbol.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
//@run-rustfix
#![deny(clippy::internal)]
#![allow(clippy::missing_clippy_version_attribute, clippy::let_unit_value)]
#![allow(
clippy::absolute_symbol_paths,
clippy::missing_clippy_version_attribute,
clippy::let_unit_value
)]
#![feature(rustc_private)]

extern crate rustc_span;
Expand Down
6 changes: 5 additions & 1 deletion tests/ui-internal/interning_defined_symbol.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
//@run-rustfix
#![deny(clippy::internal)]
#![allow(clippy::missing_clippy_version_attribute, clippy::let_unit_value)]
#![allow(
clippy::absolute_symbol_paths,
clippy::missing_clippy_version_attribute,
clippy::let_unit_value
)]
#![feature(rustc_private)]

extern crate rustc_span;
Expand Down
8 changes: 4 additions & 4 deletions tests/ui-internal/interning_defined_symbol.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:18:13
--> $DIR/interning_defined_symbol.rs:22:13
|
LL | let _ = Symbol::intern("f32");
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::f32`
Expand All @@ -12,19 +12,19 @@ LL | #![deny(clippy::internal)]
= note: `#[deny(clippy::interning_defined_symbol)]` implied by `#[deny(clippy::internal)]`

error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:21:13
--> $DIR/interning_defined_symbol.rs:25:13
|
LL | let _ = sym!(f32);
| ^^^^^^^^^ help: try: `rustc_span::sym::f32`

error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:24:13
--> $DIR/interning_defined_symbol.rs:28:13
|
LL | let _ = Symbol::intern("proc-macro");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::sym::proc_dash_macro`

error: interning a defined symbol
--> $DIR/interning_defined_symbol.rs:27:13
--> $DIR/interning_defined_symbol.rs:31:13
|
LL | let _ = Symbol::intern("self");
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `rustc_span::symbol::kw::SelfLower`
Expand Down
1 change: 1 addition & 0 deletions tests/ui-internal/unnecessary_def_path.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@run-rustfix
//@aux-build:paths.rs
#![allow(clippy::absolute_symbol_paths)]
#![deny(clippy::internal)]
#![feature(rustc_private)]

Expand Down
1 change: 1 addition & 0 deletions tests/ui-internal/unnecessary_def_path.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@run-rustfix
//@aux-build:paths.rs
#![allow(clippy::absolute_symbol_paths)]
#![deny(clippy::internal)]
#![feature(rustc_private)]

Expand Down
Loading

0 comments on commit 69d4cb3

Please sign in to comment.