Skip to content

Commit

Permalink
Auto merge of rust-lang#6377 - CDirkx:redundant-pattern-match-ipaddr,…
Browse files Browse the repository at this point in the history
… r=ebroto

Enhance `redundant_pattern_matching` to also lint on `std::net::IpAddr`

Follow-up to rust-lang#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`
  • Loading branch information
bors committed Nov 28, 2020
2 parents bc723c5 + dc075b4 commit 68cf94f
Show file tree
Hide file tree
Showing 11 changed files with 341 additions and 36 deletions.
25 changes: 23 additions & 2 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down
73 changes: 73 additions & 0 deletions tests/ui/redundant_pattern_matching_ipaddr.fixed
Original file line number Diff line number Diff line change
@@ -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();
}
91 changes: 91 additions & 0 deletions tests/ui/redundant_pattern_matching_ipaddr.rs
Original file line number Diff line number Diff line change
@@ -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,
};
}
130 changes: 130 additions & 0 deletions tests/ui/redundant_pattern_matching_ipaddr.stderr
Original file line number Diff line number Diff line change
@@ -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

5 changes: 1 addition & 4 deletions tests/ui/redundant_pattern_matching_option.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -55,8 +54,6 @@ fn gen_opt() -> Option<()> {
None
}

fn takes_bool(_: bool) {}

fn foo() {}

fn bar() {}
Expand Down
Loading

0 comments on commit 68cf94f

Please sign in to comment.