Skip to content

Fix as_deref_mut false positives in needless_option_as_deref #8646

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

Merged
merged 1 commit into from
Apr 7, 2022
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
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::MAP_COLLECT_RESULT_UNIT),
LintId::of(methods::MAP_FLATTEN),
LintId::of(methods::MAP_IDENTITY),
LintId::of(methods::NEEDLESS_OPTION_AS_DEREF),
LintId::of(methods::NEEDLESS_SPLITN),
LintId::of(methods::NEW_RET_NO_SELF),
LintId::of(methods::OK_EXPECT),
Expand Down Expand Up @@ -225,7 +226,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(needless_bool::NEEDLESS_BOOL),
LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
LintId::of(needless_late_init::NEEDLESS_LATE_INIT),
LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF),
LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
LintId::of(needless_update::NEEDLESS_UPDATE),
LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.register_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(methods::MANUAL_SPLIT_ONCE),
LintId::of(methods::MAP_FLATTEN),
LintId::of(methods::MAP_IDENTITY),
LintId::of(methods::NEEDLESS_OPTION_AS_DEREF),
LintId::of(methods::NEEDLESS_SPLITN),
LintId::of(methods::OPTION_AS_REF_DEREF),
LintId::of(methods::OPTION_FILTER_MAP),
Expand All @@ -60,7 +61,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(needless_bool::BOOL_COMPARISON),
LintId::of(needless_bool::NEEDLESS_BOOL),
LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF),
LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
LintId::of(needless_update::NEEDLESS_UPDATE),
LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ store.register_lints(&[
methods::MAP_FLATTEN,
methods::MAP_IDENTITY,
methods::MAP_UNWRAP_OR,
methods::NEEDLESS_OPTION_AS_DEREF,
methods::NEEDLESS_SPLITN,
methods::NEW_RET_NO_SELF,
methods::OK_EXPECT,
Expand Down Expand Up @@ -386,7 +387,6 @@ store.register_lints(&[
needless_continue::NEEDLESS_CONTINUE,
needless_for_each::NEEDLESS_FOR_EACH,
needless_late_init::NEEDLESS_LATE_INIT,
needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF,
needless_pass_by_value::NEEDLESS_PASS_BY_VALUE,
needless_question_mark::NEEDLESS_QUESTION_MARK,
needless_update::NEEDLESS_UPDATE,
Expand Down
2 changes: 0 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ mod needless_borrowed_ref;
mod needless_continue;
mod needless_for_each;
mod needless_late_init;
mod needless_option_as_deref;
mod needless_pass_by_value;
mod needless_question_mark;
mod needless_update;
Expand Down Expand Up @@ -536,7 +535,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(ptr::Ptr));
store.register_late_pass(|| Box::new(ptr_eq::PtrEq));
store.register_late_pass(|| Box::new(needless_bool::NeedlessBool));
store.register_late_pass(|| Box::new(needless_option_as_deref::OptionNeedlessDeref));
store.register_late_pass(|| Box::new(needless_bool::BoolComparison));
store.register_late_pass(|| Box::new(needless_for_each::NeedlessForEach));
store.register_late_pass(|| Box::new(misc::MiscLints));
Expand Down
29 changes: 29 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ mod map_collect_result_unit;
mod map_flatten;
mod map_identity;
mod map_unwrap_or;
mod needless_option_as_deref;
mod ok_expect;
mod option_as_ref_deref;
mod option_map_or_none;
Expand Down Expand Up @@ -2106,6 +2107,30 @@ declare_clippy_lint! {
"using `.collect::<Vec<String>>().join(\"\")` on an iterator"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for no-op uses of `Option::{as_deref, as_deref_mut}`,
/// for example, `Option<&T>::as_deref()` returns the same type.
///
/// ### Why is this bad?
/// Redundant code and improving readability.
///
/// ### Example
/// ```rust
/// let a = Some(&1);
/// let b = a.as_deref(); // goes from Option<&i32> to Option<&i32>
/// ```
/// Could be written as:
/// ```rust
/// let a = Some(&1);
/// let b = a;
/// ```
#[clippy::version = "1.57.0"]
pub NEEDLESS_OPTION_AS_DEREF,
complexity,
"no-op use of `deref` or `deref_mut` method to `Option`."
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
Expand Down Expand Up @@ -2193,6 +2218,7 @@ impl_lint_pass!(Methods => [
UNNECESSARY_TO_OWNED,
UNNECESSARY_JOIN,
ERR_EXPECT,
NEEDLESS_OPTION_AS_DEREF,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -2425,6 +2451,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
}
},
("as_deref" | "as_deref_mut", []) => {
needless_option_as_deref::check(cx, expr, recv, name);
},
("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv),
("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv),
("assume_init", []) => uninit_assumed_init::check(cx, expr, recv),
Expand Down
37 changes: 37 additions & 0 deletions clippy_lints/src/methods/needless_option_as_deref.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::path_res;
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::usage::local_used_after_expr;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::sym;

use super::NEEDLESS_OPTION_AS_DEREF;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, name: &str) {
let typeck = cx.typeck_results();
let outer_ty = typeck.expr_ty(expr);

if is_type_diagnostic_item(cx, outer_ty, sym::Option) && outer_ty == typeck.expr_ty(recv) {
if name == "as_deref_mut" && recv.is_syntactic_place_expr() {
let Res::Local(binding_id) = path_res(cx, recv) else { return };

if local_used_after_expr(cx, binding_id, recv) {
return;
}
}

span_lint_and_sugg(
cx,
NEEDLESS_OPTION_AS_DEREF,
expr.span,
"derefed type is same as origin",
"try this",
snippet_opt(cx, recv.span).unwrap(),
Applicability::MachineApplicable,
);
}
}
65 changes: 0 additions & 65 deletions clippy_lints/src/needless_option_as_deref.rs

This file was deleted.

30 changes: 29 additions & 1 deletion tests/ui/needless_option_as_deref.fixed
Original file line number Diff line number Diff line change
@@ -1,13 +1,41 @@
// run-rustfix

#[warn(clippy::needless_option_as_deref)]
#![allow(unused)]
#![warn(clippy::needless_option_as_deref)]

fn main() {
// should lint
let _: Option<&usize> = Some(&1);
let _: Option<&mut usize> = Some(&mut 1);

let mut y = 0;
let mut x = Some(&mut y);
let _ = x;

// should not lint
let _ = Some(Box::new(1)).as_deref();
let _ = Some(Box::new(1)).as_deref_mut();

// #7846
let mut i = 0;
let mut opt_vec = vec![Some(&mut i)];
opt_vec[0].as_deref_mut().unwrap();

let mut i = 0;
let x = &mut Some(&mut i);
(*x).as_deref_mut();

// #8047
let mut y = 0;
let mut x = Some(&mut y);
x.as_deref_mut();
dbg!(x);
}

struct S<'a> {
opt: Option<&'a mut usize>,
}

fn from_field<'a>(s: &'a mut S<'a>) -> Option<&'a mut usize> {
s.opt.as_deref_mut()
}
30 changes: 29 additions & 1 deletion tests/ui/needless_option_as_deref.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,41 @@
// run-rustfix

#[warn(clippy::needless_option_as_deref)]
#![allow(unused)]
#![warn(clippy::needless_option_as_deref)]

fn main() {
// should lint
let _: Option<&usize> = Some(&1).as_deref();
let _: Option<&mut usize> = Some(&mut 1).as_deref_mut();

let mut y = 0;
let mut x = Some(&mut y);
let _ = x.as_deref_mut();

// should not lint
let _ = Some(Box::new(1)).as_deref();
let _ = Some(Box::new(1)).as_deref_mut();

// #7846
let mut i = 0;
let mut opt_vec = vec![Some(&mut i)];
opt_vec[0].as_deref_mut().unwrap();

let mut i = 0;
let x = &mut Some(&mut i);
(*x).as_deref_mut();

// #8047
let mut y = 0;
let mut x = Some(&mut y);
x.as_deref_mut();
dbg!(x);
}

struct S<'a> {
opt: Option<&'a mut usize>,
}

fn from_field<'a>(s: &'a mut S<'a>) -> Option<&'a mut usize> {
s.opt.as_deref_mut()
}
12 changes: 9 additions & 3 deletions tests/ui/needless_option_as_deref.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
error: derefed type is same as origin
--> $DIR/needless_option_as_deref.rs:7:29
--> $DIR/needless_option_as_deref.rs:8:29
|
LL | let _: Option<&usize> = Some(&1).as_deref();
| ^^^^^^^^^^^^^^^^^^^ help: try this: `Some(&1)`
|
= note: `-D clippy::needless-option-as-deref` implied by `-D warnings`

error: derefed type is same as origin
--> $DIR/needless_option_as_deref.rs:8:33
--> $DIR/needless_option_as_deref.rs:9:33
|
LL | let _: Option<&mut usize> = Some(&mut 1).as_deref_mut();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Some(&mut 1)`

error: aborting due to 2 previous errors
error: derefed type is same as origin
--> $DIR/needless_option_as_deref.rs:13:13
|
LL | let _ = x.as_deref_mut();
| ^^^^^^^^^^^^^^^^ help: try this: `x`

error: aborting due to 3 previous errors