Added tests for map_err, ignored map_err lint on drop_ref tests

This commit is contained in:
Ricky 2020-09-01 16:59:37 -04:00
parent e49a29933b
commit 202a80c927
5 changed files with 82 additions and 38 deletions

View file

@ -1,6 +1,6 @@
use crate::utils::span_lint_and_sugg; use crate::utils::span_lint_and_sugg;
use rustc_errors::Applicability; 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_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -18,7 +18,7 @@ declare_clippy_lint! {
/// Ignore /// Ignore
///} ///}
///fn main() -> Result<(), Errors> { ///fn main() -> Result<(), Errors> {
/// ///
/// let x = u32::try_from(-123_i32); /// let x = u32::try_from(-123_i32);
/// ///
/// println!("{:?}", x.map_err(|_| Errors::Ignore)); /// println!("{:?}", x.map_err(|_| Errors::Ignore));
@ -32,7 +32,7 @@ declare_clippy_lint! {
/// WithContext(TryFromIntError) /// WithContext(TryFromIntError)
///} ///}
///fn main() -> Result<(), Errors> { ///fn main() -> Result<(), Errors> {
/// ///
/// let x = u32::try_from(-123_i32); /// let x = u32::try_from(-123_i32);
/// ///
/// println!("{:?}", x.map_err(|e| Errors::WithContext(e))); /// println!("{:?}", x.map_err(|e| Errors::WithContext(e)));
@ -48,7 +48,7 @@ declare_clippy_lint! {
declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]); declare_lint_pass!(MapErrIgnore => [MAP_ERR_IGNORE]);
impl<'tcx> LateLintPass<'tcx> for MapErrIgnore { 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<'_>) { fn check_expr(&mut self, cx: &LateContext<'_>, e: &Expr<'_>) {
if e.span.from_expansion() { if e.span.from_expansion() {
return; return;
@ -56,26 +56,34 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
// check if this is a method call (e.g. x.foo()) // check if this is a method call (e.g. x.foo())
if let ExprKind::MethodCall(ref method, _t_span, ref args, _) = e.kind { 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 { 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 // 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) // check if this is by Reference (meaning there's no move statement)
if capture == CaptureBy::Ref { if capture == CaptureBy::Ref {
// Get the closure body to check the parameters and values // Get the closure body to check the parameters and values
let closure_body = cx.tcx.hir().body(body_id); let closure_body = cx.tcx.hir().body(body_id);
// make sure there's only one parameter (`|_|`) // make sure there's only one parameter (`|_|`)
if closure_body.params.len() == 1 { if closure_body.params.len() == 1 {
// make sure that parameter is the wild token (`_`) // make sure that parameter is the wild token (`_`)
if let PatKind::Wild = closure_body.params[0].pat.kind { 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 // Check the value of the closure to see if we can build the enum we are throwing away
// make sure this is a Path // the error for make sure this is a Path
if let ExprKind::Path(q_path) = &closure_body.value.kind { if let ExprKind::Path(q_path) = &closure_body.value.kind {
// this should be a resolved path, only keep the path field // this should be a resolved path, only keep the path field
if let QPath::Resolved(_, path) = q_path { 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 ("::"") // finally get the idents for each path segment collect them as a string and
let closure_fold: String = path.segments.iter().map(|x| x.ident.as_str().to_string()).collect::<Vec<String>>().join("::"); // join them with the path separator ("::"")
//Span the body of the closure (the |...| bit) and suggest the fix by taking the error and encapsulating it in the enum 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( span_lint_and_sugg(
cx, cx,
MAP_ERR_IGNORE, MAP_ERR_IGNORE,
@ -84,10 +92,11 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
"Allow the error enum to encapsulate the original error", "Allow the error enum to encapsulate the original error",
format!("|e| {}(e)", closure_fold), format!("|e| {}(e)", closure_fold),
Applicability::HasPlaceholders, Applicability::HasPlaceholders,
); );
} }
} else { } 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( span_lint_and_sugg(
cx, cx,
MAP_ERR_IGNORE, MAP_ERR_IGNORE,
@ -96,13 +105,13 @@ impl<'tcx> LateLintPass<'tcx> for MapErrIgnore {
"Allow the error enum to encapsulate the original error", "Allow the error enum to encapsulate the original error",
"|e|".to_string(), "|e|".to_string(),
Applicability::HasPlaceholders, Applicability::HasPlaceholders,
); );
} }
} }
} }
} }
} }
} }
} }
} }
} }

