nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-28 09:44:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move MapErrIgnore into Methods lint pass

Browse Source
This commit is contained in:
Jason Newcomb 2022-06-05 15:59:24 -04:00
parent 452395485b
commit 2f0ed0a0b1
6 changed files with 140 additions and 158 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
clippy_lints/src/lib.register_lints.rs
View File

@ -246,7 +246,6 @@ store.register_lints(&[
manual_rem_euclid::MANUAL_REM_EUCLID,
manual_retain::MANUAL_RETAIN,
manual_strip::MANUAL_STRIP,
map_err_ignore::MAP_ERR_IGNORE,
map_unit_fn::OPTION_MAP_UNIT_FN,
map_unit_fn::RESULT_MAP_UNIT_FN,
match_result_ok::MATCH_RESULT_OK,
@ -326,6 +325,7 @@ store.register_lints(&[
methods::MANUAL_STR_REPEAT,
methods::MAP_CLONE,
methods::MAP_COLLECT_RESULT_UNIT,
methods::MAP_ERR_IGNORE,
methods::MAP_FLATTEN,
methods::MAP_IDENTITY,
methods::MAP_UNWRAP_OR,

2
clippy_lints/src/lib.register_restriction.rs
View File

@ -30,7 +30,6 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(large_include_file::LARGE_INCLUDE_FILE),
LintId::of(let_underscore::LET_UNDERSCORE_MUST_USE),
LintId::of(literal_representation::DECIMAL_LITERAL_REPRESENTATION),
LintId::of(map_err_ignore::MAP_ERR_IGNORE),
LintId::of(matches::REST_PAT_IN_FULLY_BOUND_STRUCTS),
LintId::of(matches::TRY_ERR),
LintId::of(matches::WILDCARD_ENUM_MATCH_ARM),
@ -39,6 +38,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(methods::EXPECT_USED),
LintId::of(methods::FILETYPE_IS_FILE),
LintId::of(methods::GET_UNWRAP),
LintId::of(methods::MAP_ERR_IGNORE),
LintId::of(methods::UNWRAP_USED),
LintId::of(misc_early::SEPARATED_LITERAL_SUFFIX),
LintId::of(misc_early::UNNEEDED_FIELD_PATTERN),

2
clippy_lints/src/lib.rs
View File

@ -273,7 +273,6 @@ mod manual_non_exhaustive;
mod manual_rem_euclid;
mod manual_retain;
mod manual_strip;
mod map_err_ignore;
mod map_unit_fn;
mod match_result_ok;
mod matches;
@ -636,7 +635,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
msrv,
))
});
store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore));
store.register_late_pass(|| Box::new(shadow::Shadow::default()));
store.register_late_pass(|| Box::new(unit_types::UnitTypes));
store.register_late_pass(|| Box::new(loops::Loops));

154
clippy_lints/src/map_err_ignore.rs
View File

@ -1,154 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_hir::{CaptureBy, Closure, Expr, ExprKind, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
declare_clippy_lint! {
/// ### What it does
/// Checks for instances of `map_err(|_| Some::Enum)`
///
/// ### Why is this bad?
/// This `map_err` throws away the original error rather than allowing the enum to contain and report the cause of the error
///
/// ### Example
/// Before:
/// ```rust
/// use std::fmt;
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible,
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {}
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(|_| Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
///
/// After:
/// ```rust
/// use std::{fmt, num::ParseIntError};
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible(ParseIntError),
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible(_) => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {
/// fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
/// match self {
/// Error::Indivisible(source) => Some(source),
/// _ => None,
/// }
/// }
/// }
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
#[clippy::version = "1.48.0"]
pub MAP_ERR_IGNORE,
restriction,
"`map_err` should not ignore the original error"
}
declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]);
impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
// do not try to lint if this is from a macro or desugaring
fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if e.span.from_expansion() {
return;
}
// check if this is a method call (e.g. x.foo())
if let ExprKind::MethodCall(method, [_, arg], _) = e.kind {
// only work if the method name is `map_err` and there are only 2 arguments (e.g. x.map_err(|_|[1]
// Enum::Variant[2]))
if method.ident.name == sym!(map_err) {
// make sure the first argument is a closure, and grab the CaptureRef, BodyId, and fn_decl_span
// fields
if let ExprKind::Closure(&Closure {
capture_clause,
body,
fn_decl_span,
..
}) = arg.kind
{
// check if this is by Reference (meaning there's no move statement)
if capture_clause == CaptureBy::Ref {
// Get the closure body to check the parameters and values
let closure_body = cx.tcx.hir().body(body);
// make sure there's only one parameter (`|_|`)
if closure_body.params.len() == 1 {
// make sure that parameter is the wild token (`_`)
if let PatKind::Wild = closure_body.params[0].pat.kind {
// 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,
fn_decl_span,
"`map_err(|_|...` wildcard pattern discards the original error",
None,
"consider storing the original error as a source in the new error, or silence this warning using an ignored identifier (`.map_err(|_foo| ...`)",
);
}
}
}
}
}
}
}
}

