From 9035264a8febac258d1def1d27978ab8cd1ff121 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Fri, 10 Jan 2020 08:34:13 +0900
Subject: [PATCH 1/7] Add suggestion in `if_let_some_result`
---
clippy_lints/src/ok_if_let.rs | 33 ++++++++++++++++++++++++++-------
tests/ui/ok_if_let.fixed | 35 +++++++++++++++++++++++++++++++++++
tests/ui/ok_if_let.rs | 20 ++++++++++++++++----
tests/ui/ok_if_let.stderr | 28 +++++++++++++++++++++++++---
4 files changed, 102 insertions(+), 14 deletions(-)
create mode 100644 tests/ui/ok_if_let.fixed
diff --git a/clippy_lints/src/ok_if_let.rs b/clippy_lints/src/ok_if_let.rs
index 6145fae3156..9696951ac0c 100644
--- a/clippy_lints/src/ok_if_let.rs
+++ b/clippy_lints/src/ok_if_let.rs
@@ -1,5 +1,6 @@
-use crate::utils::{match_type, method_chain_args, paths, snippet, span_help_and_lint};
+use crate::utils::{match_type, method_chain_args, paths, snippet, snippet_with_applicability, span_lint_and_sugg};
use if_chain::if_chain;
+use rustc_errors::Applicability;
use rustc_hir::*;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -40,18 +41,36 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! { //begin checking variables
if let ExprKind::Match(ref op, ref body, ref source) = expr.kind; //test if expr is a match
- if let MatchSource::IfLetDesugar { .. } = *source; //test if it is an If Let
- if let ExprKind::MethodCall(_, _, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
+ if let MatchSource::IfLetDesugar { contains_else_clause } = *source; //test if it is an If Let
+ if let ExprKind::MethodCall(_, ok_span, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
then {
let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT);
- let some_expr_string = snippet(cx, y[0].span, "");
+ let mut applicability = Applicability::MachineApplicable;
+ let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
+ let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
+ let mut sugg = format!(
+ "if let Ok({}) = {} {}",
+ some_expr_string,
+ // FIXME(JohnTitor): this trimming is hacky, probably can improve it
+ trimmed_ok.trim_matches('.'),
+ snippet(cx, body[0].span, ".."),
+ );
+ if contains_else_clause {
+ sugg = format!("{} else {}", sugg, snippet(cx, body[1].span, ".."));
+ }
if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type {
- span_help_and_lint(cx, IF_LET_SOME_RESULT, expr.span,
- "Matching on `Some` with `ok()` is redundant",
- &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string));
+ span_lint_and_sugg(
+ cx,
+ IF_LET_SOME_RESULT,
+ expr.span,
+ "Matching on `Some` with `ok()` is redundant",
+ &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
+ sugg,
+ applicability,
+ );
}
}
}
diff --git a/tests/ui/ok_if_let.fixed b/tests/ui/ok_if_let.fixed
new file mode 100644
index 00000000000..e7fdc972a19
--- /dev/null
+++ b/tests/ui/ok_if_let.fixed
@@ -0,0 +1,35 @@
+// run-rustfix
+
+#![warn(clippy::if_let_some_result)]
+
+fn str_to_int(x: &str) -> i32 {
+ if let Ok(y) = x.parse() {
+ y
+ } else {
+ 0
+ }
+}
+
+fn str_to_int_ok(x: &str) -> i32 {
+ if let Ok(y) = x.parse() {
+ y
+ } else {
+ 0
+ }
+}
+
+fn nested_some_no_else(x: &str) -> i32 {
+ {
+ if let Ok(y) = x.parse() {
+ return y;
+ };
+ 0
+ }
+}
+
+fn main() {
+ let x = str_to_int("1");
+ let y = str_to_int_ok("2");
+ let z = nested_some_no_else("3");
+ println!("{}{}{}", x, y, z);
+}
diff --git a/tests/ui/ok_if_let.rs b/tests/ui/ok_if_let.rs
index 61db3113052..becadf3644f 100644
--- a/tests/ui/ok_if_let.rs
+++ b/tests/ui/ok_if_let.rs
@@ -1,3 +1,5 @@
+// run-rustfix
+
#![warn(clippy::if_let_some_result)]
fn str_to_int(x: &str) -> i32 {
@@ -16,8 +18,18 @@ fn str_to_int_ok(x: &str) -> i32 {
}
}
-fn main() {
- let y = str_to_int("1");
- let z = str_to_int_ok("2");
- println!("{}{}", y, z);
+fn nested_some_no_else(x: &str) -> i32 {
+ {
+ if let Some(y) = x.parse().ok() {
+ return y;
+ };
+ 0
+ }
+}
+
+fn main() {
+ let x = str_to_int("1");
+ let y = str_to_int_ok("2");
+ let z = nested_some_no_else("3");
+ println!("{}{}{}", x, y, z);
}
diff --git a/tests/ui/ok_if_let.stderr b/tests/ui/ok_if_let.stderr
index e3e6c5c4634..4aa6057ba47 100644
--- a/tests/ui/ok_if_let.stderr
+++ b/tests/ui/ok_if_let.stderr
@@ -1,5 +1,5 @@
error: Matching on `Some` with `ok()` is redundant
- --> $DIR/ok_if_let.rs:4:5
+ --> $DIR/ok_if_let.rs:6:5
|
LL | / if let Some(y) = x.parse().ok() {
LL | | y
@@ -9,7 +9,29 @@ LL | | }
| |_____^
|
= note: `-D clippy::if-let-some-result` implied by `-D warnings`
- = help: Consider matching on `Ok(y)` and removing the call to `ok` instead
+help: Consider matching on `Ok(y)` and removing the call to `ok` instead
+ |
+LL | if let Ok(y) = x.parse() {
+LL | y
+LL | } else {
+LL | 0
+LL | }
+ |
-error: aborting due to previous error
+error: Matching on `Some` with `ok()` is redundant
+ --> $DIR/ok_if_let.rs:23:9
+ |
+LL | / if let Some(y) = x.parse().ok() {
+LL | | return y;
+LL | | };
+ | |_________^
+ |
+help: Consider matching on `Ok(y)` and removing the call to `ok` instead
+ |
+LL | if let Ok(y) = x.parse() {
+LL | return y;
+LL | };
+ |
+
+error: aborting due to 2 previous errors
From 9e55424bdc4b837e38a7f52139ad6158d7818102 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sat, 11 Jan 2020 03:05:10 +0900
Subject: [PATCH 2/7] Apply review comments
---
clippy_lints/src/ok_if_let.rs | 11 ++++++-----
tests/ui/ok_if_let.fixed | 7 +++----
tests/ui/ok_if_let.rs | 7 +++----
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/clippy_lints/src/ok_if_let.rs b/clippy_lints/src/ok_if_let.rs
index 9696951ac0c..dc129a28e37 100644
--- a/clippy_lints/src/ok_if_let.rs
+++ b/clippy_lints/src/ok_if_let.rs
@@ -4,6 +4,7 @@ use rustc_errors::Applicability;
use rustc_hir::*;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::BytePos;
declare_clippy_lint! {
/// **What it does:*** Checks for unnecessary `ok()` in if let.
@@ -40,8 +41,8 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! { //begin checking variables
- if let ExprKind::Match(ref op, ref body, ref source) = expr.kind; //test if expr is a match
- if let MatchSource::IfLetDesugar { contains_else_clause } = *source; //test if it is an If Let
+ if let ExprKind::Match(ref op, ref body, source) = expr.kind; //test if expr is a match
+ if let MatchSource::IfLetDesugar { contains_else_clause } = source; //test if it is an If Let
if let ExprKind::MethodCall(_, ok_span, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
@@ -49,13 +50,13 @@ 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)));
let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
- let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
+ let trimmed_ok = snippet_with_applicability(cx, trimed_ok_span, "", &mut applicability);
let mut sugg = format!(
"if let Ok({}) = {} {}",
some_expr_string,
- // FIXME(JohnTitor): this trimming is hacky, probably can improve it
- trimmed_ok.trim_matches('.'),
+ trimmed_ok,
snippet(cx, body[0].span, ".."),
);
if contains_else_clause {
diff --git a/tests/ui/ok_if_let.fixed b/tests/ui/ok_if_let.fixed
index e7fdc972a19..439c749f995 100644
--- a/tests/ui/ok_if_let.fixed
+++ b/tests/ui/ok_if_let.fixed
@@ -28,8 +28,7 @@ fn nested_some_no_else(x: &str) -> i32 {
}
fn main() {
- let x = str_to_int("1");
- let y = str_to_int_ok("2");
- let z = nested_some_no_else("3");
- println!("{}{}{}", x, y, z);
+ let _ = str_to_int("1");
+ let _ = str_to_int_ok("2");
+ let _ = nested_some_no_else("3");
}
diff --git a/tests/ui/ok_if_let.rs b/tests/ui/ok_if_let.rs
index becadf3644f..83f6ce1d330 100644
--- a/tests/ui/ok_if_let.rs
+++ b/tests/ui/ok_if_let.rs
@@ -28,8 +28,7 @@ fn nested_some_no_else(x: &str) -> i32 {
}
fn main() {
- let x = str_to_int("1");
- let y = str_to_int_ok("2");
- let z = nested_some_no_else("3");
- println!("{}{}{}", x, y, z);
+ let _ = str_to_int("1");
+ let _ = str_to_int_ok("2");
+ let _ = nested_some_no_else("3");
}
From ae872fe1c74d68d7d1e88079cc77c858268fcdbf Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sat, 11 Jan 2020 03:29:55 +0900
Subject: [PATCH 3/7] Rename `ok_if_let` to `if_let_some_result`
---
.../src/{ok_if_let.rs => if_let_some_result.rs} | 0
clippy_lints/src/lib.rs | 10 +++++-----
src/lintlist/mod.rs | 2 +-
tests/ui/{ok_if_let.fixed => if_let_some_result.fixed} | 0
tests/ui/{ok_if_let.rs => if_let_some_result.rs} | 0
.../ui/{ok_if_let.stderr => if_let_some_result.stderr} | 4 ++--
6 files changed, 8 insertions(+), 8 deletions(-)
rename clippy_lints/src/{ok_if_let.rs => if_let_some_result.rs} (100%)
rename tests/ui/{ok_if_let.fixed => if_let_some_result.fixed} (100%)
rename tests/ui/{ok_if_let.rs => if_let_some_result.rs} (100%)
rename tests/ui/{ok_if_let.stderr => if_let_some_result.stderr} (91%)
diff --git a/clippy_lints/src/ok_if_let.rs b/clippy_lints/src/if_let_some_result.rs
similarity index 100%
rename from clippy_lints/src/ok_if_let.rs
rename to clippy_lints/src/if_let_some_result.rs
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 5fe0d937f2f..8a90e39f2e9 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -210,6 +210,7 @@ pub mod functions;
pub mod get_last_with_len;
pub mod identity_conversion;
pub mod identity_op;
+pub mod if_let_some_result;
pub mod if_not_else;
pub mod implicit_return;
pub mod indexing_slicing;
@@ -263,7 +264,6 @@ pub mod new_without_default;
pub mod no_effect;
pub mod non_copy_const;
pub mod non_expressive_names;
-pub mod ok_if_let;
pub mod open_options;
pub mod overflow_check_conditional;
pub mod panic_unimplemented;
@@ -703,7 +703,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS,
&non_expressive_names::MANY_SINGLE_CHAR_NAMES,
&non_expressive_names::SIMILAR_NAMES,
- &ok_if_let::IF_LET_SOME_RESULT,
+ &if_let_some_result::IF_LET_SOME_RESULT,
&open_options::NONSENSICAL_OPEN_OPTIONS,
&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
&panic_unimplemented::PANIC,
@@ -904,7 +904,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box eval_order_dependence::EvalOrderDependence);
store.register_late_pass(|| box missing_doc::MissingDoc::new());
store.register_late_pass(|| box missing_inline::MissingInline);
- store.register_late_pass(|| box ok_if_let::OkIfLet);
+ store.register_late_pass(|| box if_let_some_result::OkIfLet);
store.register_late_pass(|| box redundant_pattern_matching::RedundantPatternMatching);
store.register_late_pass(|| box partialeq_ne_impl::PartialEqNeImpl);
store.register_late_pass(|| box unused_io_amount::UnusedIoAmount);
@@ -1265,7 +1265,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
- LintId::of(&ok_if_let::IF_LET_SOME_RESULT),
+ LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
LintId::of(&panic_unimplemented::PANIC_PARAMS),
@@ -1413,7 +1413,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
- LintId::of(&ok_if_let::IF_LET_SOME_RESULT),
+ LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&panic_unimplemented::PANIC_PARAMS),
LintId::of(&ptr::CMP_NULL),
LintId::of(&ptr::PTR_ARG),
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index eee16fcd312..c6339daf2eb 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -705,7 +705,7 @@ pub const ALL_LINTS: [Lint; 347] = [
group: "style",
desc: "usage of `ok()` in `if let Some(pat)` statements is unnecessary, match on `Ok(pat)` instead",
deprecation: None,
- module: "ok_if_let",
+ module: "if_let_some_result",
},
Lint {
name: "if_not_else",
diff --git a/tests/ui/ok_if_let.fixed b/tests/ui/if_let_some_result.fixed
similarity index 100%
rename from tests/ui/ok_if_let.fixed
rename to tests/ui/if_let_some_result.fixed
diff --git a/tests/ui/ok_if_let.rs b/tests/ui/if_let_some_result.rs
similarity index 100%
rename from tests/ui/ok_if_let.rs
rename to tests/ui/if_let_some_result.rs
diff --git a/tests/ui/ok_if_let.stderr b/tests/ui/if_let_some_result.stderr
similarity index 91%
rename from tests/ui/ok_if_let.stderr
rename to tests/ui/if_let_some_result.stderr
index 4aa6057ba47..6925dfa0bf9 100644
--- a/tests/ui/ok_if_let.stderr
+++ b/tests/ui/if_let_some_result.stderr
@@ -1,5 +1,5 @@
error: Matching on `Some` with `ok()` is redundant
- --> $DIR/ok_if_let.rs:6:5
+ --> $DIR/if_let_some_result.rs:6:5
|
LL | / if let Some(y) = x.parse().ok() {
LL | | y
@@ -19,7 +19,7 @@ LL | }
|
error: Matching on `Some` with `ok()` is redundant
- --> $DIR/ok_if_let.rs:23:9
+ --> $DIR/if_let_some_result.rs:23:9
|
LL | / if let Some(y) = x.parse().ok() {
LL | | return y;
From aab64a21cca6e73cee0a7d6c203aa3ac14a8ea8d Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sat, 11 Jan 2020 04:34:01 +0900
Subject: [PATCH 4/7] Reduce span range
---
clippy_lints/src/if_let_some_result.rs | 25 +++++++++++++------------
tests/ui/if_let_some_result.stderr | 10 ++--------
2 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/clippy_lints/src/if_let_some_result.rs b/clippy_lints/src/if_let_some_result.rs
index dc129a28e37..860caad2858 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, snippet_with_applicability, span_lint_and_sugg};
+use crate::utils::{match_type, method_chain_args, paths, snippet_with_applicability, span_lint_and_then};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::*;
@@ -42,7 +42,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! { //begin checking variables
if let ExprKind::Match(ref op, ref body, source) = expr.kind; //test if expr is a match
- if let MatchSource::IfLetDesugar { contains_else_clause } = source; //test if it is an If Let
+ if let MatchSource::IfLetDesugar { .. } = source; //test if it is an If Let
if let ExprKind::MethodCall(_, ok_span, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
@@ -53,24 +53,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
let trimed_ok_span = op.span.until(op.span.with_lo(ok_span.lo() - BytePos(1)));
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 mut sugg = format!(
- "if let Ok({}) = {} {}",
+ let sugg = format!(
+ "if let Ok({}) = {}",
some_expr_string,
trimmed_ok,
- snippet(cx, body[0].span, ".."),
);
- if contains_else_clause {
- sugg = format!("{} else {}", sugg, snippet(cx, body[1].span, ".."));
- }
if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type {
- span_lint_and_sugg(
+ span_lint_and_then(
cx,
IF_LET_SOME_RESULT,
expr.span,
"Matching on `Some` with `ok()` is redundant",
- &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
- sugg,
- applicability,
+ |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,
+ );
+ },
);
}
}
diff --git a/tests/ui/if_let_some_result.stderr b/tests/ui/if_let_some_result.stderr
index 6925dfa0bf9..0084260b22a 100644
--- a/tests/ui/if_let_some_result.stderr
+++ b/tests/ui/if_let_some_result.stderr
@@ -12,11 +12,7 @@ LL | | }
help: Consider matching on `Ok(y)` and removing the call to `ok` instead
|
LL | if let Ok(y) = x.parse() {
-LL | y
-LL | } else {
-LL | 0
-LL | }
- |
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
error: Matching on `Some` with `ok()` is redundant
--> $DIR/if_let_some_result.rs:23:9
@@ -29,9 +25,7 @@ LL | | };
help: Consider matching on `Ok(y)` and removing the call to `ok` instead
|
LL | if let Ok(y) = x.parse() {
-LL | return y;
-LL | };
- |
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 2 previous errors
From ce06ba3d30c2331c00596317284a0e1049b79929 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sat, 11 Jan 2020 06:02:02 +0900
Subject: [PATCH 5/7] Run `update_lints`
---
clippy_lints/src/lib.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 8a90e39f2e9..497146772a5 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -545,6 +545,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&get_last_with_len::GET_LAST_WITH_LEN,
&identity_conversion::IDENTITY_CONVERSION,
&identity_op::IDENTITY_OP,
+ &if_let_some_result::IF_LET_SOME_RESULT,
&if_not_else::IF_NOT_ELSE,
&implicit_return::IMPLICIT_RETURN,
&indexing_slicing::INDEXING_SLICING,
@@ -703,7 +704,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS,
&non_expressive_names::MANY_SINGLE_CHAR_NAMES,
&non_expressive_names::SIMILAR_NAMES,
- &if_let_some_result::IF_LET_SOME_RESULT,
&open_options::NONSENSICAL_OPEN_OPTIONS,
&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
&panic_unimplemented::PANIC,
@@ -1153,6 +1153,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&get_last_with_len::GET_LAST_WITH_LEN),
LintId::of(&identity_conversion::IDENTITY_CONVERSION),
LintId::of(&identity_op::IDENTITY_OP),
+ LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING),
LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&infinite_iter::INFINITE_ITER),
@@ -1265,7 +1266,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
- LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
LintId::of(&panic_unimplemented::PANIC_PARAMS),
@@ -1367,6 +1367,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&formatting::SUSPICIOUS_UNARY_OP_FORMATTING),
LintId::of(&functions::DOUBLE_MUST_USE),
LintId::of(&functions::MUST_USE_UNIT),
+ LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
LintId::of(&inherent_to_string::INHERENT_TO_STRING),
LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
@@ -1413,7 +1414,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
- LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
LintId::of(&panic_unimplemented::PANIC_PARAMS),
LintId::of(&ptr::CMP_NULL),
LintId::of(&ptr::PTR_ARG),
From 7e76a196649e1675e9c210f659074499e37fb353 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sun, 19 Jan 2020 10:00:34 +0900
Subject: [PATCH 6/7] 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
From c9f8d03f42f8ce0617992e6375d8c9a39ab1ca01 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sun, 19 Jan 2020 21:14:47 +0900
Subject: [PATCH 7/7] Treat more strange pattern
---
clippy_lints/src/if_let_some_result.rs | 29 ++++++++++----------------
tests/ui/if_let_some_result.rs | 2 +-
tests/ui/if_let_some_result.stderr | 4 ++--
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/clippy_lints/src/if_let_some_result.rs b/clippy_lints/src/if_let_some_result.rs
index 8a3dcb7b6e9..a9826d58633 100644
--- a/clippy_lints/src/if_let_some_result.rs
+++ b/clippy_lints/src/if_let_some_result.rs
@@ -4,7 +4,6 @@ use rustc_errors::Applicability;
use rustc_hir::*;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::BytePos;
declare_clippy_lint! {
/// **What it does:*** Checks for unnecessary `ok()` in if let.
@@ -46,15 +45,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
if let ExprKind::MethodCall(_, ok_span, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
+ let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT);
+ if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type;
then {
- let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT);
let mut applicability = Applicability::MachineApplicable;
- // 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, op.span.until(ok_span), "", &mut applicability);
let sugg = format!(
@@ -62,17 +57,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
some_expr_string,
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_sugg(
- cx,
- IF_LET_SOME_RESULT,
- expr.span.with_hi(ok_span.hi() + BytePos(2)),
- "Matching on `Some` with `ok()` is redundant",
- &format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
- sugg,
- applicability,
- );
- }
+ span_lint_and_sugg(
+ cx,
+ IF_LET_SOME_RESULT,
+ expr.span.with_hi(op.span.hi()),
+ "Matching on `Some` with `ok()` is redundant",
+ &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.rs b/tests/ui/if_let_some_result.rs
index bee8156e084..ecac1357445 100644
--- a/tests/ui/if_let_some_result.rs
+++ b/tests/ui/if_let_some_result.rs
@@ -21,7 +21,7 @@ fn str_to_int_ok(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
diff --git a/tests/ui/if_let_some_result.stderr b/tests/ui/if_let_some_result.stderr
index b4adc526efa..334ccb01016 100644
--- a/tests/ui/if_let_some_result.stderr
+++ b/tests/ui/if_let_some_result.stderr
@@ -13,8 +13,8 @@ LL | if let Ok(y) = x.parse() {
error: Matching on `Some` with `ok()` is redundant
--> $DIR/if_let_some_result.rs:24:9
|
-LL | if let Some(y) = x . parse() . ok() {
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | if let Some(y) = x . parse() . ok () {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: Consider matching on `Ok(y)` and removing the call to `ok` instead
|