From 202a80c927412a548180eca5990ee764d76ed643 Mon Sep 17 00:00:00 2001 From: Ricky Date: Tue, 1 Sep 2020 16:59:37 -0400 Subject: [PATCH] Added tests for map_err, ignored map_err lint on drop_ref tests --- clippy_lints/src/map_err_ignore.rs | 49 ++++++++++++++++++------------ tests/ui/drop_ref.rs | 1 + tests/ui/drop_ref.stderr | 36 +++++++++++----------- tests/ui/map_err.rs | 24 +++++++++++++++ tests/ui/map_err.stderr | 10 ++++++ 5 files changed, 82 insertions(+), 38 deletions(-) create mode 100644 tests/ui/map_err.rs create mode 100644 tests/ui/map_err.stderr diff --git a/clippy_lints/src/map_err_ignore.rs b/clippy_lints/src/map_err_ignore.rs index c63c201a9f3..43bfcf0b8f1 100644 --- a/clippy_lints/src/map_err_ignore.rs +++ b/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::{Expr, ExprKind, CaptureBy, PatKind, QPath}; +use rustc_hir::{CaptureBy, Expr, ExprKind, PatKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -18,7 +18,7 @@ declare_clippy_lint! { /// Ignore ///} ///fn main() -> Result<(), Errors> { - /// + /// /// let x = u32::try_from(-123_i32); /// /// println!("{:?}", x.map_err(|_| Errors::Ignore)); @@ -32,7 +32,7 @@ declare_clippy_lint! { /// WithContext(TryFromIntError) ///} ///fn main() -> Result<(), Errors> { - /// + /// /// let x = u32::try_from(-123_i32); /// /// println!("{:?}", x.map_err(|e| Errors::WithContext(e))); @@ -48,7 +48,7 @@ declare_clippy_lint! { 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 + // 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; @@ -56,26 +56,34 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore { // check if this is a method call (e.g. x.foo()) if let ExprKind::MethodCall(ref method, _t_span, ref args, _) = 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])) + // 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.as_str() == "map_err" && args.len() == 2 { // make sure the first argument is a closure, and grab the CaptureRef, body_id, and body_span fields - if let ExprKind::Closure(capture, _, body_id, body_span, _) = args[1].kind { + if let ExprKind::Closure(capture, _, body_id, body_span, _) = args[1].kind { // check if this is by Reference (meaning there's no move statement) - if capture == CaptureBy::Ref { - // Get the closure body to check the parameters and values + if capture == CaptureBy::Ref { + // Get the closure body to check the parameters and values let closure_body = cx.tcx.hir().body(body_id); // make sure there's only one parameter (`|_|`) - if closure_body.params.len() == 1 { - // make sure that parameter is the wild token (`_`) + 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 + // 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::>().join("::"); - //Span the body of the closure (the |...| bit) and suggest the fix by taking the error and encapsulating it in the enum + // 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::>() + .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, @@ -84,10 +92,11 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore { "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 + //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, @@ -96,13 +105,13 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore { "Allow the error enum to encapsulate the original error", "|e|".to_string(), Applicability::HasPlaceholders, - ); + ); } } } - } + } } } } } -} \ No newline at end of file +} diff --git a/tests/ui/drop_ref.rs b/tests/ui/drop_ref.rs index 9181d789d4f..6b5bcdaa78e 100644 --- a/tests/ui/drop_ref.rs +++ b/tests/ui/drop_ref.rs @@ -1,5 +1,6 @@ #![warn(clippy::drop_ref)] #![allow(clippy::toplevel_ref_arg)] +#![allow(clippy::map_err_ignore)] use std::mem::drop; diff --git a/tests/ui/drop_ref.stderr b/tests/ui/drop_ref.stderr index 35ae88b78a4..7974bf56d44 100644 --- a/tests/ui/drop_ref.stderr +++ b/tests/ui/drop_ref.stderr @@ -1,108 +1,108 @@ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:9:5 + --> $DIR/drop_ref.rs:10:5 | LL | drop(&SomeStruct); | ^^^^^^^^^^^^^^^^^ | = note: `-D clippy::drop-ref` implied by `-D warnings` note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:9:10 + --> $DIR/drop_ref.rs:10:10 | LL | drop(&SomeStruct); | ^^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:12:5 + --> $DIR/drop_ref.rs:13:5 | LL | drop(&owned1); | ^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:12:10 + --> $DIR/drop_ref.rs:13:10 | LL | drop(&owned1); | ^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:13:5 + --> $DIR/drop_ref.rs:14:5 | LL | drop(&&owned1); | ^^^^^^^^^^^^^^ | note: argument has type `&&SomeStruct` - --> $DIR/drop_ref.rs:13:10 + --> $DIR/drop_ref.rs:14:10 | LL | drop(&&owned1); | ^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:14:5 + --> $DIR/drop_ref.rs:15:5 | LL | drop(&mut owned1); | ^^^^^^^^^^^^^^^^^ | note: argument has type `&mut SomeStruct` - --> $DIR/drop_ref.rs:14:10 + --> $DIR/drop_ref.rs:15:10 | LL | drop(&mut owned1); | ^^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:18:5 + --> $DIR/drop_ref.rs:19:5 | LL | drop(reference1); | ^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:18:10 + --> $DIR/drop_ref.rs:19:10 | LL | drop(reference1); | ^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:21:5 + --> $DIR/drop_ref.rs:22:5 | LL | drop(reference2); | ^^^^^^^^^^^^^^^^ | note: argument has type `&mut SomeStruct` - --> $DIR/drop_ref.rs:21:10 + --> $DIR/drop_ref.rs:22:10 | LL | drop(reference2); | ^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:24:5 + --> $DIR/drop_ref.rs:25:5 | LL | drop(reference3); | ^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:24:10 + --> $DIR/drop_ref.rs:25:10 | LL | drop(reference3); | ^^^^^^^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:29:5 + --> $DIR/drop_ref.rs:30:5 | LL | drop(&val); | ^^^^^^^^^^ | note: argument has type `&T` - --> $DIR/drop_ref.rs:29:10 + --> $DIR/drop_ref.rs:30:10 | LL | drop(&val); | ^^^^ error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. - --> $DIR/drop_ref.rs:37:5 + --> $DIR/drop_ref.rs:38:5 | LL | std::mem::drop(&SomeStruct); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: argument has type `&SomeStruct` - --> $DIR/drop_ref.rs:37:20 + --> $DIR/drop_ref.rs:38:20 | LL | std::mem::drop(&SomeStruct); | ^^^^^^^^^^^ diff --git a/tests/ui/map_err.rs b/tests/ui/map_err.rs new file mode 100644 index 00000000000..f3a74ad95cd --- /dev/null +++ b/tests/ui/map_err.rs @@ -0,0 +1,24 @@ +use std::convert::TryFrom; +use std::error::Error; +use std::fmt; + +#[derive(Debug)] +enum Errors { + Ignored, +} + +impl Error for Errors {} + +impl fmt::Display for Errors { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Error") + } +} + +fn main() -> Result<(), Errors> { + let x = u32::try_from(-123_i32); + + println!("{:?}", x.map_err(|_| Errors::Ignored)); + + Ok(()) +} diff --git a/tests/ui/map_err.stderr b/tests/ui/map_err.stderr new file mode 100644 index 00000000000..8cd37d8c025 --- /dev/null +++ b/tests/ui/map_err.stderr @@ -0,0 +1,10 @@ +error: `map_err` has thrown away 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` + +error: aborting due to previous error +