Skip to content

Commit

Permalink
Support suggesting panics
Browse files Browse the repository at this point in the history
  • Loading branch information
@pksunkara
pksunkara committed Jun 12, 2023
1 parent 69af0e1 commit 6e4dc93
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 61 deletions.
16 changes: 8 additions & 8 deletions clippy_lints/src/methods/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3664,10 +3664,10 @@ impl Methods {
Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, span, err_span, &self.msrv),
_ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
}
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
},
("expect_err", [_]) => {
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests);
},
("extend", [arg]) => {
Expand Down Expand Up @@ -3874,12 +3874,12 @@ impl Methods {
},
_ => {},
}
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests);
},
("unwrap_err", []) => {
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests)
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests);
},
("unwrap_or", [u_arg]) => {
match method_call(recv) {
Expand All @@ -3894,10 +3894,10 @@ impl Methods {
},
_ => {},
}
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
},
("unwrap_or_default", []) => {
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
}
("unwrap_or_else", [u_arg]) => {
match method_call(recv) {
Expand All @@ -3908,7 +3908,7 @@ impl Methods {
unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or");
},
}
unnecessary_literal_unwrap::check(cx, expr, recv, name);
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
},
("zip", [arg]) => {
if let ExprKind::MethodCall(name, iter_recv, [], _) = recv.kind
Expand Down
91 changes: 55 additions & 36 deletions clippy_lints/src/methods/unnecessary_literal_unwrap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,49 +5,68 @@ use rustc_lint::LateContext;

use super::UNNECESSARY_LITERAL_UNWRAP;

pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, name: &str) {
pub(super) fn check(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
method: &str,
args: &[hir::Expr<'_>],
) {
let init = clippy_utils::expr_or_init(cx, recv);

if let hir::ExprKind::Call(call, [arg]) = init.kind {
let constructor = if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) {
"Some"
let (constructor, call_args) = if let hir::ExprKind::Call(call, call_args) = init.kind {
if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) {
("Some", call_args)
} else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultOk) {
"Ok"
("Ok", call_args)
} else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultErr) {
"Err"
("Err", call_args)
} else {
return;
};
}
} else if is_res_lang_ctor(cx, path_res(cx, init), hir::LangItem::OptionNone) {
let call_args: &[hir::Expr<'_>] = &[];
("None", call_args)
} else {
return;
};

if init.span == recv.span {
span_lint_and_then(
cx,
UNNECESSARY_LITERAL_UNWRAP,
expr.span,
&format!("used `{name}()` on `{constructor}` value"),
|diag| {
let suggestions = vec![
(recv.span.with_hi(arg.span.lo()), String::new()),
(expr.span.with_lo(arg.span.hi()), String::new()),
];
let help_message = format!("used `{method}()` on `{constructor}` value");
let suggestion_message = format!("remove the `{constructor}` and `{method}()`");

diag.multipart_suggestion(
format!("remove the `{constructor}` and `{name}()`"),
suggestions,
Applicability::MachineApplicable,
);
},
);
} else {
span_lint_and_then(
cx,
UNNECESSARY_LITERAL_UNWRAP,
expr.span,
&format!("used `{name}()` on `{constructor}` value"),
|diag| {
diag.span_help(init.span, format!("remove the `{constructor}` and `{name}()`"));
},
);
}
if init.span == recv.span {
span_lint_and_then(cx, UNNECESSARY_LITERAL_UNWRAP, expr.span, &help_message, |diag| {
let suggestions = match (constructor, method) {
("None", "unwrap") => vec![(expr.span, "panic!()".to_string())],
("None", "expect") => vec![
(expr.span.with_hi(args[0].span.lo()), "panic!(".to_string()),
(expr.span.with_lo(args[0].span.hi()), ")".to_string()),
],
("Ok", "unwrap_err") | ("Err", "unwrap") => vec![
(
recv.span.with_hi(call_args[0].span.lo()),
"panic!(\"{:?}\", ".to_string(),
),
(expr.span.with_lo(call_args[0].span.hi()), ")".to_string()),
],
("Ok", "expect_err") | ("Err", "expect") => vec![
(
recv.span.with_hi(call_args[0].span.lo()),
"panic!(\"{1}: {:?}\", ".to_string(),
),
(call_args[0].span.with_lo(args[0].span.lo()), ", ".to_string()),
],
_ => vec![
(recv.span.with_hi(call_args[0].span.lo()), String::new()),
(expr.span.with_lo(call_args[0].span.hi()), String::new()),
],
};

diag.multipart_suggestion(suggestion_message, suggestions, Applicability::MachineApplicable);
});
} else {
span_lint_and_then(cx, UNNECESSARY_LITERAL_UNWRAP, expr.span, &help_message, |diag| {
diag.span_help(init.span, suggestion_message);
});
}
}
15 changes: 13 additions & 2 deletions tests/ui/unnecessary_literal_unwrap.fixed
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
//run-rustfix
#![warn(clippy::unnecessary_literal_unwrap)]
#![allow(clippy::unnecessary_lazy_evaluations)]
#![allow(unreachable_code)]

fn unwrap_option() {
fn unwrap_option_some() {
let _val = 1;
let _val = 1;
}

fn unwrap_option_none() {
panic!();
panic!("this always happens");
}

fn unwrap_result_ok() {
let _val = 1;
let _val = 1;
panic!("{:?}", 1);
panic!("{1}: {:?}", 1, "this always happens");
}

fn unwrap_result_err() {
let _val = 1;
let _val = 1;
panic!("{:?}", 1);
panic!("{1}: {:?}", 1, "this always happens");
}

fn unwrap_methods_option() {
Expand All @@ -30,7 +40,8 @@ fn unwrap_methods_result() {
}

fn main() {
unwrap_option();
unwrap_option_some();
unwrap_option_none();
unwrap_result_ok();
unwrap_result_err();
unwrap_methods_option();
Expand Down
15 changes: 13 additions & 2 deletions tests/ui/unnecessary_literal_unwrap.rs
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
//run-rustfix
#![warn(clippy::unnecessary_literal_unwrap)]
#![allow(clippy::unnecessary_lazy_evaluations)]
#![allow(unreachable_code)]

fn unwrap_option() {
fn unwrap_option_some() {
let _val = Some(1).unwrap();
let _val = Some(1).expect("this never happens");
}

fn unwrap_option_none() {
None::<usize>.unwrap();
None::<usize>.expect("this always happens");
}

fn unwrap_result_ok() {
let _val = Ok::<usize, ()>(1).unwrap();
let _val = Ok::<usize, ()>(1).expect("this never happens");
Ok::<usize, ()>(1).unwrap_err();
Ok::<usize, ()>(1).expect_err("this always happens");
}

fn unwrap_result_err() {
let _val = Err::<(), usize>(1).unwrap_err();
let _val = Err::<(), usize>(1).expect_err("this never happens");
Err::<(), usize>(1).unwrap();
Err::<(), usize>(1).expect("this always happens");
}

fn unwrap_methods_option() {
Expand All @@ -30,7 +40,8 @@ fn unwrap_methods_result() {
}

fn main() {
unwrap_option();
unwrap_option_some();
unwrap_option_none();
unwrap_result_ok();
unwrap_result_err();
unwrap_methods_option();
Expand Down
Loading

0 comments on commit 6e4dc93

Please sign in to comment.