Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jfrikker committed Jun 23, 2019
1 parent 60a8084 commit 1e6c697
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 23 deletions.
31 changes: 13 additions & 18 deletions clippy_lints/src/try_err.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
use crate::utils::{match_qpath, paths, snippet, span_lint_and_sugg};
use if_chain::if_chain;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
Expand All @@ -17,17 +17,17 @@ declare_clippy_lint! {
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust,ignore
/// // Bad
/// ```rust
/// fn foo(fail: bool) -> Result<i32, String> {
/// if fail {
/// Err("failed")?;
/// }
/// Ok(0)
/// }
/// ```
/// Could be written:
///
/// // Good
/// ```rust
/// fn foo(fail: bool) -> Result<i32, String> {
/// if fail {
/// return Err("failed".into());
Expand Down Expand Up @@ -57,7 +57,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr {
if let ExprKind::Match(ref match_arg, _, MatchSource::TryDesugar) = expr.node;
if let ExprKind::Call(ref match_fun, ref try_args) = match_arg.node;
if let ExprKind::Path(ref match_fun_path) = match_fun.node;
if match_qpath(match_fun_path, &["std", "ops", "Try", "into_result"]);
if match_qpath(match_fun_path, &paths::TRY_INTO_RESULT);
if let Some(ref try_arg) = try_args.get(0);
if let ExprKind::Call(ref err_fun, ref err_args) = try_arg.node;
if let Some(ref err_arg) = err_args.get(0);
Expand All @@ -73,19 +73,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr {
format!("return Err({}.into())", snippet(cx, err_arg.span, "_"))
};

span_lint_and_then(
span_lint_and_sugg(
cx,
TRY_ERR,
expr.span,
&format!("confusing error return, consider using `{}`", suggestion),
|db| {
db.span_suggestion(
expr.span,
"try this",
suggestion,
Applicability::MaybeIncorrect
);
},
"returning an `Err(_)` with the `?` operator",
"try this",
suggestion,
Applicability::MaybeIncorrect
);
}
}
Expand All @@ -97,7 +92,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr {
// its output type.
fn find_err_return_type<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx ExprKind) -> Option<Ty<'tcx>> {
if let ExprKind::Match(_, ref arms, MatchSource::TryDesugar) = expr {
arms.iter().filter_map(|ty| find_err_return_type_arm(cx, ty)).nth(0)
arms.iter().find_map(|ty| find_err_return_type_arm(cx, ty))
} else {
None
}
Expand All @@ -109,7 +104,7 @@ fn find_err_return_type_arm<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arm: &'tcx Arm
if let ExprKind::Ret(Some(ref err_ret)) = arm.body.node;
if let ExprKind::Call(ref from_error_path, ref from_error_args) = err_ret.node;
if let ExprKind::Path(ref from_error_fn) = from_error_path.node;
if match_qpath(from_error_fn, &["std", "ops", "Try", "from_error"]);
if match_qpath(from_error_fn, &paths::TRY_FROM_ERROR);
if let Some(from_error_arg) = from_error_args.get(0);
then {
Some(cx.tables.expr_ty(from_error_arg))
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"
pub const TO_STRING: [&str; 3] = ["alloc", "string", "ToString"];
pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"];
pub const TRANSMUTE: [&str; 4] = ["core", "intrinsics", "", "transmute"];
pub const TRY_FROM_ERROR: [&str; 4] = ["std", "ops", "Try", "from_error"];
pub const TRY_INTO_RESULT: [&str; 4] = ["std", "ops", "Try", "into_result"];
pub const UNINIT: [&str; 4] = ["core", "intrinsics", "", "uninit"];
pub const VEC: [&str; 3] = ["alloc", "vec", "Vec"];
Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1823,7 +1823,7 @@ pub const ALL_LINTS: [Lint; 306] = [
Lint {
name: "try_err",
group: "style",
desc: "TODO",
desc: "return errors explicitly rather than hiding them behind a `?`",
deprecation: None,
module: "try_err",
},
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/try_err.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: confusing error return, consider using `return Err(err)`
error: returning an `Err(_)` with the `?` operator
--> $DIR/try_err.rs:7:5
|
LL | Err(err)?;
Expand All @@ -10,19 +10,19 @@ note: lint level defined here
LL | #![deny(clippy::try_err)]
| ^^^^^^^^^^^^^^^

error: confusing error return, consider using `return Err(err.into())`
error: returning an `Err(_)` with the `?` operator
--> $DIR/try_err.rs:14:5
|
LL | Err(err)?;
| ^^^^^^^^^ help: try this: `return Err(err.into())`

error: confusing error return, consider using `return Err(err)`
error: returning an `Err(_)` with the `?` operator
--> $DIR/try_err.rs:31:13
|
LL | Err(err)?;
| ^^^^^^^^^ help: try this: `return Err(err)`

error: confusing error return, consider using `return Err(err.into())`
error: returning an `Err(_)` with the `?` operator
--> $DIR/try_err.rs:46:13
|
LL | Err(err)?;
Expand Down

0 comments on commit 1e6c697

Please sign in to comment.