-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 #6123 - montrivo:less_concise_than, r=ebroto
add lint manual_unwrap_or Implements partially #5923. changelog: add lint manual_unwrap_or
- Loading branch information
Showing
12 changed files
with
405 additions
and
76 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,104 @@ | ||
| use crate::consts::constant_simple; | ||
| use crate::utils; | ||
| use if_chain::if_chain; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{def, Arm, 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::unwrap_or`. | ||
| /// | ||
| /// **Why is this bad?** | ||
| /// Concise code helps focusing on behavior instead of boilerplate. | ||
| /// | ||
| /// **Known problems:** None. | ||
| /// | ||
| /// **Example:** | ||
| /// ```rust | ||
| /// let foo: Option<i32> = None; | ||
| /// match foo { | ||
| /// Some(v) => v, | ||
| /// None => 1, | ||
| /// }; | ||
| /// ``` | ||
| /// | ||
| /// Use instead: | ||
| /// ```rust | ||
| /// let foo: Option<i32> = None; | ||
| /// foo.unwrap_or(1); | ||
| /// ``` | ||
| pub MANUAL_UNWRAP_OR, | ||
| complexity, | ||
| "finds patterns that can be encoded more concisely with `Option::unwrap_or`" | ||
| } | ||
|
|
||
| declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); | ||
|
|
||
| impl LateLintPass<'_> for ManualUnwrapOr { | ||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { | ||
| if in_external_macro(cx.sess(), expr.span) { | ||
| return; | ||
| } | ||
| lint_option_unwrap_or_case(cx, expr); | ||
| } | ||
| } | ||
|
|
||
| fn lint_option_unwrap_or_case<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { | ||
| fn applicable_none_arm<'a>(arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { | ||
| if_chain! { | ||
| if arms.len() == 2; | ||
| if arms.iter().all(|arm| arm.guard.is_none()); | ||
| if let Some((idx, none_arm)) = arms.iter().enumerate().find(|(_, arm)| | ||
| if let PatKind::Path(ref qpath) = arm.pat.kind { | ||
| utils::match_qpath(qpath, &utils::paths::OPTION_NONE) | ||
| } else { | ||
| false | ||
| } | ||
| ); | ||
| let some_arm = &arms[1 - idx]; | ||
| if let PatKind::TupleStruct(ref some_qpath, &[some_binding], _) = some_arm.pat.kind; | ||
| if utils::match_qpath(some_qpath, &utils::paths::OPTION_SOME); | ||
| if let PatKind::Binding(_, binding_hir_id, ..) = some_binding.kind; | ||
| if let ExprKind::Path(QPath::Resolved(_, body_path)) = some_arm.body.kind; | ||
| if let def::Res::Local(body_path_hir_id) = body_path.res; | ||
| if body_path_hir_id == binding_hir_id; | ||
| if !utils::usage::contains_return_break_continue_macro(none_arm.body); | ||
| then { | ||
| Some(none_arm) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if_chain! { | ||
| if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind; | ||
| let ty = cx.typeck_results().expr_ty(scrutinee); | ||
| if utils::is_type_diagnostic_item(cx, ty, sym!(option_type)); | ||
| if let Some(none_arm) = applicable_none_arm(match_arms); | ||
| if let Some(scrutinee_snippet) = utils::snippet_opt(cx, scrutinee.span); | ||
| if let Some(none_body_snippet) = utils::snippet_opt(cx, none_arm.body.span); | ||
| if let Some(indent) = utils::indent_of(cx, expr.span); | ||
| if constant_simple(cx, cx.typeck_results(), none_arm.body).is_some(); | ||
| then { | ||
| let reindented_none_body = | ||
| utils::reindent_multiline(none_body_snippet.into(), true, Some(indent)); | ||
| utils::span_lint_and_sugg( | ||
| cx, | ||
| MANUAL_UNWRAP_OR, expr.span, | ||
| "this pattern reimplements `Option::unwrap_or`", | ||
| "replace with", | ||
| format!( | ||
| "{}.unwrap_or({})", | ||
| scrutinee_snippet, | ||
| reindented_none_body, | ||
| ), | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| } | ||
| } | ||
| } |
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
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,68 @@ | ||
| // run-rustfix | ||
| #![allow(dead_code)] | ||
|
|
||
| fn unwrap_or() { | ||
| // int case | ||
| Some(1).unwrap_or(42); | ||
|
|
||
| // int case reversed | ||
| Some(1).unwrap_or(42); | ||
|
|
||
| // richer none expr | ||
| Some(1).unwrap_or(1 + 42); | ||
|
|
||
| // multiline case | ||
| #[rustfmt::skip] | ||
| Some(1).unwrap_or({ | ||
| 42 + 42 | ||
| + 42 + 42 + 42 | ||
| + 42 + 42 + 42 | ||
| }); | ||
|
|
||
| // string case | ||
| Some("Bob").unwrap_or("Alice"); | ||
|
|
||
| // don't lint | ||
| match Some(1) { | ||
| Some(i) => i + 2, | ||
| None => 42, | ||
| }; | ||
| match Some(1) { | ||
| Some(i) => i, | ||
| None => return, | ||
| }; | ||
| for j in 0..4 { | ||
| match Some(j) { | ||
| Some(i) => i, | ||
| None => continue, | ||
| }; | ||
| match Some(j) { | ||
| Some(i) => i, | ||
| None => break, | ||
| }; | ||
| } | ||
|
|
||
| // cases where the none arm isn't a constant expression | ||
| // are not linted due to potential ownership issues | ||
|
|
||
| // ownership issue example, don't lint | ||
| struct NonCopyable; | ||
| let mut option: Option<NonCopyable> = None; | ||
| match option { | ||
| Some(x) => x, | ||
| None => { | ||
| option = Some(NonCopyable); | ||
| // some more code ... | ||
| option.unwrap() | ||
| }, | ||
| }; | ||
|
|
||
| // ownership issue example, don't lint | ||
| let option: Option<&str> = None; | ||
| match option { | ||
| Some(s) => s, | ||
| None => &format!("{} {}!", "hello", "world"), | ||
| }; | ||
| } | ||
|
|
||
| fn main() {} |
Oops, something went wrong.