diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e6f00e857f7..0d6b2f5698bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6350,6 +6350,7 @@ Released 2018-09-13 [`check-inconsistent-struct-field-initializers`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-inconsistent-struct-field-initializers [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items [`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold +[`collapse-let-chains`]: https://doc.rust-lang.org/clippy/lint_configuration.html#collapse-let-chains [`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros [`disallowed-methods`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods [`disallowed-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-names diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index a677745379c2..046c774e4605 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -475,6 +475,17 @@ The maximum cognitive complexity a function can have * [`cognitive_complexity`](https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity) +## `collapse-let-chains` +Whether `if let` chains should be collapsed. This requires the use of the unstable +`let_chains` rustc feature. + +**Default Value:** `false` + +--- +**Affected lints:** +* [`collapsible_if`](https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if) + + ## `disallowed-macros` The list of disallowed macros, written as fully qualified paths. diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index a5c0ff8978d3..783014843930 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -500,6 +500,10 @@ define_Conf! { /// The maximum cognitive complexity a function can have #[lints(cognitive_complexity)] cognitive_complexity_threshold: u64 = 25, + /// Whether `if let` chains should be collapsed. This requires the use of the unstable + /// `let_chains` rustc feature. + #[lints(collapsible_if)] + collapse_let_chains: bool = false, /// DEPRECATED LINT: CYCLOMATIC_COMPLEXITY. /// /// Use the Cognitive Complexity lint instead. diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index b100e2408de3..5b17b731389a 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -79,16 +79,24 @@ declare_clippy_lint! { } pub struct CollapsibleIf { + collapse_let_chains: bool, lint_commented_code: bool, } impl CollapsibleIf { pub fn new(conf: &'static Conf) -> Self { Self { + collapse_let_chains: conf.collapse_let_chains, lint_commented_code: conf.lint_commented_code, } } + /// Prevent triggering on `if c { if let a = b { .. } }` unless the + /// `collapse_let_chains` config option is set. + fn is_collapsible(&self, cond: &ast::Expr) -> bool { + self.collapse_let_chains || !matches!(cond.kind, ast::ExprKind::Let(..)) + } + fn check_collapsible_else_if(cx: &EarlyContext<'_>, then_span: Span, else_: &ast::Expr) { if let ast::ExprKind::Block(ref block, _) = else_.kind && !block_starts_with_comment(cx, block) @@ -127,8 +135,7 @@ impl CollapsibleIf { if let Some(inner) = expr_block(then) && inner.attrs.is_empty() && let ast::ExprKind::If(check_inner, _, None) = &inner.kind - // Prevent triggering on `if c { if let a = b { .. } }`. - && !matches!(check_inner.kind, ast::ExprKind::Let(..)) + && self.is_collapsible(check_inner) && let ctxt = expr.span.ctxt() && inner.span.ctxt() == ctxt && let contains_comment = span_contains_comment(cx.sess().source_map(), check.span.to(check_inner.span)) @@ -175,8 +182,7 @@ impl EarlyLintPass for CollapsibleIf { { if let Some(else_) = else_ { Self::check_collapsible_else_if(cx, then.span, else_); - } else if !matches!(cond.kind, ast::ExprKind::Let(..)) { - // Prevent triggering on `if c { if let a = b { .. } }`. + } else if self.is_collapsible(cond) { self.check_collapsible_if_if(cx, expr, cond, then); } } diff --git a/tests/ui-toml/collapsible_if/clippy.toml b/tests/ui-toml/collapsible_if/clippy.toml index 592cea90cff5..141c71726629 100644 --- a/tests/ui-toml/collapsible_if/clippy.toml +++ b/tests/ui-toml/collapsible_if/clippy.toml @@ -1 +1,2 @@ +collapse-let-chains = true lint-commented-code = true diff --git a/tests/ui-toml/collapsible_if/collapsible_if.fixed b/tests/ui-toml/collapsible_if/collapsible_if.fixed index f695f9804d59..48d88483f90e 100644 --- a/tests/ui-toml/collapsible_if/collapsible_if.fixed +++ b/tests/ui-toml/collapsible_if/collapsible_if.fixed @@ -1,3 +1,4 @@ +#![feature(let_chains)] #![allow(clippy::eq_op, clippy::nonminimal_bool)] #[rustfmt::skip] @@ -31,4 +32,28 @@ fn main() { println!("Hello world!"); } //~^^^^^ collapsible_if + + if true + && true { + println!("No comment, linted"); + } + //~^^^^^ collapsible_if + + 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-toml/collapsible_if/collapsible_if.rs b/tests/ui-toml/collapsible_if/collapsible_if.rs index 868b4adcde50..80f2ab2ace75 100644 --- a/tests/ui-toml/collapsible_if/collapsible_if.rs +++ b/tests/ui-toml/collapsible_if/collapsible_if.rs @@ -1,3 +1,4 @@ +#![feature(let_chains)] #![allow(clippy::eq_op, clippy::nonminimal_bool)] #[rustfmt::skip] @@ -35,4 +36,32 @@ fn main() { } } //~^^^^^ collapsible_if + + if true { + if true { + println!("No comment, linted"); + } + } + //~^^^^^ collapsible_if + + 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-toml/collapsible_if/collapsible_if.stderr b/tests/ui-toml/collapsible_if/collapsible_if.stderr index a12c2112f587..e074008825c2 100644 --- a/tests/ui-toml/collapsible_if/collapsible_if.stderr +++ b/tests/ui-toml/collapsible_if/collapsible_if.stderr @@ -1,5 +1,5 @@ error: this `if` statement can be collapsed - --> tests/ui-toml/collapsible_if/collapsible_if.rs:8:5 + --> tests/ui-toml/collapsible_if/collapsible_if.rs:9:5 | LL | / if x == "hello" { LL | | // Comment must be kept @@ -21,7 +21,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui-toml/collapsible_if/collapsible_if.rs:17:5 + --> tests/ui-toml/collapsible_if/collapsible_if.rs:18:5 | LL | / if x == "hello" { // Inner comment LL | | if y == "world" { @@ -39,7 +39,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui-toml/collapsible_if/collapsible_if.rs:24:5 + --> tests/ui-toml/collapsible_if/collapsible_if.rs:25:5 | LL | / if x == "hello" { LL | | /* Inner comment */ @@ -59,7 +59,7 @@ LL ~ } | error: this `if` statement can be collapsed - --> tests/ui-toml/collapsible_if/collapsible_if.rs:32:5 + --> tests/ui-toml/collapsible_if/collapsible_if.rs:33:5 | LL | / if x == "hello" { /* Inner comment */ LL | | if y == "world" { @@ -76,5 +76,77 @@ LL | println!("Hello world!"); LL ~ } | -error: aborting due to 4 previous errors +error: this `if` statement can be collapsed + --> tests/ui-toml/collapsible_if/collapsible_if.rs:40:5 + | +LL | / if true { +LL | | if true { +LL | | println!("No comment, linted"); +LL | | } +LL | | } + | |______^ + | +help: collapse nested if block + | +LL ~ if true +LL ~ && true { +LL | println!("No comment, linted"); +LL ~ } + | + +error: this `if` statement can be collapsed + --> tests/ui-toml/collapsible_if/collapsible_if.rs:47:5 + | +LL | / if let Some(a) = Some(3) { +LL | | if let Some(b) = Some(4) { +LL | | let _ = a + b; +LL | | } +LL | | } + | |_____^ + | +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-toml/collapsible_if/collapsible_if.rs:54: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-toml/collapsible_if/collapsible_if.rs:61: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 8 previous errors diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index f2eaa66a4ae4..05e1cf42b6ca 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -34,6 +34,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect check-inconsistent-struct-field-initializers check-private-items cognitive-complexity-threshold + collapse-let-chains disallowed-macros disallowed-methods disallowed-names @@ -126,6 +127,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect check-inconsistent-struct-field-initializers check-private-items cognitive-complexity-threshold + collapse-let-chains disallowed-macros disallowed-methods disallowed-names @@ -218,6 +220,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni check-inconsistent-struct-field-initializers check-private-items cognitive-complexity-threshold + collapse-let-chains disallowed-macros disallowed-methods disallowed-names diff --git a/tests/ui/auxiliary/proc_macros.rs b/tests/ui/auxiliary/proc_macros.rs index 1a2a4ec23114..7a4cc4fa9ee8 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::().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::().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() }