get all tests passing pending some questions

rust-lang · Feb 28, 2021 · 05f8b58 · 05f8b58
1 parent a397d94
commit 05f8b58
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 50 deletions.
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -34,11 +34,12 @@ use crate::utils::usage::mutated_variables;
 use crate::utils::{
     contains_return, contains_ty, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
     in_macro, is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path,
-    match_qpath, match_trait_method, match_type, meets_msrv, method_calls, method_chain_args,
-    path_to_local_id, paths, remove_blocks, return_ty, single_segment_path, snippet, snippet_block,
-    snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
-    span_lint_and_then, strip_pat_refs, sugg, walk_ptrs_ty_depth, SpanlessEq,
+    match_qpath, match_trait_method, match_type, meets_msrv, method_calls, method_chain_args, path_to_local_id, paths,
+    remove_blocks, return_ty, single_segment_path, snippet, snippet_block, snippet_with_applicability,
+    snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, strip_pat_refs,
+    sugg, walk_ptrs_ty_depth, SpanlessEq,
 };
+use clippy_utils::paths::{OPTION_IS_SOME, OPTION_UNWRAP};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for `.unwrap()` calls on `Option`s and on `Result`s.
@@ -3213,17 +3214,12 @@ fn is_method<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, method_path: &[
     }
 }
 
-// these two paths can't be checked and thus aren't in `paths`
-const OPTION_IS_SOME: [&str; 4] = ["core", "option", "Option", "is_some"];
-const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"];
-
 fn is_option_filter_map<'tcx>(
     cx: &LateContext<'tcx>,
     filter_arg: &'tcx hir::Expr<'_>,
     map_arg: &'tcx hir::Expr<'_>,
 ) -> bool {
-    is_method(cx, map_arg, &OPTION_UNWRAP, sym!(unwrap)) &&
-    is_method(cx, filter_arg, &OPTION_IS_SOME, sym!(is_some))
+    is_method(cx, map_arg, &OPTION_UNWRAP, sym!(unwrap)) && is_method(cx, filter_arg, &OPTION_IS_SOME, sym!(is_some))
 }
 
 /// lint use of `filter().map()` for `Iterators`
@@ -3238,28 +3234,22 @@ fn lint_filter_some_map_unwrap<'tcx>(
     let iterator = match_trait_method(cx, expr, &paths::ITERATOR);
     let iterator_or_option =
         iterator || is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&filter_recv), sym::option_type);
-    if iterator_or_option {
-        if is_option_filter_map(cx, filter_arg, map_arg) {
-            let msg = "`filter` for `Some` followed by `unwrap`";
-            let help = "consider using `flatten` instead";
-            let sugg = format!(
-                "{}.flatten()",
-                snippet_block(cx, filter_recv.span, "..", Some(target_span))
-            );
-            span_lint_and_sugg(
-                cx,
-                OPTION_FILTER_MAP,
-                expr.span.with_hi(expr.span.hi()),
-                msg,
-                help,
-                sugg,
-                Applicability::MachineApplicable,
-            );
-        } else if iterator {
-            let msg = "called `filter(..).map(..)` on an `Iterator`";
-            let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead";
-            span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
-        }
+    if iterator_or_option && is_option_filter_map(cx, filter_arg, map_arg) {
+        let msg = "`filter` for `Some` followed by `unwrap`";
+        let help = "consider using `flatten` instead";
+        let sugg = format!(
+            "{}.flatten()",
+            snippet_block(cx, filter_recv.span, "..", Some(target_span))
+        );
+        span_lint_and_sugg(
+            cx,
+            OPTION_FILTER_MAP,
+            expr.span.with_hi(expr.span.hi()),
+            msg,
+            help,
+            sugg,
+            Applicability::MachineApplicable,
+        );
     }
 }
 

diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
@@ -87,7 +87,9 @@ pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"];
 pub const OPS_MODULE: [&str; 2] = ["core", "ops"];
 pub const OPTION: [&str; 3] = ["core", "option", "Option"];
 pub const OPTION_NONE: [&str; 4] = ["core", "option", "Option", "None"];
+pub const OPTION_IS_SOME: [&str; 4] = ["core", "option", "Option", "is_some"];
 pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"];
