Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-23 13:13:34 +00:00
Code Issues Projects Releases Packages Wiki Activity

Rollup merge of #5425 - xiongmao86:issue5367, r=flip1995

Browse source 
Ehance opt_as_ref_deref lint.

- [x] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Run `cargo dev fmt`

Lint on opt.as_ref().map(|x| &**x). Fixes #5367.

changelog: lint on opt.as_ref().map(|x| &**x)
This commit is contained in:
Philipp Krones 2020-04-08 15:50:28 +02:00 committed by GitHub
commit 79d152190c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 73 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
clippy_lints/src/loops.rs
View file

@ -654,7 +654,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult
fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
let stmts = block.stmts.iter().map(stmt_to_expr);
let expr = once(block.expr.as_ref().map(|p| &**p));
let expr = once(block.expr.as_deref());
let mut iter = stmts.chain(expr).filter_map(|e| e);
never_loop_expr_seq(&mut iter, main_loop_id)
}
@ -662,7 +662,7 @@ fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> {
match stmt.kind {
StmtKind::Semi(ref e, ..) | StmtKind::Expr(ref e, ..) => Some(e),
StmtKind::Local(ref local) => local.init.as_ref().map(|p| &**p),
StmtKind::Local(ref local) => local.init.as_deref(),
_ => None,
}
}

57
clippy_lints/src/methods/mod.rs
View file

