about summary refs log tree commit diff
diff options
context:
space:
mode:
authorThibsG <Thibs@debian.com>2020-01-19 11:07:13 +0100
committerThibsG <Thibs@debian.com>2020-02-04 22:49:08 +0100
commit6afd7ea147cf34eb2ce505d513664b5f4fadfb58 (patch)
treed1441fe1bfd257218c8e16f2ccf9a6a54fc0f7be
parent3445d41f07aba83d83b2093381512a24b9fa974c (diff)
downloadrust-6afd7ea147cf34eb2ce505d513664b5f4fadfb58.tar.gz
rust-6afd7ea147cf34eb2ce505d513664b5f4fadfb58.zip
Use span_lint_and_sugg + move infaillible lint
 - moving infaillible lint to prevent collisions
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/infallible_destructuring_match.rs77
-rw-r--r--clippy_lints/src/lib.rs14
-rw-r--r--clippy_lints/src/matches.rs130
-rw-r--r--src/lintlist/mod.rs9
-rw-r--r--tests/ui/escape_analysis.rs2
-rw-r--r--tests/ui/escape_analysis.stderr4
-rw-r--r--tests/ui/match_single_binding.fixed26
-rw-r--r--tests/ui/match_single_binding.rs4
-rw-r--r--tests/ui/match_single_binding.stderr11
10 files changed, 166 insertions, 112 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eb3b518cd8f..49de9df6469 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1217,6 +1217,7 @@ Released 2018-09-13
 [`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
 [`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
 [`match_same_arms`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_same_arms
+[`match_single_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
 [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
 [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
 [`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
diff --git a/clippy_lints/src/infallible_destructuring_match.rs b/clippy_lints/src/infallible_destructuring_match.rs
deleted file mode 100644
index 1d23dd115bc..00000000000
--- a/clippy_lints/src/infallible_destructuring_match.rs
+++ /dev/null
@@ -1,77 +0,0 @@
-use super::utils::{get_arg_name, match_var, remove_blocks, snippet_with_applicability, span_lint_and_sugg};
-use if_chain::if_chain;
-use rustc_errors::Applicability;
-use rustc_hir::*;
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-
-declare_clippy_lint! {
-    /// **What it does:** Checks for matches being used to destructure a single-variant enum
-    /// or tuple struct where a `let` will suffice.
-    ///
-    /// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    /// ```rust
-    /// enum Wrapper {
-    ///     Data(i32),
-    /// }
-    ///
-    /// let wrapper = Wrapper::Data(42);
-    ///
-    /// let data = match wrapper {
-    ///     Wrapper::Data(i) => i,
-    /// };
-    /// ```
-    ///
-    /// The correct use would be:
-    /// ```rust
-    /// enum Wrapper {
-    ///     Data(i32),
-    /// }
-    ///
-    /// let wrapper = Wrapper::Data(42);
-    /// let Wrapper::Data(data) = wrapper;
-    /// ```
-    pub INFALLIBLE_DESTRUCTURING_MATCH,
-    style,
-    "a `match` statement with a single infallible arm instead of a `let`"
-}
-
-declare_lint_pass!(InfallibleDestructingMatch => [INFALLIBLE_DESTRUCTURING_MATCH]);
-
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InfallibleDestructingMatch {
-    fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
-        if_chain! {
-            if let Some(ref expr) = local.init;
-            if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.kind;
-            if arms.len() == 1 && arms[0].guard.is_none();
-            if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
-            if args.len() == 1;
-            if let Some(arg) = get_arg_name(&args[0]);
-            let body = remove_blocks(&arms[0].body);
-            if match_var(body, arg);
-
-            then {
-                let mut applicability = Applicability::MachineApplicable;
-                span_lint_and_sugg(
-                    cx,
-                    INFALLIBLE_DESTRUCTURING_MATCH,
-                    local.span,
-                    "you seem to be trying to use `match` to destructure a single infallible pattern. \
-                     Consider using `let`",
-                    "try this",
-                    format!(
-                        "let {}({}) = {};",
-                        snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
-                        snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
-                        snippet_with_applicability(cx, target.span, "..", &mut applicability),
-                    ),
-                    applicability,
-                );
-            }
-        }
-    }
-}
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 2f4e6a0fd48..45342cc7e00 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -218,7 +218,6 @@ pub mod if_let_some_result;
 pub mod if_not_else;
 pub mod implicit_return;
 pub mod indexing_slicing;
-pub mod infallible_destructuring_match;
 pub mod infinite_iter;
 pub mod inherent_impl;
 pub mod inherent_to_string;
@@ -555,7 +554,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &implicit_return::IMPLICIT_RETURN,
         &indexing_slicing::INDEXING_SLICING,
         &indexing_slicing::OUT_OF_BOUNDS_INDEXING,
-        &infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
         &infinite_iter::INFINITE_ITER,
         &infinite_iter::MAYBE_INFINITE_ITER,
         &inherent_impl::MULTIPLE_INHERENT_IMPL,
@@ -600,12 +598,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &map_clone::MAP_CLONE,
         &map_unit_fn::OPTION_MAP_UNIT_FN,
         &map_unit_fn::RESULT_MAP_UNIT_FN,
+        &matches::INFALLIBLE_DESTRUCTURING_MATCH,
         &matches::MATCH_AS_REF,
         &matches::MATCH_BOOL,
         &matches::MATCH_OVERLAPPING_ARM,
         &matches::MATCH_REF_PATS,
-        &matches::MATCH_WILD_ERR_ARM,
         &matches::MATCH_SINGLE_BINDING,
+        &matches::MATCH_WILD_ERR_ARM,
         &matches::SINGLE_MATCH,
         &matches::SINGLE_MATCH_ELSE,
         &matches::WILDCARD_ENUM_MATCH_ARM,
@@ -865,7 +864,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box types::Casts);
     let type_complexity_threshold = conf.type_complexity_threshold;
     store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold));
