From 56df855025223020875374f16d0a8fec7504fd36 Mon Sep 17 00:00:00 2001 From: Paul Greenwood Date: Sat, 24 May 2025 13:37:47 -0400 Subject: [PATCH 1/5] fix(or_fun_call): respect MSRV for unwrap_or_default suggestion The `unwrap_or_default()` method was introduced in Rust 1.16, but the lint was suggesting it even when the MSRV was set to 1.15 or lower. This change adds an MSRV check to ensure we only suggest `unwrap_or_default()` when the MSRV is at least 1.16. The fix: 1. Adds MSRV check in `check_unwrap_or_default` using `msrvs::STR_REPEAT` (Rust 1.16) 2. Adds MSRV parameter to the `check` function signature 3. Updates the call site to pass the MSRV parameter Fixes #14876 (unwrap_or_default MSRV issue) --- clippy_lints/src/methods/mod.rs | 4 ++-- clippy_lints/src/methods/or_fun_call.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d2d59f0013c0..fdb096e8e5f5 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4412,7 +4412,7 @@ declare_clippy_lint! { /// Checks for calls to `Read::bytes` on types which don't implement `BufRead`. /// /// ### Why is this bad? - /// The default implementation calls `read` for each byte, which can be very inefficient for data that’s not in memory, such as `File`. + /// The default implementation calls `read` for each byte, which can be very inefficient for data that's not in memory, such as `File`. /// /// ### Example /// ```no_run @@ -4741,7 +4741,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods { }, ExprKind::MethodCall(method_call, receiver, args, _) => { let method_span = method_call.ident.span; - or_fun_call::check(cx, expr, method_span, method_call.ident.name, receiver, args); + or_fun_call::check(cx, expr, method_span, method_call.ident.name, receiver, args, self.msrv); expect_fun_call::check( cx, &self.format_args, diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index c74c42e9e5bd..179c57a00205 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/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::msrvs::{self, Msrv}; 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; @@ -26,6 +27,7 @@ pub(super) fn check<'tcx>( name: Symbol, receiver: &'tcx hir::Expr<'_>, args: &'tcx [hir::Expr<'_>], + msrv: Msrv, ) { /// Checks for `unwrap_or(T::new())`, `unwrap_or(T::default())`, /// `or_insert(T::new())` or `or_insert(T::default())`. @@ -39,7 +41,13 @@ pub(super) fn check<'tcx>( call_expr: Option<&hir::Expr<'_>>, span: Span, method_span: Span, + msrv: Msrv, ) -> bool { + // Don't suggest unwrap_or_default if MSRV is less than 1.16 + if !msrv.meets(cx, msrvs::STR_REPEAT) { + return false; + } + if !expr_type_is_certain(cx, receiver) { return false; } @@ -215,11 +223,11 @@ pub(super) fn check<'tcx>( }; (!inner_fun_has_args && !is_nested_expr - && check_unwrap_or_default(cx, name, receiver, fun, Some(ex), expr.span, method_span)) + && check_unwrap_or_default(cx, name, receiver, fun, Some(ex), expr.span, method_span, msrv)) || check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, fun_span) }, hir::ExprKind::Path(..) | hir::ExprKind::Closure(..) if !is_nested_expr => { - check_unwrap_or_default(cx, name, receiver, ex, None, expr.span, method_span) + check_unwrap_or_default(cx, name, receiver, ex, None, expr.span, method_span, msrv) }, hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => { check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, None) From 73d9ce6f84f181f87db7f0e6edb0262eb52e9ead Mon Sep 17 00:00:00 2001 From: Paul Greenwood Date: Sat, 24 May 2025 14:01:56 -0400 Subject: [PATCH 2/5] fix(or_fun_call): respect MSRV for unwrap_or_default suggestion The `unwrap_or_default()` method was introduced in Rust 1.16, but the lint was suggesting it even when the MSRV was set to 1.15 or lower. This change adds an MSRV check to ensure we only suggest `unwrap_or_default()` when the MSRV is at least 1.16. The fix: 1. Adds MSRV check in `check_unwrap_or_default` using `msrvs::STR_REPEAT` (Rust 1.16) 2. Adds MSRV parameter to the `check` function signature 3. Updates the call site to pass the MSRV parameter 4. Adds `#[allow(clippy::too_many_arguments)]` to handle the linter warning Fixes #14876 (unwrap_or_default MSRV issue) --- clippy_lints/src/methods/or_fun_call.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 179c57a00205..178852ba73e4 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -33,6 +33,7 @@ pub(super) fn check<'tcx>( /// `or_insert(T::new())` or `or_insert(T::default())`. /// Similarly checks for `unwrap_or_else(T::new)`, `unwrap_or_else(T::default)`, /// `or_insert_with(T::new)` or `or_insert_with(T::default)`. + #[allow(clippy::too_many_arguments)] fn check_unwrap_or_default( cx: &LateContext<'_>, name: Symbol, @@ -63,8 +64,8 @@ pub(super) fn check<'tcx>( let output_type_implements_default = |fun| { let fun_ty = cx.typeck_results().expr_ty(fun); - if let ty::FnDef(def_id, args) = fun_ty.kind() { - let output_ty = cx.tcx.fn_sig(def_id).instantiate(cx.tcx, args).skip_binder().output(); + if let ty::FnDef(def_id, substs) = fun_ty.kind() { + let output_ty = cx.tcx.fn_sig(def_id).instantiate(cx.tcx, substs).skip_binder().output(); cx.tcx .get_diagnostic_item(sym::Default) .is_some_and(|default_trait_id| implements_trait(cx, output_ty, default_trait_id, &[])) From 94767461412934b72c3123f06b226df7ff8ee6a5 Mon Sep 17 00:00:00 2001 From: Paul Greenwood Date: Sat, 24 May 2025 14:10:36 -0400 Subject: [PATCH 3/5] On branch Result--unwrap_or_default()-suggested-for-MSRV-<-1.16-#14876 Your branch is ahead of 'origin/Result--unwrap_or_default()-suggested-for-MSRV-<-1.16-#14876' by 1 commit. (use "git push" to publish your local commits) Changes to be committed: modified: clippy_lints/src/methods/or_fun_call.rs --- clippy_lints/src/methods/or_fun_call.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 178852ba73e4..888b4fdae79a 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -1,3 +1,4 @@ +#[allow(clippy::too_many_arguments)] use std::ops::ControlFlow; use clippy_utils::diagnostics::span_lint_and_sugg; From 90a5f53e89668a052d40e2b1f15c0b7dc48c2aed Mon Sep 17 00:00:00 2001 From: Paul Greenwood Date: Sat, 24 May 2025 14:27:48 -0400 Subject: [PATCH 4/5] On branch Result--unwrap_or_default()-suggested-for-MSRV-<-1.16-#14876 Your branch is up to date with 'origin/Result--unwrap_or_default()-suggested-for-MSRV-<-1.16-#14876'. Changes to be committed: modified: clippy_lints/src/methods/or_fun_call.rs --- clippy_lints/src/methods/or_fun_call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 888b4fdae79a..389fb2591d37 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -1,4 +1,4 @@ -#[allow(clippy::too_many_arguments)] +#![allow(clippy::too_many_arguments)] use std::ops::ControlFlow; use clippy_utils::diagnostics::span_lint_and_sugg; From 9ae6e24b64f22c41b526979939cb102240f55c29 Mon Sep 17 00:00:00 2001 From: Paul Greenwood Date: Sat, 31 May 2025 11:12:15 -0400 Subject: [PATCH 5/5] Add MSRV check for Result::unwrap_or_default() This change ensures that the `unwrap_or_default` suggestion is only made for `Result` types when the MSRV is at least 1.16, since `Result::unwrap_or_default()` was stabilized in that version. The suggestion for `Option::unwrap_or_default()` remains available for all MSRVs. - Add `RESULT_UNWRAP_OR_DEFAULT` constant to `msrv_aliases!` macro - Update `check_unwrap_or_default` to only check MSRV for `Result` types - Add test cases for both `Option` and `Result` with different MSRVs Fixes #12973 --- .gitignore | 5 +++++ clippy_lints/src/methods/or_fun_call.rs | 5 +++-- clippy_utils/src/msrvs.rs | 2 +- tests/ui/or_fun_call.rs | 30 +++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index a7c25b29021f..bef3c6d4772e 100644 --- a/.gitignore +++ b/.gitignore @@ -46,3 +46,8 @@ helper.txt # mdbook generated output /book/book +.cursorrules +.cursor/rules/data-flow-analysis.mdc +.giga/specifications.json +.cursor/rules +.cursor/rules/lint-implementation-patterns.mdc diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 389fb2591d37..693bbd00c6c1 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -45,8 +45,9 @@ pub(super) fn check<'tcx>( method_span: Span, msrv: Msrv, ) -> bool { - // Don't suggest unwrap_or_default if MSRV is less than 1.16 - if !msrv.meets(cx, msrvs::STR_REPEAT) { + // Only check MSRV for Result type + let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs(); + if is_type_diagnostic_item(cx, receiver_ty, sym::Result) && !msrv.meets(cx, msrvs::RESULT_UNWRAP_OR_DEFAULT) { return false; } diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index a5e66ad463bb..5b2fd8130495 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -76,7 +76,7 @@ msrv_aliases! { 1,24,0 { IS_ASCII_DIGIT } 1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN } 1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR } - 1,16,0 { STR_REPEAT } + 1,16,0 { STR_REPEAT, RESULT_UNWRAP_OR_DEFAULT } 1,15,0 { MAYBE_BOUND_IN_WHERE } 1,13,0 { QUESTION_MARK_OPERATOR } } diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index a7cd632bf166..86c46985c008 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -409,4 +409,34 @@ fn fn_call_in_nested_expr() { //~^ or_fun_call } +mod msrv_tests { + #[clippy::msrv = "1.15"] + fn test_option_below_msrv() { + let opt = Some(1); + let _ = opt.unwrap_or(0); + //~^ unwrap_or_default + } + + #[clippy::msrv = "1.15"] + fn test_result_below_msrv() { + let res: Result = Ok(1); + let _ = res.unwrap_or(0); + //~^ or_fun_call + } + + #[clippy::msrv = "1.16"] + fn test_option_above_msrv() { + let opt = Some(1); + let _ = opt.unwrap_or(0); + //~^ unwrap_or_default + } + + #[clippy::msrv = "1.16"] + fn test_result_above_msrv() { + let res: Result = Ok(1); + let _ = res.unwrap_or(0); + //~^ unwrap_or_default + } +} + fn main() {}