Auto merge of #6377 - CDirkx:redundant-pattern-match-ipaddr, r=ebroto

Enhance `redundant_pattern_matching` to also lint on `std::net::IpAddr` Follow-up to #6339 r? `@ebroto` (note: also contains a small cleanup of the other ui tests) changelog: Enhance [`redundant_pattern_matching`] to also lint on `std::net::IpAddr`
rust-lang · Nov 28, 2020 · 68cf94f · 68cf94f
2 parents bc723c5 + dc075b4
commit 68cf94f
Show file tree

Hide file tree

Showing 11 changed files with 341 additions and 36 deletions.
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
@@ -412,8 +412,8 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Lint for redundant pattern matching over `Result`, `Option` or
-    /// `std::task::Poll`
+    /// **What it does:** Lint for redundant pattern matching over `Result`, `Option`,
+    /// `std::task::Poll` or `std::net::IpAddr`
     ///
     /// **Why is this bad?** It's more concise and clear to just use the proper
     /// utility function
@@ -424,12 +424,15 @@ declare_clippy_lint! {
     ///
     /// ```rust
     /// # use std::task::Poll;
+    /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
     /// if let Ok(_) = Ok::<i32, i32>(42) {}
     /// if let Err(_) = Err::<i32, i32>(42) {}
     /// if let None = None::<()> {}
     /// if let Some(_) = Some(42) {}
     /// if let Poll::Pending = Poll::Pending::<()> {}
     /// if let Poll::Ready(_) = Poll::Ready(42) {}
+    /// if let IpAddr::V4(_) = IpAddr::V4(Ipv4Addr::LOCALHOST) {}
+    /// if let IpAddr::V6(_) = IpAddr::V6(Ipv6Addr::LOCALHOST) {}
     /// match Ok::<i32, i32>(42) {
     ///     Ok(_) => true,
     ///     Err(_) => false,
@@ -440,12 +443,15 @@ declare_clippy_lint! {
     ///
     /// ```rust
     /// # use std::task::Poll;
+    /// # use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
     /// if Ok::<i32, i32>(42).is_ok() {}
     /// if Err::<i32, i32>(42).is_err() {}
     /// if None::<()>.is_none() {}
     /// if Some(42).is_some() {}
     /// if Poll::Pending::<()>.is_pending() {}
     /// if Poll::Ready(42).is_ready() {}
+    /// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+    /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
     /// Ok::<i32, i32>(42).is_ok();
     /// ```
     pub REDUNDANT_PATTERN_MATCHING,
@@ -1577,6 +1583,10 @@ mod redundant_pattern_match {
                         "is_some()"
                     } else if match_qpath(path, &paths::POLL_READY) {
                         "is_ready()"
+                    } else if match_qpath(path, &paths::IPADDR_V4) {
+                        "is_ipv4()"
+                    } else if match_qpath(path, &paths::IPADDR_V6) {
+                        "is_ipv6()"
                     } else {
                         return;
                     }
@@ -1657,6 +1667,17 @@ mod redundant_pattern_match {
                             "is_ok()",
                             "is_err()",
                         )
+                        .or_else(|| {
+                            find_good_method_for_match(
+                                arms,
+                                path_left,
+                                path_right,
+                                &paths::IPADDR_V4,
+                                &paths::IPADDR_V6,
+                                "is_ipv4()",
+                                "is_ipv6()",
+                            )
+                        })
                     } else {
                         None
                     }

diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
@@ -58,6 +58,8 @@ pub const INTO: [&str; 3] = ["core", "convert", "Into"];
 pub const INTO_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "IntoIterator"];
 pub const IO_READ: [&str; 3] = ["std", "io", "Read"];
 pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"];
+pub const IPADDR_V4: [&str; 4] = ["std", "net", "IpAddr", "V4"];
+pub const IPADDR_V6: [&str; 4] = ["std", "net", "IpAddr", "V6"];
 pub const ITERATOR: [&str; 5] = ["core", "iter", "traits", "iterator", "Iterator"];
 pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
 pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"];

