Skip to content

Commit 762a91b

Browse files
SpriteOvOSpriteOvO
committed
Simplify negative Option::{is_some_and,is_none_or} 
1 parent b39323b commit 762a91b

8 files changed

+321
-20
lines changed

Diff for: ‎clippy_config/src/msrvs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ macro_rules! msrv_aliases {
1919
msrv_aliases! {
2020
1,83,0 { CONST_EXTERN_FN }
2121
1,83,0 { CONST_FLOAT_BITS_CONV }
22+
1,82,0 { IS_NONE_OR }
2223
1,81,0 { LINT_REASONS_STABILIZATION }
2324
1,80,0 { BOX_INTO_ITER}
2425
1,77,0 { C_STR_LITERALS }

Diff for: ‎clippy_lints/src/booleans.rs

+60-18
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use clippy_config::Conf;
2+
use clippy_config::msrvs::{self, Msrv};
13
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
24
use clippy_utils::eq_expr_value;
35
use clippy_utils::source::SpanRangeExt;
@@ -7,7 +9,7 @@ use rustc_errors::Applicability;
79
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr};
810
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, UnOp};
911
use rustc_lint::{LateContext, LateLintPass, Level};
10-
use rustc_session::declare_lint_pass;
12+
use rustc_session::{RustcVersion, impl_lint_pass};
1113
use rustc_span::def_id::LocalDefId;
1214
use rustc_span::{Span, sym};
1315

@@ -69,9 +71,25 @@ declare_clippy_lint! {
6971
}
7072

7173
// For each pairs, both orders are considered.
72-
const METHODS_WITH_NEGATION: [(&str, &str); 2] = [("is_some", "is_none"), ("is_err", "is_ok")];
74+
const METHODS_WITH_NEGATION: [(Option<RustcVersion>, &str, &str); 3] = [
75+
(None, "is_some", "is_none"),
76+
(None, "is_err", "is_ok"),
77+
(Some(msrvs::IS_NONE_OR), "is_some_and", "is_none_or"),
78+
];
79+
80+
pub struct NonminimalBool {
81+
msrv: Msrv,
82+
}
83+
84+
impl NonminimalBool {
85+
pub fn new(conf: &'static Conf) -> Self {
86+
Self {
87+
msrv: conf.msrv.clone(),
88+
}
89+
}
90+
}
7391

74-
declare_lint_pass!(NonminimalBool => [NONMINIMAL_BOOL, OVERLY_COMPLEX_BOOL_EXPR]);
92+
impl_lint_pass!(NonminimalBool => [NONMINIMAL_BOOL, OVERLY_COMPLEX_BOOL_EXPR]);
7593

7694
impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
7795
fn check_fn(
@@ -83,7 +101,7 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
83101
_: Span,
84102
_: LocalDefId,
85103
) {
86-
NonminimalBoolVisitor { cx }.visit_body(body);
104+
NonminimalBoolVisitor { cx, msrv: &self.msrv }.visit_body(body);
87105
}
88106

89107
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
@@ -100,6 +118,8 @@ impl<'tcx> LateLintPass<'tcx> for NonminimalBool {
100118
_ => {},
101119
}
102120
}
121+
122+
extract_msrv_attr!(LateContext);
103123
}
104124

105125
fn inverted_bin_op_eq_str(op: BinOpKind) -> Option<&'static str> {
@@ -176,11 +196,11 @@ fn check_inverted_bool_in_condition(
176196
);
177197
}
178198