-    store.register_late_pass(|| box matches::Matches);
+    store.register_late_pass(|| box matches::Matches::default());
     store.register_late_pass(|| box minmax::MinMaxPass);
     store.register_late_pass(|| box open_options::OpenOptions);
     store.register_late_pass(|| box zero_div_zero::ZeroDiv);
@@ -942,7 +941,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box question_mark::QuestionMark);
     store.register_late_pass(|| box suspicious_trait_impl::SuspiciousImpl);
     store.register_late_pass(|| box map_unit_fn::MapUnit);
-    store.register_late_pass(|| box infallible_destructuring_match::InfallibleDestructingMatch);
     store.register_late_pass(|| box inherent_impl::MultipleInherentImpl::default());
     store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
     store.register_late_pass(|| box unwrap::Unwrap);
@@ -1167,7 +1165,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&identity_op::IDENTITY_OP),
         LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
         LintId::of(&indexing_slicing::OUT_OF_BOUNDS_INDEXING),
-        LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
         LintId::of(&infinite_iter::INFINITE_ITER),
         LintId::of(&inherent_to_string::INHERENT_TO_STRING),
         LintId::of(&inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY),
@@ -1202,12 +1199,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&map_clone::MAP_CLONE),
         LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
         LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
+        LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
         LintId::of(&matches::MATCH_AS_REF),
         LintId::of(&matches::MATCH_BOOL),
         LintId::of(&matches::MATCH_OVERLAPPING_ARM),
         LintId::of(&matches::MATCH_REF_PATS),
-        LintId::of(&matches::MATCH_WILD_ERR_ARM),
         LintId::of(&matches::MATCH_SINGLE_BINDING),
+        LintId::of(&matches::MATCH_WILD_ERR_ARM),
         LintId::of(&matches::SINGLE_MATCH),
         LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
         LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
@@ -1384,7 +1382,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&functions::DOUBLE_MUST_USE),
         LintId::of(&functions::MUST_USE_UNIT),
         LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
-        LintId::of(&infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH),
         LintId::of(&inherent_to_string::INHERENT_TO_STRING),
         LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
         LintId::of(&len_zero::LEN_ZERO),
@@ -1397,6 +1394,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&loops::WHILE_LET_ON_ITERATOR),
         LintId::of(&main_recursion::MAIN_RECURSION),
         LintId::of(&map_clone::MAP_CLONE),
+        LintId::of(&matches::INFALLIBLE_DESTRUCTURING_MATCH),
         LintId::of(&matches::MATCH_BOOL),
         LintId::of(&matches::MATCH_OVERLAPPING_ARM),
         LintId::of(&matches::MATCH_REF_PATS),
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index b5cf12b0947..e4335abcea2 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -5,7 +5,7 @@ use crate::utils::usage::is_unused;
 use crate::utils::{
     span_lint_and_help, span_lint_and_note, 
     expr_block, in_macro, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks,
-    snippet, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then,
+    snippet, snippet_block, snippet_with_applicability,  span_lint_and_sugg, span_lint_and_then,
 };
 use if_chain::if_chain;
 use rustc::lint::in_external_macro;
@@ -14,7 +14,7 @@ use rustc_errors::Applicability;
 use rustc_hir::def::CtorKind;
 use rustc_hir::*;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 use std::cmp::Ordering;
 use std::collections::Bound;
