Auto merge of #12493 - y21:issue12491, r=Alexendoo

fix span calculation for non-ascii in `needless_return`

Fixes #12491

Probably fixes #12328 as well, but that one has no reproducer, so 🤷

The bug was that the lint used `rfind()` for finding the byte index of the start of the previous non-whitespace character:
```
// abc\n     return;
     ^
```
... then subtracting one to get the byte index of the actual whitespace (the `\n` here).
(Subtracting instead of adding because it treats this as the length from the `return` token to the `\n`)

That's correct for ascii, like here, and will get us to the `\n`, however for non ascii, the `c` could be multiple bytes wide, which would put us in the middle of a codepoint if we simply subtract 1 and is what caused the ICE.

There's probably a lot of ways we could fix this.
This PR changes it to iterate backwards using bytes instead of characters, so that when `rposition()` finally finds a non-whitespace byte, we *know* that we've skipped exactly 1 byte. This was *probably*(?) what the code was intending to do

changelog: Fix ICE in [`needless_return`] when previous line end in a non-ascii character

4 files changed, 36 insertions, 2 deletions

diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs
index 0b72c8a0719..77a954cff62 100644
--- a/clippy_lints/src/returns.rs
+++ b/clippy_lints/src/returns.rs
@@ -465,8 +465,8 @@ fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
 // Go backwards while encountering whitespace and extend the given Span to that point.
 fn extend_span_to_previous_non_ws(cx: &LateContext<'_>, sp: Span) -> Span {
     if let Ok(prev_source) = cx.sess().source_map().span_to_prev_source(sp) {
-        let ws = [' ', '\t', '\n'];
-        if let Some(non_ws_pos) = prev_source.rfind(|c| !ws.contains(&c)) {
+        let ws = [b' ', b'\t', b'\n'];
+        if let Some(non_ws_pos) = prev_source.bytes().rposition(|c| !ws.contains(&c)) {
             let len = prev_source.len() - non_ws_pos - 1;
             return sp.with_lo(sp.lo() - BytePos::from_usize(len));
         }
diff --git a/tests/ui/crashes/ice-12491.fixed b/tests/ui/crashes/ice-12491.fixed
new file mode 100644
index 00000000000..4ea480b0663
--- /dev/null
+++ b/tests/ui/crashes/ice-12491.fixed
@@ -0,0 +1,7 @@
+#![warn(clippy::needless_return)]
+
+fn main() {
+    if (true) {
+        // anything一些中文
+    }
+}
diff --git a/tests/ui/crashes/ice-12491.rs b/tests/ui/crashes/ice-12491.rs
new file mode 100644
index 00000000000..60add6afa2c
--- /dev/null
+++ b/tests/ui/crashes/ice-12491.rs
@@ -0,0 +1,8 @@
+#![warn(clippy::needless_return)]
+
+fn main() {
+    if (true) {
+        // anything一些中文
+        return;
+    }
+}
diff --git a/tests/ui/crashes/ice-12491.stderr b/tests/ui/crashes/ice-12491.stderr
new file mode 100644
index 00000000000..7cc418898e8
--- /dev/null
+++ b/tests/ui/crashes/ice-12491.stderr
@@ -0,0 +1,19 @@
+error: unneeded `return` statement
+  --> tests/ui/crashes/ice-12491.rs:5:24
+   |
+LL |           // anything一些中文
+   |  ____________________________^
+LL | |         return;
+   | |______________^
+   |
+   = note: `-D clippy::needless-return` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::needless_return)]`
+help: remove `return`
+   |
+LL -         // anything一些中文
+LL -         return;
+LL +         // anything一些中文
+   |
+
+error: aborting due to 1 previous error
+