View file

@ -1,5 +1,6 @@
#![warn(clippy::drop_ref)] #![warn(clippy::drop_ref)]
#![allow(clippy::toplevel_ref_arg)] #![allow(clippy::toplevel_ref_arg)]
#![allow(clippy::map_err_ignore)]
use std::mem::drop; use std::mem::drop;

View file

@ -1,108 +1,108 @@
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(&SomeStruct);
| ^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^
| |
= note: `-D clippy::drop-ref` implied by `-D warnings` = note: `-D clippy::drop-ref` implied by `-D warnings`
note: argument has type `&SomeStruct` note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:9:10 --> $DIR/drop_ref.rs:10:10
| |
LL | drop(&SomeStruct); LL | drop(&SomeStruct);
| ^^^^^^^^^^^ | ^^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(&owned1);
| ^^^^^^^^^^^^^ | ^^^^^^^^^^^^^
| |
note: argument has type `&SomeStruct` note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:12:10 --> $DIR/drop_ref.rs:13:10
| |
LL | drop(&owned1); LL | drop(&owned1);
| ^^^^^^^ | ^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(&&owned1);
| ^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^
| |
note: argument has type `&&SomeStruct` note: argument has type `&&SomeStruct`
--> $DIR/drop_ref.rs:13:10 --> $DIR/drop_ref.rs:14:10
| |
LL | drop(&&owned1); LL | drop(&&owned1);
| ^^^^^^^^ | ^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(&mut owned1);
| ^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^
| |
note: argument has type `&mut SomeStruct` note: argument has type `&mut SomeStruct`
--> $DIR/drop_ref.rs:14:10 --> $DIR/drop_ref.rs:15:10
| |
LL | drop(&mut owned1); LL | drop(&mut owned1);
| ^^^^^^^^^^^ | ^^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(reference1);
| ^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^
| |
note: argument has type `&SomeStruct` note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:18:10 --> $DIR/drop_ref.rs:19:10
| |
LL | drop(reference1); LL | drop(reference1);
| ^^^^^^^^^^ | ^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(reference2);
| ^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^
| |
note: argument has type `&mut SomeStruct` note: argument has type `&mut SomeStruct`
--> $DIR/drop_ref.rs:21:10 --> $DIR/drop_ref.rs:22:10
| |
LL | drop(reference2); LL | drop(reference2);
| ^^^^^^^^^^ | ^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(reference3);
| ^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^
| |
note: argument has type `&SomeStruct` note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:24:10 --> $DIR/drop_ref.rs:25:10
| |
LL | drop(reference3); LL | drop(reference3);
| ^^^^^^^^^^ | ^^^^^^^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | drop(&val);
| ^^^^^^^^^^ | ^^^^^^^^^^
| |
note: argument has type `&T` note: argument has type `&T`
--> $DIR/drop_ref.rs:29:10 --> $DIR/drop_ref.rs:30:10
| |
LL | drop(&val); LL | drop(&val);
| ^^^^ | ^^^^
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing. 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); LL | std::mem::drop(&SomeStruct);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
note: argument has type `&SomeStruct` note: argument has type `&SomeStruct`
--> $DIR/drop_ref.rs:37:20 --> $DIR/drop_ref.rs:38:20
| |
LL | std::mem::drop(&SomeStruct); LL | std::mem::drop(&SomeStruct);
| ^^^^^^^^^^^ | ^^^^^^^^^^^

24
tests/ui/map_err.rs Normal file
View file

@ -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(())
}

10
tests/ui/map_err.stderr Normal file
View file

@ -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