rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
10.89k stars 1.46k forks source link

`haystack.ends_with(needle)` doesn't silence `clippy::arithmetic_side_effects` caused by `haystack.len()-needle.len()` #12704

Open Scripter17 opened 3 weeks ago

Scripter17 commented 3 weeks ago

Summary

When calling str::ends_with with a &str, Rust doesn't know that, for the method to return true, the length of the haystack has to be greater than or equal to the length of the needle.

Lint Name

arithmetic_side_effects

Reproducer

I tried this code:

fn main() {
    let mut a = "abc".to_string();
    let b = "c";
    if a.ends_with(b) {a.truncate(a.len()-b.len())};
}

with --warn clippy::arithmetic_side_effects

I saw this happen:

warning: arithmetic operation that can potentially result in unexpected side-effects
 --> src/main.rs:4:35
  |
4 |     if a.ends_with(b) {a.truncate(a.len()-b.len())};
  |                                   ^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects
  = note: requested on the command line with `-W clippy::arithmetic-side-effects`

warning: `asetest` (bin "asetest") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s

I expected to see this happen:

No warning because the thing being warned about can't happen.

Version

rustc 1.77.2 (25ef9e3d8 2024-04-09)
binary: rustc
commit-hash: 25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04
commit-date: 2024-04-09
host: x86_64-unknown-linux-gnu
release: 1.77.2
LLVM version: 17.0.6

Additional Labels

No response

c410-f3r commented 2 weeks ago

By design arithmetic_side_effects mostly sees the lhs/rhs expressions of the arithmetic operation and generally ignores the surrounding environment.

The indicated example happens to be using a locally known length but that is not always to case.

// If `string_ref` is zero, then `a.len()-b.len()` will underflow.
fn foo(string_ref: &str) {
    let mut a = string_ref.to_string();
    let b = "c";
    if a.ends_with(b) {a.truncate(a.len()-b.len())};
}

If you know for sure that an operation will never introduce surprising side-effects, then you can directly call wrapping_sub.