@ -3233,6 +3233,8 @@ fn lint_option_as_ref_deref<'a, 'tcx>(
map_args: &[hir::Expr<'_>],
is_mut: bool,
) {
let same_mutability = |m| (is_mut && m == &hir::Mutability::Mut) || (!is_mut && m == &hir::Mutability::Not);
let option_ty = cx.tables.expr_ty(&as_ref_args[0]);
if !match_type(cx, option_ty, &paths::OPTION) {
return;
@ -3255,39 +3257,56 @@ fn lint_option_as_ref_deref<'a, 'tcx>(
hir::ExprKind::Closure(_, _, body_id, _, _) => {
let closure_body = cx.tcx.hir().body(body_id);
let closure_expr = remove_blocks(&closure_body.value);
if_chain! {
if let hir::ExprKind::MethodCall(_, _, args) = &closure_expr.kind;
if args.len() == 1;
if let hir::ExprKind::Path(qpath) = &args[0].kind;
if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id);
if closure_body.params[0].pat.hir_id == local_id;
let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::<Box<[_]>>();
if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj;
then {
let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap();
deref_aliases.iter().any(|path| match_def_path(cx, method_did, path))
} else {
false
}
match &closure_expr.kind {
hir::ExprKind::MethodCall(_, _, args) => {
if_chain! {
if args.len() == 1;
if let hir::ExprKind::Path(qpath) = &args[0].kind;
if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id);
if closure_body.params[0].pat.hir_id == local_id;
let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::<Box<[_]>>();
if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj;
then {
let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap();
deref_aliases.iter().any(|path| match_def_path(cx, method_did, path))
} else {
false
}
}
},
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, m, ref inner) if same_mutability(m) => {
if_chain! {
if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner1) = inner.kind;
if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner2) = inner1.kind;
if let hir::ExprKind::Path(ref qpath) = inner2.kind;
if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, inner2.hir_id);
then {
closure_body.params[0].pat.hir_id == local_id
} else {
false
}
}
},
_ => false,
}
},
_ => false,
};
if is_deref {
let current_method = if is_mut {
".as_mut().map(DerefMut::deref_mut)"
format!(".as_mut().map({})", snippet(cx, map_args[1].span, ".."))
} else {
".as_ref().map(Deref::deref)"
format!(".as_ref().map({})", snippet(cx, map_args[1].span, ".."))
};
let method_hint = if is_mut { "as_deref_mut" } else { "as_deref" };
let hint = format!("{}.{}()", snippet(cx, as_ref_args[0].span, ".."), method_hint);
let suggestion = format!("try using {} instead", method_hint);
let msg = format!(
"called `{0}` (or with one of deref aliases) on an Option value. \
This can be done more directly by calling `{1}` instead",
"called `{0}` on an Option value. This can be done more directly \
by calling `{1}` instead",
current_method, hint
);
span_lint_and_sugg(

3
tests/ui/option_as_ref_deref.fixed
View file

@ -35,4 +35,7 @@ fn main() {
let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
let _ = opt.as_deref();
let _ = opt.as_deref_mut();
}

3
tests/ui/option_as_ref_deref.rs
View file

@ -38,4 +38,7 @@ fn main() {
let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted
let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted
let _ = opt.as_ref().map(|x| &**x);
let _ = opt.as_mut().map(|x| &mut **x);
}

42
tests/ui/option_as_ref_deref.stderr
View file

@ -1,4 +1,4 @@
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead
error: called `.as_ref().map(Deref::deref)` on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead
--> $DIR/option_as_ref_deref.rs:13:13
|
LL | let _ = opt.clone().as_ref().map(Deref::deref).map(str::len);
@ -6,7 +6,7 @@ LL | let _ = opt.clone().as_ref().map(Deref::deref).map(str::len);
|
= note: `-D clippy::option-as-ref-deref` implied by `-D warnings`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead
error: called `.as_ref().map(Deref::deref)` on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead
--> $DIR/option_as_ref_deref.rs:16:13
|
LL | let _ = opt.clone()
@ -16,77 +16,89 @@ LL | | Deref::deref
LL | | )
| |_________^ help: try using as_deref instead: `opt.clone().as_deref()`
error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(DerefMut::deref_mut)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
--> $DIR/option_as_ref_deref.rs:22:13
|
LL | let _ = opt.as_mut().map(DerefMut::deref_mut);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(String::as_str)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
--> $DIR/option_as_ref_deref.rs:24:13
|
LL | let _ = opt.as_ref().map(String::as_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(|x| x.as_str())` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
--> $DIR/option_as_ref_deref.rs:25:13
|
LL | let _ = opt.as_ref().map(|x| x.as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`
error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(String::as_mut_str)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
--> $DIR/option_as_ref_deref.rs:26:13
|
LL | let _ = opt.as_mut().map(String::as_mut_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`
error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
error: called `.as_mut().map(|x| x.as_mut_str())` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
--> $DIR/option_as_ref_deref.rs:27:13
|
LL | let _ = opt.as_mut().map(|x| x.as_mut_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(CString::new(vec![]).unwrap()).as_deref()` instead
error: called `.as_ref().map(CString::as_c_str)` on an Option value. This can be done more directly by calling `Some(CString::new(vec![]).unwrap()).as_deref()` instead
--> $DIR/option_as_ref_deref.rs:28:13
|
LL | let _ = Some(CString::new(vec![]).unwrap()).as_ref().map(CString::as_c_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(CString::new(vec![]).unwrap()).as_deref()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(OsString::new()).as_deref()` instead
error: called `.as_ref().map(OsString::as_os_str)` on an Option value. This can be done more directly by calling `Some(OsString::new()).as_deref()` instead
--> $DIR/option_as_ref_deref.rs:29:13
|
LL | let _ = Some(OsString::new()).as_ref().map(OsString::as_os_str);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(OsString::new()).as_deref()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(PathBuf::new()).as_deref()` instead
error: called `.as_ref().map(PathBuf::as_path)` on an Option value. This can be done more directly by calling `Some(PathBuf::new()).as_deref()` instead
--> $DIR/option_as_ref_deref.rs:30:13
|
LL | let _ = Some(PathBuf::new()).as_ref().map(PathBuf::as_path);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(PathBuf::new()).as_deref()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref()` instead
error: called `.as_ref().map(Vec::as_slice)` on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref()` instead
--> $DIR/option_as_ref_deref.rs:31:13
|
LL | let _ = Some(Vec::<()>::new()).as_ref().map(Vec::as_slice);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(Vec::<()>::new()).as_deref()`
error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref_mut()` instead
error: called `.as_mut().map(Vec::as_mut_slice)` on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref_mut()` instead
--> $DIR/option_as_ref_deref.rs:32:13
|
LL | let _ = Some(Vec::<()>::new()).as_mut().map(Vec::as_mut_slice);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `Some(Vec::<()>::new()).as_deref_mut()`
error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead
error: called `.as_ref().map(|x| x.deref())` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
--> $DIR/option_as_ref_deref.rs:34:13
|
LL | let _ = opt.as_ref().map(|x| x.deref());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`
error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.clone().as_deref_mut()` instead
error: called `.as_mut().map(|x| x.deref_mut())` on an Option value. This can be done more directly by calling `opt.clone().as_deref_mut()` instead
--> $DIR/option_as_ref_deref.rs:35:13
|
LL | let _ = opt.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.clone().as_deref_mut()`
error: aborting due to 14 previous errors
error: called `.as_ref().map(|x| &**x)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead
--> $DIR/option_as_ref_deref.rs:42:13
|
LL | let _ = opt.as_ref().map(|x| &**x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()`
error: called `.as_mut().map(|x| &mut **x)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead
--> $DIR/option_as_ref_deref.rs:43:13
|
LL | let _ = opt.as_mut().map(|x| &mut **x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()`
error: aborting due to 16 previous errors