diff --git a/tests/ui/redundant_pattern_matching_ipaddr.fixed b/tests/ui/redundant_pattern_matching_ipaddr.fixed
@@ -0,0 +1,73 @@
+// run-rustfix
+
+#![warn(clippy::all)]
+#![warn(clippy::redundant_pattern_matching)]
+#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
+
+use std::net::{
+    IpAddr::{self, V4, V6},
+    Ipv4Addr, Ipv6Addr,
+};
+
+fn main() {
+    let ipaddr: IpAddr = V4(Ipv4Addr::LOCALHOST);
+    if ipaddr.is_ipv4() {}
+
+    if V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+
+    if V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
+
+    while V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+
+    while V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
+
+    if V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+
+    if V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
+
+    if let V4(ipaddr) = V4(Ipv4Addr::LOCALHOST) {
+        println!("{}", ipaddr);
+    }
+
+    V4(Ipv4Addr::LOCALHOST).is_ipv4();
+
+    V4(Ipv4Addr::LOCALHOST).is_ipv6();
+
+    V6(Ipv6Addr::LOCALHOST).is_ipv6();
+
+    V6(Ipv6Addr::LOCALHOST).is_ipv4();
+
+    let _ = if V4(Ipv4Addr::LOCALHOST).is_ipv4() {
+        true
+    } else {
+        false
+    };
+
+    ipaddr_const();
+
+    let _ = if gen_ipaddr().is_ipv4() {
+        1
+    } else if gen_ipaddr().is_ipv6() {
+        2
+    } else {
+        3
+    };
+}
+
+fn gen_ipaddr() -> IpAddr {
+    V4(Ipv4Addr::LOCALHOST)
+}
+
+const fn ipaddr_const() {
+    if V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+
+    if V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
+
+    while V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+
+    while V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
+
+    V4(Ipv4Addr::LOCALHOST).is_ipv4();
+
+    V6(Ipv6Addr::LOCALHOST).is_ipv6();
+}
diff --git a/tests/ui/redundant_pattern_matching_ipaddr.rs b/tests/ui/redundant_pattern_matching_ipaddr.rs
@@ -0,0 +1,91 @@
+// run-rustfix
+
+#![warn(clippy::all)]
+#![warn(clippy::redundant_pattern_matching)]
+#![allow(unused_must_use, clippy::needless_bool, clippy::match_like_matches_macro)]
+
+use std::net::{
+    IpAddr::{self, V4, V6},
+    Ipv4Addr, Ipv6Addr,
+};
+
+fn main() {
+    let ipaddr: IpAddr = V4(Ipv4Addr::LOCALHOST);
+    if let V4(_) = &ipaddr {}
+
+    if let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+
+    if let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+
+    while let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+
+    while let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+
+    if V4(Ipv4Addr::LOCALHOST).is_ipv4() {}
+
+    if V6(Ipv6Addr::LOCALHOST).is_ipv6() {}
+
+    if let V4(ipaddr) = V4(Ipv4Addr::LOCALHOST) {
+        println!("{}", ipaddr);
+    }
+
+    match V4(Ipv4Addr::LOCALHOST) {
+        V4(_) => true,
+        V6(_) => false,
+    };
+
+    match V4(Ipv4Addr::LOCALHOST) {
+        V4(_) => false,
+        V6(_) => true,
+    };
+
+    match V6(Ipv6Addr::LOCALHOST) {
+        V4(_) => false,
+        V6(_) => true,
+    };
+
+    match V6(Ipv6Addr::LOCALHOST) {
+        V4(_) => true,
+        V6(_) => false,
+    };
+
+    let _ = if let V4(_) = V4(Ipv4Addr::LOCALHOST) {
+        true
+    } else {
+        false
+    };
+
+    ipaddr_const();
+
+    let _ = if let V4(_) = gen_ipaddr() {
+        1
+    } else if let V6(_) = gen_ipaddr() {
+        2
+    } else {
+        3
+    };
+}
+
+fn gen_ipaddr() -> IpAddr {
+    V4(Ipv4Addr::LOCALHOST)
+}
+
+const fn ipaddr_const() {
+    if let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+
+    if let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+
+    while let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+
+    while let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+
+    match V4(Ipv4Addr::LOCALHOST) {
+        V4(_) => true,
+        V6(_) => false,
+    };
+
+    match V6(Ipv6Addr::LOCALHOST) {
+        V4(_) => false,
+        V6(_) => true,
+    };
+}
diff --git a/tests/ui/redundant_pattern_matching_ipaddr.stderr b/tests/ui/redundant_pattern_matching_ipaddr.stderr
@@ -0,0 +1,130 @@
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:14:12
+   |
+LL |     if let V4(_) = &ipaddr {}
+   |     -------^^^^^---------- help: try this: `if ipaddr.is_ipv4()`
+   |
+   = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:16:12
+   |
+LL |     if let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+   |     -------^^^^^-------------------------- help: try this: `if V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:18:12
+   |
+LL |     if let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+   |     -------^^^^^-------------------------- help: try this: `if V6(Ipv6Addr::LOCALHOST).is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:20:15
+   |
+LL |     while let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+   |     ----------^^^^^-------------------------- help: try this: `while V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:22:15
+   |
+LL |     while let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+   |     ----------^^^^^-------------------------- help: try this: `while V6(Ipv6Addr::LOCALHOST).is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:32:5
+   |
+LL | /     match V4(Ipv4Addr::LOCALHOST) {
+LL | |         V4(_) => true,
+LL | |         V6(_) => false,
+LL | |     };
+   | |_____^ help: try this: `V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:37:5
+   |
+LL | /     match V4(Ipv4Addr::LOCALHOST) {
+LL | |         V4(_) => false,
+LL | |         V6(_) => true,
+LL | |     };
+   | |_____^ help: try this: `V4(Ipv4Addr::LOCALHOST).is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:42:5
+   |
+LL | /     match V6(Ipv6Addr::LOCALHOST) {
+LL | |         V4(_) => false,
+LL | |         V6(_) => true,
+LL | |     };
+   | |_____^ help: try this: `V6(Ipv6Addr::LOCALHOST).is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:47:5
+   |
+LL | /     match V6(Ipv6Addr::LOCALHOST) {
+LL | |         V4(_) => true,
+LL | |         V6(_) => false,
+LL | |     };
+   | |_____^ help: try this: `V6(Ipv6Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:52:20
+   |
+LL |     let _ = if let V4(_) = V4(Ipv4Addr::LOCALHOST) {
+   |             -------^^^^^-------------------------- help: try this: `if V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:60:20
+   |
+LL |     let _ = if let V4(_) = gen_ipaddr() {
+   |             -------^^^^^--------------- help: try this: `if gen_ipaddr().is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:62:19
+   |
+LL |     } else if let V6(_) = gen_ipaddr() {
+   |            -------^^^^^--------------- help: try this: `if gen_ipaddr().is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:74:12
+   |
+LL |     if let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+   |     -------^^^^^-------------------------- help: try this: `if V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:76:12
+   |
+LL |     if let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+   |     -------^^^^^-------------------------- help: try this: `if V6(Ipv6Addr::LOCALHOST).is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:78:15
+   |
+LL |     while let V4(_) = V4(Ipv4Addr::LOCALHOST) {}
+   |     ----------^^^^^-------------------------- help: try this: `while V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:80:15
+   |
+LL |     while let V6(_) = V6(Ipv6Addr::LOCALHOST) {}
+   |     ----------^^^^^-------------------------- help: try this: `while V6(Ipv6Addr::LOCALHOST).is_ipv6()`
+
+error: redundant pattern matching, consider using `is_ipv4()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:82:5
+   |
+LL | /     match V4(Ipv4Addr::LOCALHOST) {
+LL | |         V4(_) => true,
+LL | |         V6(_) => false,
+LL | |     };
+   | |_____^ help: try this: `V4(Ipv4Addr::LOCALHOST).is_ipv4()`
+
+error: redundant pattern matching, consider using `is_ipv6()`
+  --> $DIR/redundant_pattern_matching_ipaddr.rs:87:5
+   |
+LL | /     match V6(Ipv6Addr::LOCALHOST) {
+LL | |         V4(_) => false,
+LL | |         V6(_) => true,
+LL | |     };
+   | |_____^ help: try this: `V6(Ipv6Addr::LOCALHOST).is_ipv6()`
+
+error: aborting due to 18 previous errors
+
diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed
@@ -37,8 +37,7 @@ fn main() {
     let _ = None::<()>.is_none();
 
     let opt = Some(false);
-    let x = if opt.is_some() { true } else { false };
-    takes_bool(x);
+    let _ = if opt.is_some() { true } else { false };
 
     issue6067();
 
@@ -55,8 +54,6 @@ fn gen_opt() -> Option<()> {
     None
 }
 
-fn takes_bool(_: bool) {}
-
 fn foo() {}
 
 fn bar() {}