@@ -246,11 +246,46 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
+    /// **What it does:** Checks for matches being used to destructure a single-variant enum
+    /// or tuple struct where a `let` will suffice.
+    ///
+    /// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// enum Wrapper {
+    ///     Data(i32),
+    /// }
+    ///
+    /// let wrapper = Wrapper::Data(42);
+    ///
+    /// let data = match wrapper {
+    ///     Wrapper::Data(i) => i,
+    /// };
+    /// ```
+    ///
+    /// The correct use would be:
+    /// ```rust
+    /// enum Wrapper {
+    ///     Data(i32),
+    /// }
+    ///
+    /// let wrapper = Wrapper::Data(42);
+    /// let Wrapper::Data(data) = wrapper;
+    /// ```
+    pub INFALLIBLE_DESTRUCTURING_MATCH,
+    style,
+    "a `match` statement with a single infallible arm instead of a `let`"
+}
+
+declare_clippy_lint! {
     /// **What it does:** Checks for useless match that binds to only one value.
     ///
     /// **Why is this bad?** Readability and needless complexity.
     ///
-    /// **Known problems:** This situation frequently happen in macros, so can't lint there.
+    /// **Known problems:** None.
     ///
     /// **Example:**
     /// ```rust
@@ -272,7 +307,12 @@ declare_clippy_lint! {
     "a match with a single binding instead of using `let` statement"
 }
 
-declare_lint_pass!(Matches => [
+#[derive(Default)]
+pub struct Matches {
+    infallible_destructuring_match_linted: bool,
+}
+
+impl_lint_pass!(Matches => [
     SINGLE_MATCH,
     MATCH_REF_PATS,
     MATCH_BOOL,
@@ -283,6 +323,7 @@ declare_lint_pass!(Matches => [
     WILDCARD_ENUM_MATCH_ARM,
     WILDCARD_IN_OR_PATTERNS,
     MATCH_SINGLE_BINDING,
+    INFALLIBLE_DESTRUCTURING_MATCH
 ]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
@@ -298,12 +339,51 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
             check_wild_enum_match(cx, ex, arms);
             check_match_as_ref(cx, ex, arms, expr);
             check_wild_in_or_pats(cx, arms);
-            check_match_single_binding(cx, ex, arms, expr);
+
+            if self.infallible_destructuring_match_linted {
+                self.infallible_destructuring_match_linted = false;
+            } else {
+                check_match_single_binding(cx, ex, arms, expr);
+            }
         }
         if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
             check_match_ref_pats(cx, ex, arms, expr);
         }
     }
+
+    fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local<'_>) {
+        if_chain! {
+            if let Some(ref expr) = local.init;
+            if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.kind;
+            if arms.len() == 1 && arms[0].guard.is_none();
+            if let PatKind::TupleStruct(
+                QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.kind;
+            if args.len() == 1;
+            if let Some(arg) = get_arg_name(&args[0]);
+            let body = remove_blocks(&arms[0].body);
+            if match_var(body, arg);
+
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                self.infallible_destructuring_match_linted = true;
+                span_lint_and_sugg(
+                    cx,
+                    INFALLIBLE_DESTRUCTURING_MATCH,
+                    local.span,
+                    "you seem to be trying to use `match` to destructure a single infallible pattern. \
+                    Consider using `let`",
+                    "try this",
+                    format!(
+                        "let {}({}) = {};",
+                        snippet_with_applicability(cx, variant_name.span, "..", &mut applicability),
+                        snippet_with_applicability(cx, local.pat.span, "..", &mut applicability),
+                        snippet_with_applicability(cx, target.span, "..", &mut applicability),
+                    ),
+                    applicability,
+                );
+            }
+        }
+    }
 }
 
 #[rustfmt::skip]
@@ -746,21 +826,31 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A
         return;
     }
     if arms.len() == 1 {
-        let bind_names = arms[0].pat.span;
-        let matched_vars = ex.span;
-        span_lint_and_sugg(
-            cx,
-            MATCH_SINGLE_BINDING,
-            expr.span,
-            "this match could be written as a `let` statement",
-            "try this",
-            format!(
-                "let {} = {};",
-                snippet(cx, bind_names, ".."),
-                snippet(cx, matched_vars, "..")
-            ),
-            Applicability::HasPlaceholders,
-        );
+        if is_refutable(cx, arms[0].pat) {
+            return;
+        }
+        match arms[0].pat.kind {
+            PatKind::Binding(..) | PatKind::Tuple(_, _) => {
+                let bind_names = arms[0].pat.span;
+                let matched_vars = ex.span;
+                let match_body = remove_blocks(&arms[0].body);
+                span_lint_and_sugg(
+                    cx,
+                    MATCH_SINGLE_BINDING,
+                    expr.span,
+                    "this match could be written as a `let` statement",
+                    "consider using `let` statement",
+                    format!(
+                        "let {} = {};\n{}",
+                        snippet(cx, bind_names, ".."),
+                        snippet(cx, matched_vars, ".."),
+                        snippet_block(cx, match_body.span, "..")
+                    ),
+                    Applicability::MachineApplicable,
+                );
+            },
+            _ => (),
+        }
     }
 }
 
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index eaada9961b0..b28a0917a91 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -775,7 +775,7 @@ pub const ALL_LINTS: [Lint; 351] = [
         group: "style",
         desc: "a `match` statement with a single infallible arm instead of a `let`",
         deprecation: None,
-        module: "infallible_destructuring_match",
+        module: "matches",
     },
     Lint {
         name: "infinite_iter",
@@ -1093,6 +1093,13 @@ pub const ALL_LINTS: [Lint; 351] = [
         module: "copies",
     },
     Lint {
+        name: "match_single_binding",
+        group: "complexity",
+        desc: "a match with a single binding instead of using `let` statement",
+        deprecation: None,
+        module: "matches",
+    },
+    Lint {
         name: "match_wild_err_arm",
         group: "style",
         desc: "a `match` with `Err(_)` arm and take drastic actions",
diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs
index d435484d3e3..7d4b71d09dc 100644
--- a/tests/ui/escape_analysis.rs
+++ b/tests/ui/escape_analysis.rs
@@ -3,7 +3,7 @@
     clippy::borrowed_box,
     clippy::needless_pass_by_value,
     clippy::unused_unit,
-    clippy::redundant_clone
+    clippy::redundant_clone,
 )]
 #![warn(clippy::boxed_local)]
 
