Skip to content

Commit

Permalink
add new lint [once_cell_lazy] to detect usage of static Lazy type…
Browse files Browse the repository at this point in the history
… from `once_cell`
  • Loading branch information
J-ZhengLi committed Jun 21, 2024
1 parent 3e84ca8 commit 7fba7f9
Show file tree
Hide file tree
Showing 13 changed files with 631 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5677,6 +5677,7 @@ Released 2018-09-13
[`obfuscated_if_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else
[`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
[`once_cell_lazy`]: https://rust-lang.github.io/rust-clippy/master/index.html#once_cell_lazy
[`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
[`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some
Expand Down
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ define_Conf! {
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS, MANUAL_PATTERN_CHAR_COMPARISON.
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE, ITER_KV_MAP, MANUAL_C_STR_LITERALS, ASSIGNING_CLONES, LEGACY_NUMERIC_CONSTANTS, MANUAL_PATTERN_CHAR_COMPARISON, ONCE_CELL_LAZY.
///
/// The minimum rust version that the project supports. Defaults to the `rust-version` field in `Cargo.toml`
#[default_text = ""]
Expand Down
1 change: 1 addition & 0 deletions clippy_config/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ macro_rules! msrv_aliases {

// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,80,0 { LAZY_CELL }
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY_INFO,
crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO,
crate::octal_escapes::OCTAL_ESCAPES_INFO,
crate::once_cell_lazy::ONCE_CELL_LAZY_INFO,
crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO,
crate::operators::ABSURD_EXTREME_COMPARISONS_INFO,
crate::operators::ARITHMETIC_SIDE_EFFECTS_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ mod non_octal_unix_permissions;
mod non_send_fields_in_send_ty;
mod nonstandard_macro_braces;
mod octal_escapes;
mod once_cell_lazy;
mod only_used_in_recursion;
mod operators;
mod option_env_unwrap;
Expand Down Expand Up @@ -1171,6 +1172,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
});
store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
store.register_late_pass(move |_| Box::new(once_cell_lazy::OnceCellLazy::new(msrv())));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
277 changes: 277 additions & 0 deletions clippy_lints/src/once_cell_lazy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,277 @@
use clippy_config::msrvs::Msrv;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{def_path_def_ids, fn_def_id, match_def_path, path_def_id};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, BodyId, Expr, ExprKind, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::Span;

declare_clippy_lint! {
/// ### What it does
/// Lints when a `once_cell::sync::Lazy` type is declared as static variables.
///
/// ### Why restrict this?
/// Such useage have been superseded by the `std::sync::LazyLock` type,
/// replacing them would reduce dependency of `once_cell` crate.
///
/// ### Example
/// ```rust
/// use once_cell_lazy::once_cell_lazy;
/// use once_cell::sync::Lazy;
///
/// once_cell_lazy! {
/// static ref FOO: String = "foo".to_uppercase();
/// }
/// static BAR: Lazy<String> = Lazy::new(|| "BAR".to_lowercase());
/// ```
/// Use instead:
/// ```rust
/// static FOO: std::sync::LazyLock<String> = std::sync::LazyLock::new(|| "foo".to_uppercase());
/// static BAR: std::sync::LazyLock<String> = std::sync::LazyLock::new(|| "BAR".to_lowercase());
/// ```
#[clippy::version = "1.81.0"]
pub ONCE_CELL_LAZY,
restriction,
"using `once_cell::sync::Lazy` type that could be replaced by `std::sync::LazyLock`"
}

/// A list containing functions with coresponding replacements in `LazyLock`.
///
/// Some functions could be replaced as well if we have replaced `Lazy` to `LazyLock`,
/// therefore after suggesting replace the type, we need to make sure the function calls can be
/// replaced, otherwise the suggestions cannot be applied thus the applicability should be
/// `Unspecified` or `MaybeIncorret`.
static FUNCTION_REPLACEMENTS: &[(&str, Option<&str>)] = &[
("once_cell::sync::Lazy::force", Some("std::sync::LazyLock::force")),
("once_cell::sync::Lazy::force_mut", None),
("once_cell::sync::Lazy::get", None),
("once_cell::sync::Lazy::get_mut", None),
("once_cell::sync::Lazy::new", Some("std::sync::LazyLock::new")),
// Note that `Lazy::into_value` is not in the list,
// because we only check for `static`s in this lint, and `into_value` attempts to take ownership
// of the parameter, which means it would fail natively.
// But do keep in mind that the equivalant replacement is called `LazyLock::into_inner`
// if somehow we decided to expand this lint to catch "non-static"s.
];

pub struct OnceCellLazy {
msrv: Msrv,
sugg_map: FxIndexMap<DefId, Option<String>>,
lazy_type_defs: FxIndexMap<DefId, LazyInfo>,
}

impl OnceCellLazy {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self {
msrv,
sugg_map: FxIndexMap::default(),
lazy_type_defs: FxIndexMap::default(),
}
}
}

impl_lint_pass!(OnceCellLazy => [ONCE_CELL_LAZY]);

macro_rules! ensure_prerequisite {
($msrv:expr, $cx:ident) => {
if !$msrv.meets(clippy_config::msrvs::LAZY_CELL) || clippy_utils::is_no_std_crate($cx) {
return;
}
};
}

impl<'hir> LateLintPass<'hir> for OnceCellLazy {
extract_msrv_attr!(LateContext);

fn check_crate(&mut self, cx: &LateContext<'hir>) {
// Do not link if current crate does not support `LazyLock`.
ensure_prerequisite!(self.msrv, cx);

// Convert hardcoded fn replacement list into a map with def_id
for (path, sugg) in FUNCTION_REPLACEMENTS {
let path_vec = path.split("::").collect::<Vec<_>>();
for did in def_path_def_ids(cx, &path_vec) {
self.sugg_map.insert(did, sugg.map(ToOwned::to_owned));
}
}
}

fn check_item(&mut self, cx: &LateContext<'hir>, item: &Item<'hir>) {
ensure_prerequisite!(self.msrv, cx);

if let Some(lazy_kind) = LazyInfo::from_item(cx, item) {
self.lazy_type_defs.insert(item.owner_id.to_def_id(), lazy_kind);
}
}

fn check_expr(&mut self, cx: &LateContext<'hir>, expr: &Expr<'hir>) {
ensure_prerequisite!(self.msrv, cx);

// All functions in the `FUNCTION_REPLACEMENTS` have only one args
if let ExprKind::Call(callee, [arg]) = expr.kind
&& let Some(call_def_id) = fn_def_id(cx, expr)
&& self.sugg_map.contains_key(&call_def_id)
&& let ExprKind::Path(qpath) = arg.peel_borrows().kind
&& let Some(arg_def_id) = cx.typeck_results().qpath_res(&qpath, arg.hir_id).opt_def_id()
&& let Some(lazy_info) = self.lazy_type_defs.get_mut(&arg_def_id)
{
lazy_info.calls_span_and_id.insert(callee.span, call_def_id);
}
}

fn check_crate_post(&mut self, cx: &LateContext<'hir>) {
ensure_prerequisite!(self.msrv, cx);

for (_, lazy_info) in &self.lazy_type_defs {
lazy_info.lint(cx, &self.sugg_map);
}

self.sugg_map = FxIndexMap::default();
}
}

struct LazyInfo {
/// Span of the [`hir::Ty`] where this `Lazy` type was declared,
/// without including args.
/// i.e.:
/// ```ignore
/// static FOO: Lazy<String> = Lazy::new(...);
/// // ^^^^
/// ```
ty_span_no_args: Span,
/// `Span` and `DefId` of calls on `Lazy` type.
/// i.e.:
/// ```ignore
/// static FOO: Lazy<String> = {
/// if cond {
/// Lazy::new(...)
/// // ^^^^^^^^^
/// } else {
/// Lazy::new(...)
/// // ^^^^^^^^^
/// }
/// }
/// ```
///
/// Or:
///
/// ```ignore
/// let x = Lazy::get(&FOO);
/// // ^^^^^^^^
/// ```
calls_span_and_id: FxIndexMap<Span, DefId>,
}

impl LazyInfo {
fn from_item(cx: &LateContext<'_>, item: &Item<'_>) -> Option<Self> {
// Check if item is a `once_cell:sync::Lazy` static.
if let ItemKind::Static(ty, _, body_id) = item.kind
&& let Some(path_def_id) = path_def_id(cx, ty)
&& let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
&& match_def_path(cx, path_def_id, &["once_cell", "sync", "Lazy"])
{
let ty_span_no_args = path_span_without_args(path);
let body = cx.tcx.hir().body(body_id);

// visit body to collect `Lazy::new` calls
let mut new_fn_calls = FxIndexMap::default();
for_each_expr::<(), ()>(cx, body, |ex| {
if let Some((fn_did, call_span)) = fn_def_id_and_span_from_body(cx, ex, body_id)
&& match_def_path(cx, fn_did, &["once_cell", "sync", "Lazy", "new"])
{
new_fn_calls.insert(call_span, fn_did);
}
std::ops::ControlFlow::Continue(())
});

Some(LazyInfo {
ty_span_no_args,
calls_span_and_id: new_fn_calls,
})
} else {
None
}
}

fn lint(&self, cx: &LateContext<'_>, sugg_map: &FxIndexMap<DefId, Option<String>>) {
// Applicability might get adjusted to `MachineApplicable` later if
// all calls in `calls_span_and_id` is replaceable judging by the `sugg_map`.
let mut appl = Applicability::Unspecified;
let mut suggs = vec![(self.ty_span_no_args, "std::sync::LazyLock".to_string())];

if self
.calls_span_and_id
.iter()
// Filtering the calls that DOES NOT have a suggested replacement.
.filter(|(span, def_id)| {
let maybe_sugg = sugg_map.get(*def_id).cloned().flatten();
if let Some(sugg) = maybe_sugg {
suggs.push((**span, sugg));
false
} else {
true
}
})
.collect::<Vec<_>>()
.is_empty()
{
appl = Applicability::MachineApplicable;
}

span_lint_and_then(
cx,
ONCE_CELL_LAZY,
self.ty_span_no_args,
"this type has been superceded by `LazyLock`",
|diag| {
diag.multipart_suggestion("consider using the `LazyLock` from standard library", suggs, appl);
},
);
}
}

/// Return the span of a given `Path` without including any of its args.
///
/// NB: Re-write of a private function `rustc_lint::non_local_def::path_span_without_args`.
fn path_span_without_args(path: &hir::Path<'_>) -> Span {
path.segments
.last()
.and_then(|seg| seg.args)
.map_or(path.span, |args| path.span.until(args.span_ext))
}

/// Returns the `DefId` and `Span` of the callee if the given expression is a function call.
///
/// NB: Modified from [`clippy_utils::fn_def_id`], to support calling in an static `Item`'s body.
fn fn_def_id_and_span_from_body(cx: &LateContext<'_>, expr: &Expr<'_>, body_id: BodyId) -> Option<(DefId, Span)> {
// FIXME: find a way to cache the result.
let typeck = cx.tcx.typeck_body(body_id);
match &expr.kind {
ExprKind::Call(
Expr {
kind: ExprKind::Path(qpath),
hir_id: path_hir_id,
span,
..
},
..,
) => {
// Only return Fn-like DefIds, not the DefIds of statics/consts/etc that contain or
// deref to fn pointers, dyn Fn, impl Fn - #8850
if let Res::Def(DefKind::Fn | DefKind::Ctor(..) | DefKind::AssocFn, id) =
typeck.qpath_res(qpath, *path_hir_id)
{
Some((id, *span))
} else {
None
}
},
_ => None,
}
}
52 changes: 52 additions & 0 deletions tests/ui/lazy_lock_like/auxiliary/once_cell.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//! **FAKE** once_cell crate.
pub mod sync {
use core::cell::Cell;
use std::marker::PhantomData;

pub struct OnceCell<T>(PhantomData<T>);
impl<T> OnceCell<T> {
pub const fn new() -> OnceCell<T> {
OnceCell(PhantomData)
}
}
impl<T> Default for OnceCell<T> {
fn default() -> Self {
Self::new()
}
}

pub struct Lazy<T, F = fn() -> T> {
cell: OnceCell<T>,
init: Cell<Option<F>>,
}
unsafe impl<T, F: Send> Sync for Lazy<T, F> where OnceCell<T>: Sync {}
impl<T, F> Lazy<T, F> {
pub const fn new(f: F) -> Lazy<T, F> {
Lazy {
cell: OnceCell::new(),
init: Cell::new(Some(f)),
}
}

pub fn into_value(this: Lazy<T, F>) -> Result<T, i32> {
Err(1)
}

pub fn force(_this: &Lazy<T, F>) -> i32 {
0
}

pub fn force_mut(_this: &mut Lazy<T, F>) -> i32 {
0
}

pub fn get(_this: &Lazy<T, F>) -> i32 {
0
}

pub fn get_mut(_this: &mut Lazy<T, F>) -> i32 {
0
}
}
}
Loading

0 comments on commit 7fba7f9

Please sign in to comment.