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
11.3k stars 1.53k forks source link

Complexity lint against `some_string[0..some_len].len()` #13433

Open LikeLakers2 opened 1 week ago

LikeLakers2 commented 1 week ago

What it does

I apologize if this comes across as confusing - I'm not entirely sure how to word it.

This lint would look for instances where the user is getting a slice of a string, with the starting point of the range being 0, and then calling len() on that slice.

The reason I believe this works is because impl SliceIndex<str> for Range<usize> states the following:

Returns a slice of the given string from the byte range [begin, end).

This means that, if you're getting a string slice from 0 to some_var, some_var will already need to be a length in bytes. Thus, some_string[0..some_calculated_len] will always be some_calculated_len bytes long. And thus, some_string[0..some_calculated_len].len() will always be the same as just some_calculated_len.

Advantage

Less complex-looking code, while still functioning the same.

Drawbacks

None that I'm aware of - the code should function exactly the same afterwards.

Example

let len_in_bytes = some_string[0..the_len_we_want].len();

Could be written as:

let len_in_bytes = the_len_we_want;
LikeLakers2 commented 1 week ago

As an aside to this: I don't know if we can apply this same logic to something like let my_len = my_string[start..end].len() (by suggesting it be changed to let my_len = end - start).

The math does check out the same - but with the suggested version, the panic that occurs if start > end would only occur with overflow checks enabled; whereas with the string slicing operation, that condition is always checked.

This could be solved by having clippy add an assert, but I don't know if it'd be acceptable to have clippy doing that.

antonilol commented 1 week ago

In my opinion the lint message should include something like 'if you want to count the amount of characters in this slice use .chars().count()', as that might be intended by the programmer. (inspired by warnings in docs of str::len and str::chars)

PatchMixolydic commented 1 week ago

'if you want to count the amount of characters in this slice use .chars().count()',

⚠️ str::chars().count() does not return the number of characters in a string.

(Some of these may look weird in the code editor, but will display properly in the exection output.)