Skip to content

Commit 50e0bf1

Browse files
samueltardieusamueltardieu
authored
Gate collapsible_if let_chains lints on edition 2024 and MSRV (#14723)
The MSRV being for crates targeting 1.85-1.87 on edition 2024 This enables the lint for regular nightly users without the feature gate enabled r? @samueltardieu Fixes #14678 changelog: none
2 parents c1586e1 + 8c93668 commit 50e0bf1

23 files changed

+351
-237
lines changed

‎clippy_lints/src/collapsible_if.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
3+
use clippy_utils::msrvs::{self, Msrv};
34
use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block, snippet_block_with_applicability};
45
use rustc_ast::BinOpKind;
56
use rustc_errors::Applicability;
67
use rustc_hir::{Block, Expr, ExprKind, StmtKind};
78
use rustc_lint::{LateContext, LateLintPass};
8-
use rustc_middle::ty::TyCtxt;
99
use rustc_session::impl_lint_pass;
1010
use rustc_span::Span;
1111

@@ -78,14 +78,14 @@ declare_clippy_lint! {
7878
}
7979

8080
pub struct CollapsibleIf {
81-
let_chains_enabled: bool,
81+
msrv: Msrv,
8282
lint_commented_code: bool,
8383
}
8484

8585
impl CollapsibleIf {
86-
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
86+
pub fn new(conf: &'static Conf) -> Self {
8787
Self {
88-
let_chains_enabled: tcx.features().let_chains(),
88+
msrv: conf.msrv,
8989
lint_commented_code: conf.lint_commented_code,
9090
}
9191
}
@@ -127,7 +127,7 @@ impl CollapsibleIf {
127127
if let Some(inner) = expr_block(then)
128128
&& cx.tcx.hir_attrs(inner.hir_id).is_empty()
129129
&& let ExprKind::If(check_inner, _, None) = &inner.kind
130-
&& self.eligible_condition(check_inner)
130+
&& self.eligible_condition(cx, check_inner)
131131
&& let ctxt = expr.span.ctxt()
132132
&& inner.span.ctxt() == ctxt
133133
&& (self.lint_commented_code || !block_starts_with_comment(cx, then))
@@ -163,8 +163,9 @@ impl CollapsibleIf {
163163
}
164164
}
165165

166-
pub fn eligible_condition(&self, cond: &Expr<'_>) -> bool {
167-
self.let_chains_enabled || !matches!(cond.kind, ExprKind::Let(..))
166+
fn eligible_condition(&self, cx: &LateContext<'_>, cond: &Expr<'_>) -> bool {
167+
!matches!(cond.kind, ExprKind::Let(..))
168+
|| (cx.tcx.sess.edition().at_least_rust_2024() && self.msrv.meets(cx, msrvs::LET_CHAINS))
168169
}
169170
}
170171

@@ -180,7 +181,7 @@ impl LateLintPass<'_> for CollapsibleIf {
180181
{
181182
Self::check_collapsible_else_if(cx, then.span, else_);
182183
} else if else_.is_none()
183-
&& self.eligible_condition(cond)
184+
&& self.eligible_condition(cx, cond)
184185
&& let ExprKind::Block(then, None) = then.kind
185186
{
186187
self.check_collapsible_if_if(cx, expr, cond, then);

‎clippy_lints/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
731731
store.register_early_pass(|| Box::new(unused_unit::UnusedUnit));
732732
store.register_late_pass(|_| Box::new(unused_unit::UnusedUnit));
733733
store.register_late_pass(|_| Box::new(returns::Return));
734-
store.register_late_pass(move |tcx| Box::new(collapsible_if::CollapsibleIf::new(tcx, conf)));
734+
store.register_late_pass(move |_| Box::new(collapsible_if::CollapsibleIf::new(conf)));
735735
store.register_late_pass(|_| Box::new(items_after_statements::ItemsAfterStatements));
736736
store.register_early_pass(|| Box::new(precedence::Precedence));
737737
store.register_late_pass(|_| Box::new(needless_parens_on_range_literals::NeedlessParensOnRangeLiterals));

‎clippy_utils/src/msrvs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ macro_rules! msrv_aliases {
2222

2323
// names may refer to stabilized feature flags or library items
2424
msrv_aliases! {
25+
1,88,0 { LET_CHAINS }
2526
1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT }
2627
1,85,0 { UINT_FLOAT_MIDPOINT }
2728
1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR }

‎tests/ui-toml/collapsible_if/collapsible_if_let_chains.fixed

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#![feature(let_chains)]
21
#![warn(clippy::collapsible_if)]
32

43
fn main() {

‎tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#![feature(let_chains)]
21
#![warn(clippy::collapsible_if)]
32

43
fn main() {

‎tests/ui-toml/collapsible_if/collapsible_if_let_chains.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` statement can be collapsed
2-
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:5:5
2+
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:4:5
33
|
44
LL | / if let Some(a) = Some(3) {
55
LL | | // with comment
@@ -21,7 +21,7 @@ LL ~ }
2121
|
2222

2323
error: this `if` statement can be collapsed
24-
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:13:5
24+
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:12:5
2525
|
2626
LL | / if let Some(a) = Some(3) {
2727
LL | | // with comment
@@ -41,7 +41,7 @@ LL ~ }
4141
|
4242

4343
error: this `if` statement can be collapsed
44-
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:21:5
44+
--> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:20:5
4545
|
4646
LL | / if Some(3) == Some(4).map(|x| x - 1) {
4747
LL | | // with comment

‎tests/ui/auxiliary/proc_macro_attr.rs

+39-40
Original file line numberDiff line numberDiff line change
@@ -51,45 +51,44 @@ pub fn rename_my_lifetimes(_args: TokenStream, input: TokenStream) -> TokenStrea
5151

5252
fn mut_receiver_of(sig: &mut Signature) -> Option<&mut FnArg> {
5353
let arg = sig.inputs.first_mut()?;
54-
if let FnArg::Typed(PatType { pat, .. }) = arg {
55-
if let Pat::Ident(PatIdent { ident, .. }) = &**pat {
56-
if ident == "self" {
57-
return Some(arg);
58-
}
59-
}
54+
if let FnArg::Typed(PatType { pat, .. }) = arg
55+
&& let Pat::Ident(PatIdent { ident, .. }) = &**pat
56+
&& ident == "self"
57+
{
58+
Some(arg)
59+
} else {
60+
None
6061
}
61-
None
6262
}
6363

6464
let mut elided = 0;
6565
let mut item = parse_macro_input!(input as ItemImpl);
6666

6767
// Look for methods having arbitrary self type taken by &mut ref
6868
for inner in &mut item.items {
69-
if let ImplItem::Fn(method) = inner {
70-
if let Some(FnArg::Typed(pat_type)) = mut_receiver_of(&mut method.sig) {
71-
if let box Type::Reference(reference) = &mut pat_type.ty {
72-
// Target only unnamed lifetimes
73-
let name = match &reference.lifetime {
74-
Some(lt) if lt.ident == "_" => make_name(elided),
75-
None => make_name(elided),
76-
_ => continue,
77-
};
78-
elided += 1;
79-
80-
// HACK: Syn uses `Span` from the proc_macro2 crate, and does not seem to reexport it.
81-
// In order to avoid adding the dependency, get a default span from a nonexistent token.
82-
// A default span is needed to mark the code as coming from expansion.
83-
let span = Star::default().span();
84-
85-
// Replace old lifetime with the named one
86-
let lifetime = Lifetime::new(&name, span);
87-
reference.lifetime = Some(parse_quote!(#lifetime));
88-
89-
// Add lifetime to the generics of the method
90-
method.sig.generics.params.push(parse_quote!(#lifetime));
91-
}
92-
}
69+
if let ImplItem::Fn(method) = inner
70+
&& let Some(FnArg::Typed(pat_type)) = mut_receiver_of(&mut method.sig)
71+
&& let box Type::Reference(reference) = &mut pat_type.ty
72+
{
73+
// Target only unnamed lifetimes
74+
let name = match &reference.lifetime {
75+
Some(lt) if lt.ident == "_" => make_name(elided),
76+
None => make_name(elided),
77+
_ => continue,
78+
};
79+
elided += 1;
80+
81+
// HACK: Syn uses `Span` from the proc_macro2 crate, and does not seem to reexport it.
82+
// In order to avoid adding the dependency, get a default span from a nonexistent token.
83+
// A default span is needed to mark the code as coming from expansion.
84+
let span = Star::default().span();
85+
86+
// Replace old lifetime with the named one
87+
let lifetime = Lifetime::new(&name, span);
88+
reference.lifetime = Some(parse_quote!(#lifetime));
89+
90+
// Add lifetime to the generics of the method
91+
method.sig.generics.params.push(parse_quote!(#lifetime));
9392
}
9493
}
9594

@@ -129,15 +128,15 @@ pub fn fake_desugar_await(_args: TokenStream, input: TokenStream) -> TokenStream
129128
let mut async_fn = parse_macro_input!(input as syn::ItemFn);
130129

131130
for stmt in &mut async_fn.block.stmts {
132-
if let syn::Stmt::Expr(syn::Expr::Match(syn::ExprMatch { expr: scrutinee, .. }), _) = stmt {
133-
if let syn::Expr::Await(syn::ExprAwait { base, await_token, .. }) = scrutinee.as_mut() {
134-
let blc = quote_spanned!( await_token.span => {
135-
#[allow(clippy::let_and_return)]
136-
let __pinned = #base;
137-
__pinned
138-
});
139-
*scrutinee = parse_quote!(#blc);
140-
}
131+
if let syn::Stmt::Expr(syn::Expr::Match(syn::ExprMatch { expr: scrutinee, .. }), _) = stmt
132+
&& let syn::Expr::Await(syn::ExprAwait { base, await_token, .. }) = scrutinee.as_mut()
133+
{
134+
let blc = quote_spanned!( await_token.span => {
135+
#[allow(clippy::let_and_return)]
136+
let __pinned = #base;
137+
__pinned
138+
});
139+
*scrutinee = parse_quote!(#blc);
141140
}
142141
}
143142

‎tests/ui/auxiliary/proc_macros.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#![feature(let_chains)]
21
#![feature(proc_macro_span)]
32
#![allow(clippy::needless_if, dead_code)]
43

‎tests/ui/collapsible_if.fixed

-19
Original file line numberDiff line numberDiff line change
@@ -101,27 +101,8 @@ fn main() {
101101
}
102102
}
103103

104-
// Test behavior wrt. `let_chains`.
105-
// None of the cases below should be collapsed.
106104
fn truth() -> bool { true }
107105

108-
// Prefix:
109-
if let 0 = 1 {
110-
if truth() {}
111-
}
112-
113-
// Suffix:
114-
if truth() {
115-
if let 0 = 1 {}
116-
}
117-
118-
// Midfix:
119-
if truth() {
120-
if let 0 = 1 {
121-
if truth() {}
122-
}
123-
}
124-
125106
// Fix #5962
126107
if matches!(true, true)
127108
&& matches!(true, true) {}

‎tests/ui/collapsible_if.rs

-19
Original file line numberDiff line numberDiff line change
@@ -108,27 +108,8 @@ fn main() {
108108
}
109109
}
110110

111-
// Test behavior wrt. `let_chains`.
112-
// None of the cases below should be collapsed.
113111
fn truth() -> bool { true }
114112

115-
// Prefix:
116-
if let 0 = 1 {
117-
if truth() {}
118-
}
119-
120-
// Suffix:
121-
if truth() {
122-
if let 0 = 1 {}
123-
}
124-
125-
// Midfix:
126-
if truth() {
127-
if let 0 = 1 {
128-
if truth() {}
129-
}
130-
}
131-
132113
// Fix #5962
133114
if matches!(true, true) {
134115
if matches!(true, true) {}

‎tests/ui/collapsible_if.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ LL ~ }
127127
|
128128

129129
error: this `if` statement can be collapsed
130-
--> tests/ui/collapsible_if.rs:133:5
130+
--> tests/ui/collapsible_if.rs:114:5
131131
|
132132
LL | / if matches!(true, true) {
133133
LL | | if matches!(true, true) {}
@@ -141,7 +141,7 @@ LL ~ && matches!(true, true) {}
141141
|
142142

143143
error: this `if` statement can be collapsed
144-
--> tests/ui/collapsible_if.rs:139:5
144+
--> tests/ui/collapsible_if.rs:120:5
145145
|
146146
LL | / if matches!(true, true) && truth() {
147147
LL | | if matches!(true, true) {}
@@ -155,7 +155,7 @@ LL ~ && matches!(true, true) {}
155155
|
156156

157157
error: this `if` statement can be collapsed
158-
--> tests/ui/collapsible_if.rs:151:5
158+
--> tests/ui/collapsible_if.rs:132:5
159159
|
160160
LL | / if true {
161161
LL | | if true {
@@ -173,7 +173,7 @@ LL ~ }
173173
|
174174

175175
error: this `if` statement can be collapsed
176-
--> tests/ui/collapsible_if.rs:168:5
176+
--> tests/ui/collapsible_if.rs:149:5
177177
|
178178
LL | / if true {
179179
LL | | if true {

‎tests/ui/collapsible_if_let_chains.edition2024.fixed

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
//@revisions: edition2021 edition2024
2+
//@[edition2021] edition:2021
3+
//@[edition2024] edition:2024
4+
//@[edition2021] check-pass
5+
6+
#![warn(clippy::collapsible_if)]
7+
8+
fn main() {
9+
if let Some(a) = Some(3) {
10+
// with comment, so do not lint
11+
if let Some(b) = Some(4) {
12+
let _ = a + b;
13+
}
14+
}
15+
16+
//~[edition2024]v collapsible_if
17+
if let Some(a) = Some(3)
18+
&& let Some(b) = Some(4) {
19+
let _ = a + b;
20+
}
21+
22+
//~[edition2024]v collapsible_if
23+
if let Some(a) = Some(3)
24+
&& a + 1 == 4 {
25+
let _ = a;
26+
}
27+
28+
//~[edition2024]v collapsible_if
29+
if Some(3) == Some(4).map(|x| x - 1)
30+
&& let Some(b) = Some(4) {
31+
let _ = b;
32+
}
33+
34+
fn truth() -> bool {
35+
true
36+
}
37+
38+
// Prefix:
39+
//~[edition2024]v collapsible_if
40+
if let 0 = 1
41+
&& truth() {}
42+
43+
// Suffix:
44+
//~[edition2024]v collapsible_if
45+
if truth()
46+
&& let 0 = 1 {}
47+
48+
// Midfix:
49+
//~[edition2024]vvv collapsible_if
50+
//~[edition2024]v collapsible_if
51+
if truth()
52+
&& let 0 = 1
53+
&& truth() {}
54+
}
55+
56+
#[clippy::msrv = "1.87.0"]
57+
fn msrv_1_87() {
58+
if let 0 = 1 {
59+
if true {}
60+
}
61+
}
62+
63+
#[clippy::msrv = "1.88.0"]
64+
fn msrv_1_88() {
65+
//~[edition2024]v collapsible_if
66+
if let 0 = 1
67+
&& true {}
68+
}

0 commit comments

Comments
 (0)