diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 9 | ||||
| -rw-r--r-- | clippy_lints/src/manual_unwrap_or.rs | 113 | ||||
| -rw-r--r-- | src/lintlist/mod.rs | 14 | ||||
| -rw-r--r-- | tests/ui/manual_unwrap_or.fixed (renamed from tests/ui/less_concise_than.fixed) | 4 | ||||
| -rw-r--r-- | tests/ui/manual_unwrap_or.rs (renamed from tests/ui/less_concise_than.rs) | 7 | ||||
| -rw-r--r-- | tests/ui/manual_unwrap_or.stderr (renamed from tests/ui/less_concise_than.stderr) | 29 | ||||
| -rw-r--r-- | tests/ui/shadow.rs | 2 |
8 files changed, 101 insertions, 79 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 93ce6bb85d8..d82f970b8bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1781,7 +1781,6 @@ Released 2018-09-13 [`large_stack_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_arrays [`len_without_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_without_is_empty [`len_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#len_zero -[`less_concise_than_option_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#less_concise_than_option_unwrap_or [`let_and_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return [`let_underscore_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_lock [`let_underscore_must_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_must_use @@ -1797,6 +1796,7 @@ Released 2018-09-13 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic [`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap +[`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names [`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone [`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2e9900815d9..d4d2f92a6a6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -224,7 +224,6 @@ mod large_const_arrays; mod large_enum_variant; mod large_stack_arrays; mod len_zero; -mod less_concise_than; mod let_if_seq; mod let_underscore; mod lifetimes; @@ -235,6 +234,7 @@ mod main_recursion; mod manual_async_fn; mod manual_non_exhaustive; mod manual_strip; +mod manual_unwrap_or; mod map_clone; mod map_err_ignore; mod map_identity; @@ -610,7 +610,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &large_stack_arrays::LARGE_STACK_ARRAYS, &len_zero::LEN_WITHOUT_IS_EMPTY, &len_zero::LEN_ZERO, - &less_concise_than::LESS_CONCISE_THAN_OPTION_UNWRAP_OR, &let_if_seq::USELESS_LET_IF_SEQ, &let_underscore::LET_UNDERSCORE_LOCK, &let_underscore::LET_UNDERSCORE_MUST_USE, @@ -642,6 +641,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &manual_async_fn::MANUAL_ASYNC_FN, &manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE, &manual_strip::MANUAL_STRIP, + &manual_unwrap_or::MANUAL_UNWRAP_OR, &map_clone::MAP_CLONE, &map_err_ignore::MAP_ERR_IGNORE, &map_identity::MAP_IDENTITY, @@ -1128,7 +1128,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box repeat_once::RepeatOnce); store.register_late_pass(|| box unwrap_in_result::UnwrapInResult); store.register_late_pass(|| box self_assignment::SelfAssignment); - store.register_late_pass(|| box less_concise_than::LessConciseThan); + store.register_late_pass(|| box manual_unwrap_or::ManualUnwrapOr); store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs); store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync); store.register_late_pass(|| box manual_strip::ManualStrip); @@ -1213,7 +1213,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&infinite_iter::MAYBE_INFINITE_ITER), LintId::of(&items_after_statements::ITEMS_AFTER_STATEMENTS), LintId::of(&large_stack_arrays::LARGE_STACK_ARRAYS), - LintId::of(&less_concise_than::LESS_CONCISE_THAN_OPTION_UNWRAP_OR), LintId::of(&literal_representation::LARGE_DIGIT_GROUPS), LintId::of(&literal_representation::UNREADABLE_LITERAL), LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP), @@ -1371,6 +1370,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&manual_async_fn::MANUAL_ASYNC_FN), LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), LintId::of(&manual_strip::MANUAL_STRIP), + LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(&map_clone::MAP_CLONE), LintId::of(&map_identity::MAP_IDENTITY), LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), @@ -1666,6 +1666,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::MUT_RANGE_BOUND), LintId::of(&loops::WHILE_LET_LOOP), LintId::of(&manual_strip::MANUAL_STRIP), + LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(&map_identity::MAP_IDENTITY), LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN), diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs index 097aff4b178..9d8fc863424 100644 --- a/clippy_lints/src/manual_unwrap_or.rs +++ b/clippy_lints/src/manual_unwrap_or.rs @@ -2,12 +2,14 @@ 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 can be encoded more concisely with `Option::unwrap_or`. + /// Finds patterns that reimplement `Option::unwrap_or`. /// /// **Why is this bad?** /// Concise code helps focusing on behavior instead of boilerplate. @@ -16,7 +18,7 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// match int_optional { + /// match int_option { /// Some(v) => v, /// None => 1, /// } @@ -24,39 +26,35 @@ declare_clippy_lint! { /// /// Use instead: /// ```rust - /// int_optional.unwrap_or(1) + /// int_option.unwrap_or(1) /// ``` - pub LESS_CONCISE_THAN_OPTION_UNWRAP_OR, - pedantic, - "finds patterns that can be encoded more concisely with `Option::unwrap_or`" + pub MANUAL_UNWRAP_OR, + complexity, + "finds patterns that can be encoded more concisely with `Option::unwrap_or(_else)`" } -declare_lint_pass!(LessConciseThan => [LESS_CONCISE_THAN_OPTION_UNWRAP_OR]); +declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); -impl LateLintPass<'_> for LessConciseThan { +impl LateLintPass<'_> for ManualUnwrapOr { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if utils::in_macro(expr.span) { - return; - } - if lint_option_unwrap_or_case(cx, expr) { + 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>) -> bool { - #[allow(clippy::needless_bool)] 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_chain! { - if let PatKind::Path(ref qpath) = arm.pat.kind; - if utils::match_qpath(qpath, &utils::paths::OPTION_NONE); - then { true } - else { false } - } + 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; @@ -65,43 +63,50 @@ fn lint_option_unwrap_or_case<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tc 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; - then { Some(none_arm) } - else { None } + if !utils::usage::contains_return_break_continue_macro(none_arm.body); + then { + Some(none_arm) + } + else { + None + } } } + if_chain! { - if !utils::usage::contains_return_break_continue_macro(expr); - if let ExprKind::Match (match_expr, match_arms, _) = expr.kind; - let ty = cx.typeck_results().expr_ty(match_expr); - if utils::is_type_diagnostic_item(cx, ty, sym!(option_type)); - if let Some(none_arm) = applicable_none_arm(match_arms); - if let Some(match_expr_snippet) = utils::snippet_opt(cx, match_expr.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); - then { - let reindented_none_body = - utils::reindent_multiline(none_body_snippet.into(), true, Some(indent)); - let eager_eval = utils::eager_or_lazy::is_eagerness_candidate(cx, none_arm.body); - let method = if eager_eval { - "unwrap_or" - } else { - "unwrap_or_else" - }; - utils::span_lint_and_sugg( - cx, - LESS_CONCISE_THAN_OPTION_UNWRAP_OR, expr.span, - "this pattern can be more concisely encoded with `Option::unwrap_or`", - "replace with", - format!( - "{}.{}({}{})", - match_expr_snippet, - method, - if eager_eval { ""} else { "|| " }, - reindented_none_body - ), - Applicability::MachineApplicable, - ); - true - } else { false} + 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); + then { + let reindented_none_body = + utils::reindent_multiline(none_body_snippet.into(), true, Some(indent)); + let eager_eval = utils::eager_or_lazy::is_eagerness_candidate(cx, none_arm.body); + let method = if eager_eval { + "unwrap_or" + } else { + "unwrap_or_else" + }; + utils::span_lint_and_sugg( + cx, + MANUAL_UNWRAP_OR, expr.span, + &format!("this pattern reimplements `Option::{}`", &method), + "replace with", + format!( + "{}.{}({}{})", + scrutinee_snippet, + method, + if eager_eval { ""} else { "|| " }, + reindented_none_body + ), + Applicability::MachineApplicable, + ); + true + } else { + false + } } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 6dc95fcfdb2..debd3c31d8b 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1076,13 +1076,6 @@ vec![ module: "len_zero", }, Lint { - name: "less_concise_than_option_unwrap_or", - group: "pedantic", - desc: "finds patterns that can be encoded more concisely with `Option::unwrap_or`", - deprecation: None, - module: "less_concise_than", - }, - Lint { name: "let_and_return", group: "style", desc: "creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block", @@ -1188,6 +1181,13 @@ vec![ module: "swap", }, Lint { + name: "manual_unwrap_or", + group: "complexity", + desc: "finds patterns that can be encoded more concisely with `Option::unwrap_or(_else)`", + deprecation: None, + module: "manual_unwrap_or", + }, + Lint { name: "many_single_char_names", group: "style", desc: "too many single character bindings", diff --git a/tests/ui/less_concise_than.fixed b/tests/ui/manual_unwrap_or.fixed index 52b69ebba3e..99d30360db1 100644 --- a/tests/ui/less_concise_than.fixed +++ b/tests/ui/manual_unwrap_or.fixed @@ -1,11 +1,13 @@ // run-rustfix -#![warn(clippy::less_concise_than_option_unwrap_or)] #![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_else(|| 1 + 42); diff --git a/tests/ui/less_concise_than.rs b/tests/ui/manual_unwrap_or.rs index bb2a8f2050a..5d03d9db163 100644 --- a/tests/ui/less_concise_than.rs +++ b/tests/ui/manual_unwrap_or.rs @@ -1,5 +1,4 @@ // run-rustfix -#![warn(clippy::less_concise_than_option_unwrap_or)] #![allow(dead_code)] fn unwrap_or() { @@ -9,6 +8,12 @@ fn unwrap_or() { None => 42, }; + // int case reversed + match Some(1) { + None => 42, + Some(i) => i, + }; + // richer none expr match Some(1) { Some(i) => i, diff --git a/tests/ui/less_concise_than.stderr b/tests/ui/manual_unwrap_or.stderr index e3e8a406db1..03da118a0c4 100644 --- a/tests/ui/less_concise_than.stderr +++ b/tests/ui/manual_unwrap_or.stderr @@ -1,5 +1,5 @@ -error: this pattern can be more concisely encoded with `Option::unwrap_or` - --> $DIR/less_concise_than.rs:7:5 +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:6:5 | LL | / match Some(1) { LL | | Some(i) => i, @@ -7,10 +7,19 @@ LL | | None => 42, LL | | }; | |_____^ help: replace with: `Some(1).unwrap_or(42)` | - = note: `-D clippy::less-concise-than-option-unwrap-or` implied by `-D warnings` + = note: `-D clippy::manual-unwrap-or` implied by `-D warnings` -error: this pattern can be more concisely encoded with `Option::unwrap_or` - --> $DIR/less_concise_than.rs:13:5 +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:12:5 + | +LL | / match Some(1) { +LL | | None => 42, +LL | | Some(i) => i, +LL | | }; + | |_____^ help: replace with: `Some(1).unwrap_or(42)` + +error: this pattern reimplements `Option::unwrap_or_else` + --> $DIR/manual_unwrap_or.rs:18:5 | LL | / match Some(1) { LL | | Some(i) => i, @@ -18,8 +27,8 @@ LL | | None => 1 + 42, LL | | }; | |_____^ help: replace with: `Some(1).unwrap_or_else(|| 1 + 42)` -error: this pattern can be more concisely encoded with `Option::unwrap_or` - --> $DIR/less_concise_than.rs:19:5 +error: this pattern reimplements `Option::unwrap_or_else` + --> $DIR/manual_unwrap_or.rs:24:5 | LL | / match Some(1) { LL | | Some(i) => i, @@ -39,8 +48,8 @@ LL | b + 42 LL | }); | -error: this pattern can be more concisely encoded with `Option::unwrap_or` - --> $DIR/less_concise_than.rs:29:5 +error: this pattern reimplements `Option::unwrap_or` + --> $DIR/manual_unwrap_or.rs:34:5 | LL | / match Some("Bob") { LL | | Some(i) => i, @@ -48,5 +57,5 @@ LL | | None => "Alice", LL | | }; | |_____^ help: replace with: `Some("Bob").unwrap_or("Alice")` -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors diff --git a/tests/ui/shadow.rs b/tests/ui/shadow.rs index e7441293d45..e366c75335c 100644 --- a/tests/ui/shadow.rs +++ b/tests/ui/shadow.rs @@ -8,7 +8,7 @@ #![allow( unused_parens, unused_variables, - clippy::less_concise_than_option_unwrap_or, + clippy::manual_unwrap_or, clippy::missing_docs_in_private_items, clippy::single_match )] |