Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rustup #6071

Merged
merged 8 commits into from
Sep 21, 2020
Merged

Rustup #6071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
10d272b
Update Clippy testcases
CDirkx Sep 20, 2020
292e2f7
Remove `can_suggest` check for `is_ok` and `is_err`.
CDirkx Sep 20, 2020
ac0d069
Update src/tools/clippy/clippy_lints/src/matches.rs
CDirkx Sep 20, 2020
a334ae6
Auto merge of #76136 - CDirkx:const-result, r=dtolnay
bors Sep 20, 2020
ed43385
Update Clippy testcases
CDirkx Sep 20, 2020
141b9c2
Remove `can_suggest` from Clippy.
CDirkx Sep 20, 2020
6e07247
Merge remote-tracking branch 'upstream/master' into rustup
ebroto Sep 21, 2020
4117ae1
Split redundant_pattern_matching tests
ebroto Sep 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 16 additions & 62 deletions clippy_lints/src/matches.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1440,15 +1440,12 @@ where

mod redundant_pattern_match {
use super::REDUNDANT_PATTERN_MATCHING;
use crate::utils::{in_constant, match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Expr, ExprKind, HirId, MatchSource, PatKind, QPath};
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_mir::const_eval::is_const_fn;
use rustc_span::source_map::Symbol;

pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
Expand All @@ -1468,37 +1465,24 @@ mod redundant_pattern_match {
arms: &[Arm<'_>],
keyword: &'static str,
) {
fn find_suggestion(cx: &LateContext<'_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> {
if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") {
return Some("is_ok()");
}
if match_qpath(path, &paths::RESULT_ERR) && can_suggest(cx, hir_id, sym!(result_type), "is_err") {
return Some("is_err()");
}
if match_qpath(path, &paths::OPTION_SOME) && can_suggest(cx, hir_id, sym!(option_type), "is_some") {
return Some("is_some()");
}
if match_qpath(path, &paths::OPTION_NONE) && can_suggest(cx, hir_id, sym!(option_type), "is_none") {
return Some("is_none()");
}
None
}

let hir_id = expr.hir_id;
let good_method = match arms[0].pat.kind {
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
if let PatKind::Wild = patterns[0].kind {
find_suggestion(cx, hir_id, path)
if match_qpath(path, &paths::RESULT_OK) {
"is_ok()"
} else if match_qpath(path, &paths::RESULT_ERR) {
"is_err()"
} else if match_qpath(path, &paths::OPTION_SOME) {
"is_some()"
} else {
return;
}
} else {
None
return;
}
},
PatKind::Path(ref path) => find_suggestion(cx, hir_id, path),
_ => None,
};
let good_method = match good_method {
Some(method) => method,
None => return,
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
_ => return,
};

// check that `while_let_on_iterator` lint does not trigger
Expand Down Expand Up @@ -1547,7 +1531,6 @@ mod redundant_pattern_match {
if arms.len() == 2 {
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);

let hir_id = expr.hir_id;
let found_good_method = match node_pair {
(
PatKind::TupleStruct(ref path_left, ref patterns_left, _),
Expand All @@ -1562,8 +1545,6 @@ mod redundant_pattern_match {
&paths::RESULT_ERR,
"is_ok()",
"is_err()",
|| can_suggest(cx, hir_id, sym!(result_type), "is_ok"),
|| can_suggest(cx, hir_id, sym!(result_type), "is_err"),
)
} else {
None
Expand All @@ -1582,8 +1563,6 @@ mod redundant_pattern_match {
&paths::OPTION_NONE,
"is_some()",
"is_none()",
|| can_suggest(cx, hir_id, sym!(option_type), "is_some"),
|| can_suggest(cx, hir_id, sym!(option_type), "is_none"),
)
} else {
None
Expand Down Expand Up @@ -1616,7 +1595,6 @@ mod redundant_pattern_match {
}
}

#[allow(clippy::too_many_arguments)]
fn find_good_method_for_match<'a>(
arms: &[Arm<'_>],
path_left: &QPath<'_>,
Expand All @@ -1625,8 +1603,6 @@ mod redundant_pattern_match {
expected_right: &[&str],
should_be_left: &'a str,
should_be_right: &'a str,
can_suggest_left: impl Fn() -> bool,
can_suggest_right: impl Fn() -> bool,
) -> Option<&'a str> {
let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
(&(*arms[0].body).kind, &(*arms[1].body).kind)
Expand All @@ -1638,35 +1614,13 @@ mod redundant_pattern_match {

match body_node_pair {
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) if can_suggest_left() => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) if can_suggest_right() => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
_ => None,
},
_ => None,
}
}

fn can_suggest(cx: &LateContext<'_>, hir_id: HirId, diag_item: Symbol, name: &str) -> bool {
if !in_constant(cx, hir_id) {
return true;
}

// Avoid suggesting calls to non-`const fn`s in const contexts, see #5697.
cx.tcx
.get_diagnostic_item(diag_item)
.and_then(|def_id| {
cx.tcx.inherent_impls(def_id).iter().find_map(|imp| {
cx.tcx
.associated_items(*imp)
.in_definition_order()
.find_map(|item| match item.kind {
ty::AssocKind::Fn if item.ident.name.as_str() == name => Some(item.def_id),
_ => None,
})
})
})
.map_or(false, |def_id| is_const_fn(cx.tcx, def_id))
}
}

