diff options
| author | Philipp Krones <hello@philkrones.com> | 2025-09-18 17:21:44 +0200 |
|---|---|---|
| committer | Philipp Krones <hello@philkrones.com> | 2025-09-18 17:21:44 +0200 |
| commit | 1bfe3bcfece0e4db3d748aba0ae9ec8def5c2644 (patch) | |
| tree | af1da24f2ace786199e00e8a780e4c9732bed386 /src/tools/clippy/clippy_lints | |
| parent | 32e3d9f59bae4bcf436bc1e28723c696d2c75b11 (diff) | |
| parent | 20ce69b9a63bcd2756cd906fe0964d1e901e042a (diff) | |
| download | rust-1bfe3bcfece0e4db3d748aba0ae9ec8def5c2644.tar.gz rust-1bfe3bcfece0e4db3d748aba0ae9ec8def5c2644.zip | |
Merge commit '20ce69b9a63bcd2756cd906fe0964d1e901e042a' into clippy-subtree-update
Diffstat (limited to 'src/tools/clippy/clippy_lints')
71 files changed, 1945 insertions, 1636 deletions
diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index 70184ee2ca5..51e59ae2050 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_lints" -version = "0.1.91" +version = "0.1.92" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/src/tools/clippy/clippy_lints/src/attrs/useless_attribute.rs b/src/tools/clippy/clippy_lints/src/attrs/useless_attribute.rs index b9b5cedb5aa..1cebc18edc9 100644 --- a/src/tools/clippy/clippy_lints/src/attrs/useless_attribute.rs +++ b/src/tools/clippy/clippy_lints/src/attrs/useless_attribute.rs @@ -30,6 +30,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, item: &Item, attrs: &[Attribute]) { sym::ambiguous_glob_reexports | sym::dead_code | sym::deprecated + | sym::deprecated_in_future | sym::hidden_glob_reexports | sym::unreachable_pub | sym::unused diff --git a/src/tools/clippy/clippy_lints/src/casts/as_underscore.rs b/src/tools/clippy/clippy_lints/src/casts/as_underscore.rs index 3ac486dd63f..a73e48e5fd5 100644 --- a/src/tools/clippy/clippy_lints/src/casts/as_underscore.rs +++ b/src/tools/clippy/clippy_lints/src/casts/as_underscore.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then; use rustc_errors::Applicability; use rustc_hir::{Expr, Ty, TyKind}; use rustc_lint::LateContext; -use rustc_middle::ty; +use rustc_middle::ty::IsSuggestable; use super::AS_UNDERSCORE; @@ -10,15 +10,15 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, ty: &'tc if matches!(ty.kind, TyKind::Infer(())) { span_lint_and_then(cx, AS_UNDERSCORE, expr.span, "using `as _` conversion", |diag| { let ty_resolved = cx.typeck_results().expr_ty(expr); - if let ty::Error(_) = ty_resolved.kind() { - diag.help("consider giving the type explicitly"); - } else { + if ty_resolved.is_suggestable(cx.tcx, true) { diag.span_suggestion( ty.span, "consider giving the type explicitly", ty_resolved, Applicability::MachineApplicable, ); + } else { + diag.help("consider giving the type explicitly"); } }); } diff --git a/src/tools/clippy/clippy_lints/src/casts/cast_possible_wrap.rs b/src/tools/clippy/clippy_lints/src/casts/cast_possible_wrap.rs index e26c03ccda9..9eaa6e4cf26 100644 --- a/src/tools/clippy/clippy_lints/src/casts/cast_possible_wrap.rs +++ b/src/tools/clippy/clippy_lints/src/casts/cast_possible_wrap.rs @@ -1,4 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::sugg::Sugg; +use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; use rustc_middle::ty::Ty; @@ -16,7 +19,14 @@ enum EmitState { LintOnPtrSize(u64), } -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { +pub(super) fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + cast_op: &Expr<'_>, + cast_from: Ty<'_>, + cast_to: Ty<'_>, + msrv: Msrv, +) { let (Some(from_nbits), Some(to_nbits)) = ( utils::int_ty_to_nbits(cx.tcx, cast_from), utils::int_ty_to_nbits(cx.tcx, cast_to), @@ -85,5 +95,23 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca .note("`usize` and `isize` may be as small as 16 bits on some platforms") .note("for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types"); } + + if msrv.meets(cx, msrvs::INTEGER_SIGN_CAST) + && let Some(cast) = utils::is_signedness_cast(cast_from, cast_to) + { + let method = match cast { + utils::CastTo::Signed => "cast_signed()", + utils::CastTo::Unsigned => "cast_unsigned()", + }; + let mut app = Applicability::MaybeIncorrect; + let sugg = Sugg::hir_with_context(cx, cast_op, expr.span.ctxt(), "..", &mut app); + + diag.span_suggestion( + expr.span, + format!("if this is intentional, use `{method}` instead"), + format!("{}.{method}", sugg.maybe_paren()), + app, + ); + } }); } diff --git a/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs b/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs index a70bd886191..f870d27b796 100644 --- a/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs +++ b/src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs @@ -2,15 +2,18 @@ use std::convert::Infallible; use std::ops::ControlFlow; use clippy_utils::consts::{ConstEvalCtxt, Constant}; -use clippy_utils::diagnostics::span_lint; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::sugg::Sugg; use clippy_utils::visitors::{Descend, for_each_expr_without_closures}; use clippy_utils::{method_chain_args, sext, sym}; +use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::{self, Ty}; use rustc_span::Symbol; -use super::CAST_SIGN_LOSS; +use super::{CAST_SIGN_LOSS, utils}; /// A list of methods that can never return a negative value. /// Includes methods that panic rather than returning a negative value. @@ -42,13 +45,33 @@ pub(super) fn check<'cx>( cast_op: &Expr<'_>, cast_from: Ty<'cx>, cast_to: Ty<'_>, + msrv: Msrv, ) { if should_lint(cx, cast_op, cast_from, cast_to) { - span_lint( + span_lint_and_then( cx, CAST_SIGN_LOSS, expr.span, format!("casting `{cast_from}` to `{cast_to}` may lose the sign of the value"), + |diag| { + if msrv.meets(cx, msrvs::INTEGER_SIGN_CAST) + && let Some(cast) = utils::is_signedness_cast(cast_from, cast_to) + { + let method = match cast { + utils::CastTo::Signed => "cast_signed()", + utils::CastTo::Unsigned => "cast_unsigned()", + }; + let mut app = Applicability::MaybeIncorrect; + let sugg = Sugg::hir_with_context(cx, cast_op, expr.span.ctxt(), "..", &mut app); + + diag.span_suggestion( + expr.span, + format!("if this is intentional, use `{method}` instead"), + format!("{}.{method}", sugg.maybe_paren()), + app, + ); + } + }, ); } } diff --git a/src/tools/clippy/clippy_lints/src/casts/mod.rs b/src/tools/clippy/clippy_lints/src/casts/mod.rs index d2e62ee56e4..47cc1da0a6e 100644 --- a/src/tools/clippy/clippy_lints/src/casts/mod.rs +++ b/src/tools/clippy/clippy_lints/src/casts/mod.rs @@ -890,9 +890,9 @@ impl<'tcx> LateLintPass<'tcx> for Casts { if cast_to.is_numeric() { cast_possible_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to, cast_to_hir.span); if cast_from.is_numeric() { - cast_possible_wrap::check(cx, expr, cast_from, cast_to); + cast_possible_wrap::check(cx, expr, cast_from_expr, cast_from, cast_to, self.msrv); cast_precision_loss::check(cx, expr, cast_from, cast_to); - cast_sign_loss::check(cx, expr, cast_from_expr, cast_from, cast_to); + cast_sign_loss::check(cx, expr, cast_from_expr, cast_from, cast_to, self.msrv); cast_abs_to_unsigned::check(cx, expr, cast_from_expr, cast_from, cast_to, self.msrv); cast_nan_to_int::check(cx, expr, cast_from_expr, cast_from, cast_to); } diff --git a/src/tools/clippy/clippy_lints/src/casts/utils.rs b/src/tools/clippy/clippy_lints/src/casts/utils.rs index d846d78b9ee..707fc2a8eed 100644 --- a/src/tools/clippy/clippy_lints/src/casts/utils.rs +++ b/src/tools/clippy/clippy_lints/src/casts/utils.rs @@ -60,3 +60,18 @@ pub(super) fn enum_ty_to_nbits(adt: AdtDef<'_>, tcx: TyCtxt<'_>) -> u64 { neg_bits.max(pos_bits).into() } } + +pub(super) enum CastTo { + Signed, + Unsigned, +} +/// Returns `Some` if the type cast is between 2 integral types that differ +/// only in signedness, otherwise `None`. The value of `Some` is which +/// signedness is casted to. +pub(super) fn is_signedness_cast(cast_from: Ty<'_>, cast_to: Ty<'_>) -> Option<CastTo> { + match (cast_from.kind(), cast_to.kind()) { + (ty::Int(from), ty::Uint(to)) if from.to_unsigned() == *to => Some(CastTo::Unsigned), + (ty::Uint(from), ty::Int(to)) if *from == to.to_unsigned() => Some(CastTo::Signed), + _ => None, + } +} diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index d0c7443a4a4..2a4bedc1845 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/clippy_lints/src/declared_lints.rs @@ -448,10 +448,12 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::OR_THEN_UNWRAP_INFO, crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO, crate::methods::PATH_ENDS_WITH_EXT_INFO, + crate::methods::PTR_OFFSET_WITH_CAST_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::READONLY_WRITE_LOCK_INFO, crate::methods::READ_LINE_WITHOUT_TRIM_INFO, crate::methods::REDUNDANT_AS_STR_INFO, + crate::methods::REDUNDANT_ITER_CLONED_INFO, crate::methods::REPEAT_ONCE_INFO, crate::methods::RESULT_FILTER_MAP_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, @@ -503,7 +505,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::min_ident_chars::MIN_IDENT_CHARS_INFO, crate::minmax::MIN_MAX_INFO, crate::misc::SHORT_CIRCUIT_STATEMENT_INFO, - crate::misc::TOPLEVEL_REF_ARG_INFO, crate::misc::USED_UNDERSCORE_BINDING_INFO, crate::misc::USED_UNDERSCORE_ITEMS_INFO, crate::misc_early::BUILTIN_TYPE_SHADOW_INFO, @@ -625,7 +626,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::ptr::MUT_FROM_REF_INFO, crate::ptr::PTR_ARG_INFO, crate::ptr::PTR_EQ_INFO, - crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO, crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO, crate::pub_use::PUB_USE_INFO, crate::question_mark::QUESTION_MARK_INFO, @@ -706,6 +706,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO, crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO, crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO, + crate::toplevel_ref_arg::TOPLEVEL_REF_ARG_INFO, crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO, crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO, crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO, diff --git a/src/tools/clippy/clippy_lints/src/dereference.rs b/src/tools/clippy/clippy_lints/src/dereference.rs index 9645a26a68a..a70105db194 100644 --- a/src/tools/clippy/clippy_lints/src/dereference.rs +++ b/src/tools/clippy/clippy_lints/src/dereference.rs @@ -1,10 +1,9 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; -use clippy_utils::ty::{adjust_derefs_manually_drop, implements_trait, is_manually_drop}; +use clippy_utils::ty::{adjust_derefs_manually_drop, implements_trait, is_manually_drop, peel_and_count_ty_refs}; use clippy_utils::{ DefinedTy, ExprUseNode, expr_use_ctxt, get_parent_expr, is_block_like, is_lint_allowed, path_to_local, - peel_middle_ty_refs, }; use rustc_ast::util::parser::ExprPrecedence; use rustc_data_structures::fx::FxIndexMap; @@ -942,7 +941,7 @@ fn report<'tcx>( let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app); let ty = typeck.expr_ty(expr); - let (_, ref_count) = peel_middle_ty_refs(ty); + let (_, ref_count, _) = peel_and_count_ty_refs(ty); let deref_str = if ty_changed_count >= ref_count && ref_count != 0 { // a deref call changing &T -> &U requires two deref operators the first time // this occurs. One to remove the reference, a second to call the deref impl. @@ -1045,7 +1044,7 @@ fn report<'tcx>( if let ty::Ref(_, dst, _) = data.adjusted_ty.kind() && dst.is_slice() { - let (src, n_src_refs) = peel_middle_ty_refs(ty); + let (src, n_src_refs, _) = peel_and_count_ty_refs(ty); if n_src_refs >= 2 && src.is_array() { return; } diff --git a/src/tools/clippy/clippy_lints/src/derive.rs b/src/tools/clippy/clippy_lints/src/derive.rs deleted file mode 100644 index c53a957f6a8..00000000000 --- a/src/tools/clippy/clippy_lints/src/derive.rs +++ /dev/null @@ -1,527 +0,0 @@ -use std::ops::ControlFlow; - -use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_then, span_lint_hir_and_then}; -use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy}; -use clippy_utils::{has_non_exhaustive_attr, is_lint_allowed, paths}; -use rustc_errors::Applicability; -use rustc_hir::def_id::DefId; -use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn, walk_item}; -use rustc_hir::{self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, Impl, Item, ItemKind, UnsafeSource}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::hir::nested_filter; -use rustc_middle::ty::{ - self, ClauseKind, GenericArgKind, GenericParamDefKind, ParamEnv, TraitPredicate, Ty, TyCtxt, Upcast, -}; -use rustc_session::declare_lint_pass; -use rustc_span::def_id::LocalDefId; -use rustc_span::{Span, sym}; - -declare_clippy_lint! { - /// ### What it does - /// Lints against manual `PartialEq` implementations for types with a derived `Hash` - /// implementation. - /// - /// ### Why is this bad? - /// The implementation of these traits must agree (for - /// example for use with `HashMap`) so it’s probably a bad idea to use a - /// default-generated `Hash` implementation with an explicitly defined - /// `PartialEq`. In particular, the following must hold for any type: - /// - /// ```text - /// k1 == k2 ⇒ hash(k1) == hash(k2) - /// ``` - /// - /// ### Example - /// ```ignore - /// #[derive(Hash)] - /// struct Foo; - /// - /// impl PartialEq for Foo { - /// ... - /// } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub DERIVED_HASH_WITH_MANUAL_EQ, - correctness, - "deriving `Hash` but implementing `PartialEq` explicitly" -} - -declare_clippy_lint! { - /// ### What it does - /// Lints against manual `PartialOrd` and `Ord` implementations for types with a derived `Ord` - /// or `PartialOrd` implementation. - /// - /// ### Why is this bad? - /// The implementation of these traits must agree (for - /// example for use with `sort`) so it’s probably a bad idea to use a - /// default-generated `Ord` implementation with an explicitly defined - /// `PartialOrd`. In particular, the following must hold for any type - /// implementing `Ord`: - /// - /// ```text - /// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap() - /// ``` - /// - /// ### Example - /// ```rust,ignore - /// #[derive(Ord, PartialEq, Eq)] - /// struct Foo; - /// - /// impl PartialOrd for Foo { - /// ... - /// } - /// ``` - /// Use instead: - /// ```rust,ignore - /// #[derive(PartialEq, Eq)] - /// struct Foo; - /// - /// impl PartialOrd for Foo { - /// fn partial_cmp(&self, other: &Foo) -> Option<Ordering> { - /// Some(self.cmp(other)) - /// } - /// } - /// - /// impl Ord for Foo { - /// ... - /// } - /// ``` - /// or, if you don't need a custom ordering: - /// ```rust,ignore - /// #[derive(Ord, PartialOrd, PartialEq, Eq)] - /// struct Foo; - /// ``` - #[clippy::version = "1.47.0"] - pub DERIVE_ORD_XOR_PARTIAL_ORD, - correctness, - "deriving `Ord` but implementing `PartialOrd` explicitly" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for explicit `Clone` implementations for `Copy` - /// types. - /// - /// ### Why is this bad? - /// To avoid surprising behavior, these traits should - /// agree and the behavior of `Copy` cannot be overridden. In almost all - /// situations a `Copy` type should have a `Clone` implementation that does - /// nothing more than copy the object, which is what `#[derive(Copy, Clone)]` - /// gets you. - /// - /// ### Example - /// ```rust,ignore - /// #[derive(Copy)] - /// struct Foo; - /// - /// impl Clone for Foo { - /// // .. - /// } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub EXPL_IMPL_CLONE_ON_COPY, - pedantic, - "implementing `Clone` explicitly on `Copy` types" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for deriving `serde::Deserialize` on a type that - /// has methods using `unsafe`. - /// - /// ### Why is this bad? - /// Deriving `serde::Deserialize` will create a constructor - /// that may violate invariants held by another constructor. - /// - /// ### Example - /// ```rust,ignore - /// use serde::Deserialize; - /// - /// #[derive(Deserialize)] - /// pub struct Foo { - /// // .. - /// } - /// - /// impl Foo { - /// pub fn new() -> Self { - /// // setup here .. - /// } - /// - /// pub unsafe fn parts() -> (&str, &str) { - /// // assumes invariants hold - /// } - /// } - /// ``` - #[clippy::version = "1.45.0"] - pub UNSAFE_DERIVE_DESERIALIZE, - pedantic, - "deriving `serde::Deserialize` on a type that has methods using `unsafe`" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for types that derive `PartialEq` and could implement `Eq`. - /// - /// ### Why is this bad? - /// If a type `T` derives `PartialEq` and all of its members implement `Eq`, - /// then `T` can always implement `Eq`. Implementing `Eq` allows `T` to be used - /// in APIs that require `Eq` types. It also allows structs containing `T` to derive - /// `Eq` themselves. - /// - /// ### Example - /// ```no_run - /// #[derive(PartialEq)] - /// struct Foo { - /// i_am_eq: i32, - /// i_am_eq_too: Vec<String>, - /// } - /// ``` - /// Use instead: - /// ```no_run - /// #[derive(PartialEq, Eq)] - /// struct Foo { - /// i_am_eq: i32, - /// i_am_eq_too: Vec<String>, - /// } - /// ``` - #[clippy::version = "1.63.0"] - pub DERIVE_PARTIAL_EQ_WITHOUT_EQ, - nursery, - "deriving `PartialEq` on a type that can implement `Eq`, without implementing `Eq`" -} - -declare_lint_pass!(Derive => [ - EXPL_IMPL_CLONE_ON_COPY, - DERIVED_HASH_WITH_MANUAL_EQ, - DERIVE_ORD_XOR_PARTIAL_ORD, - UNSAFE_DERIVE_DESERIALIZE, - DERIVE_PARTIAL_EQ_WITHOUT_EQ -]); - -impl<'tcx> LateLintPass<'tcx> for Derive { - fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Impl(Impl { - of_trait: Some(of_trait), - .. - }) = item.kind - { - let trait_ref = &of_trait.trait_ref; - let ty = cx.tcx.type_of(item.owner_id).instantiate_identity(); - let is_automatically_derived = cx.tcx.is_automatically_derived(item.owner_id.to_def_id()); - - check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived); - check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived); - - if is_automatically_derived { - check_unsafe_derive_deserialize(cx, item, trait_ref, ty); - check_partial_eq_without_eq(cx, item.span, trait_ref, ty); - } else { - check_copy_clone(cx, item, trait_ref, ty); - } - } - } -} - -/// Implementation of the `DERIVED_HASH_WITH_MANUAL_EQ` lint. -fn check_hash_peq<'tcx>( - cx: &LateContext<'tcx>, - span: Span, - trait_ref: &hir::TraitRef<'_>, - ty: Ty<'tcx>, - hash_is_automatically_derived: bool, -) { - if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait() - && let Some(def_id) = trait_ref.trait_def_id() - && cx.tcx.is_diagnostic_item(sym::Hash, def_id) - { - // Look for the PartialEq implementations for `ty` - cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { - let peq_is_automatically_derived = cx.tcx.is_automatically_derived(impl_id); - - if !hash_is_automatically_derived || peq_is_automatically_derived { - return; - } - - let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation"); - - // Only care about `impl PartialEq<Foo> for Foo` - // For `impl PartialEq<B> for A, input_types is [A, B] - if trait_ref.instantiate_identity().args.type_at(1) == ty { - span_lint_and_then( - cx, - DERIVED_HASH_WITH_MANUAL_EQ, - span, - "you are deriving `Hash` but have implemented `PartialEq` explicitly", - |diag| { - if let Some(local_def_id) = impl_id.as_local() { - let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id); - diag.span_note(cx.tcx.hir_span(hir_id), "`PartialEq` implemented here"); - } - }, - ); - } - }); - } -} - -/// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint. -fn check_ord_partial_ord<'tcx>( - cx: &LateContext<'tcx>, - span: Span, - trait_ref: &hir::TraitRef<'_>, - ty: Ty<'tcx>, - ord_is_automatically_derived: bool, -) { - if let Some(ord_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Ord) - && let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait() - && let Some(def_id) = &trait_ref.trait_def_id() - && *def_id == ord_trait_def_id - { - // Look for the PartialOrd implementations for `ty` - cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| { - let partial_ord_is_automatically_derived = cx.tcx.is_automatically_derived(impl_id); - - if partial_ord_is_automatically_derived == ord_is_automatically_derived { - return; - } - - let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation"); - - // Only care about `impl PartialOrd<Foo> for Foo` - // For `impl PartialOrd<B> for A, input_types is [A, B] - if trait_ref.instantiate_identity().args.type_at(1) == ty { - let mess = if partial_ord_is_automatically_derived { - "you are implementing `Ord` explicitly but have derived `PartialOrd`" - } else { - "you are deriving `Ord` but have implemented `PartialOrd` explicitly" - }; - - span_lint_and_then(cx, DERIVE_ORD_XOR_PARTIAL_ORD, span, mess, |diag| { - if let Some(local_def_id) = impl_id.as_local() { - let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id); - diag.span_note(cx.tcx.hir_span(hir_id), "`PartialOrd` implemented here"); - } - }); - } - }); - } -} - -/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint. -fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { - let clone_id = match cx.tcx.lang_items().clone_trait() { - Some(id) if trait_ref.trait_def_id() == Some(id) => id, - _ => return, - }; - let Some(copy_id) = cx.tcx.lang_items().copy_trait() else { - return; - }; - let (ty_adt, ty_subs) = match *ty.kind() { - // Unions can't derive clone. - ty::Adt(adt, subs) if !adt.is_union() => (adt, subs), - _ => return, - }; - // If the current self type doesn't implement Copy (due to generic constraints), search to see if - // there's a Copy impl for any instance of the adt. - if !is_copy(cx, ty) { - if ty_subs.non_erasable_generics().next().is_some() { - let has_copy_impl = cx.tcx.local_trait_impls(copy_id).iter().any(|&id| { - matches!(cx.tcx.type_of(id).instantiate_identity().kind(), ty::Adt(adt, _) - if ty_adt.did() == adt.did()) - }); - if !has_copy_impl { - return; - } - } else { - return; - } - } - // Derive constrains all generic types to requiring Clone. Check if any type is not constrained for - // this impl. - if ty_subs.types().any(|ty| !implements_trait(cx, ty, clone_id, &[])) { - return; - } - // `#[repr(packed)]` structs with type/const parameters can't derive `Clone`. - // https://github.com/rust-lang/rust-clippy/issues/10188 - if ty_adt.repr().packed() - && ty_subs - .iter() - .any(|arg| matches!(arg.kind(), GenericArgKind::Type(_) | GenericArgKind::Const(_))) - { - return; - } - // The presence of `unsafe` fields prevents deriving `Clone` automatically - if ty_adt.all_fields().any(|f| f.safety.is_unsafe()) { - return; - } - - span_lint_and_note( - cx, - EXPL_IMPL_CLONE_ON_COPY, - item.span, - "you are implementing `Clone` explicitly on a `Copy` type", - Some(item.span), - "consider deriving `Clone` or removing `Copy`", - ); -} - -/// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint. -fn check_unsafe_derive_deserialize<'tcx>( - cx: &LateContext<'tcx>, - item: &Item<'_>, - trait_ref: &hir::TraitRef<'_>, - ty: Ty<'tcx>, -) { - fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool { - let mut visitor = UnsafeVisitor { cx }; - walk_item(&mut visitor, item).is_break() - } - - if let Some(trait_def_id) = trait_ref.trait_def_id() - && paths::SERDE_DESERIALIZE.matches(cx, trait_def_id) - && let ty::Adt(def, _) = ty.kind() - && let Some(local_def_id) = def.did().as_local() - && let adt_hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id) - && !is_lint_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id) - && cx - .tcx - .inherent_impls(def.did()) - .iter() - .map(|imp_did| cx.tcx.hir_expect_item(imp_did.expect_local())) - .any(|imp| has_unsafe(cx, imp)) - { - span_lint_hir_and_then( - cx, - UNSAFE_DERIVE_DESERIALIZE, - adt_hir_id, - item.span, - "you are deriving `serde::Deserialize` on a type that has methods using `unsafe`", - |diag| { - diag.help( - "consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html", - ); - }, - ); - } -} - -struct UnsafeVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, -} - -impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { - type Result = ControlFlow<()>; - type NestedFilter = nested_filter::All; - - fn visit_fn( - &mut self, - kind: FnKind<'tcx>, - decl: &'tcx FnDecl<'_>, - body_id: BodyId, - _: Span, - id: LocalDefId, - ) -> Self::Result { - if let Some(header) = kind.header() - && header.is_unsafe() - { - ControlFlow::Break(()) - } else { - walk_fn(self, kind, decl, body_id, id) - } - } - - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> Self::Result { - if let ExprKind::Block(block, _) = expr.kind - && block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) - && block - .span - .source_callee() - .and_then(|expr| expr.macro_def_id) - .is_none_or(|did| !self.cx.tcx.is_diagnostic_item(sym::pin_macro, did)) - { - return ControlFlow::Break(()); - } - - walk_expr(self, expr) - } - - fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { - self.cx.tcx - } -} - -/// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint. -fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { - if let ty::Adt(adt, args) = ty.kind() - && cx.tcx.visibility(adt.did()).is_public() - && let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq) - && let Some(def_id) = trait_ref.trait_def_id() - && cx.tcx.is_diagnostic_item(sym::PartialEq, def_id) - && !has_non_exhaustive_attr(cx.tcx, *adt) - && !ty_implements_eq_trait(cx.tcx, ty, eq_trait_def_id) - && let typing_env = typing_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id) - && let Some(local_def_id) = adt.did().as_local() - // If all of our fields implement `Eq`, we can implement `Eq` too - && adt - .all_fields() - .map(|f| f.ty(cx.tcx, args)) - .all(|ty| implements_trait_with_env(cx.tcx, typing_env, ty, eq_trait_def_id, None, &[])) - { - span_lint_hir_and_then( - cx, - DERIVE_PARTIAL_EQ_WITHOUT_EQ, - cx.tcx.local_def_id_to_hir_id(local_def_id), - span.ctxt().outer_expn_data().call_site, - "you are deriving `PartialEq` and can implement `Eq`", - |diag| { - diag.span_suggestion( - span.ctxt().outer_expn_data().call_site, - "consider deriving `Eq` as well", - "PartialEq, Eq", - Applicability::MachineApplicable, - ); - }, - ); - } -} - -fn ty_implements_eq_trait<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, eq_trait_id: DefId) -> bool { - tcx.non_blanket_impls_for_ty(eq_trait_id, ty).next().is_some() -} - -/// Creates the `ParamEnv` used for the given type's derived `Eq` impl. -fn typing_env_for_derived_eq(tcx: TyCtxt<'_>, did: DefId, eq_trait_id: DefId) -> ty::TypingEnv<'_> { - // Initial map from generic index to param def. - // Vec<(param_def, needs_eq)> - let mut params = tcx - .generics_of(did) - .own_params - .iter() - .map(|p| (p, matches!(p.kind, GenericParamDefKind::Type { .. }))) - .collect::<Vec<_>>(); - - let ty_predicates = tcx.predicates_of(did).predicates; - for (p, _) in ty_predicates { - if let ClauseKind::Trait(p) = p.kind().skip_binder() - && p.trait_ref.def_id == eq_trait_id - && let ty::Param(self_ty) = p.trait_ref.self_ty().kind() - { - // Flag types which already have an `Eq` bound. - params[self_ty.index as usize].1 = false; - } - } - - let param_env = ParamEnv::new(tcx.mk_clauses_from_iter(ty_predicates.iter().map(|&(p, _)| p).chain( - params.iter().filter(|&&(_, needs_eq)| needs_eq).map(|&(param, _)| { - ClauseKind::Trait(TraitPredicate { - trait_ref: ty::TraitRef::new(tcx, eq_trait_id, [tcx.mk_param_from_def(param)]), - polarity: ty::PredicatePolarity::Positive, - }) - .upcast(tcx) - }), - ))); - ty::TypingEnv { - typing_mode: ty::TypingMode::non_body_analysis(), - param_env, - } -} diff --git a/src/tools/clippy/clippy_lints/src/derive/derive_ord_xor_partial_ord.rs b/src/tools/clippy/clippy_lints/src/derive/derive_ord_xor_partial_ord.rs new file mode 100644 index 00000000000..cbbcb2f7a3b --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/derive/derive_ord_xor_partial_ord.rs @@ -0,0 +1,50 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; +use rustc_span::{Span, sym}; + +use super::DERIVE_ORD_XOR_PARTIAL_ORD; + +/// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint. +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + span: Span, + trait_ref: &hir::TraitRef<'_>, + ty: Ty<'tcx>, + ord_is_automatically_derived: bool, +) { + if let Some(ord_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Ord) + && let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait() + && let Some(def_id) = &trait_ref.trait_def_id() + && *def_id == ord_trait_def_id + { + // Look for the PartialOrd implementations for `ty` + cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| { + let partial_ord_is_automatically_derived = cx.tcx.is_automatically_derived(impl_id); + + if partial_ord_is_automatically_derived == ord_is_automatically_derived { + return; + } + + let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation"); + + // Only care about `impl PartialOrd<Foo> for Foo` + // For `impl PartialOrd<B> for A, input_types is [A, B] + if trait_ref.instantiate_identity().args.type_at(1) == ty { + let mess = if partial_ord_is_automatically_derived { + "you are implementing `Ord` explicitly but have derived `PartialOrd`" + } else { + "you are deriving `Ord` but have implemented `PartialOrd` explicitly" + }; + + span_lint_and_then(cx, DERIVE_ORD_XOR_PARTIAL_ORD, span, mess, |diag| { + if let Some(local_def_id) = impl_id.as_local() { + let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id); + diag.span_note(cx.tcx.hir_span(hir_id), "`PartialOrd` implemented here"); + } + }); + } + }); + } +} diff --git a/src/tools/clippy/clippy_lints/src/derive/derive_partial_eq_without_eq.rs b/src/tools/clippy/clippy_lints/src/derive/derive_partial_eq_without_eq.rs new file mode 100644 index 00000000000..ed7881c461f --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/derive/derive_partial_eq_without_eq.rs @@ -0,0 +1,87 @@ +use clippy_utils::diagnostics::span_lint_hir_and_then; +use clippy_utils::has_non_exhaustive_attr; +use clippy_utils::ty::implements_trait_with_env; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, ClauseKind, GenericParamDefKind, ParamEnv, TraitPredicate, Ty, TyCtxt, Upcast}; +use rustc_span::{Span, sym}; + +use super::DERIVE_PARTIAL_EQ_WITHOUT_EQ; + +/// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint. +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { + if let ty::Adt(adt, args) = ty.kind() + && cx.tcx.visibility(adt.did()).is_public() + && let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq) + && let Some(def_id) = trait_ref.trait_def_id() + && cx.tcx.is_diagnostic_item(sym::PartialEq, def_id) + && !has_non_exhaustive_attr(cx.tcx, *adt) + && !ty_implements_eq_trait(cx.tcx, ty, eq_trait_def_id) + && let typing_env = typing_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id) + && let Some(local_def_id) = adt.did().as_local() + // If all of our fields implement `Eq`, we can implement `Eq` too + && adt + .all_fields() + .map(|f| f.ty(cx.tcx, args)) + .all(|ty| implements_trait_with_env(cx.tcx, typing_env, ty, eq_trait_def_id, None, &[])) + { + span_lint_hir_and_then( + cx, + DERIVE_PARTIAL_EQ_WITHOUT_EQ, + cx.tcx.local_def_id_to_hir_id(local_def_id), + span.ctxt().outer_expn_data().call_site, + "you are deriving `PartialEq` and can implement `Eq`", + |diag| { + diag.span_suggestion( + span.ctxt().outer_expn_data().call_site, + "consider deriving `Eq` as well", + "PartialEq, Eq", + Applicability::MachineApplicable, + ); + }, + ); + } +} + +fn ty_implements_eq_trait<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, eq_trait_id: DefId) -> bool { + tcx.non_blanket_impls_for_ty(eq_trait_id, ty).next().is_some() +} + +/// Creates the `ParamEnv` used for the given type's derived `Eq` impl. +fn typing_env_for_derived_eq(tcx: TyCtxt<'_>, did: DefId, eq_trait_id: DefId) -> ty::TypingEnv<'_> { + // Initial map from generic index to param def. + // Vec<(param_def, needs_eq)> + let mut params = tcx + .generics_of(did) + .own_params + .iter() + .map(|p| (p, matches!(p.kind, GenericParamDefKind::Type { .. }))) + .collect::<Vec<_>>(); + + let ty_predicates = tcx.predicates_of(did).predicates; + for (p, _) in ty_predicates { + if let ClauseKind::Trait(p) = p.kind().skip_binder() + && p.trait_ref.def_id == eq_trait_id + && let ty::Param(self_ty) = p.trait_ref.self_ty().kind() + { + // Flag types which already have an `Eq` bound. + params[self_ty.index as usize].1 = false; + } + } + + let param_env = ParamEnv::new(tcx.mk_clauses_from_iter(ty_predicates.iter().map(|&(p, _)| p).chain( + params.iter().filter(|&&(_, needs_eq)| needs_eq).map(|&(param, _)| { + ClauseKind::Trait(TraitPredicate { + trait_ref: ty::TraitRef::new(tcx, eq_trait_id, [tcx.mk_param_from_def(param)]), + polarity: ty::PredicatePolarity::Positive, + }) + .upcast(tcx) + }), + ))); + ty::TypingEnv { + typing_mode: ty::TypingMode::non_body_analysis(), + param_env, + } +} diff --git a/src/tools/clippy/clippy_lints/src/derive/derived_hash_with_manual_eq.rs b/src/tools/clippy/clippy_lints/src/derive/derived_hash_with_manual_eq.rs new file mode 100644 index 00000000000..6f36a58025a --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/derive/derived_hash_with_manual_eq.rs @@ -0,0 +1,49 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; +use rustc_span::{Span, sym}; + +use super::DERIVED_HASH_WITH_MANUAL_EQ; + +/// Implementation of the `DERIVED_HASH_WITH_MANUAL_EQ` lint. +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + span: Span, + trait_ref: &hir::TraitRef<'_>, + ty: Ty<'tcx>, + hash_is_automatically_derived: bool, +) { + if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait() + && let Some(def_id) = trait_ref.trait_def_id() + && cx.tcx.is_diagnostic_item(sym::Hash, def_id) + { + // Look for the PartialEq implementations for `ty` + cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| { + let peq_is_automatically_derived = cx.tcx.is_automatically_derived(impl_id); + + if !hash_is_automatically_derived || peq_is_automatically_derived { + return; + } + + let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation"); + + // Only care about `impl PartialEq<Foo> for Foo` + // For `impl PartialEq<B> for A, input_types is [A, B] + if trait_ref.instantiate_identity().args.type_at(1) == ty { + span_lint_and_then( + cx, + DERIVED_HASH_WITH_MANUAL_EQ, + span, + "you are deriving `Hash` but have implemented `PartialEq` explicitly", + |diag| { + if let Some(local_def_id) = impl_id.as_local() { + let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id); + diag.span_note(cx.tcx.hir_span(hir_id), "`PartialEq` implemented here"); + } + }, + ); + } + }); + } +} diff --git a/src/tools/clippy/clippy_lints/src/derive/expl_impl_clone_on_copy.rs b/src/tools/clippy/clippy_lints/src/derive/expl_impl_clone_on_copy.rs new file mode 100644 index 00000000000..6b97b4bd6b4 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/derive/expl_impl_clone_on_copy.rs @@ -0,0 +1,65 @@ +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::ty::{implements_trait, is_copy}; +use rustc_hir::{self as hir, Item}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, GenericArgKind, Ty}; + +use super::EXPL_IMPL_CLONE_ON_COPY; + +/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint. +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { + let clone_id = match cx.tcx.lang_items().clone_trait() { + Some(id) if trait_ref.trait_def_id() == Some(id) => id, + _ => return, + }; + let Some(copy_id) = cx.tcx.lang_items().copy_trait() else { + return; + }; + let (ty_adt, ty_subs) = match *ty.kind() { + // Unions can't derive clone. + ty::Adt(adt, subs) if !adt.is_union() => (adt, subs), + _ => return, + }; + // If the current self type doesn't implement Copy (due to generic constraints), search to see if + // there's a Copy impl for any instance of the adt. + if !is_copy(cx, ty) { + if ty_subs.non_erasable_generics().next().is_some() { + let has_copy_impl = cx.tcx.local_trait_impls(copy_id).iter().any(|&id| { + matches!(cx.tcx.type_of(id).instantiate_identity().kind(), ty::Adt(adt, _) + if ty_adt.did() == adt.did()) + }); + if !has_copy_impl { + return; + } + } else { + return; + } + } + // Derive constrains all generic types to requiring Clone. Check if any type is not constrained for + // this impl. + if ty_subs.types().any(|ty| !implements_trait(cx, ty, clone_id, &[])) { + return; + } + // `#[repr(packed)]` structs with type/const parameters can't derive `Clone`. + // https://github.com/rust-lang/rust-clippy/issues/10188 + if ty_adt.repr().packed() + && ty_subs + .iter() + .any(|arg| matches!(arg.kind(), GenericArgKind::Type(_) | GenericArgKind::Const(_))) + { + return; + } + // The presence of `unsafe` fields prevents deriving `Clone` automatically + if ty_adt.all_fields().any(|f| f.safety.is_unsafe()) { + return; + } + + span_lint_and_note( + cx, + EXPL_IMPL_CLONE_ON_COPY, + item.span, + "you are implementing `Clone` explicitly on a `Copy` type", + Some(item.span), + "consider deriving `Clone` or removing `Copy`", + ); +} diff --git a/src/tools/clippy/clippy_lints/src/derive/mod.rs b/src/tools/clippy/clippy_lints/src/derive/mod.rs new file mode 100644 index 00000000000..1d63394ce37 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/derive/mod.rs @@ -0,0 +1,215 @@ +use rustc_hir::{Impl, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; + +mod derive_ord_xor_partial_ord; +mod derive_partial_eq_without_eq; +mod derived_hash_with_manual_eq; +mod expl_impl_clone_on_copy; +mod unsafe_derive_deserialize; + +declare_clippy_lint! { + /// ### What it does + /// Lints against manual `PartialEq` implementations for types with a derived `Hash` + /// implementation. + /// + /// ### Why is this bad? + /// The implementation of these traits must agree (for + /// example for use with `HashMap`) so it’s probably a bad idea to use a + /// default-generated `Hash` implementation with an explicitly defined + /// `PartialEq`. In particular, the following must hold for any type: + /// + /// ```text + /// k1 == k2 ⇒ hash(k1) == hash(k2) + /// ``` + /// + /// ### Example + /// ```ignore + /// #[derive(Hash)] + /// struct Foo; + /// + /// impl PartialEq for Foo { + /// ... + /// } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub DERIVED_HASH_WITH_MANUAL_EQ, + correctness, + "deriving `Hash` but implementing `PartialEq` explicitly" +} + +declare_clippy_lint! { + /// ### What it does + /// Lints against manual `PartialOrd` and `Ord` implementations for types with a derived `Ord` + /// or `PartialOrd` implementation. + /// + /// ### Why is this bad? + /// The implementation of these traits must agree (for + /// example for use with `sort`) so it’s probably a bad idea to use a + /// default-generated `Ord` implementation with an explicitly defined + /// `PartialOrd`. In particular, the following must hold for any type + /// implementing `Ord`: + /// + /// ```text + /// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap() + /// ``` + /// + /// ### Example + /// ```rust,ignore + /// #[derive(Ord, PartialEq, Eq)] + /// struct Foo; + /// + /// impl PartialOrd for Foo { + /// ... + /// } + /// ``` + /// Use instead: + /// ```rust,ignore + /// #[derive(PartialEq, Eq)] + /// struct Foo; + /// + /// impl PartialOrd for Foo { + /// fn partial_cmp(&self, other: &Foo) -> Option<Ordering> { + /// Some(self.cmp(other)) + /// } + /// } + /// + /// impl Ord for Foo { + /// ... + /// } + /// ``` + /// or, if you don't need a custom ordering: + /// ```rust,ignore + /// #[derive(Ord, PartialOrd, PartialEq, Eq)] + /// struct Foo; + /// ``` + #[clippy::version = "1.47.0"] + pub DERIVE_ORD_XOR_PARTIAL_ORD, + correctness, + "deriving `Ord` but implementing `PartialOrd` explicitly" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for explicit `Clone` implementations for `Copy` + /// types. + /// + /// ### Why is this bad? + /// To avoid surprising behavior, these traits should + /// agree and the behavior of `Copy` cannot be overridden. In almost all + /// situations a `Copy` type should have a `Clone` implementation that does + /// nothing more than copy the object, which is what `#[derive(Copy, Clone)]` + /// gets you. + /// + /// ### Example + /// ```rust,ignore + /// #[derive(Copy)] + /// struct Foo; + /// + /// impl Clone for Foo { + /// // .. + /// } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub EXPL_IMPL_CLONE_ON_COPY, + pedantic, + "implementing `Clone` explicitly on `Copy` types" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for deriving `serde::Deserialize` on a type that + /// has methods using `unsafe`. + /// + /// ### Why is this bad? + /// Deriving `serde::Deserialize` will create a constructor + /// that may violate invariants held by another constructor. + /// + /// ### Example + /// ```rust,ignore + /// use serde::Deserialize; + /// + /// #[derive(Deserialize)] + /// pub struct Foo { + /// // .. + /// } + /// + /// impl Foo { + /// pub fn new() -> Self { + /// // setup here .. + /// } + /// + /// pub unsafe fn parts() -> (&str, &str) { + /// // assumes invariants hold + /// } + /// } + /// ``` + #[clippy::version = "1.45.0"] + pub UNSAFE_DERIVE_DESERIALIZE, + pedantic, + "deriving `serde::Deserialize` on a type that has methods using `unsafe`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for types that derive `PartialEq` and could implement `Eq`. + /// + /// ### Why is this bad? + /// If a type `T` derives `PartialEq` and all of its members implement `Eq`, + /// then `T` can always implement `Eq`. Implementing `Eq` allows `T` to be used + /// in APIs that require `Eq` types. It also allows structs containing `T` to derive + /// `Eq` themselves. + /// + /// ### Example + /// ```no_run + /// #[derive(PartialEq)] + /// struct Foo { + /// i_am_eq: i32, + /// i_am_eq_too: Vec<String>, + /// } + /// ``` + /// Use instead: + /// ```no_run + /// #[derive(PartialEq, Eq)] + /// struct Foo { + /// i_am_eq: i32, + /// i_am_eq_too: Vec<String>, + /// } + /// ``` + #[clippy::version = "1.63.0"] + pub DERIVE_PARTIAL_EQ_WITHOUT_EQ, + nursery, + "deriving `PartialEq` on a type that can implement `Eq`, without implementing `Eq`" +} + +declare_lint_pass!(Derive => [ + EXPL_IMPL_CLONE_ON_COPY, + DERIVED_HASH_WITH_MANUAL_EQ, + DERIVE_ORD_XOR_PARTIAL_ORD, + UNSAFE_DERIVE_DESERIALIZE, + DERIVE_PARTIAL_EQ_WITHOUT_EQ +]); + +impl<'tcx> LateLintPass<'tcx> for Derive { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + if let ItemKind::Impl(Impl { + of_trait: Some(of_trait), + .. + }) = item.kind + { + let trait_ref = &of_trait.trait_ref; + let ty = cx.tcx.type_of(item.owner_id).instantiate_identity(); + let is_automatically_derived = cx.tcx.is_automatically_derived(item.owner_id.to_def_id()); + + derived_hash_with_manual_eq::check(cx, item.span, trait_ref, ty, is_automatically_derived); + derive_ord_xor_partial_ord::check(cx, item.span, trait_ref, ty, is_automatically_derived); + + if is_automatically_derived { + unsafe_derive_deserialize::check(cx, item, trait_ref, ty); + derive_partial_eq_without_eq::check(cx, item.span, trait_ref, ty); + } else { + expl_impl_clone_on_copy::check(cx, item, trait_ref, ty); + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/derive/unsafe_derive_deserialize.rs b/src/tools/clippy/clippy_lints/src/derive/unsafe_derive_deserialize.rs new file mode 100644 index 00000000000..c391e7b6228 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/derive/unsafe_derive_deserialize.rs @@ -0,0 +1,93 @@ +use std::ops::ControlFlow; + +use clippy_utils::diagnostics::span_lint_hir_and_then; +use clippy_utils::{is_lint_allowed, paths}; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn, walk_item}; +use rustc_hir::{self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, Item, UnsafeSource}; +use rustc_lint::LateContext; +use rustc_middle::hir::nested_filter; +use rustc_middle::ty::{self, Ty}; +use rustc_span::{Span, sym}; + +use super::UNSAFE_DERIVE_DESERIALIZE; + +/// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint. +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { + fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool { + let mut visitor = UnsafeVisitor { cx }; + walk_item(&mut visitor, item).is_break() + } + + if let Some(trait_def_id) = trait_ref.trait_def_id() + && paths::SERDE_DESERIALIZE.matches(cx, trait_def_id) + && let ty::Adt(def, _) = ty.kind() + && let Some(local_def_id) = def.did().as_local() + && let adt_hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id) + && !is_lint_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id) + && cx + .tcx + .inherent_impls(def.did()) + .iter() + .map(|imp_did| cx.tcx.hir_expect_item(imp_did.expect_local())) + .any(|imp| has_unsafe(cx, imp)) + { + span_lint_hir_and_then( + cx, + UNSAFE_DERIVE_DESERIALIZE, + adt_hir_id, + item.span, + "you are deriving `serde::Deserialize` on a type that has methods using `unsafe`", + |diag| { + diag.help( + "consider implementing `serde::Deserialize` manually. See https://serde.rs/impl-deserialize.html", + ); + }, + ); + } +} + +struct UnsafeVisitor<'a, 'tcx> { + cx: &'a LateContext<'tcx>, +} + +impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { + type Result = ControlFlow<()>; + type NestedFilter = nested_filter::All; + + fn visit_fn( + &mut self, + kind: FnKind<'tcx>, + decl: &'tcx FnDecl<'_>, + body_id: BodyId, + _: Span, + id: LocalDefId, + ) -> Self::Result { + if let Some(header) = kind.header() + && header.is_unsafe() + { + ControlFlow::Break(()) + } else { + walk_fn(self, kind, decl, body_id, id) + } + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) -> Self::Result { + if let ExprKind::Block(block, _) = expr.kind + && block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) + && block + .span + .source_callee() + .and_then(|expr| expr.macro_def_id) + .is_none_or(|did| !self.cx.tcx.is_diagnostic_item(sym::pin_macro, did)) + { + return ControlFlow::Break(()); + } + + walk_expr(self, expr) + } + + fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt { + self.cx.tcx + } +} diff --git a/src/tools/clippy/clippy_lints/src/disallowed_macros.rs b/src/tools/clippy/clippy_lints/src/disallowed_macros.rs index 49cd2671dc0..1c9c971730f 100644 --- a/src/tools/clippy/clippy_lints/src/disallowed_macros.rs +++ b/src/tools/clippy/clippy_lints/src/disallowed_macros.rs @@ -1,4 +1,3 @@ - use clippy_config::Conf; use clippy_config::types::{DisallowedPath, create_disallowed_map}; use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; @@ -8,7 +7,8 @@ use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefIdMap; use rustc_hir::{ - AmbigArg, Expr, ExprKind, ForeignItem, HirId, ImplItem, ImplItemImplKind, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty, + AmbigArg, Expr, ExprKind, ForeignItem, HirId, ImplItem, ImplItemImplKind, Item, ItemKind, OwnerId, Pat, Path, Stmt, + TraitItem, Ty, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::TyCtxt; diff --git a/src/tools/clippy/clippy_lints/src/eta_reduction.rs b/src/tools/clippy/clippy_lints/src/eta_reduction.rs index 2da1c2bad11..752f39b4e6d 100644 --- a/src/tools/clippy/clippy_lints/src/eta_reduction.rs +++ b/src/tools/clippy/clippy_lints/src/eta_reduction.rs @@ -197,6 +197,18 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx // in a type which is `'static`. // For now ignore all callee types which reference a type parameter. && !generic_args.types().any(|t| matches!(t.kind(), ty::Param(_))) + // Rule out `AsyncFn*`s, because while they can be called as `|x| f(x)`, + // they can't be passed directly into a place expecting an `Fn*` (#13892) + && let Ok((closure_kind, _)) = cx + .tcx + .infer_ctxt() + .build(cx.typing_mode()) + .err_ctxt() + .type_implements_fn_trait( + cx.param_env, + Binder::bind_with_vars(callee_ty_adjusted, List::empty()), + ty::PredicatePolarity::Positive, + ) { span_lint_hir_and_then( cx, @@ -213,19 +225,10 @@ fn check_closure<'tcx>(cx: &LateContext<'tcx>, outer_receiver: Option<&Expr<'tcx // 'cuz currently nothing changes after deleting this check. local_used_in(cx, l, args) || local_used_after_expr(cx, l, expr) }) { - match cx - .tcx - .infer_ctxt() - .build(cx.typing_mode()) - .err_ctxt() - .type_implements_fn_trait( - cx.param_env, - Binder::bind_with_vars(callee_ty_adjusted, List::empty()), - ty::PredicatePolarity::Positive, - ) { + match closure_kind { // Mutable closure is used after current expr; we cannot consume it. - Ok((ClosureKind::FnMut, _)) => snippet = format!("&mut {snippet}"), - Ok((ClosureKind::Fn, _)) if !callee_ty_raw.is_ref() => { + ClosureKind::FnMut => snippet = format!("&mut {snippet}"), + ClosureKind::Fn if !callee_ty_raw.is_ref() => { snippet = format!("&{snippet}"); }, _ => (), diff --git a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs index 906bbd006d4..72f87953040 100644 --- a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs +++ b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs @@ -4,7 +4,7 @@ use rustc_middle::ty; use rustc_span::def_id::LocalDefId; use clippy_utils::diagnostics::span_lint; -use clippy_utils::ty::type_is_unsafe_function; +use clippy_utils::ty::is_unsafe_fn; use clippy_utils::visitors::for_each_expr; use clippy_utils::{iter_input_pats, path_to_local}; @@ -51,7 +51,7 @@ fn check_raw_ptr<'tcx>( let typeck = cx.tcx.typeck_body(body.id()); let _: Option<!> = for_each_expr(cx, body.value, |e| { match e.kind { - hir::ExprKind::Call(f, args) if type_is_unsafe_function(cx, typeck.expr_ty(f)) => { + hir::ExprKind::Call(f, args) if is_unsafe_fn(cx, typeck.expr_ty(f)) => { for arg in args { check_arg(cx, &raw_ptrs, arg); } diff --git a/src/tools/clippy/clippy_lints/src/functions/ref_option.rs b/src/tools/clippy/clippy_lints/src/functions/ref_option.rs index 106202d00d4..5dc1b7269b7 100644 --- a/src/tools/clippy/clippy_lints/src/functions/ref_option.rs +++ b/src/tools/clippy/clippy_lints/src/functions/ref_option.rs @@ -1,23 +1,20 @@ use crate::functions::REF_OPTION; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::is_trait_impl_item; use clippy_utils::source::snippet; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::ty::option_arg_ty; +use clippy_utils::{is_from_proc_macro, is_trait_impl_item}; use rustc_errors::Applicability; -use rustc_hir as hir; use rustc_hir::intravisit::FnKind; -use rustc_hir::{FnDecl, HirId}; -use rustc_lint::LateContext; -use rustc_middle::ty::{self, GenericArgKind, Mutability, Ty}; +use rustc_hir::{self as hir, FnDecl, HirId}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::ty::{self, Mutability, Ty}; +use rustc_span::Span; use rustc_span::def_id::LocalDefId; -use rustc_span::{Span, sym}; -fn check_ty<'a>(cx: &LateContext<'a>, param: &rustc_hir::Ty<'a>, param_ty: Ty<'a>, fixes: &mut Vec<(Span, String)>) { - if let ty::Ref(_, opt_ty, Mutability::Not) = param_ty.kind() - && is_type_diagnostic_item(cx, *opt_ty, sym::Option) - && let ty::Adt(_, opt_gen_args) = opt_ty.kind() - && let [gen_arg] = opt_gen_args.as_slice() - && let GenericArgKind::Type(gen_ty) = gen_arg.kind() +fn check_ty<'a>(cx: &LateContext<'a>, param: &hir::Ty<'a>, param_ty: Ty<'a>, fixes: &mut Vec<(Span, String)>) { + if !param.span.in_external_macro(cx.sess().source_map()) + && let ty::Ref(_, opt_ty, Mutability::Not) = param_ty.kind() + && let Some(gen_ty) = option_arg_ty(cx, *opt_ty) && !gen_ty.is_ref() // Need to gen the original spans, so first parsing mid, and hir parsing afterward && let hir::TyKind::Ref(lifetime, hir::MutTy { ty, .. }) = param.kind @@ -27,6 +24,7 @@ fn check_ty<'a>(cx: &LateContext<'a>, param: &rustc_hir::Ty<'a>, param_ty: Ty<'a args: [hir::GenericArg::Type(opt_ty)], .. }) = last.args + && !is_from_proc_macro(cx, param) { let lifetime = snippet(cx, lifetime.ident.span, ".."); fixes.push(( @@ -67,21 +65,24 @@ fn check_fn_sig<'a>(cx: &LateContext<'a>, decl: &FnDecl<'a>, span: Span, sig: ty #[allow(clippy::too_many_arguments)] pub(crate) fn check_fn<'a>( cx: &LateContext<'a>, - kind: FnKind<'_>, + kind: FnKind<'a>, decl: &FnDecl<'a>, span: Span, hir_id: HirId, def_id: LocalDefId, - body: &hir::Body<'_>, + body: &hir::Body<'a>, avoid_breaking_exported_api: bool, ) { if avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) { return; } + if span.in_external_macro(cx.sess().source_map()) { + return; + } if let FnKind::Closure = kind { // Compute the span of the closure parameters + return type if set - let span = if let hir::FnRetTy::Return(out_ty) = &decl.output { + let inputs_output_span = if let hir::FnRetTy::Return(out_ty) = &decl.output { if decl.inputs.is_empty() { out_ty.span } else { @@ -100,9 +101,18 @@ pub(crate) fn check_fn<'a>( }; let sig = args.as_closure().sig().skip_binder(); - check_fn_sig(cx, decl, span, sig); + if is_from_proc_macro(cx, &(&kind, body, hir_id, span)) { + return; + } + + check_fn_sig(cx, decl, inputs_output_span, sig); } else if !is_trait_impl_item(cx, hir_id) { let sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder(); + + if is_from_proc_macro(cx, &(&kind, body, hir_id, span)) { + return; + } + check_fn_sig(cx, decl, span, sig); } } @@ -112,8 +122,10 @@ pub(super) fn check_trait_item<'a>( trait_item: &hir::TraitItem<'a>, avoid_breaking_exported_api: bool, ) { - if let hir::TraitItemKind::Fn(ref sig, _) = trait_item.kind + if !trait_item.span.in_external_macro(cx.sess().source_map()) + && let hir::TraitItemKind::Fn(ref sig, _) = trait_item.kind && !(avoid_breaking_exported_api && cx.effective_visibilities.is_exported(trait_item.owner_id.def_id)) + && !is_from_proc_macro(cx, trait_item) { let def_id = trait_item.owner_id.def_id; let ty_sig = cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder(); diff --git a/src/tools/clippy/clippy_lints/src/future_not_send.rs b/src/tools/clippy/clippy_lints/src/future_not_send.rs index 3ccfa51ab70..596047977a9 100644 --- a/src/tools/clippy/clippy_lints/src/future_not_send.rs +++ b/src/tools/clippy/clippy_lints/src/future_not_send.rs @@ -78,66 +78,65 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend { if let ty::Alias(ty::Opaque, AliasTy { def_id, args, .. }) = *ret_ty.kind() && let Some(future_trait) = cx.tcx.lang_items().future_trait() && let Some(send_trait) = cx.tcx.get_diagnostic_item(sym::Send) + && let preds = cx.tcx.explicit_item_self_bounds(def_id) + // If is a Future + && preds + .iter_instantiated_copied(cx.tcx, args) + .filter_map(|(p, _)| p.as_trait_clause()) + .any(|trait_pred| trait_pred.skip_binder().trait_ref.def_id == future_trait) { - let preds = cx.tcx.explicit_item_self_bounds(def_id); - let is_future = preds.iter_instantiated_copied(cx.tcx, args).any(|(p, _)| { - p.as_trait_clause() - .is_some_and(|trait_pred| trait_pred.skip_binder().trait_ref.def_id == future_trait) - }); - if is_future { - let span = decl.output.span(); - let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode()); - let ocx = ObligationCtxt::new_with_diagnostics(&infcx); - let cause = traits::ObligationCause::misc(span, fn_def_id); - ocx.register_bound(cause, cx.param_env, ret_ty, send_trait); - let send_errors = ocx.select_all_or_error(); + let span = decl.output.span(); + let infcx = cx.tcx.infer_ctxt().build(cx.typing_mode()); + let ocx = ObligationCtxt::new_with_diagnostics(&infcx); + let cause = traits::ObligationCause::misc(span, fn_def_id); + ocx.register_bound(cause, cx.param_env, ret_ty, send_trait); + let send_errors = ocx.select_all_or_error(); - // Allow errors that try to prove `Send` for types that "mention" a generic parameter at the "top - // level". - // For example, allow errors that `T: Send` can't be proven, but reject `Rc<T>: Send` errors, - // which is always unconditionally `!Send` for any possible type `T`. - // - // We also allow associated type projections if the self type is either itself a projection or a - // type parameter. - // This is to prevent emitting warnings for e.g. holding a `<Fut as Future>::Output` across await - // points, where `Fut` is a type parameter. + // Allow errors that try to prove `Send` for types that "mention" a generic parameter at the "top + // level". + // For example, allow errors that `T: Send` can't be proven, but reject `Rc<T>: Send` errors, + // which is always unconditionally `!Send` for any possible type `T`. + // + // We also allow associated type projections if the self type is either itself a projection or a + // type parameter. + // This is to prevent emitting warnings for e.g. holding a `<Fut as Future>::Output` across await + // points, where `Fut` is a type parameter. - let is_send = send_errors.iter().all(|err| { - err.obligation - .predicate - .as_trait_clause() - .map(Binder::skip_binder) - .is_some_and(|pred| { - pred.def_id() == send_trait - && pred.self_ty().has_param() - && TyParamAtTopLevelVisitor.visit_ty(pred.self_ty()) == ControlFlow::Break(true) - }) - }); + let is_send = send_errors.iter().all(|err| { + err.obligation + .predicate + .as_trait_clause() + .map(Binder::skip_binder) + .is_some_and(|pred| { + pred.def_id() == send_trait + && pred.self_ty().has_param() + && TyParamAtTopLevelVisitor.visit_ty(pred.self_ty()) == ControlFlow::Break(true) + }) + }); - if !is_send { - span_lint_and_then( - cx, - FUTURE_NOT_SEND, - span, - "future cannot be sent between threads safely", - |db| { - for FulfillmentError { obligation, .. } in send_errors { - infcx - .err_ctxt() - .maybe_note_obligation_cause_for_async_await(db, &obligation); - if let PredicateKind::Clause(ClauseKind::Trait(trait_pred)) = - obligation.predicate.kind().skip_binder() - { - db.note(format!( - "`{}` doesn't implement `{}`", - trait_pred.self_ty(), - trait_pred.trait_ref.print_only_trait_path(), - )); - } + if !is_send { + span_lint_and_then( + cx, + FUTURE_NOT_SEND, + span, + "future cannot be sent between threads safely", + |db| { + for FulfillmentError { obligation, .. } in send_errors { + infcx + .err_ctxt() + .maybe_note_obligation_cause_for_async_await(db, &obligation); + if let PredicateKind::Clause(ClauseKind::Trait(trait_pred)) = + obligation.predicate.kind().skip_binder() + { + db.note(format!( + "`{}` doesn't implement `{}`", + trait_pred.self_ty(), + trait_pred.trait_ref.print_only_trait_path(), + )); } - }, - ); - } + } + }, + ); } } } diff --git a/src/tools/clippy/clippy_lints/src/invalid_upcast_comparisons.rs b/src/tools/clippy/clippy_lints/src/invalid_upcast_comparisons.rs index b0ecc5d52dd..1666e8e5ae3 100644 --- a/src/tools/clippy/clippy_lints/src/invalid_upcast_comparisons.rs +++ b/src/tools/clippy/clippy_lints/src/invalid_upcast_comparisons.rs @@ -1,3 +1,4 @@ +use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::layout::LayoutOf; @@ -9,7 +10,7 @@ use clippy_utils::comparisons; use clippy_utils::comparisons::Rel; use clippy_utils::consts::{ConstEvalCtxt, FullInt}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::source::snippet; +use clippy_utils::source::snippet_with_context; declare_clippy_lint! { /// ### What it does @@ -69,13 +70,21 @@ fn numeric_cast_precast_bounds(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option< fn err_upcast_comparison(cx: &LateContext<'_>, span: Span, expr: &Expr<'_>, always: bool) { if let ExprKind::Cast(cast_val, _) = expr.kind { + let mut applicability = Applicability::MachineApplicable; + let (cast_val_snip, _) = snippet_with_context( + cx, + cast_val.span, + expr.span.ctxt(), + "the expression", + &mut applicability, + ); span_lint( cx, INVALID_UPCAST_COMPARISONS, span, format!( "because of the numeric bounds on `{}` prior to casting, this expression is always {}", - snippet(cx, cast_val.span, "the expression"), + cast_val_snip, if always { "true" } else { "false" }, ), ); diff --git a/src/tools/clippy/clippy_lints/src/len_zero.rs b/src/tools/clippy/clippy_lints/src/len_zero.rs index f44a5fdf715..28a0fbc0511 100644 --- a/src/tools/clippy/clippy_lints/src/len_zero.rs +++ b/src/tools/clippy/clippy_lints/src/len_zero.rs @@ -371,9 +371,9 @@ fn parse_len_output<'tcx>(cx: &LateContext<'tcx>, sig: FnSig<'tcx>) -> Option<Le match *sig.output().kind() { ty::Int(_) | ty::Uint(_) => Some(LenOutput::Integral), - ty::Adt(adt, subs) if subs.type_at(0).is_integral() => match cx.tcx.get_diagnostic_name(adt.did()) { - Some(sym::Option) => Some(LenOutput::Option(adt.did())), - Some(sym::Result) => Some(LenOutput::Result(adt.did())), + ty::Adt(adt, subs) => match cx.tcx.get_diagnostic_name(adt.did()) { + Some(sym::Option) => subs.type_at(0).is_integral().then(|| LenOutput::Option(adt.did())), + Some(sym::Result) => subs.type_at(0).is_integral().then(|| LenOutput::Result(adt.did())), _ => None, }, _ => None, diff --git a/src/tools/clippy/clippy_lints/src/let_with_type_underscore.rs b/src/tools/clippy/clippy_lints/src/let_with_type_underscore.rs index 5b0f95ffc37..24a4c321bda 100644 --- a/src/tools/clippy/clippy_lints/src/let_with_type_underscore.rs +++ b/src/tools/clippy/clippy_lints/src/let_with_type_underscore.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_from_proc_macro; use clippy_utils::source::{IntoSpan, SpanRangeExt}; +use rustc_ast::{Local, TyKind}; use rustc_errors::Applicability; -use rustc_hir::{LetStmt, TyKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; use rustc_session::declare_lint_pass; declare_clippy_lint! { @@ -26,14 +26,14 @@ declare_clippy_lint! { } declare_lint_pass!(UnderscoreTyped => [LET_WITH_TYPE_UNDERSCORE]); -impl<'tcx> LateLintPass<'tcx> for UnderscoreTyped { - fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx LetStmt<'_>) { - if let Some(ty) = local.ty // Ensure that it has a type defined - && let TyKind::Infer(()) = &ty.kind // that type is '_' +impl EarlyLintPass for UnderscoreTyped { + fn check_local(&mut self, cx: &EarlyContext<'_>, local: &Local) { + if let Some(ty) = &local.ty // Ensure that it has a type defined + && let TyKind::Infer = ty.kind // that type is '_' && local.span.eq_ctxt(ty.span) - && let sm = cx.tcx.sess.source_map() + && let sm = cx.sess().source_map() && !local.span.in_external_macro(sm) - && !is_from_proc_macro(cx, ty) + && !is_from_proc_macro(cx, &**ty) { let span_to_remove = sm .span_extend_to_prev_char_before(ty.span, ':', true) diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index a89cf3fdc1e..c56fa257b06 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -302,7 +302,6 @@ mod permissions_set_readonly_false; mod pointers_in_nomem_asm_block; mod precedence; mod ptr; -mod ptr_offset_with_cast; mod pub_underscore_fields; mod pub_use; mod question_mark; @@ -360,6 +359,7 @@ mod temporary_assignment; mod tests_outside_test_module; mod to_digit_is_some; mod to_string_trait_impl; +mod toplevel_ref_arg; mod trailing_empty_array; mod trait_bounds; mod transmute; @@ -592,7 +592,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(unwrap::Unwrap)); store.register_late_pass(move |_| Box::new(indexing_slicing::IndexingSlicing::new(conf))); store.register_late_pass(move |tcx| Box::new(non_copy_const::NonCopyConst::new(tcx, conf))); - store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast)); store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone)); store.register_late_pass(|_| Box::new(slow_vector_initialization::SlowVectorInit)); store.register_late_pass(move |_| Box::new(unnecessary_wraps::UnnecessaryWraps::new(conf))); @@ -744,7 +743,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(missing_assert_message::MissingAssertMessage)); store.register_late_pass(|_| Box::new(needless_maybe_sized::NeedlessMaybeSized)); store.register_late_pass(|_| Box::new(redundant_async_block::RedundantAsyncBlock)); - store.register_late_pass(|_| Box::new(let_with_type_underscore::UnderscoreTyped)); + store.register_early_pass(|| Box::new(let_with_type_underscore::UnderscoreTyped)); store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(conf))); store.register_late_pass(|_| Box::new(unnecessary_struct_initialization::UnnecessaryStruct)); store.register_late_pass(move |_| Box::new(unnecessary_box_returns::UnnecessaryBoxReturns::new(conf))); @@ -831,5 +830,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf))); store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom)); store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny)); + store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/src/tools/clippy/clippy_lints/src/lifetimes.rs b/src/tools/clippy/clippy_lints/src/lifetimes.rs index 149ae5e710c..d8b186b6787 100644 --- a/src/tools/clippy/clippy_lints/src/lifetimes.rs +++ b/src/tools/clippy/clippy_lints/src/lifetimes.rs @@ -745,7 +745,7 @@ fn report_elidable_impl_lifetimes<'tcx>( impl_: &'tcx Impl<'_>, map: &FxIndexMap<LocalDefId, Vec<Usage>>, ) { - let single_usages = map + let (elidable_lts, usages): (Vec<_>, Vec<_>) = map .iter() .filter_map(|(def_id, usages)| { if let [ @@ -762,14 +762,12 @@ fn report_elidable_impl_lifetimes<'tcx>( None } }) - .collect::<Vec<_>>(); + .unzip(); - if single_usages.is_empty() { + if elidable_lts.is_empty() { return; } - let (elidable_lts, usages): (Vec<_>, Vec<_>) = single_usages.into_iter().unzip(); - report_elidable_lifetimes(cx, impl_.generics, &elidable_lts, &usages, true); } @@ -795,9 +793,7 @@ fn report_elidable_lifetimes( // In principle, the result of the call to `Node::ident` could be `unwrap`ped, as `DefId` should refer to a // `Node::GenericParam`. .filter_map(|&def_id| cx.tcx.hir_node_by_def_id(def_id).ident()) - .map(|ident| ident.to_string()) - .collect::<Vec<_>>() - .join(", "); + .format(", "); let elidable_usages: Vec<ElidableUsage> = usages .iter() @@ -860,36 +856,89 @@ fn elision_suggestions( .filter(|param| !param.is_elided_lifetime() && !param.is_impl_trait()) .collect::<Vec<_>>(); - let mut suggestions = if elidable_lts.len() == explicit_params.len() { + if !elidable_lts + .iter() + .all(|lt| explicit_params.iter().any(|param| param.def_id == *lt)) + { + return None; + } + + let mut suggestions = if elidable_lts.is_empty() { + vec![] + } else if elidable_lts.len() == explicit_params.len() { // if all the params are elided remove the whole generic block // // fn x<'a>() {} // ^^^^ vec![(generics.span, String::new())] } else { - elidable_lts - .iter() - .map(|&id| { - let pos = explicit_params.iter().position(|param| param.def_id == id)?; - let param = explicit_params.get(pos)?; - - let span = if let Some(next) = explicit_params.get(pos + 1) { - // fn x<'prev, 'a, 'next>() {} - // ^^^^ - param.span.until(next.span) + match &explicit_params[..] { + // no params, nothing to elide + [] => unreachable!("handled by `elidable_lts.is_empty()`"), + [param] => { + if elidable_lts.contains(¶m.def_id) { + unreachable!("handled by `elidable_lts.len() == explicit_params.len()`") } else { - // `pos` should be at least 1 here, because the param in position 0 would either have a `next` - // param or would have taken the `elidable_lts.len() == explicit_params.len()` branch. - let prev = explicit_params.get(pos - 1)?; - - // fn x<'prev, 'a>() {} - // ^^^^ - param.span.with_lo(prev.span.hi()) + unreachable!("handled by `elidable_lts.is_empty()`") + } + }, + [_, _, ..] => { + // Given a list like `<'a, 'b, 'c, 'd, ..>`, + // + // If there is a cluster of elidable lifetimes at the beginning, say `'a` and `'b`, we should + // suggest removing them _and_ the trailing comma. The span for that is `a.span.until(c.span)`: + // <'a, 'b, 'c, 'd, ..> => <'a, 'b, 'c, 'd, ..> + // ^^ ^^ ^^^^^^^^ + // + // And since we know that `'c` isn't elidable--otherwise it would've been in the cluster--we can go + // over all the lifetimes after it, and for each elidable one, add a suggestion spanning the + // lifetime itself and the comma before, because each individual suggestion is guaranteed to leave + // the list valid: + // <.., 'c, 'd, 'e, 'f, 'g, ..> => <.., 'c, 'd, 'e, 'f, 'g, ..> + // ^^ ^^ ^^ ^^^^ ^^^^^^^^ + // + // In case there is no such starting cluster, we only need to do the second part of the algorithm: + // <'a, 'b, 'c, 'd, 'e, 'f, 'g, ..> => <'a, 'b , 'c, 'd, 'e, 'f, 'g, ..> + // ^^ ^^ ^^ ^^ ^^^^^^^^^ ^^^^^^^^ + + // Split off the starting cluster + // TODO: use `slice::split_once` once stabilized (github.com/rust-lang/rust/issues/112811): + // ``` + // let Some(split) = explicit_params.split_once(|param| !elidable_lts.contains(¶m.def_id)) else { + // // there were no lifetime param that couldn't be elided + // unreachable!("handled by `elidable_lts.len() == explicit_params.len()`") + // }; + // match split { /* .. */ } + // ``` + let Some(split_pos) = explicit_params + .iter() + .position(|param| !elidable_lts.contains(¶m.def_id)) + else { + // there were no lifetime param that couldn't be elided + unreachable!("handled by `elidable_lts.len() == explicit_params.len()`") }; - - Some((span, String::new())) - }) - .collect::<Option<Vec<_>>>()? + let split = explicit_params + .split_at_checked(split_pos) + .expect("got `split_pos` from `position` on the same Vec"); + + match split { + ([..], []) => unreachable!("handled by `elidable_lts.len() == explicit_params.len()`"), + ([], [_]) => unreachable!("handled by `explicit_params.len() == 1`"), + (cluster, rest @ [rest_first, ..]) => { + // the span for the cluster + (cluster.first().map(|fw| fw.span.until(rest_first.span)).into_iter()) + // the span for the remaining lifetimes (calculations independent of the cluster) + .chain( + rest.array_windows() + .filter(|[_, curr]| elidable_lts.contains(&curr.def_id)) + .map(|[prev, curr]| curr.span.with_lo(prev.span.hi())), + ) + .map(|sp| (sp, String::new())) + .collect() + }, + } + }, + } }; suggestions.extend(usages.iter().map(|&usage| { diff --git a/src/tools/clippy/clippy_lints/src/loops/explicit_iter_loop.rs b/src/tools/clippy/clippy_lints/src/loops/explicit_iter_loop.rs index 6ce7f0b1f0b..af475c40586 100644 --- a/src/tools/clippy/clippy_lints/src/loops/explicit_iter_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/explicit_iter_loop.rs @@ -138,9 +138,9 @@ fn is_ref_iterable<'tcx>( return Some((AdjustKind::None, self_ty)); } - let res_ty = cx - .tcx - .erase_and_anonymize_regions(EarlyBinder::bind(req_res_ty).instantiate(cx.tcx, typeck.node_args(call_expr.hir_id))); + let res_ty = cx.tcx.erase_and_anonymize_regions( + EarlyBinder::bind(req_res_ty).instantiate(cx.tcx, typeck.node_args(call_expr.hir_id)), + ); let mutbl = if let ty::Ref(_, _, mutbl) = *req_self_ty.kind() { Some(mutbl) } else { diff --git a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs index 2ccff768097..544c3c34d02 100644 --- a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs @@ -22,7 +22,10 @@ pub(super) fn check<'tcx>( for_loop: Option<&ForLoop<'_>>, ) { match never_loop_block(cx, block, &mut Vec::new(), loop_id) { - NeverLoopResult::Diverging { ref break_spans } => { + NeverLoopResult::Diverging { + ref break_spans, + ref never_spans, + } => { span_lint_and_then(cx, NEVER_LOOP, span, "this loop never actually loops", |diag| { if let Some(ForLoop { arg: iterator, @@ -34,12 +37,16 @@ pub(super) fn check<'tcx>( { // If the block contains a break or continue, or if the loop has a label, `MachineApplicable` is not // appropriate. - let app = if !contains_any_break_or_continue(block) && label.is_none() { + let mut app = if !contains_any_break_or_continue(block) && label.is_none() { Applicability::MachineApplicable } else { Applicability::Unspecified }; + if !never_spans.is_empty() { + app = Applicability::HasPlaceholders; + } + let mut suggestions = vec![( for_span.with_hi(iterator.span.hi()), for_to_if_let_sugg(cx, iterator, pat), @@ -51,6 +58,13 @@ pub(super) fn check<'tcx>( suggestions, app, ); + + for span in never_spans { + diag.span_help( + *span, + "this code is unreachable. Consider moving the reachable parts out", + ); + } } }); }, @@ -77,13 +91,16 @@ fn contains_any_break_or_continue(block: &Block<'_>) -> bool { /// The first two bits of information are in this enum, and the last part is in the /// `local_labels` variable, which contains a list of `(block_id, reachable)` pairs ordered by /// scope. -#[derive(Clone)] +#[derive(Clone, Debug)] enum NeverLoopResult { /// A continue may occur for the main loop. MayContinueMainLoop, /// We have not encountered any main loop continue, /// but we are diverging (subsequent control flow is not reachable) - Diverging { break_spans: Vec<Span> }, + Diverging { + break_spans: Vec<Span>, + never_spans: Vec<Span>, + }, /// We have not encountered any main loop continue, /// and subsequent control flow is (possibly) reachable Normal, @@ -128,14 +145,18 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult ( NeverLoopResult::Diverging { break_spans: mut break_spans1, + never_spans: mut never_spans1, }, NeverLoopResult::Diverging { break_spans: mut break_spans2, + never_spans: mut never_spans2, }, ) => { break_spans1.append(&mut break_spans2); + never_spans1.append(&mut never_spans2); NeverLoopResult::Diverging { break_spans: break_spans1, + never_spans: never_spans1, } }, } @@ -207,6 +228,8 @@ fn all_spans_after_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> Vec<Span> { } return vec![stmt.span]; + } else if let Node::Block(_) = cx.tcx.parent_hir_node(expr.hir_id) { + return vec![expr.span]; } vec![] @@ -270,10 +293,13 @@ fn never_loop_expr<'tcx>( ExprKind::Match(e, arms, _) => { let e = never_loop_expr(cx, e, local_labels, main_loop_id); combine_seq(e, || { - arms.iter() - .fold(NeverLoopResult::Diverging { break_spans: vec![] }, |a, b| { - combine_branches(a, never_loop_expr(cx, b.body, local_labels, main_loop_id)) - }) + arms.iter().fold( + NeverLoopResult::Diverging { + break_spans: vec![], + never_spans: vec![], + }, + |a, b| combine_branches(a, never_loop_expr(cx, b.body, local_labels, main_loop_id)), + ) }) }, ExprKind::Block(b, _) => { @@ -296,6 +322,7 @@ fn never_loop_expr<'tcx>( } else { NeverLoopResult::Diverging { break_spans: all_spans_after_expr(cx, expr), + never_spans: vec![], } } }, @@ -306,7 +333,10 @@ fn never_loop_expr<'tcx>( combine_seq(first, || { // checks if break targets a block instead of a loop mark_block_as_reachable(expr, local_labels); - NeverLoopResult::Diverging { break_spans: vec![] } + NeverLoopResult::Diverging { + break_spans: vec![], + never_spans: vec![], + } }) }, ExprKind::Break(dest, e) => { @@ -322,11 +352,15 @@ fn never_loop_expr<'tcx>( } else { all_spans_after_expr(cx, expr) }, + never_spans: vec![], } }) }, ExprKind::Become(e) => combine_seq(never_loop_expr(cx, e, local_labels, main_loop_id), || { - NeverLoopResult::Diverging { break_spans: vec![] } + NeverLoopResult::Diverging { + break_spans: vec![], + never_spans: vec![], + } }), ExprKind::InlineAsm(asm) => combine_seq_many(asm.operands.iter().map(|(o, _)| match o { InlineAsmOperand::In { expr, .. } | InlineAsmOperand::InOut { expr, .. } => { @@ -356,7 +390,10 @@ fn never_loop_expr<'tcx>( }; let result = combine_seq(result, || { if cx.typeck_results().expr_ty(expr).is_never() { - NeverLoopResult::Diverging { break_spans: vec![] } + NeverLoopResult::Diverging { + break_spans: vec![], + never_spans: all_spans_after_expr(cx, expr), + } } else { NeverLoopResult::Normal } diff --git a/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs b/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs index 288f27db8ca..5814b6815a1 100644 --- a/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs +++ b/src/tools/clippy/clippy_lints/src/manual_abs_diff.rs @@ -4,8 +4,8 @@ use clippy_utils::higher::If; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::HasSession as _; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{eq_expr_value, peel_blocks, peel_middle_ty_refs, span_contains_comment}; +use clippy_utils::ty::{is_type_diagnostic_item, peel_and_count_ty_refs}; +use clippy_utils::{eq_expr_value, peel_blocks, span_contains_comment}; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -107,7 +107,7 @@ impl ManualAbsDiff { |ty| is_type_diagnostic_item(cx, ty, sym::Duration) && self.msrv.meets(cx, msrvs::DURATION_ABS_DIFF); let a_ty = cx.typeck_results().expr_ty(a).peel_refs(); - let (b_ty, b_n_refs) = peel_middle_ty_refs(cx.typeck_results().expr_ty(b)); + let (b_ty, b_n_refs, _) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(b)); (a_ty == b_ty && (is_int(a_ty) || is_duration(a_ty))).then_some((a_ty, b_n_refs)) } diff --git a/src/tools/clippy/clippy_lints/src/manual_option_as_slice.rs b/src/tools/clippy/clippy_lints/src/manual_option_as_slice.rs index 922db174e3d..b036e78cded 100644 --- a/src/tools/clippy/clippy_lints/src/manual_option_as_slice.rs +++ b/src/tools/clippy/clippy_lints/src/manual_option_as_slice.rs @@ -1,7 +1,7 @@ use clippy_config::Conf; use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; use clippy_utils::msrvs::Msrv; -use clippy_utils::{is_none_arm, msrvs, peel_hir_expr_refs, sym}; +use clippy_utils::{is_none_pattern, msrvs, peel_hir_expr_refs, sym}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Arm, Expr, ExprKind, LangItem, Pat, PatKind, QPath, is_range_literal}; @@ -60,8 +60,8 @@ impl LateLintPass<'_> for ManualOptionAsSlice { } match expr.kind { ExprKind::Match(scrutinee, [arm1, arm2], _) => { - if is_none_arm(cx, arm2) && check_arms(cx, arm2, arm1) - || is_none_arm(cx, arm1) && check_arms(cx, arm1, arm2) + if is_none_pattern(cx, arm2.pat) && check_arms(cx, arm2, arm1) + || is_none_pattern(cx, arm1.pat) && check_arms(cx, arm1, arm2) { check_as_ref(cx, scrutinee, span, self.msrv); } diff --git a/src/tools/clippy/clippy_lints/src/manual_slice_size_calculation.rs b/src/tools/clippy/clippy_lints/src/manual_slice_size_calculation.rs index 0c09a47c965..de12fa29d02 100644 --- a/src/tools/clippy/clippy_lints/src/manual_slice_size_calculation.rs +++ b/src/tools/clippy/clippy_lints/src/manual_slice_size_calculation.rs @@ -2,6 +2,7 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::snippet_with_context; +use clippy_utils::ty::peel_and_count_ty_refs; use clippy_utils::{expr_or_init, is_in_const_context, std_or_core}; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; @@ -102,7 +103,7 @@ fn simplify_half<'tcx>( && let ExprKind::MethodCall(method_path, receiver, [], _) = expr1.kind && method_path.ident.name == sym::len && let receiver_ty = cx.typeck_results().expr_ty(receiver) - && let (receiver_ty, refs_count) = clippy_utils::ty::walk_ptrs_ty_depth(receiver_ty) + && let (receiver_ty, refs_count, _) = peel_and_count_ty_refs(receiver_ty) && let ty::Slice(ty1) = receiver_ty.kind() // expr2 is `size_of::<T2>()`? && let ExprKind::Call(func, []) = expr2.kind diff --git a/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs b/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs index edbb556fd97..a8490d6aa7d 100644 --- a/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs +++ b/src/tools/clippy/clippy_lints/src/matches/manual_ok_err.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::{option_arg_ty, peel_mid_ty_refs_is_mutable}; +use clippy_utils::ty::{option_arg_ty, peel_and_count_ty_refs}; use clippy_utils::{get_parent_expr, is_res_lang_ctor, path_res, peel_blocks, span_contains_comment}; use rustc_ast::{BindingMode, Mutability}; use rustc_errors::Applicability; @@ -135,15 +135,11 @@ fn apply_lint(cx: &LateContext<'_>, expr: &Expr<'_>, scrutinee: &Expr<'_>, is_ok let scrut = Sugg::hir_with_applicability(cx, scrutinee, "..", &mut app).maybe_paren(); let scrutinee_ty = cx.typeck_results().expr_ty(scrutinee); - let (_, n_ref, mutability) = peel_mid_ty_refs_is_mutable(scrutinee_ty); - let prefix = if n_ref > 0 { - if mutability == Mutability::Mut { - ".as_mut()" - } else { - ".as_ref()" - } - } else { - "" + let (_, _, mutability) = peel_and_count_ty_refs(scrutinee_ty); + let prefix = match mutability { + Some(Mutability::Mut) => ".as_mut()", + Some(Mutability::Not) => ".as_ref()", + None => "", }; let sugg = format!("{scrut}{prefix}.{method}()"); diff --git a/src/tools/clippy/clippy_lints/src/matches/manual_utils.rs b/src/tools/clippy/clippy_lints/src/matches/manual_utils.rs index dbae71bbb1b..d4bfdb7e440 100644 --- a/src/tools/clippy/clippy_lints/src/matches/manual_utils.rs +++ b/src/tools/clippy/clippy_lints/src/matches/manual_utils.rs @@ -2,7 +2,7 @@ use crate::map_unit_fn::OPTION_MAP_UNIT_FN; use crate::matches::MATCH_AS_REF; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::{is_copy, is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function}; +use clippy_utils::ty::{is_copy, is_type_diagnostic_item, is_unsafe_fn, peel_and_count_ty_refs}; use clippy_utils::{ CaptureKind, can_move_expr_to_closure, expr_requires_coercion, is_else_clause, is_lint_allowed, is_res_lang_ctor, path_res, path_to_local_id, peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, @@ -30,8 +30,9 @@ pub(super) fn check_with<'tcx, F>( where F: Fn(&LateContext<'tcx>, &'tcx Pat<'_>, &'tcx Expr<'_>, SyntaxContext) -> Option<SomeExpr<'tcx>>, { - let (scrutinee_ty, ty_ref_count, ty_mutability) = - peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee)); + let (scrutinee_ty, ty_ref_count, ty_mutability) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(scrutinee)); + let ty_mutability = ty_mutability.unwrap_or(Mutability::Mut); + if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::Option) && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Option)) { @@ -191,7 +192,7 @@ fn can_pass_as_func<'tcx>(cx: &LateContext<'tcx>, binding: HirId, expr: &'tcx Ex ExprKind::Call(func, [arg]) if path_to_local_id(arg, binding) && cx.typeck_results().expr_adjustments(arg).is_empty() - && !type_is_unsafe_function(cx, cx.typeck_results().expr_ty(func).peel_refs()) => + && !is_unsafe_fn(cx, cx.typeck_results().expr_ty(func).peel_refs()) => { Some(func) }, diff --git a/src/tools/clippy/clippy_lints/src/matches/needless_match.rs b/src/tools/clippy/clippy_lints/src/matches/needless_match.rs index b04db03f8d2..3a2097c3df2 100644 --- a/src/tools/clippy/clippy_lints/src/matches/needless_match.rs +++ b/src/tools/clippy/clippy_lints/src/matches/needless_match.rs @@ -1,7 +1,7 @@ use super::NEEDLESS_MATCH; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts}; +use clippy_utils::ty::{is_type_diagnostic_item, same_type_modulo_regions}; use clippy_utils::{ SpanlessEq, eq_expr_value, get_parent_expr_for_hir, higher, is_else_clause, is_res_lang_ctor, over, path_res, peel_blocks_with_stmt, @@ -122,7 +122,7 @@ fn expr_ty_matches_p_ty(cx: &LateContext<'_>, expr: &Expr<'_>, p_expr: &Expr<'_> // Compare match_expr ty with local in `let local = match match_expr {..}` Node::LetStmt(local) => { let results = cx.typeck_results(); - return same_type_and_consts(results.node_type(local.hir_id), results.expr_ty(expr)); + return same_type_modulo_regions(results.node_type(local.hir_id), results.expr_ty(expr)); }, // compare match_expr ty with RetTy in `fn foo() -> RetTy` Node::Item(item) => { @@ -133,7 +133,7 @@ fn expr_ty_matches_p_ty(cx: &LateContext<'_>, expr: &Expr<'_>, p_expr: &Expr<'_> .instantiate_identity() .output() .skip_binder(); - return same_type_and_consts(output, cx.typeck_results().expr_ty(expr)); + return same_type_modulo_regions(output, cx.typeck_results().expr_ty(expr)); } }, // check the parent expr for this whole block `{ match match_expr {..} }` diff --git a/src/tools/clippy/clippy_lints/src/matches/rest_pat_in_fully_bound_struct.rs b/src/tools/clippy/clippy_lints/src/matches/rest_pat_in_fully_bound_struct.rs index ae09c2e87d6..1130d82ab78 100644 --- a/src/tools/clippy/clippy_lints/src/matches/rest_pat_in_fully_bound_struct.rs +++ b/src/tools/clippy/clippy_lints/src/matches/rest_pat_in_fully_bound_struct.rs @@ -7,7 +7,7 @@ use super::REST_PAT_IN_FULLY_BOUND_STRUCTS; pub(crate) fn check(cx: &LateContext<'_>, pat: &Pat<'_>) { if !pat.span.from_expansion() - && let PatKind::Struct(QPath::Resolved(_, path), fields, Some(_)) = pat.kind + && let PatKind::Struct(QPath::Resolved(_, path), fields, Some(dotdot)) = pat.kind && let Some(def_id) = path.res.opt_def_id() && let ty = cx.tcx.type_of(def_id).instantiate_identity() && let ty::Adt(def, _) = ty.kind() @@ -15,14 +15,18 @@ pub(crate) fn check(cx: &LateContext<'_>, pat: &Pat<'_>) { && fields.len() == def.non_enum_variant().fields.len() && !def.non_enum_variant().is_field_list_non_exhaustive() { - #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")] span_lint_and_then( cx, REST_PAT_IN_FULLY_BOUND_STRUCTS, pat.span, "unnecessary use of `..` pattern in struct binding. All fields were already bound", |diag| { - diag.help("consider removing `..` from this binding"); + diag.span_suggestion_verbose( + dotdot, + "consider removing `..` from this binding", + "", + rustc_errors::Applicability::MachineApplicable, + ); }, ); } diff --git a/src/tools/clippy/clippy_lints/src/matches/single_match.rs b/src/tools/clippy/clippy_lints/src/matches/single_match.rs index bcf079b7007..83939d32579 100644 --- a/src/tools/clippy/clippy_lints/src/matches/single_match.rs +++ b/src/tools/clippy/clippy_lints/src/matches/single_match.rs @@ -2,10 +2,8 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::{ SpanRangeExt, expr_block, snippet, snippet_block_with_context, snippet_with_applicability, snippet_with_context, }; -use clippy_utils::ty::implements_trait; -use clippy_utils::{ - is_lint_allowed, is_unit_expr, peel_blocks, peel_hir_pat_refs, peel_middle_ty_refs, peel_n_hir_expr_refs, -}; +use clippy_utils::ty::{implements_trait, peel_and_count_ty_refs}; +use clippy_utils::{is_lint_allowed, is_unit_expr, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs}; use core::ops::ControlFlow; use rustc_arena::DroplessArena; use rustc_errors::{Applicability, Diag}; @@ -133,7 +131,7 @@ fn report_single_pattern( let (pat, pat_ref_count) = peel_hir_pat_refs(arm.pat); let (msg, sugg) = if let PatKind::Expr(_) = pat.kind - && let (ty, ty_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(ex)) + && let (ty, ty_ref_count, _) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(ex)) && let Some(spe_trait_id) = cx.tcx.lang_items().structural_peq_trait() && let Some(pe_trait_id) = cx.tcx.lang_items().eq_trait() && (ty.is_integral() diff --git a/src/tools/clippy/clippy_lints/src/methods/implicit_clone.rs b/src/tools/clippy/clippy_lints/src/methods/implicit_clone.rs index 8a976d1b4dc..0ba84919395 100644 --- a/src/tools/clippy/clippy_lints/src/methods/implicit_clone.rs +++ b/src/tools/clippy/clippy_lints/src/methods/implicit_clone.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_context; -use clippy_utils::ty::implements_trait; -use clippy_utils::{is_diag_item_method, is_diag_trait_item, peel_middle_ty_refs, sym}; +use clippy_utils::ty::{implements_trait, peel_and_count_ty_refs}; +use clippy_utils::{is_diag_item_method, is_diag_trait_item, sym}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -14,7 +14,7 @@ pub fn check(cx: &LateContext<'_>, method_name: Symbol, expr: &hir::Expr<'_>, re && is_clone_like(cx, method_name, method_def_id) && let return_type = cx.typeck_results().expr_ty(expr) && let input_type = cx.typeck_results().expr_ty(recv) - && let (input_type, ref_count) = peel_middle_ty_refs(input_type) + && let (input_type, ref_count, _) = peel_and_count_ty_refs(input_type) && !(ref_count > 0 && is_diag_trait_item(cx, method_def_id, sym::ToOwned)) && let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did())) && return_type == input_type diff --git a/src/tools/clippy/clippy_lints/src/methods/inefficient_to_string.rs b/src/tools/clippy/clippy_lints/src/methods/inefficient_to_string.rs index 4ed7de81ea3..47195fdd65f 100644 --- a/src/tools/clippy/clippy_lints/src/methods/inefficient_to_string.rs +++ b/src/tools/clippy/clippy_lints/src/methods/inefficient_to_string.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::{is_type_lang_item, walk_ptrs_ty_depth}; +use clippy_utils::ty::{is_type_lang_item, peel_and_count_ty_refs}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -24,7 +24,7 @@ pub fn check( && let Some(args) = cx.typeck_results().node_args_opt(expr.hir_id) && let arg_ty = cx.typeck_results().expr_ty_adjusted(receiver) && let self_ty = args.type_at(0) - && let (deref_self_ty, deref_count) = walk_ptrs_ty_depth(self_ty) + && let (deref_self_ty, deref_count, _) = peel_and_count_ty_refs(self_ty) && deref_count >= 1 && specializes_tostring(cx, deref_self_ty) { diff --git a/src/tools/clippy/clippy_lints/src/methods/iter_overeager_cloned.rs b/src/tools/clippy/clippy_lints/src/methods/iter_overeager_cloned.rs index f851ebe91f3..d43dc23a86b 100644 --- a/src/tools/clippy/clippy_lints/src/methods/iter_overeager_cloned.rs +++ b/src/tools/clippy/clippy_lints/src/methods/iter_overeager_cloned.rs @@ -10,8 +10,7 @@ use rustc_middle::mir::{FakeReadCause, Mutability}; use rustc_middle::ty::{self, BorrowKind}; use rustc_span::{Symbol, sym}; -use super::ITER_OVEREAGER_CLONED; -use crate::redundant_clone::REDUNDANT_CLONE; +use super::{ITER_OVEREAGER_CLONED, REDUNDANT_ITER_CLONED}; #[derive(Clone, Copy)] pub(super) enum Op<'a> { @@ -96,7 +95,7 @@ pub(super) fn check<'tcx>( } let (lint, msg, trailing_clone) = match op { - Op::RmCloned | Op::NeedlessMove(_) => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""), + Op::RmCloned | Op::NeedlessMove(_) => (REDUNDANT_ITER_CLONED, "unneeded cloning of iterator items", ""), Op::LaterCloned | Op::FixClosure(_, _) => ( ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 49ca81dafc2..8679689c8ad 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -91,6 +91,7 @@ mod or_fun_call; mod or_then_unwrap; mod path_buf_push_overwrite; mod path_ends_with_ext; +mod ptr_offset_with_cast; mod range_zip_with_len; mod read_line_without_trim; mod readonly_write_lock; @@ -1727,6 +1728,43 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for usage of the `offset` pointer method with a `usize` casted to an + /// `isize`. + /// + /// ### Why is this bad? + /// If we’re always increasing the pointer address, we can avoid the numeric + /// cast by using the `add` method instead. + /// + /// ### Example + /// ```no_run + /// let vec = vec![b'a', b'b', b'c']; + /// let ptr = vec.as_ptr(); + /// let offset = 1_usize; + /// + /// unsafe { + /// ptr.offset(offset as isize); + /// } + /// ``` + /// + /// Could be written: + /// + /// ```no_run + /// let vec = vec![b'a', b'b', b'c']; + /// let ptr = vec.as_ptr(); + /// let offset = 1_usize; + /// + /// unsafe { + /// ptr.add(offset); + /// } + /// ``` + #[clippy::version = "1.30.0"] + pub PTR_OFFSET_WITH_CAST, + complexity, + "unneeded pointer offset cast" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for `FileType::is_file()`. /// /// ### Why restrict this? @@ -4576,6 +4614,31 @@ declare_clippy_lint! { "hardcoded localhost IP address" } +declare_clippy_lint! { + /// ### What it does + /// Checks for calls to `Iterator::cloned` where the original value could be used + /// instead. + /// + /// ### Why is this bad? + /// It is not always possible for the compiler to eliminate useless allocations and + /// deallocations generated by redundant `clone()`s. + /// + /// ### Example + /// ```no_run + /// let x = vec![String::new()]; + /// let _ = x.iter().cloned().map(|x| x.len()); + /// ``` + /// Use instead: + /// ```no_run + /// let x = vec![String::new()]; + /// let _ = x.iter().map(|x| x.len()); + /// ``` + #[clippy::version = "1.90.0"] + pub REDUNDANT_ITER_CLONED, + perf, + "detects redundant calls to `Iterator::cloned`" +} + #[expect(clippy::struct_excessive_bools)] pub struct Methods { avoid_breaking_exported_api: bool, @@ -4665,6 +4728,7 @@ impl_lint_pass!(Methods => [ UNINIT_ASSUMED_INIT, MANUAL_SATURATING_ARITHMETIC, ZST_OFFSET, + PTR_OFFSET_WITH_CAST, FILETYPE_IS_FILE, OPTION_AS_REF_DEREF, UNNECESSARY_LAZY_EVALUATIONS, @@ -4755,6 +4819,7 @@ impl_lint_pass!(Methods => [ IO_OTHER_ERROR, SWAP_WITH_TEMPORARY, IP_CONSTANT, + REDUNDANT_ITER_CLONED, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4960,10 +5025,7 @@ impl Methods { // Handle method calls whose receiver and arguments may not come from expansion if let Some((name, recv, args, span, call_span)) = method_call(expr) { match (name, args) { - ( - sym::add | sym::offset | sym::sub | sym::wrapping_offset | sym::wrapping_add | sym::wrapping_sub, - [_arg], - ) => { + (sym::add | sym::sub | sym::wrapping_add | sym::wrapping_sub, [_arg]) => { zst_offset::check(cx, expr, recv); }, (sym::all, [arg]) => { @@ -5334,6 +5396,11 @@ impl Methods { }, _ => iter_nth_zero::check(cx, expr, recv, n_arg), }, + (sym::offset | sym::wrapping_offset, [arg]) => { + zst_offset::check(cx, expr, recv); + + ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv); + }, (sym::ok_or_else, [arg]) => { unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"); }, diff --git a/src/tools/clippy/clippy_lints/src/methods/mut_mutex_lock.rs b/src/tools/clippy/clippy_lints/src/methods/mut_mutex_lock.rs index 4235af882b0..9d2c5e6232d 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mut_mutex_lock.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mut_mutex_lock.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::expr_custom_deref_adjustment; -use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable}; +use clippy_utils::ty::{is_type_diagnostic_item, peel_and_count_ty_refs}; use rustc_errors::Applicability; use rustc_hir::{Expr, Mutability}; use rustc_lint::LateContext; @@ -10,8 +10,7 @@ use super::MUT_MUTEX_LOCK; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, ex: &'tcx Expr<'tcx>, recv: &'tcx Expr<'tcx>, name_span: Span) { if matches!(expr_custom_deref_adjustment(cx, recv), None | Some(Mutability::Mut)) - && let (_, ref_depth, Mutability::Mut) = peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(recv)) - && ref_depth >= 1 + && let (_, _, Some(Mutability::Mut)) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(recv)) && let Some(method_id) = cx.typeck_results().type_dependent_def_id(ex.hir_id) && let Some(impl_id) = cx.tcx.impl_of_assoc(method_id) && is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id).instantiate_identity(), sym::Mutex) diff --git a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs index 04f0e3c0479..71b2f251ede 100644 --- a/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs +++ b/src/tools/clippy/clippy_lints/src/methods/or_fun_call.rs @@ -2,6 +2,7 @@ use std::ops::ControlFlow; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::eager_or_lazy::switch_to_lazy_eval; +use clippy_utils::higher::VecArgs; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::{expr_type_is_certain, implements_trait, is_type_diagnostic_item}; use clippy_utils::visitors::for_each_expr; @@ -97,6 +98,12 @@ pub(super) fn check<'tcx>( return false; } + // `.unwrap_or(vec![])` is as readable as `.unwrap_or_default()`. And if the expression is a + // non-empty `Vec`, then it will not be a default value anyway. Bail out in all cases. + if call_expr.and_then(|call_expr| VecArgs::hir(cx, call_expr)).is_some() { + return false; + } + // needs to target Default::default in particular or be *::new and have a Default impl // available if (is_new(fun) && output_type_implements_default(fun)) diff --git a/src/tools/clippy/clippy_lints/src/methods/ptr_offset_with_cast.rs b/src/tools/clippy/clippy_lints/src/methods/ptr_offset_with_cast.rs new file mode 100644 index 00000000000..d19d3b8eb89 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/methods/ptr_offset_with_cast.rs @@ -0,0 +1,82 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::sym; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::Symbol; +use std::fmt; + +use super::PTR_OFFSET_WITH_CAST; + +pub(super) fn check( + cx: &LateContext<'_>, + method: Symbol, + expr: &Expr<'_>, + recv: &Expr<'_>, + arg: &Expr<'_>, + msrv: Msrv, +) { + // `pointer::add` and `pointer::wrapping_add` are only stable since 1.26.0. These functions + // became const-stable in 1.61.0, the same version that `pointer::offset` became const-stable. + if !msrv.meets(cx, msrvs::POINTER_ADD_SUB_METHODS) { + return; + } + + let method = match method { + sym::offset => Method::Offset, + sym::wrapping_offset => Method::WrappingOffset, + _ => return, + }; + + if !cx.typeck_results().expr_ty_adjusted(recv).is_raw_ptr() { + return; + } + + // Check if the argument to the method call is a cast from usize. + let cast_lhs_expr = match arg.kind { + ExprKind::Cast(lhs, _) if cx.typeck_results().expr_ty(lhs).is_usize() => lhs, + _ => return, + }; + + let ExprKind::MethodCall(method_name, _, _, _) = expr.kind else { + return; + }; + + let msg = format!("use of `{method}` with a `usize` casted to an `isize`"); + span_lint_and_then(cx, PTR_OFFSET_WITH_CAST, expr.span, msg, |diag| { + diag.multipart_suggestion( + format!("use `{}` instead", method.suggestion()), + vec![ + (method_name.ident.span, method.suggestion().to_string()), + (arg.span.with_lo(cast_lhs_expr.span.hi()), String::new()), + ], + Applicability::MachineApplicable, + ); + }); +} + +#[derive(Copy, Clone)] +enum Method { + Offset, + WrappingOffset, +} + +impl Method { + #[must_use] + fn suggestion(self) -> &'static str { + match self { + Self::Offset => "add", + Self::WrappingOffset => "wrapping_add", + } + } +} + +impl fmt::Display for Method { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Offset => write!(f, "offset"), + Self::WrappingOffset => write!(f, "wrapping_offset"), + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs index c1f4904af7c..640931a8289 100644 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -3,11 +3,12 @@ use super::unnecessary_iter_cloned::{self, is_into_iter}; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{SpanRangeExt, snippet}; -use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy, is_type_diagnostic_item, is_type_lang_item}; +use clippy_utils::ty::{ + get_iterator_item_ty, implements_trait, is_copy, is_type_diagnostic_item, is_type_lang_item, peel_and_count_ty_refs, +}; use clippy_utils::visitors::find_all_ret_expressions; use clippy_utils::{ - fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, is_expr_temporary_value, peel_middle_ty_refs, - return_ty, sym, + fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, is_expr_temporary_value, return_ty, sym, }; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; @@ -119,8 +120,8 @@ fn check_addr_of_expr( }, ] = adjustments[..] && let receiver_ty = cx.typeck_results().expr_ty(receiver) - && let (target_ty, n_target_refs) = peel_middle_ty_refs(*target_ty) - && let (receiver_ty, n_receiver_refs) = peel_middle_ty_refs(receiver_ty) + && let (target_ty, n_target_refs, _) = peel_and_count_ty_refs(*target_ty) + && let (receiver_ty, n_receiver_refs, _) = peel_and_count_ty_refs(receiver_ty) // Only flag cases satisfying at least one of the following three conditions: // * the referent and receiver types are distinct // * the referent/receiver type is a copyable array @@ -385,7 +386,7 @@ fn check_other_call_arg<'tcx>( && let fn_sig = cx.tcx.fn_sig(callee_def_id).instantiate_identity().skip_binder() && let Some(i) = recv.into_iter().chain(call_args).position(|arg| arg.hir_id == maybe_arg.hir_id) && let Some(input) = fn_sig.inputs().get(i) - && let (input, n_refs) = peel_middle_ty_refs(*input) + && let (input, n_refs, _) = peel_and_count_ty_refs(*input) && let (trait_predicates, _) = get_input_traits_and_projections(cx, callee_def_id, input) && let Some(sized_def_id) = cx.tcx.lang_items().sized_trait() && let Some(meta_sized_def_id) = cx.tcx.lang_items().meta_sized_trait() diff --git a/src/tools/clippy/clippy_lints/src/methods/useless_asref.rs b/src/tools/clippy/clippy_lints/src/methods/useless_asref.rs index 38fad239f67..e56f4b80d01 100644 --- a/src/tools/clippy/clippy_lints/src/methods/useless_asref.rs +++ b/src/tools/clippy/clippy_lints/src/methods/useless_asref.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::ty::{implements_trait, should_call_clone_as_function, walk_ptrs_ty_depth}; +use clippy_utils::ty::{implements_trait, peel_and_count_ty_refs, should_call_clone_as_function}; use clippy_utils::{get_parent_expr, is_diag_trait_item, path_to_local_id, peel_blocks, strip_pat_refs}; use rustc_errors::Applicability; use rustc_hir::{self as hir, LangItem}; @@ -50,8 +50,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: Symbo // check if the type after `as_ref` or `as_mut` is the same as before let rcv_ty = cx.typeck_results().expr_ty(recvr); let res_ty = cx.typeck_results().expr_ty(expr); - let (base_res_ty, res_depth) = walk_ptrs_ty_depth(res_ty); - let (base_rcv_ty, rcv_depth) = walk_ptrs_ty_depth(rcv_ty); + let (base_res_ty, res_depth, _) = peel_and_count_ty_refs(res_ty); + let (base_rcv_ty, rcv_depth, _) = peel_and_count_ty_refs(rcv_ty); if base_rcv_ty == base_res_ty && rcv_depth >= res_depth { if let Some(parent) = get_parent_expr(cx, expr) { // allow the `as_ref` or `as_mut` if it is followed by another method call diff --git a/src/tools/clippy/clippy_lints/src/min_ident_chars.rs b/src/tools/clippy/clippy_lints/src/min_ident_chars.rs index 6258e408217..1ceee836732 100644 --- a/src/tools/clippy/clippy_lints/src/min_ident_chars.rs +++ b/src/tools/clippy/clippy_lints/src/min_ident_chars.rs @@ -256,7 +256,11 @@ fn is_not_in_trait_impl(cx: &LateContext<'_>, pat: &Pat<'_>, ident: Ident) -> bo } fn get_param_name(impl_item: &ImplItem<'_>, cx: &LateContext<'_>, ident: Ident) -> Option<Symbol> { - if let ImplItemImplKind::Trait { trait_item_def_id: Ok(trait_item_def_id), .. } = impl_item.impl_kind { + if let ImplItemImplKind::Trait { + trait_item_def_id: Ok(trait_item_def_id), + .. + } = impl_item.impl_kind + { let trait_param_names = cx.tcx.fn_arg_idents(trait_item_def_id); let ImplItemKind::Fn(_, body_id) = impl_item.kind else { diff --git a/src/tools/clippy/clippy_lints/src/misc.rs b/src/tools/clippy/clippy_lints/src/misc.rs index 09ee6f7037c..19e9910dfe9 100644 --- a/src/tools/clippy/clippy_lints/src/misc.rs +++ b/src/tools/clippy/clippy_lints/src/misc.rs @@ -1,57 +1,11 @@ -use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir, span_lint_hir_and_then}; -use clippy_utils::source::{snippet, snippet_with_context}; +use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::sugg::Sugg; -use clippy_utils::{ - SpanlessEq, fulfill_or_allowed, get_parent_expr, in_automatically_derived, is_lint_allowed, iter_input_pats, - last_path_segment, -}; +use clippy_utils::{SpanlessEq, fulfill_or_allowed, get_parent_expr, in_automatically_derived, last_path_segment}; use rustc_errors::Applicability; use rustc_hir::def::Res; -use rustc_hir::intravisit::FnKind; -use rustc_hir::{ - BinOpKind, BindingMode, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, QPath, Stmt, StmtKind, -}; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::declare_lint_pass; -use rustc_span::Span; -use rustc_span::def_id::LocalDefId; - -use crate::ref_patterns::REF_PATTERNS; - -declare_clippy_lint! { - /// ### What it does - /// Checks for function arguments and let bindings denoted as - /// `ref`. - /// - /// ### Why is this bad? - /// The `ref` declaration makes the function take an owned - /// value, but turns the argument into a reference (which means that the value - /// is destroyed when exiting the function). This adds not much value: either - /// take a reference type, or take an owned value and create references in the - /// body. - /// - /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The - /// type of `x` is more obvious with the former. - /// - /// ### Known problems - /// If the argument is dereferenced within the function, - /// removing the `ref` will lead to errors. This can be fixed by removing the - /// dereferences, e.g., changing `*x` to `x` within the function. - /// - /// ### Example - /// ```no_run - /// fn foo(ref _x: u8) {} - /// ``` - /// - /// Use instead: - /// ```no_run - /// fn foo(_x: &u8) {} - /// ``` - #[clippy::version = "pre 1.29.0"] - pub TOPLEVEL_REF_ARG, - style, - "an entire binding declared as `ref`, in a function argument or a `let` statement" -} declare_clippy_lint! { /// ### What it does @@ -140,79 +94,13 @@ declare_clippy_lint! { } declare_lint_pass!(LintPass => [ - TOPLEVEL_REF_ARG, USED_UNDERSCORE_BINDING, USED_UNDERSCORE_ITEMS, SHORT_CIRCUIT_STATEMENT, ]); impl<'tcx> LateLintPass<'tcx> for LintPass { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - k: FnKind<'tcx>, - decl: &'tcx FnDecl<'_>, - body: &'tcx Body<'_>, - _: Span, - _: LocalDefId, - ) { - if !matches!(k, FnKind::Closure) { - for arg in iter_input_pats(decl, body) { - if let PatKind::Binding(BindingMode(ByRef::Yes(_), _), ..) = arg.pat.kind - && is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) - && !arg.span.in_external_macro(cx.tcx.sess.source_map()) - { - span_lint_hir( - cx, - TOPLEVEL_REF_ARG, - arg.hir_id, - arg.pat.span, - "`ref` directly on a function parameter does not prevent taking ownership of the passed argument. \ - Consider using a reference type instead", - ); - } - } - } - } - fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { - if let StmtKind::Let(local) = stmt.kind - && let PatKind::Binding(BindingMode(ByRef::Yes(mutabl), _), .., name, None) = local.pat.kind - && let Some(init) = local.init - // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. - && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id) - && !stmt.span.in_external_macro(cx.tcx.sess.source_map()) - { - let ctxt = local.span.ctxt(); - let mut app = Applicability::MachineApplicable; - let sugg_init = Sugg::hir_with_context(cx, init, ctxt, "..", &mut app); - let (mutopt, initref) = if mutabl == Mutability::Mut { - ("mut ", sugg_init.mut_addr()) - } else { - ("", sugg_init.addr()) - }; - let tyopt = if let Some(ty) = local.ty { - let ty_snip = snippet_with_context(cx, ty.span, ctxt, "_", &mut app).0; - format!(": &{mutopt}{ty_snip}") - } else { - String::new() - }; - span_lint_hir_and_then( - cx, - TOPLEVEL_REF_ARG, - init.hir_id, - local.pat.span, - "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", - |diag| { - diag.span_suggestion( - stmt.span, - "try", - format!("let {name}{tyopt} = {initref};", name = snippet(cx, name.span, ".."),), - app, - ); - }, - ); - } if let StmtKind::Semi(expr) = stmt.kind && let ExprKind::Binary(binop, a, b) = &expr.kind && matches!(binop.node, BinOpKind::And | BinOpKind::Or) diff --git a/src/tools/clippy/clippy_lints/src/missing_doc.rs b/src/tools/clippy/clippy_lints/src/missing_doc.rs index 39b5964bd87..1c62caa1c82 100644 --- a/src/tools/clippy/clippy_lints/src/missing_doc.rs +++ b/src/tools/clippy/clippy_lints/src/missing_doc.rs @@ -250,7 +250,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { AssocContainer::Trait | AssocContainer::TraitImpl(_) => { note_prev_span_then_ret!(self.prev_span, impl_item.span); }, - AssocContainer::InherentImpl => {} + AssocContainer::InherentImpl => {}, } let (article, desc) = cx.tcx.article_and_description(impl_item.owner_id.to_def_id()); diff --git a/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs b/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs index c6c27e22b90..bc5e72270f4 100644 --- a/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs +++ b/src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs @@ -1,3 +1,4 @@ +use clippy_utils::desugar_await; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::visitors::{Descend, Visitable, for_each_expr}; use core::ops::ControlFlow::Continue; @@ -97,6 +98,13 @@ fn collect_unsafe_exprs<'tcx>( ) { for_each_expr(cx, node, |expr| { match expr.kind { + // The `await` itself will desugar to two unsafe calls, but we should ignore those. + // Instead, check the expression that is `await`ed + _ if let Some(e) = desugar_await(expr) => { + collect_unsafe_exprs(cx, e, unsafe_ops); + return Continue(Descend::No); + }, + ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)), ExprKind::Field(e, _) => { diff --git a/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs b/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs index c4cad592e36..a21c361356e 100644 --- a/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs +++ b/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs @@ -252,7 +252,9 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { { ( trait_item_id, - FnKind::ImplTraitFn(std::ptr::from_ref(cx.tcx.erase_and_anonymize_regions(trait_ref.args)) as usize), + FnKind::ImplTraitFn( + std::ptr::from_ref(cx.tcx.erase_and_anonymize_regions(trait_ref.args)) as usize + ), usize::from(sig.decl.implicit_self.has_implicit_self()), ) } else { diff --git a/src/tools/clippy/clippy_lints/src/operators/erasing_op.rs b/src/tools/clippy/clippy_lints/src/operators/erasing_op.rs index e3fc8d8fea7..8f5ee390f72 100644 --- a/src/tools/clippy/clippy_lints/src/operators/erasing_op.rs +++ b/src/tools/clippy/clippy_lints/src/operators/erasing_op.rs @@ -1,6 +1,6 @@ use clippy_utils::consts::{ConstEvalCtxt, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::ty::same_type_and_consts; +use clippy_utils::ty::same_type_modulo_regions; use rustc_hir::{BinOpKind, Expr}; use rustc_lint::LateContext; @@ -29,7 +29,7 @@ pub(super) fn check<'tcx>( fn different_types(tck: &TypeckResults<'_>, input: &Expr<'_>, output: &Expr<'_>) -> bool { let input_ty = tck.expr_ty(input).peel_refs(); let output_ty = tck.expr_ty(output).peel_refs(); - !same_type_and_consts(input_ty, output_ty) + !same_type_modulo_regions(input_ty, output_ty) } fn check_op<'tcx>( diff --git a/src/tools/clippy/clippy_lints/src/ptr_offset_with_cast.rs b/src/tools/clippy/clippy_lints/src/ptr_offset_with_cast.rs deleted file mode 100644 index d8d813f9846..00000000000 --- a/src/tools/clippy/clippy_lints/src/ptr_offset_with_cast.rs +++ /dev/null @@ -1,151 +0,0 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; -use clippy_utils::source::SpanRangeExt; -use clippy_utils::sym; -use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; -use std::fmt; - -declare_clippy_lint! { - /// ### What it does - /// Checks for usage of the `offset` pointer method with a `usize` casted to an - /// `isize`. - /// - /// ### Why is this bad? - /// If we’re always increasing the pointer address, we can avoid the numeric - /// cast by using the `add` method instead. - /// - /// ### Example - /// ```no_run - /// let vec = vec![b'a', b'b', b'c']; - /// let ptr = vec.as_ptr(); - /// let offset = 1_usize; - /// - /// unsafe { - /// ptr.offset(offset as isize); - /// } - /// ``` - /// - /// Could be written: - /// - /// ```no_run - /// let vec = vec![b'a', b'b', b'c']; - /// let ptr = vec.as_ptr(); - /// let offset = 1_usize; - /// - /// unsafe { - /// ptr.add(offset); - /// } - /// ``` - #[clippy::version = "1.30.0"] - pub PTR_OFFSET_WITH_CAST, - complexity, - "unneeded pointer offset cast" -} - -declare_lint_pass!(PtrOffsetWithCast => [PTR_OFFSET_WITH_CAST]); - -impl<'tcx> LateLintPass<'tcx> for PtrOffsetWithCast { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - // Check if the expressions is a ptr.offset or ptr.wrapping_offset method call - let Some((receiver_expr, arg_expr, method)) = expr_as_ptr_offset_call(cx, expr) else { - return; - }; - - // Check if the argument to the method call is a cast from usize - let Some(cast_lhs_expr) = expr_as_cast_from_usize(cx, arg_expr) else { - return; - }; - - let msg = format!("use of `{method}` with a `usize` casted to an `isize`"); - if let Some(sugg) = build_suggestion(cx, method, receiver_expr, cast_lhs_expr) { - span_lint_and_sugg( - cx, - PTR_OFFSET_WITH_CAST, - expr.span, - msg, - "try", - sugg, - Applicability::MachineApplicable, - ); - } else { - span_lint(cx, PTR_OFFSET_WITH_CAST, expr.span, msg); - } - } -} - -// If the given expression is a cast from a usize, return the lhs of the cast -fn expr_as_cast_from_usize<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> { - if let ExprKind::Cast(cast_lhs_expr, _) = expr.kind - && is_expr_ty_usize(cx, cast_lhs_expr) - { - return Some(cast_lhs_expr); - } - None -} - -// If the given expression is a ptr::offset or ptr::wrapping_offset method call, return the -// receiver, the arg of the method call, and the method. -fn expr_as_ptr_offset_call<'tcx>( - cx: &LateContext<'tcx>, - expr: &'tcx Expr<'_>, -) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>, Method)> { - if let ExprKind::MethodCall(path_segment, arg_0, [arg_1], _) = &expr.kind - && is_expr_ty_raw_ptr(cx, arg_0) - { - if path_segment.ident.name == sym::offset { - return Some((arg_0, arg_1, Method::Offset)); - } - if path_segment.ident.name == sym::wrapping_offset { - return Some((arg_0, arg_1, Method::WrappingOffset)); - } - } - None -} - -// Is the type of the expression a usize? -fn is_expr_ty_usize(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - cx.typeck_results().expr_ty(expr) == cx.tcx.types.usize -} - -// Is the type of the expression a raw pointer? -fn is_expr_ty_raw_ptr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - cx.typeck_results().expr_ty(expr).is_raw_ptr() -} - -fn build_suggestion( - cx: &LateContext<'_>, - method: Method, - receiver_expr: &Expr<'_>, - cast_lhs_expr: &Expr<'_>, -) -> Option<String> { - let receiver = receiver_expr.span.get_source_text(cx)?; - let cast_lhs = cast_lhs_expr.span.get_source_text(cx)?; - Some(format!("{receiver}.{}({cast_lhs})", method.suggestion())) -} - -#[derive(Copy, Clone)] -enum Method { - Offset, - WrappingOffset, -} - -impl Method { - #[must_use] - fn suggestion(self) -> &'static str { - match self { - Self::Offset => "add", - Self::WrappingOffset => "wrapping_add", - } - } -} - -impl fmt::Display for Method { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Offset => write!(f, "offset"), - Self::WrappingOffset => write!(f, "wrapping_offset"), - } - } -} diff --git a/src/tools/clippy/clippy_lints/src/question_mark.rs b/src/tools/clippy/clippy_lints/src/question_mark.rs index de12a25b03d..4aa100a50e0 100644 --- a/src/tools/clippy/clippy_lints/src/question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/question_mark.rs @@ -8,9 +8,9 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; use clippy_utils::{ - eq_expr_value, higher, is_else_clause, is_in_const_context, is_lint_allowed, is_path_lang_item, is_res_lang_ctor, - pat_and_expr_can_be_question_mark, path_res, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt, - span_contains_cfg, span_contains_comment, sym, + eq_expr_value, fn_def_id_with_node_args, higher, is_else_clause, is_in_const_context, is_lint_allowed, + is_path_lang_item, is_res_lang_ctor, pat_and_expr_can_be_question_mark, path_res, path_to_local, path_to_local_id, + peel_blocks, peel_blocks_with_stmt, span_contains_cfg, span_contains_comment, sym, }; use rustc_errors::Applicability; use rustc_hir::LangItem::{self, OptionNone, OptionSome, ResultErr, ResultOk}; @@ -393,8 +393,8 @@ fn check_arm_is_none_or_err<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm: &A && let ExprKind::Ret(Some(wrapped_ret_expr)) = arm_body.kind && let ExprKind::Call(ok_ctor, [ret_expr]) = wrapped_ret_expr.kind && is_res_lang_ctor(cx, path_res(cx, ok_ctor), ResultErr) - // check `...` is `val` from binding - && path_to_local_id(ret_expr, ok_val) + // check if `...` is `val` from binding or `val.into()` + && is_local_or_local_into(cx, ret_expr, ok_val) { true } else { @@ -417,6 +417,17 @@ fn check_arm_is_none_or_err<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm: &A } } +/// Check if `expr` is `val` or `val.into()` +fn is_local_or_local_into(cx: &LateContext<'_>, expr: &Expr<'_>, val: HirId) -> bool { + let is_into_call = fn_def_id_with_node_args(cx, expr) + .and_then(|(fn_def_id, _)| cx.tcx.trait_of_assoc(fn_def_id)) + .is_some_and(|trait_def_id| cx.tcx.is_diagnostic_item(sym::Into, trait_def_id)); + match expr.kind { + ExprKind::MethodCall(_, recv, [], _) | ExprKind::Call(_, [recv]) => is_into_call && path_to_local_id(recv, val), + _ => path_to_local_id(expr, val), + } +} + fn check_arms_are_try<'tcx>(cx: &LateContext<'tcx>, mode: TryMode, arm1: &Arm<'tcx>, arm2: &Arm<'tcx>) -> bool { (check_arm_is_some_or_ok(cx, mode, arm1) && check_arm_is_none_or_err(cx, mode, arm2)) || (check_arm_is_some_or_ok(cx, mode, arm2) && check_arm_is_none_or_err(cx, mode, arm1)) diff --git a/src/tools/clippy/clippy_lints/src/read_zero_byte_vec.rs b/src/tools/clippy/clippy_lints/src/read_zero_byte_vec.rs index acd840401c6..b8d4e7c4651 100644 --- a/src/tools/clippy/clippy_lints/src/read_zero_byte_vec.rs +++ b/src/tools/clippy/clippy_lints/src/read_zero_byte_vec.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::higher::{VecInitKind, get_vec_init_kind}; -use clippy_utils::source::snippet; +use clippy_utils::source::{indent_of, snippet}; use clippy_utils::{get_enclosing_block, sym}; use rustc_errors::Applicability; @@ -83,10 +83,12 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { expr.span, "reading zero byte data to `Vec`", |diag| { + let span = first_stmt_containing_expr(cx, expr).map_or(expr.span, |stmt| stmt.span); + let indent = indent_of(cx, span).unwrap_or(0); diag.span_suggestion( - expr.span, + span.shrink_to_lo(), "try", - format!("{}.resize({len}, 0); {}", ident, snippet(cx, expr.span, "..")), + format!("{ident}.resize({len}, 0);\n{}", " ".repeat(indent)), applicability, ); }, @@ -100,14 +102,15 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { expr.span, "reading zero byte data to `Vec`", |diag| { + let span = first_stmt_containing_expr(cx, expr).map_or(expr.span, |stmt| stmt.span); + let indent = indent_of(cx, span).unwrap_or(0); diag.span_suggestion( - expr.span, + span.shrink_to_lo(), "try", format!( - "{}.resize({}, 0); {}", - ident, + "{ident}.resize({}, 0);\n{}", snippet(cx, e.span, ".."), - snippet(cx, expr.span, "..") + " ".repeat(indent) ), applicability, ); @@ -130,6 +133,16 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { } } +fn first_stmt_containing_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx hir::Stmt<'tcx>> { + cx.tcx.hir_parent_iter(expr.hir_id).find_map(|(_, node)| { + if let hir::Node::Stmt(stmt) = node { + Some(stmt) + } else { + None + } + }) +} + struct ReadVecVisitor<'tcx> { local_id: HirId, read_zero_expr: Option<&'tcx Expr<'tcx>>, diff --git a/src/tools/clippy/clippy_lints/src/redundant_clone.rs b/src/tools/clippy/clippy_lints/src/redundant_clone.rs index 1d58cdd26d8..de6766cbe94 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_clone.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_clone.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::fn_has_unsatisfiable_preds; use clippy_utils::mir::{LocalUsage, PossibleBorrowerMap, visit_local_usage}; use clippy_utils::source::SpanRangeExt; -use clippy_utils::ty::{has_drop, is_copy, is_type_lang_item, walk_ptrs_ty_depth}; +use clippy_utils::ty::{has_drop, is_copy, is_type_lang_item, peel_and_count_ty_refs}; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, FnDecl, LangItem, def_id}; @@ -263,7 +263,7 @@ fn is_call_with_ref_arg<'tcx>( && args.len() == 1 && let mir::Operand::Move(mir::Place { local, .. }) = &args[0].node && let ty::FnDef(def_id, _) = *func.ty(mir, cx.tcx).kind() - && let (inner_ty, 1) = walk_ptrs_ty_depth(args[0].node.ty(mir, cx.tcx)) + && let (inner_ty, 1, _) = peel_and_count_ty_refs(args[0].node.ty(mir, cx.tcx)) && !is_copy(cx, inner_ty) { Some((def_id, *local, inner_ty, destination.as_local()?)) diff --git a/src/tools/clippy/clippy_lints/src/redundant_slicing.rs b/src/tools/clippy/clippy_lints/src/redundant_slicing.rs index 324a05cdcc0..a358eff2ce5 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_slicing.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_slicing.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::get_parent_expr; use clippy_utils::source::snippet_with_context; -use clippy_utils::ty::is_type_lang_item; -use clippy_utils::{get_parent_expr, peel_middle_ty_refs}; +use clippy_utils::ty::{is_type_lang_item, peel_and_count_ty_refs}; use rustc_ast::util::parser::ExprPrecedence; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability}; @@ -82,8 +82,8 @@ impl<'tcx> LateLintPass<'tcx> for RedundantSlicing { && let ExprKind::Index(indexed, range, _) = addressee.kind && is_type_lang_item(cx, cx.typeck_results().expr_ty_adjusted(range), LangItem::RangeFull) { - let (expr_ty, expr_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(expr)); - let (indexed_ty, indexed_ref_count) = peel_middle_ty_refs(cx.typeck_results().expr_ty(indexed)); + let (expr_ty, expr_ref_count, _) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(expr)); + let (indexed_ty, indexed_ref_count, _) = peel_and_count_ty_refs(cx.typeck_results().expr_ty(indexed)); let parent_expr = get_parent_expr(cx, expr); let needs_parens_for_prefix = parent_expr.is_some_and(|parent| cx.precedence(parent) > ExprPrecedence::Prefix); diff --git a/src/tools/clippy/clippy_lints/src/returns.rs b/src/tools/clippy/clippy_lints/src/returns.rs deleted file mode 100644 index e0c93153a77..00000000000 --- a/src/tools/clippy/clippy_lints/src/returns.rs +++ /dev/null @@ -1,513 +0,0 @@ -use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then}; -use clippy_utils::source::{SpanRangeExt, snippet_with_context}; -use clippy_utils::sugg::has_enclosing_paren; -use clippy_utils::visitors::for_each_expr; -use clippy_utils::{ - binary_expr_needs_parentheses, fn_def_id, is_from_proc_macro, is_inside_let_else, is_res_lang_ctor, - leaks_droppable_temporary_with_limited_lifetime, path_res, path_to_local_id, span_contains_cfg, - span_find_starting_semi, sym, -}; -use core::ops::ControlFlow; -use rustc_ast::MetaItemInner; -use rustc_errors::Applicability; -use rustc_hir::LangItem::ResultErr; -use rustc_hir::intravisit::FnKind; -use rustc_hir::{ - Block, Body, Expr, ExprKind, FnDecl, HirId, ItemKind, LangItem, MatchSource, Node, OwnerNode, PatKind, QPath, Stmt, - StmtKind, -}; -use rustc_lint::{LateContext, LateLintPass, Level, LintContext}; -use rustc_middle::ty::adjustment::Adjust; -use rustc_middle::ty::{self, GenericArgKind, Ty}; -use rustc_session::declare_lint_pass; -use rustc_span::def_id::LocalDefId; -use rustc_span::edition::Edition; -use rustc_span::{BytePos, Pos, Span}; -use std::borrow::Cow; -use std::fmt::Display; - -declare_clippy_lint! { - /// ### What it does - /// Checks for `let`-bindings, which are subsequently - /// returned. - /// - /// ### Why is this bad? - /// It is just extraneous code. Remove it to make your code - /// more rusty. - /// - /// ### Known problems - /// In the case of some temporaries, e.g. locks, eliding the variable binding could lead - /// to deadlocks. See [this issue](https://github.com/rust-lang/rust/issues/37612). - /// This could become relevant if the code is later changed to use the code that would have been - /// bound without first assigning it to a let-binding. - /// - /// ### Example - /// ```no_run - /// fn foo() -> String { - /// let x = String::new(); - /// x - /// } - /// ``` - /// instead, use - /// ```no_run - /// fn foo() -> String { - /// String::new() - /// } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub LET_AND_RETURN, - style, - "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for return statements at the end of a block. - /// - /// ### Why is this bad? - /// Removing the `return` and semicolon will make the code - /// more rusty. - /// - /// ### Example - /// ```no_run - /// fn foo(x: usize) -> usize { - /// return x; - /// } - /// ``` - /// simplify to - /// ```no_run - /// fn foo(x: usize) -> usize { - /// x - /// } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub NEEDLESS_RETURN, - // This lint requires some special handling in `check_final_expr` for `#[expect]`. - // This handling needs to be updated if the group gets changed. This should also - // be caught by tests. - style, - "using a return statement like `return expr;` where an expression would suffice" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks for return statements on `Err` paired with the `?` operator. - /// - /// ### Why is this bad? - /// The `return` is unnecessary. - /// - /// Returns may be used to add attributes to the return expression. Return - /// statements with attributes are therefore be accepted by this lint. - /// - /// ### Example - /// ```rust,ignore - /// fn foo(x: usize) -> Result<(), Box<dyn Error>> { - /// if x == 0 { - /// return Err(...)?; - /// } - /// Ok(()) - /// } - /// ``` - /// simplify to - /// ```rust,ignore - /// fn foo(x: usize) -> Result<(), Box<dyn Error>> { - /// if x == 0 { - /// Err(...)?; - /// } - /// Ok(()) - /// } - /// ``` - /// if paired with `try_err`, use instead: - /// ```rust,ignore - /// fn foo(x: usize) -> Result<(), Box<dyn Error>> { - /// if x == 0 { - /// return Err(...); - /// } - /// Ok(()) - /// } - /// ``` - #[clippy::version = "1.73.0"] - pub NEEDLESS_RETURN_WITH_QUESTION_MARK, - style, - "using a return statement like `return Err(expr)?;` where removing it would suffice" -} - -#[derive(PartialEq, Eq)] -enum RetReplacement<'tcx> { - Empty, - Block, - Unit, - NeedsPar(Cow<'tcx, str>, Applicability), - Expr(Cow<'tcx, str>, Applicability), -} - -impl RetReplacement<'_> { - fn sugg_help(&self) -> &'static str { - match self { - Self::Empty | Self::Expr(..) => "remove `return`", - Self::Block => "replace `return` with an empty block", - Self::Unit => "replace `return` with a unit value", - Self::NeedsPar(..) => "remove `return` and wrap the sequence with parentheses", - } - } - - fn applicability(&self) -> Applicability { - match self { - Self::Expr(_, ap) | Self::NeedsPar(_, ap) => *ap, - _ => Applicability::MachineApplicable, - } - } -} - -impl Display for RetReplacement<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Empty => write!(f, ""), - Self::Block => write!(f, "{{}}"), - Self::Unit => write!(f, "()"), - Self::NeedsPar(inner, _) => write!(f, "({inner})"), - Self::Expr(inner, _) => write!(f, "{inner}"), - } - } -} - -declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN, NEEDLESS_RETURN_WITH_QUESTION_MARK]); - -/// Checks if a return statement is "needed" in the middle of a block, or if it can be removed. This -/// is the case when the enclosing block expression is coerced to some other type, which only works -/// because of the never-ness of `return` expressions -fn stmt_needs_never_type(cx: &LateContext<'_>, stmt_hir_id: HirId) -> bool { - cx.tcx - .hir_parent_iter(stmt_hir_id) - .find_map(|(_, node)| if let Node::Expr(expr) = node { Some(expr) } else { None }) - .is_some_and(|e| { - cx.typeck_results() - .expr_adjustments(e) - .iter() - .any(|adjust| adjust.target != cx.tcx.types.unit && matches!(adjust.kind, Adjust::NeverToAny)) - }) -} - -impl<'tcx> LateLintPass<'tcx> for Return { - fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { - if !stmt.span.in_external_macro(cx.sess().source_map()) - && let StmtKind::Semi(expr) = stmt.kind - && let ExprKind::Ret(Some(ret)) = expr.kind - // return Err(...)? desugars to a match - // over a Err(...).branch() - // which breaks down to a branch call, with the callee being - // the constructor of the Err variant - && let ExprKind::Match(maybe_cons, _, MatchSource::TryDesugar(_)) = ret.kind - && let ExprKind::Call(_, [maybe_result_err]) = maybe_cons.kind - && let ExprKind::Call(maybe_constr, _) = maybe_result_err.kind - && is_res_lang_ctor(cx, path_res(cx, maybe_constr), ResultErr) - - // Ensure this is not the final stmt, otherwise removing it would cause a compile error - && let OwnerNode::Item(item) = cx.tcx.hir_owner_node(cx.tcx.hir_get_parent_item(expr.hir_id)) - && let ItemKind::Fn { body, .. } = item.kind - && let block = cx.tcx.hir_body(body).value - && let ExprKind::Block(block, _) = block.kind - && !is_inside_let_else(cx.tcx, expr) - && let [.., final_stmt] = block.stmts - && final_stmt.hir_id != stmt.hir_id - && !is_from_proc_macro(cx, expr) - && !stmt_needs_never_type(cx, stmt.hir_id) - { - span_lint_and_sugg( - cx, - NEEDLESS_RETURN_WITH_QUESTION_MARK, - expr.span.until(ret.span), - "unneeded `return` statement with `?` operator", - "remove it", - String::new(), - Applicability::MachineApplicable, - ); - } - } - - fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) { - // we need both a let-binding stmt and an expr - if let Some(retexpr) = block.expr - && let Some(stmt) = block.stmts.iter().last() - && let StmtKind::Let(local) = &stmt.kind - && local.ty.is_none() - && cx.tcx.hir_attrs(local.hir_id).is_empty() - && let Some(initexpr) = &local.init - && let PatKind::Binding(_, local_id, _, _) = local.pat.kind - && path_to_local_id(retexpr, local_id) - && (cx.sess().edition() >= Edition::Edition2024 || !last_statement_borrows(cx, initexpr)) - && !initexpr.span.in_external_macro(cx.sess().source_map()) - && !retexpr.span.in_external_macro(cx.sess().source_map()) - && !local.span.from_expansion() - && !span_contains_cfg(cx, stmt.span.between(retexpr.span)) - { - span_lint_hir_and_then( - cx, - LET_AND_RETURN, - retexpr.hir_id, - retexpr.span, - "returning the result of a `let` binding from a block", - |err| { - err.span_label(local.span, "unnecessary `let` binding"); - - if let Some(src) = initexpr.span.get_source_text(cx) { - let sugg = if binary_expr_needs_parentheses(initexpr) { - if has_enclosing_paren(&src) { - src.to_owned() - } else { - format!("({src})") - } - } else if !cx.typeck_results().expr_adjustments(retexpr).is_empty() { - if has_enclosing_paren(&src) { - format!("{src} as _") - } else { - format!("({src}) as _") - } - } else { - src.to_owned() - }; - err.multipart_suggestion( - "return the expression directly", - vec![(local.span, String::new()), (retexpr.span, sugg)], - Applicability::MachineApplicable, - ); - } else { - err.span_help(initexpr.span, "this expression can be directly returned"); - } - }, - ); - } - } - - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - kind: FnKind<'tcx>, - _: &'tcx FnDecl<'tcx>, - body: &'tcx Body<'tcx>, - sp: Span, - _: LocalDefId, - ) { - if sp.from_expansion() { - return; - } - - match kind { - FnKind::Closure => { - // when returning without value in closure, replace this `return` - // with an empty block to prevent invalid suggestion (see #6501) - let replacement = if let ExprKind::Ret(None) = &body.value.kind { - RetReplacement::Block - } else { - RetReplacement::Empty - }; - check_final_expr(cx, body.value, vec![], replacement, None); - }, - FnKind::ItemFn(..) | FnKind::Method(..) => { - check_block_return(cx, &body.value.kind, sp, vec![]); - }, - } - } -} - -// if `expr` is a block, check if there are needless returns in it -fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>, sp: Span, mut semi_spans: Vec<Span>) { - if let ExprKind::Block(block, _) = expr_kind { - if let Some(block_expr) = block.expr { - check_final_expr(cx, block_expr, semi_spans, RetReplacement::Empty, None); - } else if let Some(stmt) = block.stmts.iter().last() { - match stmt.kind { - StmtKind::Expr(expr) => { - check_final_expr(cx, expr, semi_spans, RetReplacement::Empty, None); - }, - StmtKind::Semi(semi_expr) => { - // Remove ending semicolons and any whitespace ' ' in between. - // Without `return`, the suggestion might not compile if the semicolon is retained - if let Some(semi_span) = stmt.span.trim_start(semi_expr.span) { - let semi_span_to_remove = - span_find_starting_semi(cx.sess().source_map(), semi_span.with_hi(sp.hi())); - semi_spans.push(semi_span_to_remove); - } - check_final_expr(cx, semi_expr, semi_spans, RetReplacement::Empty, None); - }, - _ => (), - } - } - } -} - -fn check_final_expr<'tcx>( - cx: &LateContext<'tcx>, - expr: &'tcx Expr<'tcx>, - semi_spans: Vec<Span>, /* containing all the places where we would need to remove semicolons if finding an - * needless return */ - replacement: RetReplacement<'tcx>, - match_ty_opt: Option<Ty<'_>>, -) { - let peeled_drop_expr = expr.peel_drop_temps(); - match &peeled_drop_expr.kind { - // simple return is always "bad" - ExprKind::Ret(inner) => { - // check if expr return nothing - let ret_span = if inner.is_none() && replacement == RetReplacement::Empty { - extend_span_to_previous_non_ws(cx, peeled_drop_expr.span) - } else { - peeled_drop_expr.span - }; - - let replacement = if let Some(inner_expr) = inner { - // if desugar of `do yeet`, don't lint - if let ExprKind::Call(path_expr, [_]) = inner_expr.kind - && let ExprKind::Path(QPath::LangItem(LangItem::TryTraitFromYeet, ..)) = path_expr.kind - { - return; - } - - let mut applicability = Applicability::MachineApplicable; - let (snippet, _) = snippet_with_context(cx, inner_expr.span, ret_span.ctxt(), "..", &mut applicability); - if binary_expr_needs_parentheses(inner_expr) { - RetReplacement::NeedsPar(snippet, applicability) - } else { - RetReplacement::Expr(snippet, applicability) - } - } else { - match match_ty_opt { - Some(match_ty) => { - match match_ty.kind() { - // If the code got till here with - // tuple not getting detected before it, - // then we are sure it's going to be Unit - // type - ty::Tuple(_) => RetReplacement::Unit, - // We don't want to anything in this case - // cause we can't predict what the user would - // want here - _ => return, - } - }, - None => replacement, - } - }; - - if inner.is_some_and(|inner| leaks_droppable_temporary_with_limited_lifetime(cx, inner)) { - return; - } - - if ret_span.from_expansion() || is_from_proc_macro(cx, expr) { - return; - } - - // Returns may be used to turn an expression into a statement in rustc's AST. - // This allows the addition of attributes, like `#[allow]` (See: clippy#9361) - // `#[expect(clippy::needless_return)]` needs to be handled separately to - // actually fulfill the expectation (clippy::#12998) - match cx.tcx.hir_attrs(expr.hir_id) { - [] => {}, - [attr] => { - if matches!(Level::from_attr(attr), Some((Level::Expect, _))) - && let metas = attr.meta_item_list() - && let Some(lst) = metas - && let [MetaItemInner::MetaItem(meta_item), ..] = lst.as_slice() - && let [tool, lint_name] = meta_item.path.segments.as_slice() - && tool.ident.name == sym::clippy - && matches!( - lint_name.ident.name, - sym::needless_return | sym::style | sym::all | sym::warnings - ) - { - // This is an expectation of the `needless_return` lint - } else { - return; - } - }, - _ => return, - } - - emit_return_lint( - cx, - peeled_drop_expr.span, - ret_span, - semi_spans, - &replacement, - expr.hir_id, - ); - }, - ExprKind::If(_, then, else_clause_opt) => { - check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone()); - if let Some(else_clause) = else_clause_opt { - // The `RetReplacement` won't be used there as `else_clause` will be either a block or - // a `if` expression. - check_final_expr(cx, else_clause, semi_spans, RetReplacement::Empty, match_ty_opt); - } - }, - // a match expr, check all arms - // an if/if let expr, check both exprs - // note, if without else is going to be a type checking error anyways - // (except for unit type functions) so we don't match it - ExprKind::Match(_, arms, MatchSource::Normal) => { - let match_ty = cx.typeck_results().expr_ty(peeled_drop_expr); - for arm in *arms { - check_final_expr(cx, arm.body, semi_spans.clone(), RetReplacement::Unit, Some(match_ty)); - } - }, - // if it's a whole block, check it - other_expr_kind => check_block_return(cx, other_expr_kind, peeled_drop_expr.span, semi_spans), - } -} - -fn emit_return_lint( - cx: &LateContext<'_>, - lint_span: Span, - ret_span: Span, - semi_spans: Vec<Span>, - replacement: &RetReplacement<'_>, - at: HirId, -) { - span_lint_hir_and_then( - cx, - NEEDLESS_RETURN, - at, - lint_span, - "unneeded `return` statement", - |diag| { - let suggestions = std::iter::once((ret_span, replacement.to_string())) - .chain(semi_spans.into_iter().map(|span| (span, String::new()))) - .collect(); - - diag.multipart_suggestion_verbose(replacement.sugg_help(), suggestions, replacement.applicability()); - }, - ); -} - -fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { - for_each_expr(cx, expr, |e| { - if let Some(def_id) = fn_def_id(cx, e) - && cx - .tcx - .fn_sig(def_id) - .instantiate_identity() - .skip_binder() - .output() - .walk() - .any(|arg| matches!(arg.kind(), GenericArgKind::Lifetime(re) if !re.is_static())) - { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } - }) - .is_some() -} - -// Go backwards while encountering whitespace and extend the given Span to that point. -fn extend_span_to_previous_non_ws(cx: &LateContext<'_>, sp: Span) -> Span { - if let Ok(prev_source) = cx.sess().source_map().span_to_prev_source(sp) { - let ws = [b' ', b'\t', b'\n']; - if let Some(non_ws_pos) = prev_source.bytes().rposition(|c| !ws.contains(&c)) { - let len = prev_source.len() - non_ws_pos - 1; - return sp.with_lo(sp.lo() - BytePos::from_usize(len)); - } - } - - sp -} diff --git a/src/tools/clippy/clippy_lints/src/returns/let_and_return.rs b/src/tools/clippy/clippy_lints/src/returns/let_and_return.rs new file mode 100644 index 00000000000..e2002fb36e5 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/returns/let_and_return.rs @@ -0,0 +1,86 @@ +use clippy_utils::diagnostics::span_lint_hir_and_then; +use clippy_utils::source::SpanRangeExt; +use clippy_utils::sugg::has_enclosing_paren; +use clippy_utils::visitors::for_each_expr; +use clippy_utils::{binary_expr_needs_parentheses, fn_def_id, path_to_local_id, span_contains_cfg}; +use core::ops::ControlFlow; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, PatKind, StmtKind}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::ty::GenericArgKind; +use rustc_span::edition::Edition; + +use super::LET_AND_RETURN; + +pub(super) fn check_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'_>) { + // we need both a let-binding stmt and an expr + if let Some(retexpr) = block.expr + && let Some(stmt) = block.stmts.last() + && let StmtKind::Let(local) = &stmt.kind + && local.ty.is_none() + && cx.tcx.hir_attrs(local.hir_id).is_empty() + && let Some(initexpr) = &local.init + && let PatKind::Binding(_, local_id, _, _) = local.pat.kind + && path_to_local_id(retexpr, local_id) + && (cx.sess().edition() >= Edition::Edition2024 || !last_statement_borrows(cx, initexpr)) + && !initexpr.span.in_external_macro(cx.sess().source_map()) + && !retexpr.span.in_external_macro(cx.sess().source_map()) + && !local.span.from_expansion() + && !span_contains_cfg(cx, stmt.span.between(retexpr.span)) + { + span_lint_hir_and_then( + cx, + LET_AND_RETURN, + retexpr.hir_id, + retexpr.span, + "returning the result of a `let` binding from a block", + |err| { + err.span_label(local.span, "unnecessary `let` binding"); + + if let Some(src) = initexpr.span.get_source_text(cx) { + let sugg = if binary_expr_needs_parentheses(initexpr) { + if has_enclosing_paren(&src) { + src.to_owned() + } else { + format!("({src})") + } + } else if !cx.typeck_results().expr_adjustments(retexpr).is_empty() { + if has_enclosing_paren(&src) { + format!("{src} as _") + } else { + format!("({src}) as _") + } + } else { + src.to_owned() + }; + err.multipart_suggestion( + "return the expression directly", + vec![(local.span, String::new()), (retexpr.span, sugg)], + Applicability::MachineApplicable, + ); + } else { + err.span_help(initexpr.span, "this expression can be directly returned"); + } + }, + ); + } +} +fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + for_each_expr(cx, expr, |e| { + if let Some(def_id) = fn_def_id(cx, e) + && cx + .tcx + .fn_sig(def_id) + .instantiate_identity() + .skip_binder() + .output() + .walk() + .any(|arg| matches!(arg.kind(), GenericArgKind::Lifetime(re) if !re.is_static())) + { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) + } + }) + .is_some() +} diff --git a/src/tools/clippy/clippy_lints/src/returns/mod.rs b/src/tools/clippy/clippy_lints/src/returns/mod.rs new file mode 100644 index 00000000000..47c6332b9b8 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/returns/mod.rs @@ -0,0 +1,140 @@ +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Block, Body, FnDecl, Stmt}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; +use rustc_span::Span; +use rustc_span::def_id::LocalDefId; + +mod let_and_return; +mod needless_return; +mod needless_return_with_question_mark; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `let`-bindings, which are subsequently + /// returned. + /// + /// ### Why is this bad? + /// It is just extraneous code. Remove it to make your code + /// more rusty. + /// + /// ### Known problems + /// In the case of some temporaries, e.g. locks, eliding the variable binding could lead + /// to deadlocks. See [this issue](https://github.com/rust-lang/rust/issues/37612). + /// This could become relevant if the code is later changed to use the code that would have been + /// bound without first assigning it to a let-binding. + /// + /// ### Example + /// ```no_run + /// fn foo() -> String { + /// let x = String::new(); + /// x + /// } + /// ``` + /// instead, use + /// ```no_run + /// fn foo() -> String { + /// String::new() + /// } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub LET_AND_RETURN, + style, + "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for return statements at the end of a block. + /// + /// ### Why is this bad? + /// Removing the `return` and semicolon will make the code + /// more rusty. + /// + /// ### Example + /// ```no_run + /// fn foo(x: usize) -> usize { + /// return x; + /// } + /// ``` + /// simplify to + /// ```no_run + /// fn foo(x: usize) -> usize { + /// x + /// } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub NEEDLESS_RETURN, + // This lint requires some special handling in `check_final_expr` for `#[expect]`. + // This handling needs to be updated if the group gets changed. This should also + // be caught by tests. + style, + "using a return statement like `return expr;` where an expression would suffice" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for return statements on `Err` paired with the `?` operator. + /// + /// ### Why is this bad? + /// The `return` is unnecessary. + /// + /// Returns may be used to add attributes to the return expression. Return + /// statements with attributes are therefore be accepted by this lint. + /// + /// ### Example + /// ```rust,ignore + /// fn foo(x: usize) -> Result<(), Box<dyn Error>> { + /// if x == 0 { + /// return Err(...)?; + /// } + /// Ok(()) + /// } + /// ``` + /// simplify to + /// ```rust,ignore + /// fn foo(x: usize) -> Result<(), Box<dyn Error>> { + /// if x == 0 { + /// Err(...)?; + /// } + /// Ok(()) + /// } + /// ``` + /// if paired with `try_err`, use instead: + /// ```rust,ignore + /// fn foo(x: usize) -> Result<(), Box<dyn Error>> { + /// if x == 0 { + /// return Err(...); + /// } + /// Ok(()) + /// } + /// ``` + #[clippy::version = "1.73.0"] + pub NEEDLESS_RETURN_WITH_QUESTION_MARK, + style, + "using a return statement like `return Err(expr)?;` where removing it would suffice" +} + +declare_lint_pass!(Return => [LET_AND_RETURN, NEEDLESS_RETURN, NEEDLESS_RETURN_WITH_QUESTION_MARK]); + +impl<'tcx> LateLintPass<'tcx> for Return { + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + needless_return_with_question_mark::check_stmt(cx, stmt); + } + + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) { + let_and_return::check_block(cx, block); + } + + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + sp: Span, + _: LocalDefId, + ) { + needless_return::check_fn(cx, kind, body, sp); + } +} diff --git a/src/tools/clippy/clippy_lints/src/returns/needless_return.rs b/src/tools/clippy/clippy_lints/src/returns/needless_return.rs new file mode 100644 index 00000000000..04739fc1b22 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/returns/needless_return.rs @@ -0,0 +1,269 @@ +use clippy_utils::diagnostics::span_lint_hir_and_then; +use clippy_utils::source::snippet_with_context; +use clippy_utils::{ + binary_expr_needs_parentheses, is_from_proc_macro, leaks_droppable_temporary_with_limited_lifetime, + span_contains_cfg, span_find_starting_semi, sym, +}; +use rustc_ast::MetaItemInner; +use rustc_errors::Applicability; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, Expr, ExprKind, HirId, LangItem, MatchSource, QPath, StmtKind}; +use rustc_lint::{LateContext, Level, LintContext}; +use rustc_middle::ty::{self, Ty}; +use rustc_span::{BytePos, Pos, Span}; +use std::borrow::Cow; +use std::fmt::Display; + +use super::NEEDLESS_RETURN; + +#[derive(PartialEq, Eq)] +enum RetReplacement<'tcx> { + Empty, + Block, + Unit, + NeedsPar(Cow<'tcx, str>, Applicability), + Expr(Cow<'tcx, str>, Applicability), +} + +impl RetReplacement<'_> { + fn sugg_help(&self) -> &'static str { + match self { + Self::Empty | Self::Expr(..) => "remove `return`", + Self::Block => "replace `return` with an empty block", + Self::Unit => "replace `return` with a unit value", + Self::NeedsPar(..) => "remove `return` and wrap the sequence with parentheses", + } + } + + fn applicability(&self) -> Applicability { + match self { + Self::Expr(_, ap) | Self::NeedsPar(_, ap) => *ap, + _ => Applicability::MachineApplicable, + } + } +} + +impl Display for RetReplacement<'_> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Empty => f.write_str(""), + Self::Block => f.write_str("{}"), + Self::Unit => f.write_str("()"), + Self::NeedsPar(inner, _) => write!(f, "({inner})"), + Self::Expr(inner, _) => write!(f, "{inner}"), + } + } +} + +pub(super) fn check_fn<'tcx>(cx: &LateContext<'tcx>, kind: FnKind<'tcx>, body: &'tcx Body<'tcx>, sp: Span) { + if sp.from_expansion() { + return; + } + + match kind { + FnKind::Closure => { + // when returning without value in closure, replace this `return` + // with an empty block to prevent invalid suggestion (see #6501) + let replacement = if let ExprKind::Ret(None) = &body.value.kind { + RetReplacement::Block + } else { + RetReplacement::Empty + }; + check_final_expr(cx, body.value, vec![], replacement, None); + }, + FnKind::ItemFn(..) | FnKind::Method(..) => { + check_block_return(cx, &body.value.kind, sp, vec![]); + }, + } +} + +// if `expr` is a block, check if there are needless returns in it +fn check_block_return<'tcx>(cx: &LateContext<'tcx>, expr_kind: &ExprKind<'tcx>, sp: Span, mut semi_spans: Vec<Span>) { + if let ExprKind::Block(block, _) = expr_kind { + if let Some(block_expr) = block.expr { + check_final_expr(cx, block_expr, semi_spans, RetReplacement::Empty, None); + } else if let Some(stmt) = block.stmts.last() { + if span_contains_cfg( + cx, + Span::between( + stmt.span, + cx.sess().source_map().end_point(block.span), // the closing brace of the block + ), + ) { + return; + } + match stmt.kind { + StmtKind::Expr(expr) => { + check_final_expr(cx, expr, semi_spans, RetReplacement::Empty, None); + }, + StmtKind::Semi(semi_expr) => { + // Remove ending semicolons and any whitespace ' ' in between. + // Without `return`, the suggestion might not compile if the semicolon is retained + if let Some(semi_span) = stmt.span.trim_start(semi_expr.span) { + let semi_span_to_remove = + span_find_starting_semi(cx.sess().source_map(), semi_span.with_hi(sp.hi())); + semi_spans.push(semi_span_to_remove); + } + check_final_expr(cx, semi_expr, semi_spans, RetReplacement::Empty, None); + }, + _ => (), + } + } + } +} + +fn check_final_expr<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + semi_spans: Vec<Span>, /* containing all the places where we would need to remove semicolons if finding an + * needless return */ + replacement: RetReplacement<'tcx>, + match_ty_opt: Option<Ty<'_>>, +) { + let peeled_drop_expr = expr.peel_drop_temps(); + match &peeled_drop_expr.kind { + // simple return is always "bad" + ExprKind::Ret(inner) => { + // check if expr return nothing + let ret_span = if inner.is_none() && replacement == RetReplacement::Empty { + extend_span_to_previous_non_ws(cx, peeled_drop_expr.span) + } else { + peeled_drop_expr.span + }; + + let replacement = if let Some(inner_expr) = inner { + // if desugar of `do yeet`, don't lint + if let ExprKind::Call(path_expr, [_]) = inner_expr.kind + && let ExprKind::Path(QPath::LangItem(LangItem::TryTraitFromYeet, ..)) = path_expr.kind + { + return; + } + + let mut applicability = Applicability::MachineApplicable; + let (snippet, _) = snippet_with_context(cx, inner_expr.span, ret_span.ctxt(), "..", &mut applicability); + if binary_expr_needs_parentheses(inner_expr) { + RetReplacement::NeedsPar(snippet, applicability) + } else { + RetReplacement::Expr(snippet, applicability) + } + } else { + match match_ty_opt { + Some(match_ty) => { + match match_ty.kind() { + // If the code got till here with + // tuple not getting detected before it, + // then we are sure it's going to be Unit + // type + ty::Tuple(_) => RetReplacement::Unit, + // We don't want to anything in this case + // cause we can't predict what the user would + // want here + _ => return, + } + }, + None => replacement, + } + }; + + if inner.is_some_and(|inner| leaks_droppable_temporary_with_limited_lifetime(cx, inner)) { + return; + } + + if ret_span.from_expansion() || is_from_proc_macro(cx, expr) { + return; + } + + // Returns may be used to turn an expression into a statement in rustc's AST. + // This allows the addition of attributes, like `#[allow]` (See: clippy#9361) + // `#[expect(clippy::needless_return)]` needs to be handled separately to + // actually fulfill the expectation (clippy::#12998) + match cx.tcx.hir_attrs(expr.hir_id) { + [] => {}, + [attr] => { + if matches!(Level::from_attr(attr), Some((Level::Expect, _))) + && let metas = attr.meta_item_list() + && let Some(lst) = metas + && let [MetaItemInner::MetaItem(meta_item), ..] = lst.as_slice() + && let [tool, lint_name] = meta_item.path.segments.as_slice() + && tool.ident.name == sym::clippy + && matches!( + lint_name.ident.name, + sym::needless_return | sym::style | sym::all | sym::warnings + ) + { + // This is an expectation of the `needless_return` lint + } else { + return; + } + }, + _ => return, + } + + emit_return_lint( + cx, + peeled_drop_expr.span, + ret_span, + semi_spans, + &replacement, + expr.hir_id, + ); + }, + ExprKind::If(_, then, else_clause_opt) => { + check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone()); + if let Some(else_clause) = else_clause_opt { + // The `RetReplacement` won't be used there as `else_clause` will be either a block or + // a `if` expression. + check_final_expr(cx, else_clause, semi_spans, RetReplacement::Empty, match_ty_opt); + } + }, + // a match expr, check all arms + // an if/if let expr, check both exprs + // note, if without else is going to be a type checking error anyways + // (except for unit type functions) so we don't match it + ExprKind::Match(_, arms, MatchSource::Normal) => { + let match_ty = cx.typeck_results().expr_ty(peeled_drop_expr); + for arm in *arms { + check_final_expr(cx, arm.body, semi_spans.clone(), RetReplacement::Unit, Some(match_ty)); + } + }, + // if it's a whole block, check it + other_expr_kind => check_block_return(cx, other_expr_kind, peeled_drop_expr.span, semi_spans), + } +} + +fn emit_return_lint( + cx: &LateContext<'_>, + lint_span: Span, + ret_span: Span, + semi_spans: Vec<Span>, + replacement: &RetReplacement<'_>, + at: HirId, +) { + span_lint_hir_and_then( + cx, + NEEDLESS_RETURN, + at, + lint_span, + "unneeded `return` statement", + |diag| { + let suggestions = std::iter::once((ret_span, replacement.to_string())) + .chain(semi_spans.into_iter().map(|span| (span, String::new()))) + .collect(); + + diag.multipart_suggestion_verbose(replacement.sugg_help(), suggestions, replacement.applicability()); + }, + ); +} + +// Go backwards while encountering whitespace and extend the given Span to that point. +fn extend_span_to_previous_non_ws(cx: &LateContext<'_>, sp: Span) -> Span { + if let Ok(prev_source) = cx.sess().source_map().span_to_prev_source(sp) { + let ws = [b' ', b'\t', b'\n']; + if let Some(non_ws_pos) = prev_source.bytes().rposition(|c| !ws.contains(&c)) { + let len = prev_source.len() - non_ws_pos - 1; + return sp.with_lo(sp.lo() - BytePos::from_usize(len)); + } + } + + sp +} diff --git a/src/tools/clippy/clippy_lints/src/returns/needless_return_with_question_mark.rs b/src/tools/clippy/clippy_lints/src/returns/needless_return_with_question_mark.rs new file mode 100644 index 00000000000..c05038cd1e5 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/returns/needless_return_with_question_mark.rs @@ -0,0 +1,60 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::{is_from_proc_macro, is_inside_let_else, is_res_lang_ctor, path_res}; +use rustc_errors::Applicability; +use rustc_hir::LangItem::ResultErr; +use rustc_hir::{ExprKind, HirId, ItemKind, MatchSource, Node, OwnerNode, Stmt, StmtKind}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::ty::adjustment::Adjust; + +use super::NEEDLESS_RETURN_WITH_QUESTION_MARK; + +pub(super) fn check_stmt<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + if !stmt.span.in_external_macro(cx.sess().source_map()) + && let StmtKind::Semi(expr) = stmt.kind + && let ExprKind::Ret(Some(ret)) = expr.kind + // return Err(...)? desugars to a match + // over a Err(...).branch() + // which breaks down to a branch call, with the callee being + // the constructor of the Err variant + && let ExprKind::Match(maybe_cons, _, MatchSource::TryDesugar(_)) = ret.kind + && let ExprKind::Call(_, [maybe_result_err]) = maybe_cons.kind + && let ExprKind::Call(maybe_constr, _) = maybe_result_err.kind + && is_res_lang_ctor(cx, path_res(cx, maybe_constr), ResultErr) + + // Ensure this is not the final stmt, otherwise removing it would cause a compile error + && let OwnerNode::Item(item) = cx.tcx.hir_owner_node(cx.tcx.hir_get_parent_item(expr.hir_id)) + && let ItemKind::Fn { body, .. } = item.kind + && let block = cx.tcx.hir_body(body).value + && let ExprKind::Block(block, _) = block.kind + && !is_inside_let_else(cx.tcx, expr) + && let [.., final_stmt] = block.stmts + && final_stmt.hir_id != stmt.hir_id + && !is_from_proc_macro(cx, expr) + && !stmt_needs_never_type(cx, stmt.hir_id) + { + span_lint_and_sugg( + cx, + NEEDLESS_RETURN_WITH_QUESTION_MARK, + expr.span.until(ret.span), + "unneeded `return` statement with `?` operator", + "remove it", + String::new(), + Applicability::MachineApplicable, + ); + } +} + +/// Checks if a return statement is "needed" in the middle of a block, or if it can be removed. +/// This is the case when the enclosing block expression is coerced to some other type, +/// which only works because of the never-ness of `return` expressions +fn stmt_needs_never_type(cx: &LateContext<'_>, stmt_hir_id: HirId) -> bool { + cx.tcx + .hir_parent_iter(stmt_hir_id) + .find_map(|(_, node)| if let Node::Expr(expr) = node { Some(expr) } else { None }) + .is_some_and(|e| { + cx.typeck_results() + .expr_adjustments(e) + .iter() + .any(|adjust| adjust.target != cx.tcx.types.unit && matches!(adjust.kind, Adjust::NeverToAny)) + }) +} diff --git a/src/tools/clippy/clippy_lints/src/semicolon_block.rs b/src/tools/clippy/clippy_lints/src/semicolon_block.rs index 1dea8f17c34..371d62a0684 100644 --- a/src/tools/clippy/clippy_lints/src/semicolon_block.rs +++ b/src/tools/clippy/clippy_lints/src/semicolon_block.rs @@ -1,5 +1,6 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::SpanRangeExt; use rustc_errors::Applicability; use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -82,6 +83,19 @@ impl SemicolonBlock { let insert_span = tail.span.source_callsite().shrink_to_hi(); let remove_span = semi_span.with_lo(block.span.hi()); + // If the block is surrounded by parens (`({ 0 });`), the author probably knows what + // they're doing and why, so don't get in their way. + // + // This has the additional benefit of stopping the block being parsed as a function call: + // ``` + // fn foo() { + // ({ 0 }); // if we remove this `;`, this will parse as a `({ 0 })(5);` function call + // (5); + // } + if remove_span.check_source_text(cx, |src| src.contains(')')) { + return; + } + if self.semicolon_inside_block_ignore_singleline && get_line(cx, remove_span) == get_line(cx, insert_span) { return; } diff --git a/src/tools/clippy/clippy_lints/src/size_of_ref.rs b/src/tools/clippy/clippy_lints/src/size_of_ref.rs index 60d923bcd77..606e852aae9 100644 --- a/src/tools/clippy/clippy_lints/src/size_of_ref.rs +++ b/src/tools/clippy/clippy_lints/src/size_of_ref.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{path_def_id, peel_middle_ty_refs}; +use clippy_utils::path_def_id; +use clippy_utils::ty::peel_and_count_ty_refs; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -59,7 +60,7 @@ impl LateLintPass<'_> for SizeOfRef { && let Some(def_id) = path_def_id(cx, path) && cx.tcx.is_diagnostic_item(sym::mem_size_of_val, def_id) && let arg_ty = cx.typeck_results().expr_ty(arg) - && peel_middle_ty_refs(arg_ty).1 > 1 + && peel_and_count_ty_refs(arg_ty).1 > 1 { span_lint_and_help( cx, diff --git a/src/tools/clippy/clippy_lints/src/toplevel_ref_arg.rs b/src/tools/clippy/clippy_lints/src/toplevel_ref_arg.rs new file mode 100644 index 00000000000..074b79263d3 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/toplevel_ref_arg.rs @@ -0,0 +1,119 @@ +use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; +use clippy_utils::source::{snippet, snippet_with_context}; +use clippy_utils::sugg::Sugg; +use clippy_utils::{is_lint_allowed, iter_input_pats}; +use rustc_errors::Applicability; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{BindingMode, Body, ByRef, FnDecl, Mutability, PatKind, Stmt, StmtKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; +use rustc_span::Span; +use rustc_span::def_id::LocalDefId; + +use crate::ref_patterns::REF_PATTERNS; + +declare_clippy_lint! { + /// ### What it does + /// Checks for function arguments and let bindings denoted as + /// `ref`. + /// + /// ### Why is this bad? + /// The `ref` declaration makes the function take an owned + /// value, but turns the argument into a reference (which means that the value + /// is destroyed when exiting the function). This adds not much value: either + /// take a reference type, or take an owned value and create references in the + /// body. + /// + /// For let bindings, `let x = &foo;` is preferred over `let ref x = foo`. The + /// type of `x` is more obvious with the former. + /// + /// ### Known problems + /// If the argument is dereferenced within the function, + /// removing the `ref` will lead to errors. This can be fixed by removing the + /// dereferences, e.g., changing `*x` to `x` within the function. + /// + /// ### Example + /// ```no_run + /// fn foo(ref _x: u8) {} + /// ``` + /// + /// Use instead: + /// ```no_run + /// fn foo(_x: &u8) {} + /// ``` + #[clippy::version = "pre 1.29.0"] + pub TOPLEVEL_REF_ARG, + style, + "an entire binding declared as `ref`, in a function argument or a `let` statement" +} + +declare_lint_pass!(ToplevelRefArg => [TOPLEVEL_REF_ARG]); + +impl<'tcx> LateLintPass<'tcx> for ToplevelRefArg { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + k: FnKind<'tcx>, + decl: &'tcx FnDecl<'_>, + body: &'tcx Body<'_>, + _: Span, + _: LocalDefId, + ) { + if !matches!(k, FnKind::Closure) { + for arg in iter_input_pats(decl, body) { + if let PatKind::Binding(BindingMode(ByRef::Yes(_), _), ..) = arg.pat.kind + && is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) + && !arg.span.in_external_macro(cx.tcx.sess.source_map()) + { + span_lint_hir( + cx, + TOPLEVEL_REF_ARG, + arg.hir_id, + arg.pat.span, + "`ref` directly on a function parameter does not prevent taking ownership of the passed argument. \ + Consider using a reference type instead", + ); + } + } + } + } + + fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { + if let StmtKind::Let(local) = stmt.kind + && let PatKind::Binding(BindingMode(ByRef::Yes(mutabl), _), .., name, None) = local.pat.kind + && let Some(init) = local.init + // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. + && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id) + && !stmt.span.in_external_macro(cx.tcx.sess.source_map()) + { + let ctxt = local.span.ctxt(); + let mut app = Applicability::MachineApplicable; + let sugg_init = Sugg::hir_with_context(cx, init, ctxt, "..", &mut app); + let (mutopt, initref) = match mutabl { + Mutability::Mut => ("mut ", sugg_init.mut_addr()), + Mutability::Not => ("", sugg_init.addr()), + }; + let tyopt = if let Some(ty) = local.ty { + let ty_snip = snippet_with_context(cx, ty.span, ctxt, "_", &mut app).0; + format!(": &{mutopt}{ty_snip}") + } else { + String::new() + }; + span_lint_hir_and_then( + cx, + TOPLEVEL_REF_ARG, + init.hir_id, + local.pat.span, + "`ref` on an entire `let` pattern is discouraged, take a reference with `&` instead", + |diag| { + diag.span_suggestion( + stmt.span, + "try", + format!("let {name}{tyopt} = {initref};", name = snippet(cx, name.span, ".."),), + app, + ); + }, + ); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs index e58212fae15..e67ab6a73d2 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs @@ -28,16 +28,27 @@ pub(super) fn check<'tcx>( format!("transmute from a pointer type (`{from_ty}`) to a reference type (`{to_ty}`)"), |diag| { let arg = sugg::Sugg::hir(cx, arg, ".."); - let (deref, cast) = if *mutbl == Mutability::Mut { - ("&mut *", "*mut") - } else { - ("&*", "*const") + let (deref, cast) = match mutbl { + Mutability::Mut => ("&mut *", "*mut"), + Mutability::Not => ("&*", "*const"), }; let mut app = Applicability::MachineApplicable; let sugg = if let Some(ty) = get_explicit_type(path) { let ty_snip = snippet_with_applicability(cx, ty.span, "..", &mut app); - if msrv.meets(cx, msrvs::POINTER_CAST) { + if !to_ref_ty.is_sized(cx.tcx, cx.typing_env()) { + // We can't suggest `.cast()`, because that requires `to_ref_ty` to be Sized. + if from_ptr_ty.has_erased_regions() { + // We can't suggest `as *mut/const () as *mut/const to_ref_ty`, because the former is a + // thin pointer, whereas the latter is a wide pointer, due of its pointee, `to_ref_ty`, + // being !Sized. + // + // The only remaining option is be to skip `*mut/const ()`, but that might not be safe + // to do because of the erased regions in `from_ptr_ty`, so reduce the applicability. + app = Applicability::MaybeIncorrect; + } + sugg::make_unop(deref, arg.as_ty(format!("{cast} {ty_snip}"))).to_string() + } else if msrv.meets(cx, msrvs::POINTER_CAST) { format!("{deref}{}.cast::<{ty_snip}>()", arg.maybe_paren()) } else if from_ptr_ty.has_erased_regions() { sugg::make_unop(deref, arg.as_ty(format!("{cast} () as {cast} {ty_snip}"))).to_string() @@ -45,15 +56,22 @@ pub(super) fn check<'tcx>( sugg::make_unop(deref, arg.as_ty(format!("{cast} {ty_snip}"))).to_string() } } else if *from_ptr_ty == *to_ref_ty { - if from_ptr_ty.has_erased_regions() { - if msrv.meets(cx, msrvs::POINTER_CAST) { - format!("{deref}{}.cast::<{to_ref_ty}>()", arg.maybe_paren()) - } else { - sugg::make_unop(deref, arg.as_ty(format!("{cast} () as {cast} {to_ref_ty}"))) - .to_string() - } - } else { + if !from_ptr_ty.has_erased_regions() { sugg::make_unop(deref, arg).to_string() + } else if !to_ref_ty.is_sized(cx.tcx, cx.typing_env()) { + // 1. We can't suggest `.cast()`, because that requires `to_ref_ty` to be Sized. + // 2. We can't suggest `as *mut/const () as *mut/const to_ref_ty`, because the former is a + // thin pointer, whereas the latter is a wide pointer, due of its pointee, `to_ref_ty`, + // being !Sized. + // + // The only remaining option is be to skip `*mut/const ()`, but that might not be safe to do + // because of the erased regions in `from_ptr_ty`, so reduce the applicability. + app = Applicability::MaybeIncorrect; + sugg::make_unop(deref, arg.as_ty(format!("{cast} {to_ref_ty}"))).to_string() + } else if msrv.meets(cx, msrvs::POINTER_CAST) { + format!("{deref}{}.cast::<{to_ref_ty}>()", arg.maybe_paren()) + } else { + sugg::make_unop(deref, arg.as_ty(format!("{cast} () as {cast} {to_ref_ty}"))).to_string() } } else { sugg::make_unop(deref, arg.as_ty(format!("{cast} {to_ref_ty}"))).to_string() diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_ref_to_ref.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_ref_to_ref.rs index 6aeb22d41a7..70c2a73ce6e 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_ref_to_ref.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_ref_to_ref.rs @@ -45,7 +45,9 @@ pub(super) fn check<'tcx>( Applicability::MaybeIncorrect, ); triggered = true; - } else if (cx.tcx.erase_and_anonymize_regions(from_ty) != cx.tcx.erase_and_anonymize_regions(to_ty)) && !const_context { + } else if (cx.tcx.erase_and_anonymize_regions(from_ty) != cx.tcx.erase_and_anonymize_regions(to_ty)) + && !const_context + { span_lint_and_then( cx, TRANSMUTE_PTR_TO_PTR, diff --git a/src/tools/clippy/clippy_lints/src/uninit_vec.rs b/src/tools/clippy/clippy_lints/src/uninit_vec.rs index cee4a53f03c..51116b5eba9 100644 --- a/src/tools/clippy/clippy_lints/src/uninit_vec.rs +++ b/src/tools/clippy/clippy_lints/src/uninit_vec.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; use clippy_utils::higher::{VecInitKind, get_vec_init_kind}; use clippy_utils::ty::{is_type_diagnostic_item, is_uninit_value_valid_for_ty}; use clippy_utils::{SpanlessEq, is_integer_literal, is_lint_allowed, path_to_local_id, peel_hir_expr_while, sym}; @@ -95,16 +95,13 @@ fn handle_uninit_vec_pair<'tcx>( // Check T of Vec<T> if !is_uninit_value_valid_for_ty(cx, args.type_at(0)) { - // FIXME: #7698, false positive of the internal lints - #[expect(clippy::collapsible_span_lint_calls)] - span_lint_and_then( + span_lint_and_help( cx, UNINIT_VEC, vec![call_span, maybe_init_or_reserve.span], "calling `set_len()` immediately after reserving a buffer creates uninitialized values", - |diag| { - diag.help("initialize the buffer or wrap the content in `MaybeUninit`"); - }, + None, + "initialize the buffer or wrap the content in `MaybeUninit`", ); } } else { diff --git a/src/tools/clippy/clippy_lints/src/unused_peekable.rs b/src/tools/clippy/clippy_lints/src/unused_peekable.rs index 1f5351e32aa..5224b62e9fc 100644 --- a/src/tools/clippy/clippy_lints/src/unused_peekable.rs +++ b/src/tools/clippy/clippy_lints/src/unused_peekable.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_hir_and_then; -use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable}; +use clippy_utils::ty::{is_type_diagnostic_item, peel_and_count_ty_refs}; use clippy_utils::{fn_def_id, is_trait_method, path_to_local_id, peel_ref_operators, sym}; use rustc_ast::Mutability; use rustc_hir::intravisit::{Visitor, walk_expr}; @@ -61,7 +61,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedPeekable { && let Some(init) = local.init && !init.span.from_expansion() && let Some(ty) = cx.typeck_results().expr_ty_opt(init) - && let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty) + && let (ty, _, None | Some(Mutability::Mut)) = peel_and_count_ty_refs(ty) && is_type_diagnostic_item(cx, ty, sym::IterPeekable) { let mut vis = PeekableVisitor::new(cx, binding); @@ -211,7 +211,7 @@ impl<'tcx> Visitor<'tcx> for PeekableVisitor<'_, 'tcx> { fn arg_is_mut_peekable(cx: &LateContext<'_>, arg: &Expr<'_>) -> bool { if let Some(ty) = cx.typeck_results().expr_ty_opt(arg) - && let (ty, _, Mutability::Mut) = peel_mid_ty_refs_is_mutable(ty) + && let (ty, _, None | Some(Mutability::Mut)) = peel_and_count_ty_refs(ty) && is_type_diagnostic_item(cx, ty, sym::IterPeekable) { true diff --git a/src/tools/clippy/clippy_lints/src/unwrap.rs b/src/tools/clippy/clippy_lints/src/unwrap.rs index 490da4f1e03..34dfe5b6546 100644 --- a/src/tools/clippy/clippy_lints/src/unwrap.rs +++ b/src/tools/clippy/clippy_lints/src/unwrap.rs @@ -292,6 +292,7 @@ impl<'tcx> Visitor<'tcx> for UnwrappableVariablesVisitor<'_, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { // Shouldn't lint when `expr` is in macro. if expr.span.in_external_macro(self.cx.tcx.sess.source_map()) { + walk_expr(self, expr); return; } // Skip checking inside closures since they are visited through `Unwrap::check_fn()` already. diff --git a/src/tools/clippy/clippy_lints/src/use_self.rs b/src/tools/clippy/clippy_lints/src/use_self.rs index 8252e6d4869..9d5be922f43 100644 --- a/src/tools/clippy/clippy_lints/src/use_self.rs +++ b/src/tools/clippy/clippy_lints/src/use_self.rs @@ -2,20 +2,21 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::ty::{same_type_and_consts, ty_from_hir_ty}; +use clippy_utils::ty::{same_type_modulo_regions, ty_from_hir_ty}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{InferKind, Visitor, VisitorExt, walk_ty}; use rustc_hir::{ - self as hir, AmbigArg, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParam, GenericParamKind, - HirId, Impl, ImplItemKind, Item, ItemKind, Pat, PatExpr, PatExprKind, PatKind, Path, QPath, Ty, TyKind, + self as hir, AmbigArg, Expr, ExprKind, FnRetTy, FnSig, GenericArgsParentheses, GenericParamKind, HirId, Impl, + ImplItemKind, Item, ItemKind, Pat, PatExpr, PatExprKind, PatKind, Path, QPath, Ty, TyKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::Ty as MiddleTy; use rustc_session::impl_lint_pass; use rustc_span::Span; +use std::iter; declare_clippy_lint! { /// ### What it does @@ -101,17 +102,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { let types_to_skip = generics .params .iter() - .filter_map(|param| match param { - GenericParam { - kind: - GenericParamKind::Const { - ty: Ty { hir_id, .. }, .. - }, - .. - } => Some(*hir_id), + .filter_map(|param| match param.kind { + GenericParamKind::Const { ty, .. } => Some(ty.hir_id), _ => None, }) - .chain(std::iter::once(self_ty.hir_id)) + .chain([self_ty.hir_id]) .collect(); StackItem::Check { impl_id: item.owner_id.def_id, @@ -209,11 +204,11 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { && !types_to_skip.contains(&hir_ty.hir_id) && let ty = ty_from_hir_ty(cx, hir_ty.as_unambig_ty()) && let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity() - && same_type_and_consts(ty, impl_ty) + && same_type_modulo_regions(ty, impl_ty) // Ensure the type we encounter and the one from the impl have the same lifetime parameters. It may be that - // the lifetime parameters of `ty` are elided (`impl<'a> Foo<'a> { fn new() -> Self { Foo{..} } }`, in + // the lifetime parameters of `ty` are elided (`impl<'a> Foo<'a> { fn new() -> Self { Foo{..} } }`), in // which case we must still trigger the lint. - && (has_no_lifetime(ty) || same_lifetimes(ty, impl_ty)) + && same_lifetimes(ty, impl_ty) && self.msrv.meets(cx, msrvs::TYPE_ALIAS_ENUM_VARIANTS) { span_lint(cx, hir_ty.span); @@ -226,18 +221,16 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { && cx.typeck_results().expr_ty(expr) == cx.tcx.type_of(impl_id).instantiate_identity() && self.msrv.meets(cx, msrvs::TYPE_ALIAS_ENUM_VARIANTS) { - } else { - return; - } - match expr.kind { - ExprKind::Struct(QPath::Resolved(_, path), ..) => check_path(cx, path), - ExprKind::Call(fun, _) => { - if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind { - check_path(cx, path); - } - }, - ExprKind::Path(QPath::Resolved(_, path)) => check_path(cx, path), - _ => (), + match expr.kind { + ExprKind::Struct(QPath::Resolved(_, path), ..) => check_path(cx, path), + ExprKind::Call(fun, _) => { + if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind { + check_path(cx, path); + } + }, + ExprKind::Path(QPath::Resolved(_, path)) => check_path(cx, path), + _ => (), + } } } @@ -307,36 +300,20 @@ fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) { } } -/// Returns `true` if types `a` and `b` have the same lifetime parameters, otherwise returns -/// `false`. +/// Checks whether types `a` and `b` have the same lifetime parameters. /// /// This function does not check that types `a` and `b` are the same types. fn same_lifetimes<'tcx>(a: MiddleTy<'tcx>, b: MiddleTy<'tcx>) -> bool { use rustc_middle::ty::{Adt, GenericArgKind}; - match (&a.kind(), &b.kind()) { - (&Adt(_, args_a), &Adt(_, args_b)) => { - args_a - .iter() - .zip(args_b.iter()) - .all(|(arg_a, arg_b)| match (arg_a.kind(), arg_b.kind()) { - // TODO: Handle inferred lifetimes - (GenericArgKind::Lifetime(inner_a), GenericArgKind::Lifetime(inner_b)) => inner_a == inner_b, - (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => same_lifetimes(type_a, type_b), - _ => true, - }) + match (a.kind(), b.kind()) { + (Adt(_, args_a), Adt(_, args_b)) => { + iter::zip(*args_a, *args_b).all(|(arg_a, arg_b)| match (arg_a.kind(), arg_b.kind()) { + // TODO: Handle inferred lifetimes + (GenericArgKind::Lifetime(inner_a), GenericArgKind::Lifetime(inner_b)) => inner_a == inner_b, + (GenericArgKind::Type(type_a), GenericArgKind::Type(type_b)) => same_lifetimes(type_a, type_b), + _ => true, + }) }, _ => a == b, } } - -/// Returns `true` if `ty` has no lifetime parameter, otherwise returns `false`. -fn has_no_lifetime(ty: MiddleTy<'_>) -> bool { - use rustc_middle::ty::{Adt, GenericArgKind}; - match ty.kind() { - &Adt(_, args) => !args - .iter() - // TODO: Handle inferred lifetimes - .any(|arg| matches!(arg.kind(), GenericArgKind::Lifetime(..))), - _ => true, - } -} diff --git a/src/tools/clippy/clippy_lints/src/useless_conversion.rs b/src/tools/clippy/clippy_lints/src/useless_conversion.rs index e45f884cfcb..1b137017ecb 100644 --- a/src/tools/clippy/clippy_lints/src/useless_conversion.rs +++ b/src/tools/clippy/clippy_lints/src/useless_conversion.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, snippet_with_context}; use clippy_utils::sugg::{DiagExt as _, Sugg}; -use clippy_utils::ty::{get_type_diagnostic_name, is_copy, is_type_diagnostic_item, same_type_and_consts}; +use clippy_utils::ty::{get_type_diagnostic_name, is_copy, is_type_diagnostic_item, same_type_modulo_regions}; use clippy_utils::{ get_parent_expr, is_inherent_method_call, is_trait_item, is_trait_method, is_ty_alias, path_to_local, sym, }; @@ -184,7 +184,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { && (is_trait_item(cx, arg, sym::Into) || is_trait_item(cx, arg, sym::From)) && let ty::FnDef(_, args) = cx.typeck_results().expr_ty(arg).kind() && let &[from_ty, to_ty] = args.into_type_list(cx.tcx).as_slice() - && same_type_and_consts(from_ty, to_ty) + && same_type_modulo_regions(from_ty, to_ty) { span_lint_and_then( cx, @@ -207,7 +207,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { if is_trait_method(cx, e, sym::Into) && name.ident.name == sym::into { let a = cx.typeck_results().expr_ty(e); let b = cx.typeck_results().expr_ty(recv); - if same_type_and_consts(a, b) { + if same_type_modulo_regions(a, b) { let mut app = Applicability::MachineApplicable; let sugg = snippet_with_context(cx, recv.span, e.span.ctxt(), "<expr>", &mut app).0; span_lint_and_sugg( @@ -324,7 +324,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { // If the types are identical then .into_iter() can be removed, unless the type // implements Copy, in which case .into_iter() returns a copy of the receiver and // cannot be safely omitted. - if same_type_and_consts(a, b) && !is_copy(cx, b) { + if same_type_modulo_regions(a, b) && !is_copy(cx, b) { // Below we check if the parent method call meets the following conditions: // 1. First parameter is `&mut self` (requires mutable reference) // 2. Second parameter implements the `FnMut` trait (e.g., Iterator::any) @@ -371,7 +371,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { && is_type_diagnostic_item(cx, a, sym::Result) && let ty::Adt(_, args) = a.kind() && let Some(a_type) = args.types().next() - && same_type_and_consts(a_type, b) + && same_type_modulo_regions(a_type, b) { span_lint_and_help( cx, @@ -396,7 +396,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { && is_type_diagnostic_item(cx, a, sym::Result) && let ty::Adt(_, args) = a.kind() && let Some(a_type) = args.types().next() - && same_type_and_consts(a_type, b) + && same_type_modulo_regions(a_type, b) { let hint = format!("consider removing `{}()`", snippet(cx, path.span, "TryFrom::try_from")); span_lint_and_help( @@ -407,7 +407,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { None, hint, ); - } else if name == sym::from_fn && same_type_and_consts(a, b) { + } else if name == sym::from_fn && same_type_modulo_regions(a, b) { let mut app = Applicability::MachineApplicable; let sugg = Sugg::hir_with_context(cx, arg, e.span.ctxt(), "<expr>", &mut app).maybe_paren(); let sugg_msg = format!("consider removing `{}()`", snippet(cx, path.span, "From::from")); |