mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 15:14:29 +00:00
Add new lint to prevent usage of unwrap in fns that return result
This commit is contained in:
parent
e45c59e922
commit
91024f1fde
6 changed files with 237 additions and 0 deletions
|
@ -1778,6 +1778,7 @@ Released 2018-09-13
|
|||
[`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label
|
||||
[`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self
|
||||
[`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit
|
||||
[`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result
|
||||
[`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used
|
||||
[`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
|
||||
[`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
|
||||
|
|
|
@ -314,6 +314,7 @@ mod unused_io_amount;
|
|||
mod unused_self;
|
||||
mod unused_unit;
|
||||
mod unwrap;
|
||||
mod unwrap_in_result;
|
||||
mod use_self;
|
||||
mod useless_conversion;
|
||||
mod vec;
|
||||
|
@ -850,6 +851,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
&unused_unit::UNUSED_UNIT,
|
||||
&unwrap::PANICKING_UNWRAP,
|
||||
&unwrap::UNNECESSARY_UNWRAP,
|
||||
&unwrap_in_result::UNWRAP_IN_RESULT,
|
||||
&use_self::USE_SELF,
|
||||
&useless_conversion::USELESS_CONVERSION,
|
||||
&utils::internal_lints::CLIPPY_LINTS_INTERNAL,
|
||||
|
@ -1094,6 +1096,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(|| box pattern_type_mismatch::PatternTypeMismatch);
|
||||
store.register_late_pass(|| box stable_sort_primitive::StableSortPrimitive);
|
||||
store.register_late_pass(|| box repeat_once::RepeatOnce);
|
||||
store.register_late_pass(|| box unwrap_in_result::UnwrapInResult);
|
||||
store.register_late_pass(|| box self_assignment::SelfAssignment);
|
||||
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
|
||||
|
||||
|
@ -1133,6 +1136,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&shadow::SHADOW_REUSE),
|
||||
LintId::of(&shadow::SHADOW_SAME),
|
||||
LintId::of(&strings::STRING_ADD),
|
||||
LintId::of(&unwrap_in_result::UNWRAP_IN_RESULT),
|
||||
LintId::of(&verbose_file_reads::VERBOSE_FILE_READS),
|
||||
LintId::of(&write::PRINT_STDOUT),
|
||||
LintId::of(&write::USE_DEBUG),
|
||||
|
|
140
clippy_lints/src/unwrap_in_result.rs
Normal file
140
clippy_lints/src/unwrap_in_result.rs
Normal file
|
@ -0,0 +1,140 @@
|
|||
use crate::utils::{is_type_diagnostic_item, method_chain_args, return_ty, span_lint_and_then, walk_ptrs_ty};
|
||||
use if_chain::if_chain;
|
||||
use rustc_hir as hir;
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::hir::map::Map;
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for functions of type Result that contain `expect()` or `unwrap()`
|
||||
///
|
||||
/// **Why is this bad?** These functions promote recoverable errors to non-recoverable errors which may be undesirable in code bases which wish to avoid panics.
|
||||
///
|
||||
/// **Known problems:** This can cause false positives in functions that handle both recoverable and non recoverable errors.
|
||||
///
|
||||
/// **Example:**
|
||||
/// Before:
|
||||
/// ```rust
|
||||
/// fn divisible_by_3(i_str: String) -> Result<(), String> {
|
||||
/// let i = i_str
|
||||
/// .parse::<i32>()
|
||||
/// .expect("cannot divide the input by three");
|
||||
///
|
||||
/// if i % 3 != 0 {
|
||||
/// Err("Number is not divisible by 3")?
|
||||
/// }
|
||||
///
|
||||
/// Ok(())
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// After:
|
||||
/// ```rust
|
||||
/// fn divisible_by_3(i_str: String) -> Result<(), String> {
|
||||
/// let i = i_str
|
||||
/// .parse::<i32>()
|
||||
/// .map_err(|e| format!("cannot divide the input by three: {}", e))?;
|
||||
///
|
||||
/// if i % 3 != 0 {
|
||||
/// Err("Number is not divisible by 3")?
|
||||
/// }
|
||||
///
|
||||
/// Ok(())
|
||||
/// }
|
||||
/// ```
|
||||
pub UNWRAP_IN_RESULT,
|
||||
restriction,
|
||||
"functions of type `Result<..>` or `Option`<...> that contain `expect()` or `unwrap()`"
|
||||
}
|
||||
|
||||
declare_lint_pass!(UnwrapInResult=> [UNWRAP_IN_RESULT]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for UnwrapInResult {
|
||||
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::ImplItem<'_>) {
|
||||
if_chain! {
|
||||
// first check if it's a method or function
|
||||
if let hir::ImplItemKind::Fn(ref _signature, _) = impl_item.kind;
|
||||
// checking if its return type is `result` or `option`
|
||||
if is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(result_type))
|
||||
|| is_type_diagnostic_item(cx, return_ty(cx, impl_item.hir_id), sym!(option_type));
|
||||
then {
|
||||
lint_impl_body(cx, impl_item.span, impl_item);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
|
||||
use rustc_hir::{Expr, ImplItemKind};
|
||||
|
||||
struct FindExpectUnwrap<'a, 'tcx> {
|
||||
lcx: &'a LateContext<'tcx>,
|
||||
typeck_results: &'tcx ty::TypeckResults<'tcx>,
|
||||
result: Vec<Span>,
|
||||
}
|
||||
|
||||
impl<'a, 'tcx> Visitor<'tcx> for FindExpectUnwrap<'a, 'tcx> {
|
||||
type Map = Map<'tcx>;
|
||||
|
||||
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
|
||||
// check for `expect`
|
||||
if let Some(arglists) = method_chain_args(expr, &["expect"]) {
|
||||
let reciever_ty = walk_ptrs_ty(self.typeck_results.expr_ty(&arglists[0][0]));
|
||||
if is_type_diagnostic_item(self.lcx, reciever_ty, sym!(option_type))
|
||||
|| is_type_diagnostic_item(self.lcx, reciever_ty, sym!(result_type))
|
||||
{
|
||||
self.result.push(expr.span);
|
||||
}
|
||||
}
|
||||
|
||||
// check for `unwrap`
|
||||
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
|
||||
let reciever_ty = walk_ptrs_ty(self.typeck_results.expr_ty(&arglists[0][0]));
|
||||
if is_type_diagnostic_item(self.lcx, reciever_ty, sym!(option_type))
|
||||
|| is_type_diagnostic_item(self.lcx, reciever_ty, sym!(result_type))
|
||||
{
|
||||
self.result.push(expr.span);
|
||||
}
|
||||
}
|
||||
|
||||
// and check sub-expressions
|
||||
intravisit::walk_expr(self, expr);
|
||||
}
|
||||
|
||||
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
|
||||
NestedVisitorMap::None
|
||||
}
|
||||
}
|
||||
|
||||
fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) {
|
||||
if_chain! {
|
||||
|
||||
if let ImplItemKind::Fn(_, body_id) = impl_item.kind;
|
||||
then {
|
||||
let body = cx.tcx.hir().body(body_id);
|
||||
let impl_item_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
|
||||
let mut fpu = FindExpectUnwrap {
|
||||
lcx: cx,
|
||||
typeck_results: cx.tcx.typeck(impl_item_def_id),
|
||||
result: Vec::new(),
|
||||
};
|
||||
fpu.visit_expr(&body.value);
|
||||
|
||||
// if we've found one, lint
|
||||
if !fpu.result.is_empty() {
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
UNWRAP_IN_RESULT,
|
||||
impl_span,
|
||||
"used unwrap or expect in a function that returns result or option",
|
||||
move |diag| {
|
||||
diag.help(
|
||||
"unwrap and expect should not be used in a function that returns result or option" );
|
||||
diag.span_note(fpu.result, "potential non-recoverable error(s)");
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
|
@ -2516,6 +2516,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
|
|||
deprecation: None,
|
||||
module: "unused_unit",
|
||||
},
|
||||
Lint {
|
||||
name: "unwrap_in_result",
|
||||
group: "restriction",
|
||||
desc: "functions of type `Result<..>` or `Option`<...> that contain `expect()` or `unwrap()`",
|
||||
deprecation: None,
|
||||
module: "unwrap_in_result",
|
||||
},
|
||||
Lint {
|
||||
name: "unwrap_used",
|
||||
group: "restriction",
|
||||
|
|
44
tests/ui/unwrap_in_result.rs
Normal file
44
tests/ui/unwrap_in_result.rs
Normal file
|
@ -0,0 +1,44 @@
|
|||
#![warn(clippy::unwrap_in_result)]
|
||||
|
||||
struct A;
|
||||
|
||||
impl A {
|
||||
// should not be detected
|
||||
fn good_divisible_by_3(i_str: String) -> Result<bool, String> {
|
||||
// checks whether a string represents a number divisible by 3
|
||||
let i_result = i_str.parse::<i32>();
|
||||
match i_result {
|
||||
Err(_e) => Err("Not a number".to_string()),
|
||||
Ok(i) => {
|
||||
if i % 3 == 0 {
|
||||
return Ok(true);
|
||||
}
|
||||
Err("Number is not divisible by 3".to_string())
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// should be detected
|
||||
fn bad_divisible_by_3(i_str: String) -> Result<bool, String> {
|
||||
// checks whether a string represents a number divisible by 3
|
||||
let i = i_str.parse::<i32>().unwrap();
|
||||
if i % 3 == 0 {
|
||||
Ok(true)
|
||||
} else {
|
||||
Err("Number is not divisible by 3".to_string())
|
||||
}
|
||||
}
|
||||
|
||||
fn example_option_expect(i_str: String) -> Option<bool> {
|
||||
let i = i_str.parse::<i32>().expect("not a number");
|
||||
if i % 3 == 0 {
|
||||
return Some(true);
|
||||
}
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
A::bad_divisible_by_3("3".to_string());
|
||||
A::good_divisible_by_3("3".to_string());
|
||||
}
|
41
tests/ui/unwrap_in_result.stderr
Normal file
41
tests/ui/unwrap_in_result.stderr
Normal file
|
@ -0,0 +1,41 @@
|
|||
error: used unwrap or expect in a function that returns result or option
|
||||
--> $DIR/unwrap_in_result.rs:22:5
|
||||
|
|
||||
LL | / fn bad_divisible_by_3(i_str: String) -> Result<bool, String> {
|
||||
LL | | // checks whether a string represents a number divisible by 3
|
||||
LL | | let i = i_str.parse::<i32>().unwrap();
|
||||
LL | | if i % 3 == 0 {
|
||||
... |
|
||||
LL | | }
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
= note: `-D clippy::unwrap-in-result` implied by `-D warnings`
|
||||
= help: unwrap and expect should not be used in a function that returns result or option
|
||||
note: potential non-recoverable error(s)
|
||||
--> $DIR/unwrap_in_result.rs:24:17
|
||||
|
|
||||
LL | let i = i_str.parse::<i32>().unwrap();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: used unwrap or expect in a function that returns result or option
|
||||
--> $DIR/unwrap_in_result.rs:32:5
|
||||
|
|
||||
LL | / fn example_option_expect(i_str: String) -> Option<bool> {
|
||||
LL | | let i = i_str.parse::<i32>().expect("not a number");
|
||||
LL | | if i % 3 == 0 {
|
||||
LL | | return Some(true);
|
||||
LL | | }
|
||||
LL | | None
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
= help: unwrap and expect should not be used in a function that returns result or option
|
||||
note: potential non-recoverable error(s)
|
||||
--> $DIR/unwrap_in_result.rs:33:17
|
||||
|
|
||||
LL | let i = i_str.parse::<i32>().expect("not a number");
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
|
Loading…
Reference in a new issue