Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new manual_unwrap_or_default lint #12440

Merged
merged 4 commits into from
Mar 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5377,6 +5377,7 @@ Released 2018-09-13
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
[`manual_try_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold
[`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
[`manual_unwrap_or_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or_default
[`manual_while_let_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_while_let_some
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
crate::manual_string_new::MANUAL_STRING_NEW_INFO,
crate::manual_strip::MANUAL_STRIP_INFO,
crate::manual_unwrap_or_default::MANUAL_UNWRAP_OR_DEFAULT_INFO,
crate::map_unit_fn::OPTION_MAP_UNIT_FN_INFO,
crate::map_unit_fn::RESULT_MAP_UNIT_FN_INFO,
crate::match_result_ok::MATCH_RESULT_OK_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ mod manual_retain;
mod manual_slice_size_calculation;
mod manual_string_new;
mod manual_strip;
mod manual_unwrap_or_default;
mod map_unit_fn;
mod match_result_ok;
mod matches;
Expand Down Expand Up @@ -1122,6 +1123,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
store.register_late_pass(|_| Box::new(zero_repeat_side_effects::ZeroRepeatSideEffects));
store.register_late_pass(|_| Box::new(manual_unwrap_or_default::ManualUnwrapOrDefault));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
181 changes: 181 additions & 0 deletions clippy_lints/src/manual_unwrap_or_default.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
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};
use rustc_session::declare_lint_pass;
use rustc_span::sym;

use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_default_equivalent;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::implements_trait;

declare_clippy_lint! {
/// ### What it does
/// Checks if a `match` or `if let` expression can be simplified using
/// `.unwrap_or_default()`.
///
/// ### Why is this bad?
/// It can be done in one call with `.unwrap_or_default()`.
///
/// ### Example
/// ```no_run
/// let x: Option<String> = Some(String::new());
/// let y: String = match x {
/// Some(v) => v,
/// None => String::new(),
/// };
///
/// let x: Option<Vec<String>> = Some(Vec::new());
/// let y: Vec<String> = if let Some(v) = x {
/// v
/// } else {
/// Vec::new()
/// };
/// ```
/// Use instead:
/// ```no_run
/// let x: Option<String> = Some(String::new());
/// let y: String = x.unwrap_or_default();
///
/// let x: Option<Vec<String>> = Some(Vec::new());
/// let y: Vec<String> = x.unwrap_or_default();
/// ```
#[clippy::version = "1.78.0"]
pub MANUAL_UNWRAP_OR_DEFAULT,
suspicious,
"check if a `match` or `if let` can be simplified with `unwrap_or_default`"
}

declare_lint_pass!(ManualUnwrapOrDefault => [MANUAL_UNWRAP_OR_DEFAULT]);

fn get_some<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<HirId> {
if let PatKind::TupleStruct(QPath::Resolved(_, path), &[pat], _) = pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& cx.tcx.lang_items().get(LangItem::OptionSome) == Some(def_id)
{
let mut bindings = Vec::new();
pat.each_binding(|_, id, _, _| bindings.push(id));
if let &[id] = bindings.as_slice() {
Some(id)
} else {
None
}
} else {
None
}
}

fn get_none<'tcx>(cx: &LateContext<'tcx>, arm: &Arm<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if let PatKind::Path(QPath::Resolved(_, path)) = arm.pat.kind
&& let Some(def_id) = path.res.opt_def_id()
// Since it comes from a pattern binding, we need to get the parent to actually match
// against it.
&& let Some(def_id) = cx.tcx.opt_parent(def_id)
&& cx.tcx.lang_items().get(LangItem::OptionNone) == Some(def_id)
{
Some(arm.body)
} else if let PatKind::Wild = arm.pat.kind {
// We consider that the `Some` check will filter it out if it's not right.
Some(arm.body)
} else {
None
}
}

fn get_some_and_none_bodies<'tcx>(
cx: &LateContext<'tcx>,
arm1: &'tcx Arm<'tcx>,
arm2: &'tcx Arm<'tcx>,
) -> Option<((&'tcx Expr<'tcx>, HirId), &'tcx Expr<'tcx>)> {
if let Some(binding_id) = get_some(cx, arm1.pat)
&& let Some(body_none) = get_none(cx, arm2)
{
Some(((arm1.body, binding_id), body_none))
} else if let Some(binding_id) = get_some(cx, arm2.pat)
&& let Some(body_none) = get_none(cx, arm1)
{
Some(((arm2.body, binding_id), body_none))
} else {
None
}
}

fn handle_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
let ExprKind::Match(match_expr, [arm1, arm2], MatchSource::Normal | MatchSource::ForLoopDesugar) = expr.kind else {
return false;
};
// We don't want conditions on the arms to simplify things.
if arm1.guard.is_none()
&& arm2.guard.is_none()
// We check that the returned type implements the `Default` trait.
&& let match_ty = cx.typeck_results().expr_ty(expr)
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
&& implements_trait(cx, match_ty, default_trait_id, &[])
// We now get the bodies for both the `Some` and `None` arms.
&& let Some(((body_some, binding_id), body_none)) = get_some_and_none_bodies(cx, arm1, arm2)
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
&& let ExprKind::Path(QPath::Resolved(_, path)) = body_some.peel_blocks().kind
&& let Res::Local(local_id) = path.res
&& local_id == binding_id
// We now check the `None` arm is calling a method equivalent to `Default::default`.
&& let body_none = body_none.peel_blocks()
&& is_default_equivalent(cx, body_none)
&& let Some(match_expr_snippet) = snippet_opt(cx, match_expr.span)
{
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
"match can be simplified with `.unwrap_or_default()`",
"replace it with",
format!("{match_expr_snippet}.unwrap_or_default()"),
Applicability::MachineApplicable,
);
}
true
}

fn handle_if_let<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let ExprKind::If(cond, if_block, Some(else_expr)) = expr.kind
&& let ExprKind::Let(let_) = cond.kind
&& let ExprKind::Block(_, _) = else_expr.kind
// We check that the returned type implements the `Default` trait.
&& let match_ty = cx.typeck_results().expr_ty(expr)
&& let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
&& implements_trait(cx, match_ty, default_trait_id, &[])
&& let Some(binding_id) = get_some(cx, let_.pat)
// We check that the `Some(x) => x` doesn't do anything apart "returning" the value in `Some`.
&& let ExprKind::Path(QPath::Resolved(_, path)) = if_block.peel_blocks().kind
&& let Res::Local(local_id) = path.res
&& local_id == binding_id
// We now check the `None` arm is calling a method equivalent to `Default::default`.
&& let body_else = else_expr.peel_blocks()
&& is_default_equivalent(cx, body_else)
&& let Some(if_let_expr_snippet) = snippet_opt(cx, let_.init.span)
{
span_lint_and_sugg(
cx,
MANUAL_UNWRAP_OR_DEFAULT,
expr.span,
"if let can be simplified with `.unwrap_or_default()`",
"replace it with",
format!("{if_let_expr_snippet}.unwrap_or_default()"),
Applicability::MachineApplicable,
);
}
}

impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOrDefault {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if expr.span.from_expansion() {
return;
}
if !handle_match(cx, expr) {
handle_if_let(cx, expr);
}
}
}
3 changes: 2 additions & 1 deletion tests/ui/manual_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
clippy::never_loop,
clippy::needless_if,
clippy::diverging_sub_expression,
clippy::single_match
clippy::single_match,
clippy::manual_unwrap_or_default
)]
#![warn(clippy::manual_let_else)]
//@no-rustfix
Expand Down
Loading
Loading