diff options
| -rw-r--r-- | clippy_lints/src/collapsible_if.rs | 17 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | tests/ui-toml/collapsible_if/collapsible_if_let_chains.fixed | 25 | ||||
| -rw-r--r-- | tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs | 28 | ||||
| -rw-r--r-- | tests/ui-toml/collapsible_if/collapsible_if_let_chains.stderr | 64 | ||||
| -rw-r--r-- | tests/ui/auxiliary/proc_macros.rs | 12 | ||||
| -rw-r--r-- | tests/ui/collapsible_if_let_chains.fixed | 29 | ||||
| -rw-r--r-- | tests/ui/collapsible_if_let_chains.rs | 32 | ||||
| -rw-r--r-- | tests/ui/collapsible_if_let_chains.stderr | 58 |
9 files changed, 254 insertions, 13 deletions
diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index 8728f7f2eb5..20fae8a6775 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -5,6 +5,7 @@ use rustc_ast::BinOpKind; use rustc_errors::Applicability; use rustc_hir::{Block, Expr, ExprKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::TyCtxt; use rustc_session::impl_lint_pass; use rustc_span::Span; @@ -77,12 +78,14 @@ declare_clippy_lint! { } pub struct CollapsibleIf { + let_chains_enabled: bool, lint_commented_code: bool, } impl CollapsibleIf { - pub fn new(conf: &'static Conf) -> Self { + pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { Self { + let_chains_enabled: tcx.features().let_chains(), lint_commented_code: conf.lint_commented_code, } } @@ -124,8 +127,7 @@ impl CollapsibleIf { if let Some(inner) = expr_block(then) && cx.tcx.hir_attrs(inner.hir_id).is_empty() && let ExprKind::If(check_inner, _, None) = &inner.kind - // Prevent triggering on `if c { if let a = b { .. } }`. - && !matches!(check_inner.kind, ExprKind::Let(..)) + && self.eligible_condition(check_inner) && let ctxt = expr.span.ctxt() && inner.span.ctxt() == ctxt && (self.lint_commented_code || !block_starts_with_comment(cx, then)) @@ -160,6 +162,10 @@ impl CollapsibleIf { ); } } + + pub fn eligible_condition(&self, cond: &Expr<'_>) -> bool { + self.let_chains_enabled || !matches!(cond.kind, ExprKind::Let(..)) + } } impl_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF, COLLAPSIBLE_ELSE_IF]); @@ -174,11 +180,10 @@ impl LateLintPass<'_> for CollapsibleIf { { Self::check_collapsible_else_if(cx, then.span, else_); } else if else_.is_none() - && !matches!(cond.kind, ExprKind::Let(..)) + && self.eligible_condition(cond) && let ExprKind::Block(then, None) = then.kind { - // Prevent triggering on `if c { if let a = b { .. } }`. - self.check_collapsible_if_if(cx, expr, cond, block!(then)); + self.check_collapsible_if_if(cx, expr, cond, then); } } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d2d8a1c4ff4..ba80e122448 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -771,7 +771,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(redundant_closure_call::RedundantClosureCall)); store.register_early_pass(|| Box::new(unused_unit::UnusedUnit)); store.register_late_pass(|_| Box::new(returns::Return)); - store.register_late_pass(move |_| Box::new(collapsible_if::CollapsibleIf::new(conf))); + store.register_late_pass(move |tcx| Box::new(collapsible_if::CollapsibleIf::new(tcx, conf))); store.register_late_pass(|_| Box::new(items_after_statements::ItemsAfterStatements)); store.register_early_pass(|| Box::new(precedence::Precedence)); store.register_late_pass(|_| Box::new(needless_parens_on_range_literals::NeedlessParensOnRangeLiterals)); diff --git a/tests/ui-toml/collapsible_if/collapsible_if_let_chains.fixed b/tests/ui-toml/collapsible_if/collapsible_if_let_chains.fixed new file mode 100644 index 00000000000..f12273954c6 --- /dev/null +++ b/tests/ui-toml/collapsible_if/collapsible_if_let_chains.fixed @@ -0,0 +1,25 @@ +#![feature(let_chains)] +#![warn(clippy::collapsible_if)] + +fn main() { + if let Some(a) = Some(3) + // with comment + && let Some(b) = Some(4) { + let _ = a + b; + } + //~^^^^^^ collapsible_if + + if let Some(a) = Some(3) + // with comment + && a + 1 == 4 { + let _ = a; + } + //~^^^^^^ collapsible_if + + if Some(3) == Some(4).map(|x| x - 1) + // with comment + && let Some(b) = Some(4) { + let _ = b; + } + //~^^^^^^ collapsible_if +} diff --git a/tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs b/tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs new file mode 100644 index 00000000000..5a984d7a3cb --- /dev/null +++ b/tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs @@ -0,0 +1,28 @@ +#![feature(let_chains)] +#![warn(clippy::collapsible_if)] + +fn main() { + if let Some(a) = Some(3) { + // with comment + if let Some(b) = Some(4) { + let _ = a + b; + } + } + //~^^^^^^ collapsible_if + + if let Some(a) = Some(3) { + // with comment + if a + 1 == 4 { + let _ = a; + } + } + //~^^^^^^ collapsible_if + + if Some(3) == Some(4).map(|x| x - 1) { + // with comment + if let Some(b) = Some(4) { + let _ = b; + } + } + //~^^^^^^ collapsible_if +} diff --git a/tests/ui-toml/collapsible_if/collapsible_if_let_chains.stderr b/tests/ui-toml/collapsible_if/collapsible_if_let_chains.stderr new file mode 100644 index 00000000000..c22a65a4473 --- /dev/null +++ b/tests/ui-toml/collapsible_if/collapsible_if_let_chains.stderr @@ -0,0 +1,64 @@ +error: this `if` statement can be collapsed + --> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:5:5 + | +LL | / if let Some(a) = Some(3) { +LL | | // with comment +LL | | if let Some(b) = Some(4) { +LL | | let _ = a + b; +LL | | } +LL | | } + | |_____^ + | + = note: `-D clippy::collapsible-if` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::collapsible_if)]` +help: collapse nested if block + | +LL ~ if let Some(a) = Some(3) +LL | // with comment +LL ~ && let Some(b) = Some(4) { +LL | let _ = a + b; +LL ~ } + | + +error: this `if` statement can be collapsed + --> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:13:5 + | +LL | / if let Some(a) = Some(3) { +LL | | // with comment +LL | | if a + 1 == 4 { +LL | | let _ = a; +LL | | } +LL | | } + | |_____^ + | +help: collapse nested if block + | +LL ~ if let Some(a) = Some(3) +LL | // with comment +LL ~ && a + 1 == 4 { +LL | let _ = a; +LL ~ } + | + +error: this `if` statement can be collapsed + --> tests/ui-toml/collapsible_if/collapsible_if_let_chains.rs:21:5 + | +LL | / if Some(3) == Some(4).map(|x| x - 1) { +LL | | // with comment +LL | | if let Some(b) = Some(4) { +LL | | let _ = b; +LL | | } +LL | | } + | |_____^ + | +help: collapse nested if block + | +LL ~ if Some(3) == Some(4).map(|x| x - 1) +LL | // with comment +LL ~ && let Some(b) = Some(4) { +LL | let _ = b; +LL ~ } + | + +error: aborting due to 3 previous errors + diff --git a/tests/ui/auxiliary/proc_macros.rs b/tests/ui/auxiliary/proc_macros.rs index 1a2a4ec2311..7a4cc4fa9ee 100644 --- a/tests/ui/auxiliary/proc_macros.rs +++ b/tests/ui/auxiliary/proc_macros.rs @@ -131,12 +131,12 @@ fn write_with_span(s: Span, mut input: IntoIter, out: &mut TokenStream) -> Resul pub fn make_it_big(input: TokenStream) -> TokenStream { let mut expr_repeat = syn::parse_macro_input!(input as syn::ExprRepeat); let len_span = expr_repeat.len.span(); - if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len { - if let syn::Lit::Int(lit_int) = &expr_lit.lit { - let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter"); - let new_val = orig_val.saturating_mul(10); - expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val); - } + if let syn::Expr::Lit(expr_lit) = &mut *expr_repeat.len + && let syn::Lit::Int(lit_int) = &expr_lit.lit + { + let orig_val = lit_int.base10_parse::<usize>().expect("not a valid length parameter"); + let new_val = orig_val.saturating_mul(10); + expr_lit.lit = syn::parse_quote_spanned!( len_span => #new_val); } quote::quote!(#expr_repeat).into() } diff --git a/tests/ui/collapsible_if_let_chains.fixed b/tests/ui/collapsible_if_let_chains.fixed new file mode 100644 index 00000000000..3dd9498a4c9 --- /dev/null +++ b/tests/ui/collapsible_if_let_chains.fixed @@ -0,0 +1,29 @@ +#![feature(let_chains)] +#![warn(clippy::collapsible_if)] + +fn main() { + if let Some(a) = Some(3) { + // with comment, so do not lint + if let Some(b) = Some(4) { + let _ = a + b; + } + } + + if let Some(a) = Some(3) + && let Some(b) = Some(4) { + let _ = a + b; + } + //~^^^^^ collapsible_if + + if let Some(a) = Some(3) + && a + 1 == 4 { + let _ = a; + } + //~^^^^^ collapsible_if + + if Some(3) == Some(4).map(|x| x - 1) + && let Some(b) = Some(4) { + let _ = b; + } + //~^^^^^ collapsible_if +} diff --git a/tests/ui/collapsible_if_let_chains.rs b/tests/ui/collapsible_if_let_chains.rs new file mode 100644 index 00000000000..064b9a0be48 --- /dev/null +++ b/tests/ui/collapsible_if_let_chains.rs @@ -0,0 +1,32 @@ +#![feature(let_chains)] +#![warn(clippy::collapsible_if)] + +fn main() { + if let Some(a) = Some(3) { + // with comment, so do not lint + if let Some(b) = Some(4) { + let _ = a + b; + } + } + + if let Some(a) = Some(3) { + if let Some(b) = Some(4) { + let _ = a + b; + } + } + //~^^^^^ collapsible_if + + if let Some(a) = Some(3) { + if a + 1 == 4 { + let _ = a; + } + } + //~^^^^^ collapsible_if + + if Some(3) == Some(4).map(|x| x - 1) { + if let Some(b) = Some(4) { + let _ = b; + } + } + //~^^^^^ collapsible_if +} diff --git a/tests/ui/collapsible_if_let_chains.stderr b/tests/ui/collapsible_if_let_chains.stderr new file mode 100644 index 00000000000..64a88114c47 --- /dev/null +++ b/tests/ui/collapsible_if_let_chains.stderr @@ -0,0 +1,58 @@ +error: this `if` statement can be collapsed + --> tests/ui/collapsible_if_let_chains.rs:12:5 + | +LL | / if let Some(a) = Some(3) { +LL | | if let Some(b) = Some(4) { +LL | | let _ = a + b; +LL | | } +LL | | } + | |_____^ + | + = note: `-D clippy::collapsible-if` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::collapsible_if)]` +help: collapse nested if block + | +LL ~ if let Some(a) = Some(3) +LL ~ && let Some(b) = Some(4) { +LL | let _ = a + b; +LL ~ } + | + +error: this `if` statement can be collapsed + --> tests/ui/collapsible_if_let_chains.rs:19:5 + | +LL | / if let Some(a) = Some(3) { +LL | | if a + 1 == 4 { +LL | | let _ = a; +LL | | } +LL | | } + | |_____^ + | +help: collapse nested if block + | +LL ~ if let Some(a) = Some(3) +LL ~ && a + 1 == 4 { +LL | let _ = a; +LL ~ } + | + +error: this `if` statement can be collapsed + --> tests/ui/collapsible_if_let_chains.rs:26:5 + | +LL | / if Some(3) == Some(4).map(|x| x - 1) { +LL | | if let Some(b) = Some(4) { +LL | | let _ = b; +LL | | } +LL | | } + | |_____^ + | +help: collapse nested if block + | +LL ~ if Some(3) == Some(4).map(|x| x - 1) +LL ~ && let Some(b) = Some(4) { +LL | let _ = b; +LL ~ } + | + +error: aborting due to 3 previous errors + |