Skip to content

Commit

Permalink
Print out type for uncertain if-let condition
Browse files Browse the repository at this point in the history
  • Loading branch information
@profetia
profetia committed Feb 4, 2025
1 parent 5414e0f commit f21f329
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 80 deletions.
72 changes: 33 additions & 39 deletions clippy_lints/src/manual_unwrap_or_default.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::{Arm, Expr, ExprKind, HirId, LangItem, MatchSource, Pat, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::ty::{self, GenericArgKind};
use rustc_middle::ty::GenericArgKind;
use rustc_session::declare_lint_pass;
use rustc_span::sym;

use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::higher::IfLetOrMatch;
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::implements_trait;
use clippy_utils::{is_default_equivalent, is_in_const_context, peel_blocks, span_contains_comment};
use clippy_utils::ty::{expr_type_is_certain, implements_trait};
use clippy_utils::{is_default_equivalent, is_in_const_context, path_res, peel_blocks, span_contains_comment};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -152,48 +152,42 @@ fn handle<'tcx>(cx: &LateContext<'tcx>, if_let_or_match: IfLetOrMatch<'tcx>, exp
&& is_default_equivalent(cx, body_none)
&& let Some(receiver) = Sugg::hir_opt(cx, condition).map(Sugg::maybe_par)
{
// We check if the expression is not a None variant
if let Some(none_def_id) = cx.tcx.lang_items().option_none_variant() {
if let ExprKind::Path(QPath::Resolved(_, path)) = &condition.kind {
if let Some(def_id) = path.res.opt_def_id() {
if cx.tcx.parent(def_id) == none_def_id {
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
"replace it with",
format!("{receiver}::</* Type */>.unwrap_or_default()"),
Applicability::HasPlaceholders,
);
}
}
}
}

// We check if the expression is not a method or function with a unspecified return type
if let ExprKind::MethodCall(_, expr, _, _) = &condition.kind {
if let ty::Adt(_, substs) = cx.typeck_results().expr_ty(expr).kind() {
if let Some(ok_type) = substs.first() {
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
format!("explicit the type {ok_type} and replace your expression with"),
format!("{receiver}.unwrap_or_default()"),
Applicability::Unspecified,
);
}
}
}

// Machine applicable only if there are no comments present
let applicability = if span_contains_comment(cx.sess().source_map(), expr.span) {
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
};

// We now check if the condition is a None variant, in which case we need to specify the type
if path_res(cx, condition)
.opt_def_id()
.is_some_and(|id| Some(cx.tcx.parent(id)) == cx.tcx.lang_items().option_none_variant())
{
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
"replace it with",
format!("{receiver}::<{expr_type}>.unwrap_or_default()"),
applicability,
);
}

// We check if the expression type is still uncertain, in which case we ask the user to specify it
if !expr_type_is_certain(cx, condition) {
return span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
format!("{expr_name} can be simplified with `.unwrap_or_default()`"),
format!("ascribe the type {expr_type} and replace your expression with"),
format!("{receiver}.unwrap_or_default()"),
Applicability::Unspecified,
);
}

span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
Expand Down
16 changes: 0 additions & 16 deletions tests/ui/manual_unwrap_or_default.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,3 @@ fn issue_12928() {
let y = if let Some(Y(a, _)) = x { a } else { 0 };
let y = if let Some(Y(a, ..)) = x { a } else { 0 };
}

//@no-rustfix
fn issue_12670() {
// no auto: type not found
#[allow(clippy::match_result_ok)]
let _ = if let Some(x) = "1".parse().ok() {
x
} else {
i32::default()
};
let _ = if let Some(x) = None { x } else { i32::default() };
// auto fix with unwrap_or_default
let a: Option<i32> = None;
let _ = if let Some(x) = a { x } else { i32::default() };
let _ = if let Some(x) = Some(99) { x } else { i32::default() };
}
26 changes: 1 addition & 25 deletions tests/ui/manual_unwrap_or_default.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,29 +83,5 @@ LL | | _ => 0,
LL | | },
| |_________^ help: replace it with: `(*b).unwrap_or_default()`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default.rs:104:30
|
LL | let _ = if let Some(x) = "1".parse().ok() {
| ^^^^^^^^^^^ help: explicit the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default.rs:109:13
|
LL | let _ = if let Some(x) = None { x } else { i32::default() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::</* Type */>.unwrap_or_default()`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default.rs:112:13
|
LL | let _ = if let Some(x) = a { x } else { i32::default() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.unwrap_or_default()`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default.rs:113:13
|
LL | let _ = if let Some(x) = Some(99) { x } else { i32::default() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(99).unwrap_or_default()`

error: aborting due to 10 previous errors
error: aborting due to 8 previous errors

15 changes: 15 additions & 0 deletions tests/ui/manual_unwrap_or_default_unfixable.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@no-rustfix
fn issue_12670() {
// no auto: type not found
#[allow(clippy::match_result_ok)]
let _ = if let Some(x) = "1".parse().ok() {
x
} else {
i32::default()
};
let _ = if let Some(x) = None { x } else { i32::default() };
// auto fix with unwrap_or_default
let a: Option<i32> = None;
let _ = if let Some(x) = a { x } else { i32::default() };
let _ = if let Some(x) = Some(99) { x } else { i32::default() };
}
34 changes: 34 additions & 0 deletions tests/ui/manual_unwrap_or_default_unfixable.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default_unfixable.rs:5:13
|
LL | let _ = if let Some(x) = "1".parse().ok() {
| _____________^
LL | | x
LL | | } else {
LL | | i32::default()
LL | | };
| |_____^ help: ascribe the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()`
|
= note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default_unfixable.rs:10:13
|
LL | let _ = if let Some(x) = None { x } else { i32::default() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::<i32>.unwrap_or_default()`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default_unfixable.rs:13:13
|
LL | let _ = if let Some(x) = a { x } else { i32::default() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.unwrap_or_default()`

error: if let can be simplified with `.unwrap_or_default()`
--> tests/ui/manual_unwrap_or_default_unfixable.rs:14:13
|
LL | let _ = if let Some(x) = Some(99) { x } else { i32::default() };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(99).unwrap_or_default()`

error: aborting due to 4 previous errors

0 comments on commit f21f329

Please sign in to comment.