Auto merge of #13443 - SpriteOvO:simplify-option-neg-methods, r=xFrednet

Simplify negative `Option::{is_some_and,is_none_or}`

Closes #13436.

Improved based on the existing lint `nonminimal_bool`, since there is already handling of similar methods `Option::{is_some,is_none}` and `Result::{is_ok,is_err}`, and there is a lot of reusable code.

When `is_some_and` or `is_none_or` have a negation, we invert it into another method by removing the Not sign and inverting the expression in the closure.

For the case where the closure block has statements, currently no simplification is implemented. (Should we do it?)

```rust
// Currently will not simplify this
_ = !opt.is_some_and(|x| {
    let complex_block = 100;
    x == complex_block
});
```

changelog: [`nonminimal_bool`]: Simplify negative `Option::{is_some_and,is_none_or}`
This commit is contained in:
bors 2024-10-02 08:54:12 +00:00
commit 398be8c842
8 changed files with 321 additions and 20 deletions

View File

@ -19,6 +19,7 @@ macro_rules! msrv_aliases {
msrv_aliases! {
1,83,0 { CONST_EXTERN_FN }
1,83,0 { CONST_FLOAT_BITS_CONV }
1,82,0 { IS_NONE_OR }
1,81,0 { LINT_REASONS_STABILIZATION }
1,80,0 { BOX_INTO_ITER}
1,77,0 { C_STR_LITERALS }

View File

@ -1,3 +1,5 @@
use clippy_config::Conf;
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::eq_expr_value;
use clippy_utils::source::SpanRangeExt;
@ -7,7 +9,7 @@
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr};
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp};
use rustc_lint::{LateContext, LateLintPass, Level};
use rustc_session::declare_lint_pass;
use rustc_session::{RustcVersion, impl_lint_pass};
use rustc_span::def_id::LocalDefId;
use rustc_span::{Span, sym};
@ -69,9 +71,25 @@
}
// For each pairs, both orders are considered.
const METHODS_WITH_NEGATION: [(&str, &str); 2] = [("is_some", "is_none"), ("is_err", "is_ok")];
const METHODS_WITH_NEGATION: [(Option<RustcVersion>, &str, &str); 3] = [
(None, "is_some", "is_none"),
(None, "is_err", "is_ok"),
(Some(msrvs::IS_NONE_OR), "is_some_and", "is_none_or"),
];
declare_lint_pass!(NonminimalBool => [NONMINIMAL_BOOL, OVERLY_COMPLEX_BOOL_EXPR]);
pub struct NonminimalBool {
msrv: Msrv,
}
impl NonminimalBool {
pub fn new(conf: &'static Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}
impl_lint_pass!(NonminimalBool => [NONMINIMAL_BOOL, OVERLY_COMPLEX_BOOL_EXPR]);
impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
fn check_fn(
@ -83,7 +101,7 @@ fn check_fn(
_: Span,
_: LocalDefId,
) {
NonminimalBoolVisitor { cx }.visit_body(body);
NonminimalBoolVisitor { cx, msrv: &self.msrv }.visit_body(body);
}
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
@ -100,6 +118,8 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
_ => {},
}
}
extract_msrv_attr!(LateContext);
}
fn inverted_bin_op_eq_str(op: BinOpKind) -> Option<&'static str> {
@ -176,11 +196,11 @@ fn check_inverted_bool_in_condition(
);
}
fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
fn check_simplify_not(cx: &LateContext<'_>, msrv: &Msrv, expr: &Expr<'_>) {
if let ExprKind::Unary(UnOp::Not, inner) = &expr.kind
&& !expr.span.from_expansion()
&& !inner.span.from_expansion()
&& let Some(suggestion) = simplify_not(cx, inner)
&& let Some(suggestion) = simplify_not(cx, msrv, inner)
&& cx.tcx.lint_level_at_node(NONMINIMAL_BOOL, expr.hir_id).0 != Level::Allow
{
span_lint_and_sugg(
@ -197,6 +217,7 @@ fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
struct NonminimalBoolVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
msrv: &'a Msrv,
}
use quine_mc_cluskey::Bool;
@ -289,6 +310,7 @@ fn negate(bin_op_kind: BinOpKind) -> Option<BinOpKind> {
struct SuggestContext<'a, 'tcx, 'v> {
terminals: &'v [&'v Expr<'v>],
cx: &'a LateContext<'tcx>,
msrv: &'a Msrv,
output: String,
}
@ -311,7 +333,7 @@ fn recurse(&mut self, suggestion: &Bool) -> Option<()> {
},
Term(n) => {
let terminal = self.terminals[n as usize];
if let Some(str) = simplify_not(self.cx, terminal) {
if let Some(str) = simplify_not(self.cx, self.msrv, terminal) {
self.output.push_str(&str);
} else {
self.output.push('!');
@ -358,7 +380,7 @@ fn recurse(&mut self, suggestion: &Bool) -> Option<()> {
}
}
fn simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
fn simplify_not(cx: &LateContext<'_>, curr_msrv: &Msrv, expr: &Expr<'_>) -> Option<String> {
match &expr.kind {
ExprKind::Binary(binop, lhs, rhs) => {
if !implements_ord(cx, lhs) {
@ -389,7 +411,7 @@ fn simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
Some(format!("{lhs_snippet}{op}{rhs_snippet}"))
})
},
ExprKind::MethodCall(path, receiver, [], _) => {
ExprKind::MethodCall(path, receiver, args, _) => {
let type_of_receiver = cx.typeck_results().expr_ty(receiver);
if !is_type_diagnostic_item(cx, type_of_receiver, sym::Option)
&& !is_type_diagnostic_item(cx, type_of_receiver, sym::Result)
@ -399,21 +421,41 @@ fn simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
METHODS_WITH_NEGATION
.iter()
.copied()
.flat_map(|(a, b)| vec![(a, b), (b, a)])
.find(|&(a, _)| {
let path: &str = path.ident.name.as_str();
a == path
.flat_map(|(msrv, a, b)| vec![(msrv, a, b), (msrv, b, a)])
.find(|&(msrv, a, _)| msrv.is_none_or(|msrv| curr_msrv.meets(msrv)) && a == path.ident.name.as_str())
.and_then(|(_, _, neg_method)| {
let negated_args = args
.iter()
.map(|arg| simplify_not(cx, curr_msrv, arg))
.collect::<Option<Vec<_>>>()?
.join(", ");
Some(format!(
"{}.{neg_method}({negated_args})",
receiver.span.get_source_text(cx)?
))
})
.and_then(|(_, neg_method)| Some(format!("{}.{neg_method}()", receiver.span.get_source_text(cx)?)))
},
ExprKind::Closure(closure) => {
let body = cx.tcx.hir().body(closure.body);
let params = body
.params
.iter()
.map(|param| param.span.get_source_text(cx).map(|t| t.to_string()))
.collect::<Option<Vec<_>>>()?
.join(", ");
let negated = simplify_not(cx, curr_msrv, body.value)?;
Some(format!("|{params}| {negated}"))
},
ExprKind::Unary(UnOp::Not, expr) => expr.span.get_source_text(cx).map(|t| t.to_string()),
_ => None,
}
}
fn suggest(cx: &LateContext<'_>, suggestion: &Bool, terminals: &[&Expr<'_>]) -> String {
fn suggest(cx: &LateContext<'_>, msrv: &Msrv, suggestion: &Bool, terminals: &[&Expr<'_>]) -> String {
let mut suggest_context = SuggestContext {
terminals,
cx,
msrv,
output: String::new(),
};
suggest_context.recurse(suggestion);
@ -526,7 +568,7 @@ fn bool_expr(&self, e: &'tcx Expr<'_>) {
diag.span_suggestion(
e.span,
"it would look like the following",
suggest(self.cx, suggestion, &h2q.terminals),
suggest(self.cx, self.msrv, suggestion, &h2q.terminals),
// nonminimal_bool can produce minimal but
// not human readable expressions (#3141)
Applicability::Unspecified,
@ -569,12 +611,12 @@ fn bool_expr(&self, e: &'tcx Expr<'_>) {
}
};
if improvements.is_empty() {
check_simplify_not(self.cx, e);
check_simplify_not(self.cx, self.msrv, e);
} else {
nonminimal_bool_lint(
improvements
.into_iter()
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals))
.map(|suggestion| suggest(self.cx, self.msrv, suggestion, &h2q.terminals))
.collect(),
);
}

View File

@ -609,7 +609,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |tcx| Box::new(await_holding_invalid::AwaitHolding::new(tcx, conf)));
store.register_late_pass(|_| Box::new(serde_api::SerdeApi));
store.register_late_pass(move |_| Box::new(types::Types::new(conf)));
store.register_late_pass(|_| Box::new(booleans::NonminimalBool));
store.register_late_pass(move |_| Box::new(booleans::NonminimalBool::new(conf)));
store.register_late_pass(|_| Box::new(enum_clike::UnportableVariant));
store.register_late_pass(|_| Box::new(float_literal::FloatLiteral));
store.register_late_pass(|_| Box::new(ptr::Ptr));

View File

@ -115,4 +115,66 @@ fn issue_12625() {
if a as u64 > b {} //~ ERROR: this boolean expression can be simplified
}
fn issue_13436() {
fn not_zero(x: i32) -> bool {
x != 0
}
let opt = Some(500);
_ = opt.is_some_and(|x| x < 1000);
_ = opt.is_some_and(|x| x <= 1000);
_ = opt.is_some_and(|x| x > 1000);
_ = opt.is_some_and(|x| x >= 1000);
_ = opt.is_some_and(|x| x == 1000);
_ = opt.is_some_and(|x| x != 1000);
_ = opt.is_some_and(not_zero);
_ = opt.is_none_or(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(not_zero);
_ = opt.is_none_or(|x| x < 1000);
_ = opt.is_none_or(|x| x <= 1000);
_ = opt.is_none_or(|x| x > 1000);
_ = opt.is_none_or(|x| x >= 1000);
_ = opt.is_none_or(|x| x == 1000);
_ = opt.is_none_or(|x| x != 1000);
_ = opt.is_none_or(not_zero);
_ = opt.is_some_and(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(not_zero);
let opt = Some(true);
_ = opt.is_some_and(|x| x);
_ = opt.is_some_and(|x| !x);
_ = !opt.is_some_and(|x| x);
_ = opt.is_none_or(|x| x); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x);
_ = opt.is_none_or(|x| !x);
_ = !opt.is_none_or(|x| x);
_ = opt.is_some_and(|x| x); //~ ERROR: this boolean expression can be simplified
let opt: Option<Result<i32, i32>> = Some(Ok(123));
_ = opt.is_some_and(|x| x.is_ok());
_ = opt.is_some_and(|x| x.is_err());
_ = opt.is_none_or(|x| x.is_ok());
_ = opt.is_none_or(|x| x.is_err());
_ = opt.is_none_or(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
_ = opt.is_some_and(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
#[clippy::msrv = "1.81"]
fn before_stabilization() {
let opt = Some(500);
_ = !opt.is_some_and(|x| x < 1000);
}
}
fn main() {}

View File

@ -115,4 +115,66 @@ fn issue_12625() {
if !(a as u64 <= b) {} //~ ERROR: this boolean expression can be simplified
}
fn issue_13436() {
fn not_zero(x: i32) -> bool {
x != 0
}
let opt = Some(500);
_ = opt.is_some_and(|x| x < 1000);
_ = opt.is_some_and(|x| x <= 1000);
_ = opt.is_some_and(|x| x > 1000);
_ = opt.is_some_and(|x| x >= 1000);
_ = opt.is_some_and(|x| x == 1000);
_ = opt.is_some_and(|x| x != 1000);
_ = opt.is_some_and(not_zero);
_ = !opt.is_some_and(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(not_zero);
_ = opt.is_none_or(|x| x < 1000);
_ = opt.is_none_or(|x| x <= 1000);
_ = opt.is_none_or(|x| x > 1000);
_ = opt.is_none_or(|x| x >= 1000);
_ = opt.is_none_or(|x| x == 1000);
_ = opt.is_none_or(|x| x != 1000);
_ = opt.is_none_or(not_zero);
_ = !opt.is_none_or(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(not_zero);
let opt = Some(true);
_ = opt.is_some_and(|x| x);
_ = opt.is_some_and(|x| !x);
_ = !opt.is_some_and(|x| x);
_ = !opt.is_some_and(|x| !x); //~ ERROR: this boolean expression can be simplified
_ = opt.is_none_or(|x| x);
_ = opt.is_none_or(|x| !x);
_ = !opt.is_none_or(|x| x);
_ = !opt.is_none_or(|x| !x); //~ ERROR: this boolean expression can be simplified
let opt: Option<Result<i32, i32>> = Some(Ok(123));
_ = opt.is_some_and(|x| x.is_ok());
_ = opt.is_some_and(|x| x.is_err());
_ = opt.is_none_or(|x| x.is_ok());
_ = opt.is_none_or(|x| x.is_err());
_ = !opt.is_some_and(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_some_and(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
_ = !opt.is_none_or(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
#[clippy::msrv = "1.81"]
fn before_stabilization() {
let opt = Some(500);
_ = !opt.is_some_and(|x| x < 1000);
}
}
fn main() {}

View File

@ -97,5 +97,113 @@ error: this boolean expression can be simplified
LL | if !(a as u64 <= b) {}
| ^^^^^^^^^^^^^^^^ help: try: `a as u64 > b`
error: aborting due to 16 previous errors
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:131:9
|
LL | _ = !opt.is_some_and(|x| x < 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x >= 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:132:9
|
LL | _ = !opt.is_some_and(|x| x <= 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x > 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:133:9
|
LL | _ = !opt.is_some_and(|x| x > 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x <= 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:134:9
|
LL | _ = !opt.is_some_and(|x| x >= 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x < 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:135:9
|
LL | _ = !opt.is_some_and(|x| x == 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x != 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:136:9
|
LL | _ = !opt.is_some_and(|x| x != 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x == 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:145:9
|
LL | _ = !opt.is_none_or(|x| x < 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x >= 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:146:9
|
LL | _ = !opt.is_none_or(|x| x <= 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x > 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:147:9
|
LL | _ = !opt.is_none_or(|x| x > 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x <= 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:148:9
|
LL | _ = !opt.is_none_or(|x| x >= 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x < 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:149:9
|
LL | _ = !opt.is_none_or(|x| x == 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x != 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:150:9
|
LL | _ = !opt.is_none_or(|x| x != 1000);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x == 1000)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:157:9
|
LL | _ = !opt.is_some_and(|x| !x);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:161:9
|
LL | _ = !opt.is_none_or(|x| !x);
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x)`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:168:9
|
LL | _ = !opt.is_some_and(|x| x.is_ok());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x.is_err())`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:169:9
|
LL | _ = !opt.is_some_and(|x| x.is_err());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_none_or(|x| x.is_ok())`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:170:9
|
LL | _ = !opt.is_none_or(|x| x.is_ok());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x.is_err())`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods.rs:171:9
|
LL | _ = !opt.is_none_or(|x| x.is_err());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.is_some_and(|x| x.is_ok())`
error: aborting due to 34 previous errors

View File

@ -0,0 +1,9 @@
#![warn(clippy::nonminimal_bool)]
//@no-rustfix
fn issue_13436() {
let opt_opt = Some(Some(500));
_ = !opt_opt.is_some_and(|x| !x.is_some_and(|y| y != 1000)); //~ ERROR: this boolean expression can be simplified
}
fn main() {}

View File

@ -0,0 +1,17 @@
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods_unfixable.rs:6:9
|
LL | _ = !opt_opt.is_some_and(|x| !x.is_some_and(|y| y != 1000));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt_opt.is_none_or(|x| x.is_some_and(|y| y != 1000))`
|
= note: `-D clippy::nonminimal-bool` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]`
error: this boolean expression can be simplified
--> tests/ui/nonminimal_bool_methods_unfixable.rs:6:34
|
LL | _ = !opt_opt.is_some_and(|x| !x.is_some_and(|y| y != 1000));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.is_none_or(|y| y == 1000)`
error: aborting due to 2 previous errors