diff --git a/tests/ui/escape_analysis.stderr b/tests/ui/escape_analysis.stderr
index 19342fe1be7..c86a769a3da 100644
--- a/tests/ui/escape_analysis.stderr
+++ b/tests/ui/escape_analysis.stderr
@@ -1,5 +1,5 @@
 error: local variable doesn't need to be boxed here
-  --> $DIR/escape_analysis.rs:39:13
+  --> $DIR/escape_analysis.rs:40:13
    |
 LL | fn warn_arg(x: Box<A>) {
    |             ^
@@ -7,7 +7,7 @@ LL | fn warn_arg(x: Box<A>) {
    = note: `-D clippy::boxed-local` implied by `-D warnings`
 
 error: local variable doesn't need to be boxed here
-  --> $DIR/escape_analysis.rs:130:12
+  --> $DIR/escape_analysis.rs:131:12
    |
 LL | pub fn new(_needs_name: Box<PeekableSeekable<&()>>) -> () {}
    |            ^^^^^^^^^^^
diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed
new file mode 100644
index 00000000000..41faa1e1c21
--- /dev/null
+++ b/tests/ui/match_single_binding.fixed
@@ -0,0 +1,26 @@
+// run-rustfix
+
+#![warn(clippy::match_single_binding)]
+#[allow(clippy::many_single_char_names)]
+
+fn main() {
+    let a = 1;
+    let b = 2;
+    let c = 3;
+    // Lint
+    let (x, y, z) = (a, b, c);
+{
+    println!("{} {} {}", x, y, z);
+}
+    // Ok
+    match a {
+        2 => println!("2"),
+        _ => println!("Not 2"),
+    }
+    // Ok
+    let d = Some(5);
+    match d {
+        Some(d) => println!("{}", d),
+        _ => println!("None"),
+    }
+}
diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs
index 816a7e7197c..06b924d0471 100644
--- a/tests/ui/match_single_binding.rs
+++ b/tests/ui/match_single_binding.rs
@@ -1,3 +1,5 @@
+// run-rustfix
+
 #![warn(clippy::match_single_binding)]
 #[allow(clippy::many_single_char_names)]
 
@@ -19,7 +21,7 @@ fn main() {
     // Ok
     let d = Some(5);
     match d {
-        Some(d) => println!("5"),
+        Some(d) => println!("{}", d),
         _ => println!("None"),
     }
 }
diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr
index 2fad6aab32c..64216a72ef7 100644
--- a/tests/ui/match_single_binding.stderr
+++ b/tests/ui/match_single_binding.stderr
@@ -1,14 +1,21 @@
 error: this match could be written as a `let` statement
-  --> $DIR/match_single_binding.rs:9:5
+  --> $DIR/match_single_binding.rs:11:5
    |
 LL | /     match (a, b, c) {
 LL | |         (x, y, z) => {
 LL | |             println!("{} {} {}", x, y, z);
 LL | |         },
 LL | |     }
-   | |_____^ help: try this: `let (x, y, z) = (a, b, c);`
+   | |_____^
    |
    = note: `-D clippy::match-single-binding` implied by `-D warnings`
+help: consider using `let` statement
+   |
+LL |     let (x, y, z) = (a, b, c);
+LL | {
+LL |     println!("{} {} {}", x, y, z);
+LL | }
+   |
 
 error: aborting due to previous error