Fix collapsible_else_if FP on conditionally compiled stmt

clippy_lints/src/collapsible_if.rs

@@ -1,13 +1,14 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block, snippet_block_with_applicability};
+use clippy_utils::source::{IntoSpan as _, SpanRangeExt, snippet, snippet_block_with_applicability};
+use clippy_utils::span_contains_non_whitespace;
 use rustc_ast::BinOpKind;
 use rustc_errors::Applicability;
 use rustc_hir::{Block, Expr, ExprKind, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
-use rustc_span::Span;
+use rustc_span::{BytePos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -90,12 +91,12 @@ impl CollapsibleIf {
         }
     }
 
-    fn check_collapsible_else_if(cx: &LateContext<'_>, then_span: Span, else_block: &Block<'_>) {
-        if !block_starts_with_comment(cx, else_block)
-            && let Some(else_) = expr_block(else_block)
+    fn check_collapsible_else_if(&self, cx: &LateContext<'_>, then_span: Span, else_block: &Block<'_>) {
+        if let Some(else_) = expr_block(else_block)
             && cx.tcx.hir_attrs(else_.hir_id).is_empty()
             && !else_.span.from_expansion()
             && let ExprKind::If(..) = else_.kind
+            && !block_starts_with_significant_tokens(cx, else_block, else_, self.lint_commented_code)
         {
             // Prevent "elseif"
             // Check that the "else" is followed by whitespace
@@ -130,7 +131,7 @@ impl CollapsibleIf {
             && self.eligible_condition(cx, check_inner)
             && let ctxt = expr.span.ctxt()
             && inner.span.ctxt() == ctxt
-            && (self.lint_commented_code || !block_starts_with_comment(cx, then))
+            && !block_starts_with_significant_tokens(cx, then, inner, self.lint_commented_code)
         {
             span_lint_and_then(
                 cx,
@@ -141,7 +142,7 @@ impl CollapsibleIf {
                     let then_open_bracket = then.span.split_at(1).0.with_leading_whitespace(cx).into_span();
                     let then_closing_bracket = {
                         let end = then.span.shrink_to_hi();
-                        end.with_lo(end.lo() - rustc_span::BytePos(1))
+                        end.with_lo(end.lo() - BytePos(1))
                             .with_leading_whitespace(cx)
                             .into_span()
                     };
@@ -179,7 +180,7 @@ impl LateLintPass<'_> for CollapsibleIf {
             if let Some(else_) = else_
                 && let ExprKind::Block(else_, None) = else_.kind
             {
-                Self::check_collapsible_else_if(cx, then.span, else_);
+                self.check_collapsible_else_if(cx, then.span, else_);
             } else if else_.is_none()
                 && self.eligible_condition(cx, cond)
                 && let ExprKind::Block(then, None) = then.kind
@@ -190,12 +191,16 @@ impl LateLintPass<'_> for CollapsibleIf {
     }
 }
 
-fn block_starts_with_comment(cx: &LateContext<'_>, block: &Block<'_>) -> bool {
-    // We trim all opening braces and whitespaces and then check if the next string is a comment.
-    let trimmed_block_text = snippet_block(cx, block.span, "..", None)
-        .trim_start_matches(|c: char| c.is_whitespace() || c == '{')
-        .to_owned();
-    trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
+// Check that nothing significant can be found but whitespaces between the initial `{` of `block`
+// and the beginning of `stop_at`.
+fn block_starts_with_significant_tokens(
+    cx: &LateContext<'_>,
+    block: &Block<'_>,
+    stop_at: &Expr<'_>,
+    lint_commented_code: bool,
+) -> bool {
+    let span = block.span.split_at(1).1.until(stop_at.span);
+    span_contains_non_whitespace(cx, span, lint_commented_code)
 }
 
 /// If `block` is a block with either one expression or a statement containing an expression,

clippy_lints/src/loops/same_item_push.rs

@@ -163,15 +163,12 @@ impl<'tcx> Visitor<'tcx> for SameItemPushVisitor<'_, 'tcx> {
                 StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(expr),
                 _ => {},
             }
+        } else if self.vec_push.is_none() {
+            // Current statement is a push ...check whether another push had been previously done
+            self.vec_push = vec_push_option;
         } else {
-            // Current statement is a push ...check whether another
-            // push had been previously done
-            if self.vec_push.is_none() {
-                self.vec_push = vec_push_option;
-            } else {
-                // There are multiple pushes ... don't lint
-                self.multiple_pushes = true;
-            }
+            // There are multiple pushes ... don't lint
+            self.multiple_pushes = true;
         }
     }
 }

clippy_lints/src/single_component_path_imports.rs

@@ -219,22 +219,21 @@ impl SingleComponentPathImports {
                             }
                         }
                     }
-                } else {
+                } else if segments[0].ident.name == kw::SelfLower {
                     // keep track of `use self::some_module` usages
-                    if segments[0].ident.name == kw::SelfLower {
-                        // simple case such as `use self::module::SomeStruct`
-                        if segments.len() > 1 {
-                            imports_reused_with_self.push(segments[1].ident.name);
-                            return;
-                        }
 
-                        // nested case such as `use self::{module1::Struct1, module2::Struct2}`
-                        if let UseTreeKind::Nested { items, .. } = &use_tree.kind {
-                            for tree in items {
-                                let segments = &tree.0.prefix.segments;
-                                if !segments.is_empty() {
-                                    imports_reused_with_self.push(segments[0].ident.name);
-                                }
+                    // simple case such as `use self::module::SomeStruct`
+                    if segments.len() > 1 {
+                        imports_reused_with_self.push(segments[1].ident.name);
+                        return;
+                    }
+
+                    // nested case such as `use self::{module1::Struct1, module2::Struct2}`
+                    if let UseTreeKind::Nested { items, .. } = &use_tree.kind {
+                        for tree in items {
+                            let segments = &tree.0.prefix.segments;
+                            if !segments.is_empty() {
+                                imports_reused_with_self.push(segments[0].ident.name);
                             }
                         }
                     }

clippy_lints/src/undocumented_unsafe_blocks.rs

@@ -606,32 +606,30 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span
     let ctxt = span.ctxt();
     if ctxt == SyntaxContext::root() {
         HasSafetyComment::Maybe
-    } else {
-        // From a macro expansion. Get the text from the start of the macro declaration to start of the
-        // unsafe block.
+    } else if let Ok(unsafe_line) = source_map.lookup_line(span.lo())
+        // From a macro expansion. Get the text from the start of the macro declaration to start of
+        // the unsafe block.
         //     macro_rules! foo { () => { stuff }; (x) => { unsafe { stuff } }; }
         //     ^--------------------------------------------^
-        if let Ok(unsafe_line) = source_map.lookup_line(span.lo())
-            && let Ok(macro_line) = source_map.lookup_line(ctxt.outer_expn_data().def_site.lo())
-            && Arc::ptr_eq(&unsafe_line.sf, &macro_line.sf)
-            && let Some(src) = unsafe_line.sf.src.as_deref()
-        {
-            if macro_line.line < unsafe_line.line {
-                match text_has_safety_comment(
-                    src,
-                    &unsafe_line.sf.lines()[macro_line.line + 1..=unsafe_line.line],
-                    unsafe_line.sf.start_pos,
-                ) {
-                    Some(b) => HasSafetyComment::Yes(b),
-                    None => HasSafetyComment::No,
-                }
-            } else {
-                HasSafetyComment::No
+        && let Ok(macro_line) = source_map.lookup_line(ctxt.outer_expn_data().def_site.lo())
+        && Arc::ptr_eq(&unsafe_line.sf, &macro_line.sf)
+        && let Some(src) = unsafe_line.sf.src.as_deref()
+    {
+        if macro_line.line < unsafe_line.line {
+            match text_has_safety_comment(
+                src,
+                &unsafe_line.sf.lines()[macro_line.line + 1..=unsafe_line.line],
+                unsafe_line.sf.start_pos,
+            ) {
+                Some(b) => HasSafetyComment::Yes(b),
+                None => HasSafetyComment::No,
             }
         } else {
-            // Problem getting source text. Pretend a comment was found.
-            HasSafetyComment::Maybe
+            HasSafetyComment::No
         }
+    } else {
+        // Problem getting source text. Pretend a comment was found.
+        HasSafetyComment::Maybe
     }
 }
 

clippy_utils/src/lib.rs

@@ -122,7 +122,7 @@ use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::{Ident, Symbol, kw};
 use rustc_span::{InnerSpan, Span};
-use source::walk_span_to_context;
+use source::{SpanRangeExt, walk_span_to_context};
 use visitors::{Visitable, for_each_unconsumed_temporary};
 
 use crate::consts::{ConstEvalCtxt, Constant, mir_to_const};
@@ -2788,6 +2788,19 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool {
     });
 }
 
+/// Checks whether a given span has any significant token. A significant token is a non-whitespace
+/// token, including comments unless `skip_comments` is set.
+/// This is useful to determine if there are any actual code tokens in the span that are omitted in
+/// the late pass, such as platform-specific code.
+pub fn span_contains_non_whitespace(cx: &impl source::HasSession, span: Span, skip_comments: bool) -> bool {
+    matches!(span.get_source_text(cx), Some(snippet) if tokenize_with_text(&snippet).any(|(token, _, _)|
+        match token {
+            TokenKind::Whitespace => false,
+            TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => !skip_comments,
+            _ => true,
+        }
+    ))
+}
 /// Returns all the comments a given span contains
 ///
 /// Comments are returned wrapped with their relevant delimiters

tests/ui-toml/collapsible_if/collapsible_else_if.fixed

@@ -0,0 +1,35 @@
+#![allow(clippy::eq_op, clippy::nonminimal_bool)]
+
+#[rustfmt::skip]
+#[warn(clippy::collapsible_if)]
+fn main() {
+    let (x, y) = ("hello", "world");
+
+    if x == "hello" {
+        todo!()
+    } else if y == "world" {
+        println!("Hello world!");
+    }
+    //~^^^^^^ collapsible_else_if
+
+    if x == "hello" {
+        todo!()
+    } else if y == "world" {
+        println!("Hello world!");
+    }
+    //~^^^^^ collapsible_else_if
+
+    if x == "hello" {
+        todo!()
+    } else if y == "world" {
+        println!("Hello world!");
+    }
+    //~^^^^^^ collapsible_else_if
+
+    if x == "hello" { 
+        todo!()
+    } else if y == "world" {
+        println!("Hello world!");
+    }
+    //~^^^^^ collapsible_else_if
+}

tests/ui-toml/collapsible_if/collapsible_else_if.rs

@@ -0,0 +1,45 @@
+#![allow(clippy::eq_op, clippy::nonminimal_bool)]
+
+#[rustfmt::skip]
+#[warn(clippy::collapsible_if)]
+fn main() {
+    let (x, y) = ("hello", "world");
+
+    if x == "hello" {
+        todo!()
+    } else {
+        // Comment must be kept
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+    //~^^^^^^ collapsible_else_if
+
+    if x == "hello" {
+        todo!()
+    } else { // Inner comment
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+    //~^^^^^ collapsible_else_if
+
+    if x == "hello" {
+        todo!()
+    } else {
+        /* Inner comment */
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+    //~^^^^^^ collapsible_else_if
+
+    if x == "hello" { 
+        todo!()
+    } else { /* Inner comment */
+        if y == "world" {
+            println!("Hello world!");
+        }
+    }
+    //~^^^^^ collapsible_else_if
+}

tests/ui-toml/collapsible_if/collapsible_else_if.stderr

@@ -0,0 +1,78 @@
+error: this `else { if .. }` block can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_else_if.rs:10:12
+   |
+LL |       } else {
+   |  ____________^
+LL | |         // Comment must be kept
+LL | |         if y == "world" {
+LL | |             println!("Hello world!");
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+   = note: `-D clippy::collapsible-else-if` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::collapsible_else_if)]`
+help: collapse nested if block
+   |
+LL ~     } else if y == "world" {
+LL +         println!("Hello world!");
+LL +     }
+   |
+
+error: this `else { if .. }` block can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_else_if.rs:20:12
+   |
+LL |       } else { // Inner comment
+   |  ____________^
+LL | |         if y == "world" {
+LL | |             println!("Hello world!");
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     } else if y == "world" {
+LL +         println!("Hello world!");
+LL +     }
+   |
+
+error: this `else { if .. }` block can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_else_if.rs:29:12
+   |
+LL |       } else {
+   |  ____________^
+LL | |         /* Inner comment */
+LL | |         if y == "world" {
+LL | |             println!("Hello world!");
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     } else if y == "world" {
+LL +         println!("Hello world!");
+LL +     }
+   |
+
+error: this `else { if .. }` block can be collapsed
+  --> tests/ui-toml/collapsible_if/collapsible_else_if.rs:39:12
+   |
+LL |       } else { /* Inner comment */
+   |  ____________^
+LL | |         if y == "world" {
+LL | |             println!("Hello world!");
+LL | |         }
+LL | |     }
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     } else if y == "world" {
+LL +         println!("Hello world!");
+LL +     }
+   |
+
+error: aborting due to 4 previous errors
+