#[test]
Expand Down
99 changes: 14 additions & 85 deletions tests/ui/redundant_pattern_matching.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,39 +18,14 @@ fn main() {

if Err::<i32, i32>(42).is_err() {}

if None::<()>.is_none() {}

if Some(42).is_some() {}

if Some(42).is_some() {
foo();
} else {
bar();
}

while Some(42).is_some() {}

while Some(42).is_none() {}

while None::<()>.is_none() {}

while Ok::<i32, i32>(10).is_ok() {}

while Ok::<i32, i32>(10).is_err() {}

let mut v = vec![1, 2, 3];
while v.pop().is_some() {
foo();
}

if Ok::<i32, i32>(42).is_ok() {}

if Err::<i32, i32>(42).is_err() {}

if None::<i32>.is_none() {}

if Some(42).is_some() {}

if let Ok(x) = Ok::<i32, i32>(42) {
println!("{}", x);
}
Expand All @@ -63,48 +38,24 @@ fn main() {

Err::<i32, i32>(42).is_ok();

Some(42).is_some();

None::<()>.is_none();

let _ = None::<()>.is_none();

let _ = if Ok::<usize, ()>(4).is_ok() { true } else { false };

let opt = Some(false);
let x = if opt.is_some() { true } else { false };
takes_bool(x);

issue5504();
issue5697();
issue6067();

let _ = if gen_opt().is_some() {
let _ = if gen_res().is_ok() {
1
} else if gen_opt().is_none() {
2
} else if gen_res().is_ok() {
3
} else if gen_res().is_err() {
4
2
} else {
5
3
};
}

fn gen_opt() -> Option<()> {
None
}

fn gen_res() -> Result<(), ()> {
Ok(())
}

fn takes_bool(_: bool) {}

fn foo() {}

fn bar() {}

macro_rules! m {
() => {
Some(42u32)
Expand All @@ -128,41 +79,19 @@ fn issue5504() {
while m!().is_some() {}
}

// None of these should be linted because none of the suggested methods
// are `const fn` without toggling a feature.
const fn issue5697() {
if let Ok(_) = Ok::<i32, i32>(42) {}

if let Err(_) = Err::<i32, i32>(42) {}

if let Some(_) = Some(42) {}

if let None = None::<()> {}

while let Ok(_) = Ok::<i32, i32>(10) {}

while let Err(_) = Ok::<i32, i32>(10) {}
// Methods that are unstable const should not be suggested within a const context, see issue #5697.
// However, in Rust 1.48.0 the methods `is_ok` and `is_err` of `Result` were stabilized as const,
// so the following should be linted.
const fn issue6067() {
if Ok::<i32, i32>(42).is_ok() {}

while let Some(_) = Some(42) {}
if Err::<i32, i32>(42).is_err() {}

while let None = None::<()> {}
while Ok::<i32, i32>(10).is_ok() {}

match Ok::<i32, i32>(42) {
Ok(_) => true,
Err(_) => false,
};
while Ok::<i32, i32>(10).is_err() {}

match Err::<i32, i32>(42) {
Ok(_) => false,
Err(_) => true,
};
match Some(42) {
Some(_) => true,
None => false,
};
Ok::<i32, i32>(42).is_ok();

match None::<()> {
Some(_) => false,
None => true,
};
Err::<i32, i32>(42).is_err();
}
Loading