From 990f8bf5a6354d98a7e0b628f9f1b810644c3ad8 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Sat, 4 Jun 2022 16:55:33 +0900 Subject: [PATCH] refactor: Add some methods --- clippy_lints/src/methods/filter_map.rs | 19 ++++++++++-- tests/ui/manual_filter_map.fixed | 22 +++++++++++++- tests/ui/manual_filter_map.rs | 27 ++++++++++++++++- tests/ui/manual_filter_map.stderr | 42 +++++++++++++++++++++++++- tests/ui/manual_find_map.fixed | 20 +++++++++++- tests/ui/manual_find_map.rs | 24 ++++++++++++++- tests/ui/manual_find_map.stderr | 40 +++++++++++++++++++++++- 7 files changed, 185 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 04ca295dc69..6cabe74fdef 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -6,7 +6,7 @@ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_hir::{Expr, ExprKind, PatKind, QPath, UnOp}; +use rustc_hir::{Expr, ExprKind, PatKind, PathSegment, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::source_map::Span; use rustc_span::symbol::{sym, Symbol}; @@ -157,8 +157,8 @@ pub(super) fn check<'tcx>( }; if match map_arg.kind { - ExprKind::MethodCall(clone, [original_arg], _) => { - clone.ident.name == sym::clone + ExprKind::MethodCall(method, [original_arg], _) => { + acceptable_methods(method) && SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, original_arg) }, _ => SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg) @@ -179,3 +179,16 @@ pub(super) fn check<'tcx>( } } } + +fn acceptable_methods(method: &PathSegment<'_>) -> bool { + let methods: [Symbol; 6] = [ + sym::clone, + sym::as_ref, + sym!(as_deref), + sym!(as_mut), + sym!(as_deref_mut), + sym!(to_owned), + ]; + + methods.contains(&method.ident.name) +} diff --git a/tests/ui/manual_filter_map.fixed b/tests/ui/manual_filter_map.fixed index c88a4f43de2..9de7040000a 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -52,8 +52,28 @@ fn issue_8920() { .iter() .filter_map(|f| f.field.clone()); - let vec = vec![ResultFoo { + let mut vec = vec![ResultFoo { field: Ok(String::from("str")), }]; let _ = vec.iter().filter_map(|f| f.field.clone().ok()); + + let _ = vec + .iter() + .filter_map(|f| f.field.as_ref().ok()); + + let _ = vec + .iter() + .filter_map(|f| f.field.as_deref().ok()); + + let _ = vec + .iter_mut() + .filter_map(|f| f.field.as_mut().ok()); + + let _ = vec + .iter_mut() + .filter_map(|f| f.field.as_deref_mut().ok()); + + let _ = vec + .iter() + .filter_map(|f| f.field.to_owned().ok()); } diff --git a/tests/ui/manual_filter_map.rs b/tests/ui/manual_filter_map.rs index bb859ebe315..6766078d694 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -53,8 +53,33 @@ fn issue_8920() { .filter(|f| f.field.is_some()) .map(|f| f.field.clone().unwrap()); - let vec = vec![ResultFoo { + let mut vec = vec![ResultFoo { field: Ok(String::from("str")), }]; let _ = vec.iter().filter(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_ref().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_deref().unwrap()); + + let _ = vec + .iter_mut() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_mut().unwrap()); + + let _ = vec + .iter_mut() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_deref_mut().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.to_owned().unwrap()); } diff --git a/tests/ui/manual_filter_map.stderr b/tests/ui/manual_filter_map.stderr index a78343e882f..5e67ca348fa 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -32,5 +32,45 @@ error: `filter(..).map(..)` can be simplified as `filter_map(..)` LL | let _ = vec.iter().filter(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|f| f.field.clone().ok())` -error: aborting due to 5 previous errors +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:63:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_ref().unwrap()); + | |___________________________________________^ help: try: `filter_map(|f| f.field.as_ref().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:68:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref().unwrap()); + | |_____________________________________________^ help: try: `filter_map(|f| f.field.as_deref().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:73:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_mut().unwrap()); + | |___________________________________________^ help: try: `filter_map(|f| f.field.as_mut().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:78:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref_mut().unwrap()); + | |_________________________________________________^ help: try: `filter_map(|f| f.field.as_deref_mut().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:83:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.to_owned().unwrap()); + | |_____________________________________________^ help: try: `filter_map(|f| f.field.to_owned().ok())` + +error: aborting due to 10 previous errors diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed index d78d72d8e8f..e7b5081ea59 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -50,8 +50,26 @@ fn issue_8920() { }]; let _ = vec.iter().find_map(|f| f.field.clone()); - let vec = vec![ResultFoo { + let mut vec = vec![ResultFoo { field: Ok(String::from("str")), }]; let _ = vec.iter().find_map(|f| f.field.clone().ok()); + + let _ = vec.iter().find_map(|f| f.field.as_ref().ok()); + + let _ = vec + .iter() + .find_map(|f| f.field.as_deref().ok()); + + let _ = vec + .iter_mut() + .find_map(|f| f.field.as_mut().ok()); + + let _ = vec + .iter_mut() + .find_map(|f| f.field.as_deref_mut().ok()); + + let _ = vec + .iter() + .find_map(|f| f.field.to_owned().ok()); } diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs index 74e8e52ed16..46badbb9a18 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -50,8 +50,30 @@ fn issue_8920() { }]; let _ = vec.iter().find(|f| f.field.is_some()).map(|f| f.field.clone().unwrap()); - let vec = vec![ResultFoo { + let mut vec = vec![ResultFoo { field: Ok(String::from("str")), }]; let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); + + let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.as_ref().unwrap()); + + let _ = vec + .iter() + .find(|f| f.field.is_ok()) + .map(|f| f.field.as_deref().unwrap()); + + let _ = vec + .iter_mut() + .find(|f| f.field.is_ok()) + .map(|f| f.field.as_mut().unwrap()); + + let _ = vec + .iter_mut() + .find(|f| f.field.is_ok()) + .map(|f| f.field.as_deref_mut().unwrap()); + + let _ = vec + .iter() + .find(|f| f.field.is_ok()) + .map(|f| f.field.to_owned().unwrap()); } diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr index 0b1465a8422..2b6955a17df 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -30,5 +30,43 @@ error: `find(..).map(..)` can be simplified as `find_map(..)` LL | let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|f| f.field.clone().ok())` -error: aborting due to 5 previous errors +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:58:24 + | +LL | let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.as_ref().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|f| f.field.as_ref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:62:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref().unwrap()); + | |_____________________________________________^ help: try: `find_map(|f| f.field.as_deref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:67:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_mut().unwrap()); + | |___________________________________________^ help: try: `find_map(|f| f.field.as_mut().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:72:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref_mut().unwrap()); + | |_________________________________________________^ help: try: `find_map(|f| f.field.as_deref_mut().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:77:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.to_owned().unwrap()); + | |_____________________________________________^ help: try: `find_map(|f| f.field.to_owned().ok())` + +error: aborting due to 10 previous errors