Skip to content

Commit

Permalink
Removed map_err suggestion in lint, and updated lint documentation ex…
Browse files Browse the repository at this point in the history
…ample
  • Loading branch information
Ricky authored and Ricky committed Sep 2, 2020
1 parent 3377291 commit 2387f68
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 62 deletions.
117 changes: 57 additions & 60 deletions clippy_lints/src/map_err_ignore.rs
@@ -1,6 +1,6 @@
use crate::utils::span_lint_and_sugg;
use rustc_errors::Applicability;
use rustc_hir::{CaptureBy, Expr, ExprKind, PatKind, QPath};
use crate::utils::span_lint_and_help;

use rustc_hir::{CaptureBy, Expr, ExprKind, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

Expand All @@ -12,33 +12,58 @@ declare_clippy_lint! {
/// **Known problems:** None.
///
/// **Example:**
///
/// Before:
/// ```rust
/// use std::convert::TryFrom;
///
/// #[derive(Debug)]
/// enum Errors {
/// Ignore
///}
///fn main() -> Result<(), Errors> {
/// Ignored
/// }
///
/// let x = u32::try_from(-123_i32);
/// fn divisible_by_3(inp: i32) -> Result<u32, Errors> {
/// let i = u32::try_from(inp).map_err(|_| Errors::Ignored)?;
///
/// println!("{:?}", x.map_err(|_| Errors::Ignore));
/// Ok(i)
/// }
/// ```
///
/// Ok(())
///}
/// ```
/// Use instead:
/// ```rust
/// enum Errors {
/// WithContext(TryFromIntError)
///}
///fn main() -> Result<(), Errors> {
/// After:
/// ```rust
/// use std::convert::TryFrom;
/// use std::num::TryFromIntError;
/// use std::fmt;
/// use std::error::Error;
///
/// #[derive(Debug)]
/// enum ParseError {
/// Indivisible {
/// source: TryFromIntError,
/// input: String,
/// }
/// }
///
/// impl fmt::Display for ParseError {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match &self {
/// ParseError::Indivisible{source: _, input} => write!(f, "Error: {}", input)
/// }
/// }
/// }
///
/// impl Error for ParseError {}
///
/// let x = u32::try_from(-123_i32);
/// impl ParseError {
/// fn new(source: TryFromIntError, input: String) -> ParseError {
/// ParseError::Indivisible{source, input}
/// }
/// }
///
/// println!("{:?}", x.map_err(|e| Errors::WithContext(e)));
/// fn divisible_by_3(inp: i32) -> Result<u32, ParseError> {
/// let i = u32::try_from(inp).map_err(|e| ParseError::new(e, e.to_string()))?;
///
/// Ok(())
///}
/// Ok(i)
/// }
/// ```
pub MAP_ERR_IGNORE,
style,
Expand Down Expand Up @@ -69,44 +94,16 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
if closure_body.params.len() == 1 {
// make sure that parameter is the wild token (`_`)
if let PatKind::Wild = closure_body.params[0].pat.kind {
// Check the value of the closure to see if we can build the enum we are throwing away
// the error for make sure this is a Path
if let ExprKind::Path(q_path) = &closure_body.value.kind {
// this should be a resolved path, only keep the path field
if let QPath::Resolved(_, path) = q_path {
// finally get the idents for each path segment collect them as a string and
// join them with the path separator ("::"")
let closure_fold: String = path
.segments
.iter()
.map(|x| x.ident.as_str().to_string())
.collect::<Vec<String>>()
.join("::");
//Span the body of the closure (the |...| bit) and suggest the fix by taking
// the error and encapsulating it in the enum
span_lint_and_sugg(
cx,
MAP_ERR_IGNORE,
body_span,
"`map_err` has thrown away the original error",
"Allow the error enum to encapsulate the original error",
format!("|e| {}(e)", closure_fold),
Applicability::HasPlaceholders,
);
}
} else {
//If we cannot build the enum in a human readable way just suggest not throwing way
// the error
span_lint_and_sugg(
cx,
MAP_ERR_IGNORE,
body_span,
"`map_err` has thrown away the original error",
"Allow the error enum to encapsulate the original error",
"|e|".to_string(),
Applicability::HasPlaceholders,
);
}
// span the area of the closure capture and warn that the
// original error will be thrown away
span_lint_and_help(
cx,
MAP_ERR_IGNORE,
body_span,
"`map_else(|_|...` ignores the original error",
None,
"Consider wrapping the error in an enum variant",
);
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions tests/ui/map_err.stderr
@@ -1,10 +1,11 @@
error: `map_err` has thrown away the original error
error: `map_else(|_|...` ignores the original error
--> $DIR/map_err.rs:21:32
|
LL | println!("{:?}", x.map_err(|_| Errors::Ignored));
| ^^^ help: Allow the error enum to encapsulate the original error: `|e| Errors::Ignored(e)`
| ^^^
|
= note: `-D clippy::map-err-ignore` implied by `-D warnings`
= help: Consider wrapping the error in an enum variant

error: aborting due to previous error

0 comments on commit 2387f68

Please sign in to comment.