Prevent including preceeding whitespaces if line contains non blanks (rust-lang#14480)

This extra condition prevents a problem when removing the '}' in:
```rust
  ( // There was an opening bracket after the parenthesis, which has been removed
    // This is a comment
   })
```
Removing the whitespaces, including the linefeed, before the '}', would
put the closing parenthesis at the end of the `// This is a comment`
line, which would make it part of the comment as well. In this case, it
is best to keep the span on the '}' alone.

changelog: none

Note to reviewer: `manual_inspect` and `collapsible_if` were two lints
exhibiting this problem in cooked up test cases, which have been
included as non-regression tests.

Author: Manishearth

clippy_utils/src/source.rs

@@ -142,7 +142,19 @@ pub trait SpanRangeExt: SpanRange {
         map_range(cx.sess().source_map(), self.into_range(), f)
     }
 
-    /// Extends the range to include all preceding whitespace characters.
+    /// Extends the range to include all preceding whitespace characters, unless there
+    /// are non-whitespace characters left on the same line after `self`.
+    ///
+    /// This extra condition prevents a problem when removing the '}' in:
+    /// ```ignore
+    ///   ( // There was an opening bracket after the parenthesis, which has been removed
+    ///     // This is a comment
+    ///    })
+    /// ```
+    /// Removing the whitespaces, including the linefeed, before the '}', would put the
+    /// closing parenthesis at the end of the `// This is a comment` line, which would
+    /// make it part of the comment as well. In this case, it is best to keep the span
+    /// on the '}' alone.
     fn with_leading_whitespace(self, cx: &impl HasSession) -> Range<BytePos> {
         with_leading_whitespace(cx.sess().source_map(), self.into_range())
     }
@@ -263,10 +275,15 @@ fn map_range(
 }
 
 fn with_leading_whitespace(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> {
-    map_range(sm, sp.clone(), |src, range| {
-        Some(src.get(..range.start)?.trim_end().len()..range.end)
+    map_range(sm, sp, |src, range| {
+        let non_blank_after = src.len() - src.get(range.end..)?.trim_start().len();
+        if src.get(range.end..non_blank_after)?.contains(['\r', '\n']) {
+            Some(src.get(..range.start)?.trim_end().len()..range.end)
+        } else {
+            Some(range)
+        }
     })
-    .unwrap_or(sp)
+    .unwrap()
 }
 
 fn trim_start(sm: &SourceMap, sp: Range<BytePos>) -> Range<BytePos> {

tests/ui-toml/collapsible_if/collapsible_if.fixed

@@ -13,7 +13,7 @@ fn main() {
     //~^^^^^^ collapsible_if
 
     // The following tests check for the fix of https://github.com/rust-lang/rust-clippy/issues/798
-    if x == "hello" // Inner comment
+    if x == "hello"  // Inner comment
         && y == "world" {
             println!("Hello world!");
         }
@@ -26,7 +26,7 @@ fn main() {
         }
     //~^^^^^^ collapsible_if
 
-    if x == "hello" /* Inner comment */
+    if x == "hello"  /* Inner comment */
         && y == "world" {
             println!("Hello world!");
         }

tests/ui-toml/collapsible_if/collapsible_if.stderr

@@ -32,7 +32,7 @@ LL | |     }
    |
 help: collapse nested if block
    |
-LL ~     if x == "hello" // Inner comment
+LL ~     if x == "hello"  // Inner comment
 LL ~         && y == "world" {
 LL |             println!("Hello world!");
 LL ~         }
@@ -70,7 +70,7 @@ LL | |     }
    |
 help: collapse nested if block
    |
-LL ~     if x == "hello" /* Inner comment */
+LL ~     if x == "hello"  /* Inner comment */
 LL ~         && y == "world" {
 LL |             println!("Hello world!");
 LL ~         }

tests/ui/collapsible_if.fixed

@@ -152,3 +152,13 @@ fn main() {
         }
     }
 }
+
+#[rustfmt::skip]
+fn layout_check() -> u32 {
+    if true
+        && true {
+        }
+        // This is a comment, do not collapse code to it
+    ; 3
+    //~^^^^^ collapsible_if
+}

tests/ui/collapsible_if.rs

@@ -162,3 +162,13 @@ fn main() {
         }
     }
 }
+
+#[rustfmt::skip]
+fn layout_check() -> u32 {
+    if true {
+        if true {
+        }
+        // This is a comment, do not collapse code to it
+    }; 3
+    //~^^^^^ collapsible_if
+}

tests/ui/collapsible_if.stderr

@@ -172,5 +172,23 @@ LL |             println!("No comment, linted");
 LL ~         }
    |
 
-error: aborting due to 10 previous errors
+error: this `if` statement can be collapsed
+  --> tests/ui/collapsible_if.rs:168:5
+   |
+LL | /     if true {
+LL | |         if true {
+...  |
+LL | |     }; 3
+   | |_____^
+   |
+help: collapse nested if block
+   |
+LL ~     if true
+LL ~         && true {
+LL |         }
+LL |         // This is a comment, do not collapse code to it
+LL ~     ; 3
+   |
+
+error: aborting due to 11 previous errors
 

tests/ui/manual_inspect.fixed

@@ -107,7 +107,7 @@ fn main() {
             let _ = || {
                 let _x = x;
             };
-            return;
+            return ;
         }
         println!("test");
     });
@@ -185,3 +185,12 @@ fn main() {
         });
     }
 }
+
+#[rustfmt::skip]
+fn layout_check() {
+    if let Some(x) = Some(1).inspect(|&x| { println!("{x}"); //~ manual_inspect
+        // Do not collapse code into this comment
+         }) {
+        println!("{x}");
+    }
+}

tests/ui/manual_inspect.rs

@@ -197,3 +197,12 @@ fn main() {
         });
     }
 }
+
+#[rustfmt::skip]
+fn layout_check() {
+    if let Some(x) = Some(1).map(|x| { println!("{x}"); //~ manual_inspect
+        // Do not collapse code into this comment
+        x }) {
+        println!("{x}");
+    }
+}

tests/ui/manual_inspect.stderr

@@ -98,7 +98,7 @@ LL |         if x.is_empty() {
 LL |             let _ = || {
 LL ~                 let _x = x;
 LL |             };
-LL ~             return;
+LL ~             return ;
 LL |         }
 LL ~         println!("test");
    |
@@ -187,5 +187,18 @@ LL |
 LL ~             println!("{}", x);
    |
 
-error: aborting due to 13 previous errors
+error: using `map` over `inspect`
+  --> tests/ui/manual_inspect.rs:203:30
+   |
+LL |     if let Some(x) = Some(1).map(|x| { println!("{x}");
+   |                              ^^^
+   |
+help: try
+   |
+LL ~     if let Some(x) = Some(1).inspect(|&x| { println!("{x}");
+LL |         // Do not collapse code into this comment
+LL ~          }) {
+   |
+
+error: aborting due to 14 previous errors