34
clippy_lints/src/methods/map_err_ignore.rs Normal file
View File

@ -0,0 +1,34 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use rustc_hir::{CaptureBy, Closure, Expr, ExprKind, PatKind};
use rustc_lint::LateContext;
use rustc_span::sym;
use super::MAP_ERR_IGNORE;
pub(super) fn check<'tcx>(cx: &LateContext<'_>, e: &Expr<'_>, arg: &'tcx Expr<'_>) {
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(e.hir_id)
&& let Some(impl_id) = cx.tcx.impl_of_method(method_id)
&& is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id), sym::Result)
&& let ExprKind::Closure(&Closure {
capture_clause: CaptureBy::Ref,
body,
fn_decl_span,
..
}) = arg.kind
&& let closure_body = cx.tcx.hir().body(body)
&& let [param] = closure_body.params
&& let PatKind::Wild = param.pat.kind
{
// 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,
fn_decl_span,
"`map_err(|_|...` wildcard pattern discards the original error",
None,
"consider storing the original error as a source in the new error, or silence this warning using an ignored identifier (`.map_err(|_foo| ...`)",
);
}
}

104
clippy_lints/src/methods/mod.rs
View File

@ -47,6 +47,7 @@ mod manual_saturating_arithmetic;
mod manual_str_repeat;
mod map_clone;
mod map_collect_result_unit;
mod map_err_ignore;
mod map_flatten;
mod map_identity;
mod map_unwrap_or;
@ -2541,6 +2542,106 @@ declare_clippy_lint! {
"using `iterator.map(|x| x.clone())`, or dereferencing closures for `Copy` types"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for instances of `map_err(|_| Some::Enum)`
///
/// ### Why is this bad?
/// This `map_err` throws away the original error rather than allowing the enum to contain and report the cause of the error
///
/// ### Example
/// Before:
/// ```rust
/// use std::fmt;
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible,
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {}
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(|_| Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
///
/// After:
/// ```rust
/// use std::{fmt, num::ParseIntError};
///
/// #[derive(Debug)]
/// enum Error {
/// Indivisible(ParseIntError),
/// Remainder(u8),
/// }
///
/// impl fmt::Display for Error {
/// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// match self {
/// Error::Indivisible(_) => write!(f, "could not divide input by three"),
/// Error::Remainder(remainder) => write!(
/// f,
/// "input is not divisible by three, remainder = {}",
/// remainder
/// ),
/// }
/// }
/// }
///
/// impl std::error::Error for Error {
/// fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
/// match self {
/// Error::Indivisible(source) => Some(source),
/// _ => None,
/// }
/// }
/// }
///
/// fn divisible_by_3(input: &str) -> Result<(), Error> {
/// input
/// .parse::<i32>()
/// .map_err(Error::Indivisible)
/// .map(|v| v % 3)
/// .and_then(|remainder| {
/// if remainder == 0 {
/// Ok(())
/// } else {
/// Err(Error::Remainder(remainder as u8))
/// }
/// })
/// }
/// ```
#[clippy::version = "1.48.0"]
pub MAP_ERR_IGNORE,
restriction,
"`map_err` should not ignore the original error"
}
pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Option<RustcVersion>,
@ -2651,6 +2752,7 @@ impl_lint_pass!(Methods => [
GET_FIRST,
MANUAL_OK_OR,
MAP_CLONE,
MAP_ERR_IGNORE,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -2982,6 +3084,8 @@ impl Methods {
(name @ ("map" | "map_err"), [m_arg]) => {
if name == "map" {
map_clone::check(cx, expr, recv, m_arg, self.msrv);
} else {
map_err_ignore::check(cx, expr, m_arg);
}
if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) {
match (name, args) {