Auto merge of #5032 - JohnTitor:add-sugg-some-result, r=flip1995
Add suggestions for `if_let_some_result` Fixes #4991 This approach may be fragile though... changelog: Add suggestions for `if_let_some_result`
This commit is contained in:
commit
fd0428f622
@ -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_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};
|
||||
@ -39,20 +40,32 @@ 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 { .. } = *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 ExprKind::Match(ref op, ref body, 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(_, 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 some_expr_string = snippet(cx, y[0].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,
|
||||
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 sugg = format!(
|
||||
"if let Ok({}) = {}",
|
||||
some_expr_string,
|
||||
trimmed_ok.trim().trim_end_matches('.'),
|
||||
);
|
||||
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));
|
||||
}
|
||||
&format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
|
||||
sugg,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
@ -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;
|
||||
@ -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,
|
||||
&ok_if_let::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);
|
||||
@ -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(&ok_if_let::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(&ok_if_let::IF_LET_SOME_RESULT),
|
||||
LintId::of(&panic_unimplemented::PANIC_PARAMS),
|
||||
LintId::of(&ptr::CMP_NULL),
|
||||
LintId::of(&ptr::PTR_ARG),
|
||||
|
@ -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",
|
||||
|
35
tests/ui/if_let_some_result.fixed
Normal file
35
tests/ui/if_let_some_result.fixed
Normal file
@ -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
|
||||
}
|
||||
}
|
||||
|
||||
#[rustfmt::skip]
|
||||
fn strange_some_no_else(x: &str) -> i32 {
|
||||
{
|
||||
if let Ok(y) = x . parse() {
|
||||
return y;
|
||||
};
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let _ = str_to_int("1");
|
||||
let _ = str_to_int_ok("2");
|
||||
let _ = strange_some_no_else("3");
|
||||
}
|
35
tests/ui/if_let_some_result.rs
Normal file
35
tests/ui/if_let_some_result.rs
Normal file
@ -0,0 +1,35 @@
|
||||
// run-rustfix
|
||||
|
||||
#![warn(clippy::if_let_some_result)]
|
||||
|
||||
fn str_to_int(x: &str) -> i32 {
|
||||
if let Some(y) = x.parse().ok() {
|
||||
y
|
||||
} else {
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
fn str_to_int_ok(x: &str) -> i32 {
|
||||
if let Ok(y) = x.parse() {
|
||||
y
|
||||
} else {
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
#[rustfmt::skip]
|
||||
fn strange_some_no_else(x: &str) -> i32 {
|
||||
{
|
||||
if let Some(y) = x . parse() . ok () {
|
||||
return y;
|
||||
};
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let _ = str_to_int("1");
|
||||
let _ = str_to_int_ok("2");
|
||||
let _ = strange_some_no_else("3");
|
||||
}
|
25
tests/ui/if_let_some_result.stderr
Normal file
25
tests/ui/if_let_some_result.stderr
Normal file
@ -0,0 +1,25 @@
|
||||
error: Matching on `Some` with `ok()` is redundant
|
||||
--> $DIR/if_let_some_result.rs:6:5
|
||||
|
|
||||
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
|
||||
|
|
||||
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 () {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: Consider matching on `Ok(y)` and removing the call to `ok` instead
|
||||
|
|
||||
LL | if let Ok(y) = x . parse() {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
|
@ -1,23 +0,0 @@
|
||||
#![warn(clippy::if_let_some_result)]
|
||||
|
||||
fn str_to_int(x: &str) -> i32 {
|
||||
if let Some(y) = x.parse().ok() {
|
||||
y
|
||||
} else {
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
fn str_to_int_ok(x: &str) -> i32 {
|
||||
if let Ok(y) = x.parse() {
|
||||
y
|
||||
} else {
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let y = str_to_int("1");
|
||||
let z = str_to_int_ok("2");
|
||||
println!("{}{}", y, z);
|
||||
}
|
@ -1,15 +0,0 @@
|
||||
error: Matching on `Some` with `ok()` is redundant
|
||||
--> $DIR/ok_if_let.rs:4:5
|
||||
|
|
||||
LL | / if let Some(y) = x.parse().ok() {
|
||||
LL | | y
|
||||
LL | | } else {
|
||||
LL | | 0
|
||||
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
|
||||
|
||||
error: aborting due to previous error
|
||||
|
Loading…
Reference in New Issue
Block a user