+pub const OPTION_UNWRAP: [&str; 4] = ["core", "option", "Option", "unwrap"];
 pub const ORD: [&str; 3] = ["core", "cmp", "Ord"];
 pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"];
 pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"];

diff --git a/tests/ui/filter_methods.stderr b/tests/ui/filter_methods.stderr
@@ -1,12 +1,3 @@
-error: called `filter(..).map(..)` on an `Iterator`
-  --> $DIR/filter_methods.rs:6:21
-   |
-LL |     let _: Vec<_> = vec![5; 6].into_iter().filter(|&x| x == 0).map(|x| x * 2).collect();
-   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::filter-map` implied by `-D warnings`
-   = help: this is more succinctly expressed by calling `.filter_map(..)` instead
-
 error: called `filter(..).flat_map(..)` on an `Iterator`
   --> $DIR/filter_methods.rs:8:21
    |
@@ -17,6 +8,7 @@ LL | |         .filter(|&x| x == 0)
 LL | |         .flat_map(|x| x.checked_mul(2))
    | |_______________________________________^
    |
+   = note: `-D clippy::filter-map` implied by `-D warnings`
    = help: this is more succinctly expressed by calling `.flat_map(..)` and filtering by returning `iter::empty()`
 
 error: called `filter_map(..).flat_map(..)` on an `Iterator`
@@ -43,5 +35,5 @@ LL | |         .map(|x| x.checked_mul(2))
    |
    = help: this is more succinctly expressed by only calling `.filter_map(..)` instead
 
-error: aborting due to 4 previous errors
+error: aborting due to 3 previous errors
 
diff --git a/tests/ui/option_filter_map.fixed b/tests/ui/option_filter_map.fixed
@@ -1,3 +1,4 @@
+#![warn(clippy::option_filter_map)]
 // run-rustfix
 fn odds_out(x: i32) -> Option<i32> {
     if x % 2 == 0 {

diff --git a/tests/ui/option_filter_map.rs b/tests/ui/option_filter_map.rs
@@ -1,3 +1,4 @@
+#![warn(clippy::option_filter_map)]
 // run-rustfix
 fn odds_out(x: i32) -> Option<i32> {
     if x % 2 == 0 {

diff --git a/tests/ui/option_filter_map.stderr b/tests/ui/option_filter_map.stderr
@@ -1,43 +1,43 @@
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:11:13
+  --> $DIR/option_filter_map.rs:12:13
    |
 LL |     let _ = Some(Some(1)).filter(Option::is_some).map(Option::unwrap);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `Some(Some(1)).flatten()`
    |
    = note: `-D clippy::option-filter-map` implied by `-D warnings`
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:12:13
+  --> $DIR/option_filter_map.rs:13:13
    |
 LL |     let _ = Some(Some(1)).filter(|o| o.is_some()).map(|o| o.unwrap());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `Some(Some(1)).flatten()`
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:13:13
+  --> $DIR/option_filter_map.rs:14:13
    |
 LL |     let _ = Some(1).map(odds_out).filter(Option::is_some).map(Option::unwrap);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `Some(1).map(odds_out).flatten()`
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:14:13
+  --> $DIR/option_filter_map.rs:15:13
    |
 LL |     let _ = Some(1).map(odds_out).filter(|o| o.is_some()).map(|o| o.unwrap());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `Some(1).map(odds_out).flatten()`
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:16:13
+  --> $DIR/option_filter_map.rs:17:13
    |
 LL |     let _ = vec![Some(1)].into_iter().filter(Option::is_some).map(Option::unwrap);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `vec![Some(1)].into_iter().flatten()`
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:17:13
+  --> $DIR/option_filter_map.rs:18:13
    |
 LL |     let _ = vec![Some(1)].into_iter().filter(|o| o.is_some()).map(|o| o.unwrap());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `vec![Some(1)].into_iter().flatten()`
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:18:13
+  --> $DIR/option_filter_map.rs:19:13
    |
 LL |       let _ = vec![1]
    |  _____________^
@@ -55,7 +55,7 @@ LL |         .map(odds_out).flatten();
    |
 
 error: `filter` for `Some` followed by `unwrap`
-  --> $DIR/option_filter_map.rs:23:13
+  --> $DIR/option_filter_map.rs:24:13
    |
 LL |       let _ = vec![1]
    |  _____________^