From 7e76a196649e1675e9c210f659074499e37fb353 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sun, 19 Jan 2020 10:00:34 +0900 Subject: [PATCH] Apply review comments --- clippy_lints/src/if_let_some_result.rs | 27 +++++++++++++------------- tests/ui/if_let_some_result.fixed | 7 ++++--- tests/ui/if_let_some_result.rs | 7 ++++--- tests/ui/if_let_some_result.stderr | 20 +++++++------------ 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