about summary refs log tree commit diff
diff options
context:
space:
mode:
authorYuki Okushi <huyuumi.dev@gmail.com>2020-01-19 10:00:34 +0900
committerYuki Okushi <huyuumi.dev@gmail.com>2020-01-19 10:14:03 +0900
commit7e76a196649e1675e9c210f659074499e37fb353 (patch)
treeabb5aa6c605b7defad1593e024cfc7a9a1360d17
parentce06ba3d30c2331c00596317284a0e1049b79929 (diff)
downloadrust-7e76a196649e1675e9c210f659074499e37fb353.tar.gz
rust-7e76a196649e1675e9c210f659074499e37fb353.zip
Apply review comments
-rw-r--r--clippy_lints/src/if_let_some_result.rs27
-rw-r--r--tests/ui/if_let_some_result.fixed7
-rw-r--r--tests/ui/if_let_some_result.rs7
-rw-r--r--tests/ui/if_let_some_result.stderr20
4 files changed, 28 insertions, 33 deletions
diff --git a/clippy_lints/src/if_let_some_result.rs b/clippy_lints/src/if_let_some_result.rs
index 860caad2858..8a3dcb7b6e9 100644
--- a/clippy_lints/src/if_let_some_result.rs
+++ b/clippy_lints/src/if_let_some_result.rs
@@ -1,4 +1,4 @@
-use crate::utils::{match_type, method_chain_args, paths, snippet_with_applicability, span_lint_and_then};
+use crate::utils::{match_type, method_chain_args, paths, snippet_with_applicability, span_lint_and_sugg};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::*;
@@ -50,28 +50,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
             then {
                 let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT);
                 let mut applicability = Applicability::MachineApplicable;
-                let trimed_ok_span = op.span.until(op.span.with_lo(ok_span.lo() - BytePos(1)));
+                // ok_span = `ok`
+                // op.span = `x.parse()   .   ok()`
+                // op.span.until(op.span.with_lo(ok_span.lo() - BytePos(1))) = `x.parse()   .`
+                // op.span.with_lo(ok_span.lo() - BytePos(1)) = ` ok()`
+                // op.span.with_hi(ok_span.hi() - BytePos(1)) = `x.parse()   .   o`
                 let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
-                let trimmed_ok = snippet_with_applicability(cx, trimed_ok_span, "", &mut applicability);
+                let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
                 let sugg = format!(
                     "if let Ok({}) = {}",
                     some_expr_string,
-                    trimmed_ok,
+                    trimmed_ok.trim().trim_end_matches('.'),
                 );
                 if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type {
-                    span_lint_and_then(
+                    span_lint_and_sugg(
                         cx,
                         IF_LET_SOME_RESULT,
-                        expr.span,
+                        expr.span.with_hi(ok_span.hi() + BytePos(2)),
                         "Matching on `Some` with `ok()` is redundant",
-                        |db| {
-                            db.span_suggestion(
-                                expr.span.shrink_to_lo().to(ok_span.with_hi(ok_span.hi() + BytePos(2))),
-                                &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
-                                sugg,
-                                applicability,
-                            );
-                        },
+                        &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
+                        sugg,
+                        applicability,
                     );
                 }
             }
diff --git a/tests/ui/if_let_some_result.fixed b/tests/ui/if_let_some_result.fixed
index 439c749f995..80505fd997f 100644
--- a/tests/ui/if_let_some_result.fixed
+++ b/tests/ui/if_let_some_result.fixed
@@ -18,9 +18,10 @@ fn str_to_int_ok(x: &str) -> i32 {
     }
 }
 
-fn nested_some_no_else(x: &str) -> i32 {
+#[rustfmt::skip]
+fn strange_some_no_else(x: &str) -> i32 {
     {
-        if let Ok(y) = x.parse() {
+        if let Ok(y) = x   .   parse()       {
             return y;
         };
         0
@@ -30,5 +31,5 @@ fn nested_some_no_else(x: &str) -> i32 {
 fn main() {
     let _ = str_to_int("1");
     let _ = str_to_int_ok("2");
-    let _ = nested_some_no_else("3");
+    let _ = strange_some_no_else("3");
 }
diff --git a/tests/ui/if_let_some_result.rs b/tests/ui/if_let_some_result.rs
index 83f6ce1d330..bee8156e084 100644
--- a/tests/ui/if_let_some_result.rs
+++ b/tests/ui/if_let_some_result.rs
@@ -18,9 +18,10 @@ fn str_to_int_ok(x: &str) -> i32 {
     }
 }
 
-fn nested_some_no_else(x: &str) -> i32 {
+#[rustfmt::skip]
+fn strange_some_no_else(x: &str) -> i32 {
     {
-        if let Some(y) = x.parse().ok() {
+        if let Some(y) = x   .   parse()   .   ok()    {
             return y;
         };
         0
@@ -30,5 +31,5 @@ fn nested_some_no_else(x: &str) -> i32 {
 fn main() {
     let _ = str_to_int("1");
     let _ = str_to_int_ok("2");
-    let _ = nested_some_no_else("3");
+    let _ = strange_some_no_else("3");
 }
diff --git a/tests/ui/if_let_some_result.stderr b/tests/ui/if_let_some_result.stderr
index 0084260b22a..b4adc526efa 100644
--- a/tests/ui/if_let_some_result.stderr
+++ b/tests/ui/if_let_some_result.stderr
@@ -1,12 +1,8 @@
 error: Matching on `Some` with `ok()` is redundant
   --> $DIR/if_let_some_result.rs:6:5
    |
-LL | /     if let Some(y) = x.parse().ok() {
-LL | |         y
-LL | |     } else {
-LL | |         0
-LL | |     }
-   | |_____^
+LL |     if let Some(y) = x.parse().ok() {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::if-let-some-result` implied by `-D warnings`
 help: Consider matching on `Ok(y)` and removing the call to `ok` instead
@@ -15,17 +11,15 @@ LL |     if let Ok(y) = x.parse() {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: Matching on `Some` with `ok()` is redundant
-  --> $DIR/if_let_some_result.rs:23:9
+  --> $DIR/if_let_some_result.rs:24:9
    |
-LL | /         if let Some(y) = x.parse().ok() {
-LL | |             return y;
-LL | |         };
-   | |_________^
+LL |         if let Some(y) = x   .   parse()   .   ok()    {
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 help: Consider matching on `Ok(y)` and removing the call to `ok` instead
    |
-LL |         if let Ok(y) = x.parse() {
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         if let Ok(y) = x   .   parse()       {
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 2 previous errors