forked from rust-lang/rust-clippy
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of rust-lang#6233 - montrivo:manual_ok_or, r=flip1995
add manual_ok_or lint Implements partially rust-lang#5923 changelog: add lint manual_ok_or
- Loading branch information
Showing
7 changed files
with
236 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| use crate::utils::{ | ||
| indent_of, is_type_diagnostic_item, match_qpath, paths, reindent_multiline, snippet_opt, span_lint_and_sugg, | ||
| }; | ||
| use if_chain::if_chain; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{def, Expr, ExprKind, PatKind, QPath}; | ||
| use rustc_lint::LintContext; | ||
| use rustc_lint::{LateContext, LateLintPass}; | ||
| use rustc_middle::lint::in_external_macro; | ||
| use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** | ||
| /// Finds patterns that reimplement `Option::ok_or`. | ||
| /// | ||
| /// **Why is this bad?** | ||
| /// Concise code helps focusing on behavior instead of boilerplate. | ||
| /// | ||
| /// **Known problems:** None. | ||
| /// | ||
| /// **Examples:** | ||
| /// ```rust | ||
| /// let foo: Option<i32> = None; | ||
| /// foo.map_or(Err("error"), |v| Ok(v)); | ||
| /// | ||
| /// let foo: Option<i32> = None; | ||
| /// foo.map_or(Err("error"), |v| Ok(v)); | ||
| /// ``` | ||
| /// | ||
| /// Use instead: | ||
| /// ```rust | ||
| /// let foo: Option<i32> = None; | ||
| /// foo.ok_or("error"); | ||
| /// ``` | ||
| pub MANUAL_OK_OR, | ||
| pedantic, | ||
| "finds patterns that can be encoded more concisely with `Option::ok_or`" | ||
| } | ||
|
|
||
| declare_lint_pass!(ManualOkOr => [MANUAL_OK_OR]); | ||
|
|
||
| impl LateLintPass<'_> for ManualOkOr { | ||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, scrutinee: &'tcx Expr<'tcx>) { | ||
| if in_external_macro(cx.sess(), scrutinee.span) { | ||
| return; | ||
| } | ||
|
|
||
| if_chain! { | ||
| if let ExprKind::MethodCall(method_segment, _, args, _) = scrutinee.kind; | ||
| if method_segment.ident.name == sym!(map_or); | ||
| if args.len() == 3; | ||
| let method_receiver = &args[0]; | ||
| let ty = cx.typeck_results().expr_ty(method_receiver); | ||
| if is_type_diagnostic_item(cx, ty, sym!(option_type)); | ||
| let or_expr = &args[1]; | ||
| if is_ok_wrapping(cx, &args[2]); | ||
| if let ExprKind::Call(Expr { kind: ExprKind::Path(err_path), .. }, &[ref err_arg]) = or_expr.kind; | ||
| if match_qpath(err_path, &paths::RESULT_ERR); | ||
| if let Some(method_receiver_snippet) = snippet_opt(cx, method_receiver.span); | ||
| if let Some(err_arg_snippet) = snippet_opt(cx, err_arg.span); | ||
| if let Some(indent) = indent_of(cx, scrutinee.span); | ||
| then { | ||
| let reindented_err_arg_snippet = | ||
| reindent_multiline(err_arg_snippet.into(), true, Some(indent + 4)); | ||
| span_lint_and_sugg( | ||
| cx, | ||
| MANUAL_OK_OR, | ||
| scrutinee.span, | ||
| "this pattern reimplements `Option::ok_or`", | ||
| "replace with", | ||
| format!( | ||
| "{}.ok_or({})", | ||
| method_receiver_snippet, | ||
| reindented_err_arg_snippet | ||
| ), | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn is_ok_wrapping(cx: &LateContext<'_>, map_expr: &Expr<'_>) -> bool { | ||
| if let ExprKind::Path(ref qpath) = map_expr.kind { | ||
| if match_qpath(qpath, &paths::RESULT_OK) { | ||
| return true; | ||
| } | ||
| } | ||
| if_chain! { | ||
| if let ExprKind::Closure(_, _, body_id, ..) = map_expr.kind; | ||
| let body = cx.tcx.hir().body(body_id); | ||
| if let PatKind::Binding(_, param_id, ..) = body.params[0].pat.kind; | ||
| if let ExprKind::Call(Expr { kind: ExprKind::Path(ok_path), .. }, &[ref ok_arg]) = body.value.kind; | ||
| if match_qpath(ok_path, &paths::RESULT_OK); | ||
| if let ExprKind::Path(QPath::Resolved(_, ok_arg_path)) = ok_arg.kind; | ||
| if let def::Res::Local(ok_arg_path_id) = ok_arg_path.res; | ||
| then { param_id == ok_arg_path_id } else { false } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| // run-rustfix | ||
| #![warn(clippy::manual_ok_or)] | ||
| #![allow(clippy::blacklisted_name)] | ||
| #![allow(clippy::redundant_closure)] | ||
| #![allow(dead_code)] | ||
| #![allow(unused_must_use)] | ||
|
|
||
| fn main() { | ||
| // basic case | ||
| let foo: Option<i32> = None; | ||
| foo.ok_or("error"); | ||
|
|
||
| // eta expansion case | ||
| foo.ok_or("error"); | ||
|
|
||
| // turbo fish syntax | ||
| None::<i32>.ok_or("error"); | ||
|
|
||
| // multiline case | ||
| #[rustfmt::skip] | ||
| foo.ok_or(&format!( | ||
| "{}{}{}{}{}{}{}", | ||
| "Alice", "Bob", "Sarah", "Marc", "Sandra", "Eric", "Jenifer")); | ||
|
|
||
| // not applicable, closure isn't direct `Ok` wrapping | ||
| foo.map_or(Err("error"), |v| Ok(v + 1)); | ||
|
|
||
| // not applicable, or side isn't `Result::Err` | ||
| foo.map_or(Ok::<i32, &str>(1), |v| Ok(v)); | ||
|
|
||
| // not applicatble, expr is not a `Result` value | ||
| foo.map_or(42, |v| v); | ||
|
|
||
| // TODO patterns not covered yet | ||
| match foo { | ||
| Some(v) => Ok(v), | ||
| None => Err("error"), | ||
| }; | ||
| foo.map_or_else(|| Err("error"), |v| Ok(v)); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // run-rustfix | ||
| #![warn(clippy::manual_ok_or)] | ||
| #![allow(clippy::blacklisted_name)] | ||
| #![allow(clippy::redundant_closure)] | ||
| #![allow(dead_code)] | ||
| #![allow(unused_must_use)] | ||
|
|
||
| fn main() { | ||
| // basic case | ||
| let foo: Option<i32> = None; | ||
| foo.map_or(Err("error"), |v| Ok(v)); | ||
|
|
||
| // eta expansion case | ||
| foo.map_or(Err("error"), Ok); | ||
|
|
||
| // turbo fish syntax | ||
| None::<i32>.map_or(Err("error"), |v| Ok(v)); | ||
|
|
||
| // multiline case | ||
| #[rustfmt::skip] | ||
| foo.map_or(Err::<i32, &str>( | ||
| &format!( | ||
| "{}{}{}{}{}{}{}", | ||
| "Alice", "Bob", "Sarah", "Marc", "Sandra", "Eric", "Jenifer") | ||
| ), | ||
| |v| Ok(v), | ||
| ); | ||
|
|
||
| // not applicable, closure isn't direct `Ok` wrapping | ||
| foo.map_or(Err("error"), |v| Ok(v + 1)); | ||
|
|
||
| // not applicable, or side isn't `Result::Err` | ||
| foo.map_or(Ok::<i32, &str>(1), |v| Ok(v)); | ||
|
|
||
| // not applicatble, expr is not a `Result` value | ||
| foo.map_or(42, |v| v); | ||
|
|
||
| // TODO patterns not covered yet | ||
| match foo { | ||
| Some(v) => Ok(v), | ||
| None => Err("error"), | ||
| }; | ||
| foo.map_or_else(|| Err("error"), |v| Ok(v)); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| error: this pattern reimplements `Option::ok_or` | ||
| --> $DIR/manual_ok_or.rs:11:5 | ||
| | | ||
| LL | foo.map_or(Err("error"), |v| Ok(v)); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")` | ||
| | | ||
| = note: `-D clippy::manual-ok-or` implied by `-D warnings` | ||
|
|
||
| error: this pattern reimplements `Option::ok_or` | ||
| --> $DIR/manual_ok_or.rs:14:5 | ||
| | | ||
| LL | foo.map_or(Err("error"), Ok); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `foo.ok_or("error")` | ||
|
|
||
| error: this pattern reimplements `Option::ok_or` | ||
| --> $DIR/manual_ok_or.rs:17:5 | ||
| | | ||
| LL | None::<i32>.map_or(Err("error"), |v| Ok(v)); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `None::<i32>.ok_or("error")` | ||
|
|
||
| error: this pattern reimplements `Option::ok_or` | ||
| --> $DIR/manual_ok_or.rs:21:5 | ||
| | | ||
| LL | / foo.map_or(Err::<i32, &str>( | ||
| LL | | &format!( | ||
| LL | | "{}{}{}{}{}{}{}", | ||
| LL | | "Alice", "Bob", "Sarah", "Marc", "Sandra", "Eric", "Jenifer") | ||
| LL | | ), | ||
| LL | | |v| Ok(v), | ||
| LL | | ); | ||
| | |_____^ | ||
| | | ||
| help: replace with | ||
| | | ||
| LL | foo.ok_or(&format!( | ||
| LL | "{}{}{}{}{}{}{}", | ||
| LL | "Alice", "Bob", "Sarah", "Marc", "Sandra", "Eric", "Jenifer")); | ||
| | | ||
|
|
||
| error: aborting due to 4 previous errors | ||
|
|