179-
fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
199+
fn check_simplify_not(cx: &LateContext<'_>, msrv: &Msrv, expr: &Expr<'_>) {
180200
if let ExprKind::Unary(UnOp::Not, inner) = &expr.kind
181201
&& !expr.span.from_expansion()
182202
&& !inner.span.from_expansion()
183-
&& let Some(suggestion) = simplify_not(cx, inner)
203+
&& let Some(suggestion) = simplify_not(cx, msrv, inner)
184204
&& cx.tcx.lint_level_at_node(NONMINIMAL_BOOL, expr.hir_id).0 != Level::Allow
185205
{
186206
span_lint_and_sugg(
@@ -197,6 +217,7 @@ fn check_simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) {
197217

198218
struct NonminimalBoolVisitor<'a, 'tcx> {
199219
cx: &'a LateContext<'tcx>,
220+
msrv: &'a Msrv,
200221
}
201222

202223
use quine_mc_cluskey::Bool;
@@ -289,6 +310,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
289310
struct SuggestContext<'a, 'tcx, 'v> {
290311
terminals: &'v [&'v Expr<'v>],
291312
cx: &'a LateContext<'tcx>,
313+
msrv: &'a Msrv,
292314
output: String,
293315
}
294316

@@ -311,7 +333,7 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
311333
},
312334
Term(n) => {
313335
let terminal = self.terminals[n as usize];
314-
if let Some(str) = simplify_not(self.cx, terminal) {
336+
if let Some(str) = simplify_not(self.cx, self.msrv, terminal) {
315337
self.output.push_str(&str);
316338
} else {
317339
self.output.push('!');
@@ -358,7 +380,7 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
358380
}
359381
}
360382

361-
fn simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
383+
fn simplify_not(cx: &LateContext<'_>, curr_msrv: &Msrv, expr: &Expr<'_>) -> Option<String> {
362384
match &expr.kind {
363385
ExprKind::Binary(binop, lhs, rhs) => {
364386
if !implements_ord(cx, lhs) {
@@ -389,7 +411,7 @@ fn simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
389411
Some(format!("{lhs_snippet}{op}{rhs_snippet}"))
390412
})
391413
},
392-
ExprKind::MethodCall(path, receiver, [], _) => {
414+
ExprKind::MethodCall(path, receiver, args, _) => {
393415
let type_of_receiver = cx.typeck_results().expr_ty(receiver);
394416
if !is_type_diagnostic_item(cx, type_of_receiver, sym::Option)
395417
&& !is_type_diagnostic_item(cx, type_of_receiver, sym::Result)
@@ -399,21 +421,41 @@ fn simplify_not(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
399421
METHODS_WITH_NEGATION
400422
.iter()
401423
.copied()
402-
.flat_map(|(a, b)| vec![(a, b), (b, a)])
403-
.find(|&(a, _)| {
404-
let path: &str = path.ident.name.as_str();
405-
a == path
424+
.flat_map(|(msrv, a, b)| vec![(msrv, a, b), (msrv, b, a)])
425+
.find(|&(msrv, a, _)| msrv.is_none_or(|msrv| curr_msrv.meets(msrv)) && a == path.ident.name.as_str())
426+
.and_then(|(_, _, neg_method)| {
427+
let negated_args = args
428+
.iter()
429+
.map(|arg| simplify_not(cx, curr_msrv, arg))
430+
.collect::<Option<Vec<_>>>()?
431+
.join(", ");
432+
Some(format!(
433+
"{}.{neg_method}({negated_args})",
434+
receiver.span.get_source_text(cx)?
435+
))
406436
})
407-
.and_then(|(_, neg_method)| Some(format!("{}.{neg_method}()", receiver.span.get_source_text(cx)?)))
408437
},
438+
ExprKind::Closure(closure) => {
439+
let body = cx.tcx.hir().body(closure.body);
440+
let params = body
441+
.params
442+
.iter()
443+
.map(|param| param.span.get_source_text(cx).map(|t| t.to_string()))
444+
.collect::<Option<Vec<_>>>()?
445+
.join(", ");
446+
let negated = simplify_not(cx, curr_msrv, body.value)?;
447+
Some(format!("|{params}| {negated}"))
448+
},
449+
ExprKind::Unary(UnOp::Not, expr) => expr.span.get_source_text(cx).map(|t| t.to_string()),
409450
_ => None,
410451
}
411452
}
412453

413-
fn suggest(cx: &LateContext<'_>, suggestion: &Bool, terminals: &[&Expr<'_>]) -> String {
454+
fn suggest(cx: &LateContext<'_>, msrv: &Msrv, suggestion: &Bool, terminals: &[&Expr<'_>]) -> String {
414455
let mut suggest_context = SuggestContext {
415456
terminals,
416457
cx,
458+
msrv,
417459
output: String::new(),
418460
};
419461
suggest_context.recurse(suggestion);
@@ -526,7 +568,7 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
526568
diag.span_suggestion(
527569
e.span,
528570
"it would look like the following",
529-
suggest(self.cx, suggestion, &h2q.terminals),
571+
suggest(self.cx, self.msrv, suggestion, &h2q.terminals),
530572
// nonminimal_bool can produce minimal but
531573
// not human readable expressions (#3141)
532574
Applicability::Unspecified,
@@ -569,12 +611,12 @@ impl<'a, 'tcx> NonminimalBoolVisitor<'a, 'tcx> {
569611
}
570612
};
571613
if improvements.is_empty() {
572-
check_simplify_not(self.cx, e);
614+
check_simplify_not(self.cx, self.msrv, e);
573615
} else {
574616
nonminimal_bool_lint(
575617
improvements
576618
.into_iter()
577-
.map(|suggestion| suggest(self.cx, suggestion, &h2q.terminals))
619+
.map(|suggestion| suggest(self.cx, self.msrv, suggestion, &h2q.terminals))
578620
.collect(),
579621
);
580622
}

Diff for: ‎clippy_lints/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
609609
store.register_late_pass(move |tcx| Box::new(await_holding_invalid::AwaitHolding::new(tcx, conf)));
610610
store.register_late_pass(|_| Box::new(serde_api::SerdeApi));
611611
store.register_late_pass(move |_| Box::new(types::Types::new(conf)));
612-
store.register_late_pass(|_| Box::new(booleans::NonminimalBool));
612+
store.register_late_pass(move |_| Box::new(booleans::NonminimalBool::new(conf)));
613613
store.register_late_pass(|_| Box::new(enum_clike::UnportableVariant));
614614
store.register_late_pass(|_| Box::new(float_literal::FloatLiteral));
615615
store.register_late_pass(|_| Box::new(ptr::Ptr));

Diff for: ‎tests/ui/nonminimal_bool_methods.fixed

+62
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,66 @@ fn issue_12625() {
115115
if a as u64 > b {} //~ ERROR: this boolean expression can be simplified
116116
}
117117

118+
fn issue_13436() {
119+
fn not_zero(x: i32) -> bool {
120+
x != 0
121+
}
122+
123+
let opt = Some(500);
124+
_ = opt.is_some_and(|x| x < 1000);
125+
_ = opt.is_some_and(|x| x <= 1000);
126+
_ = opt.is_some_and(|x| x > 1000);
127+
_ = opt.is_some_and(|x| x >= 1000);
128+
_ = opt.is_some_and(|x| x == 1000);
129+
_ = opt.is_some_and(|x| x != 1000);
130+
_ = opt.is_some_and(not_zero);
131+
_ = opt.is_none_or(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
132+
_ = opt.is_none_or(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
133+
_ = opt.is_none_or(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
134+
_ = opt.is_none_or(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
135+
_ = opt.is_none_or(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
136+
_ = opt.is_none_or(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
137+
_ = !opt.is_some_and(not_zero);
138+
_ = opt.is_none_or(|x| x < 1000);
139+
_ = opt.is_none_or(|x| x <= 1000);
140+
_ = opt.is_none_or(|x| x > 1000);
141+
_ = opt.is_none_or(|x| x >= 1000);
142+
_ = opt.is_none_or(|x| x == 1000);
143+
_ = opt.is_none_or(|x| x != 1000);
144+
_ = opt.is_none_or(not_zero);
145+
_ = opt.is_some_and(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
146+
_ = opt.is_some_and(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
147+
_ = opt.is_some_and(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
148+
_ = opt.is_some_and(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
149+
_ = opt.is_some_and(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
150+
_ = opt.is_some_and(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
151+
_ = !opt.is_none_or(not_zero);
152+
153+
let opt = Some(true);
154+
_ = opt.is_some_and(|x| x);
155+
_ = opt.is_some_and(|x| !x);
156+
_ = !opt.is_some_and(|x| x);
157+
_ = opt.is_none_or(|x| x); //~ ERROR: this boolean expression can be simplified
158+
_ = opt.is_none_or(|x| x);
159+
_ = opt.is_none_or(|x| !x);
160+
_ = !opt.is_none_or(|x| x);
161+
_ = opt.is_some_and(|x| x); //~ ERROR: this boolean expression can be simplified
162+
163+
let opt: Option<Result<i32, i32>> = Some(Ok(123));
164+
_ = opt.is_some_and(|x| x.is_ok());
165+
_ = opt.is_some_and(|x| x.is_err());
166+
_ = opt.is_none_or(|x| x.is_ok());
167+
_ = opt.is_none_or(|x| x.is_err());
168+
_ = opt.is_none_or(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
169+
_ = opt.is_none_or(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
170+
_ = opt.is_some_and(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
171+
_ = opt.is_some_and(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
172+
173+
#[clippy::msrv = "1.81"]
174+
fn before_stabilization() {
175+
let opt = Some(500);
176+
_ = !opt.is_some_and(|x| x < 1000);
177+
}
178+
}
179+
118180
fn main() {}

Diff for: ‎tests/ui/nonminimal_bool_methods.rs

+62
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,66 @@ fn issue_12625() {
115115
if !(a as u64 <= b) {} //~ ERROR: this boolean expression can be simplified
116116
}
117117

118+
fn issue_13436() {
119+
fn not_zero(x: i32) -> bool {
120+
x != 0
121+
}
122+
123+
let opt = Some(500);
124+
_ = opt.is_some_and(|x| x < 1000);
125+
_ = opt.is_some_and(|x| x <= 1000);
126+
_ = opt.is_some_and(|x| x > 1000);
127+
_ = opt.is_some_and(|x| x >= 1000);
128+
_ = opt.is_some_and(|x| x == 1000);
129+
_ = opt.is_some_and(|x| x != 1000);
130+
_ = opt.is_some_and(not_zero);
131+
_ = !opt.is_some_and(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
132+
_ = !opt.is_some_and(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
133+
_ = !opt.is_some_and(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
134+
_ = !opt.is_some_and(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
135+
_ = !opt.is_some_and(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
136+
_ = !opt.is_some_and(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
137+
_ = !opt.is_some_and(not_zero);
138+
_ = opt.is_none_or(|x| x < 1000);
139+
_ = opt.is_none_or(|x| x <= 1000);
140+
_ = opt.is_none_or(|x| x > 1000);
141+
_ = opt.is_none_or(|x| x >= 1000);
142+
_ = opt.is_none_or(|x| x == 1000);
143+
_ = opt.is_none_or(|x| x != 1000);
144+
_ = opt.is_none_or(not_zero);
145+
_ = !opt.is_none_or(|x| x < 1000); //~ ERROR: this boolean expression can be simplified
146+
_ = !opt.is_none_or(|x| x <= 1000); //~ ERROR: this boolean expression can be simplified
147+
_ = !opt.is_none_or(|x| x > 1000); //~ ERROR: this boolean expression can be simplified
148+
_ = !opt.is_none_or(|x| x >= 1000); //~ ERROR: this boolean expression can be simplified
149+
_ = !opt.is_none_or(|x| x == 1000); //~ ERROR: this boolean expression can be simplified
150+
_ = !opt.is_none_or(|x| x != 1000); //~ ERROR: this boolean expression can be simplified
151+
_ = !opt.is_none_or(not_zero);
152+
153+
let opt = Some(true);
154+
_ = opt.is_some_and(|x| x);
155+
_ = opt.is_some_and(|x| !x);
156+
_ = !opt.is_some_and(|x| x);
157+
_ = !opt.is_some_and(|x| !x); //~ ERROR: this boolean expression can be simplified
158+
_ = opt.is_none_or(|x| x);
159+
_ = opt.is_none_or(|x| !x);
160+
_ = !opt.is_none_or(|x| x);
161+
_ = !opt.is_none_or(|x| !x); //~ ERROR: this boolean expression can be simplified
162+
163+
let opt: Option<Result<i32, i32>> = Some(Ok(123));
164+
_ = opt.is_some_and(|x| x.is_ok());
165+
_ = opt.is_some_and(|x| x.is_err());
166+
_ = opt.is_none_or(|x| x.is_ok());
167+
_ = opt.is_none_or(|x| x.is_err());
168+
_ = !opt.is_some_and(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
169+
_ = !opt.is_some_and(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
170+
_ = !opt.is_none_or(|x| x.is_ok()); //~ ERROR: this boolean expression can be simplified
171+
_ = !opt.is_none_or(|x| x.is_err()); //~ ERROR: this boolean expression can be simplified
172+
173+
#[clippy::msrv = "1.81"]
174+
fn before_stabilization() {
175+
let opt = Some(500);
176+
_ = !opt.is_some_and(|x| x < 1000);
177+
}
178+
}
179+
118180
fn main() {}

0 commit comments

Comments
 (0)