Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable lint clippy::string_slice #1056

Open
EdJoPaTo opened this issue Apr 22, 2024 · 2 comments · May be fixed by #1064
Open

Enable lint clippy::string_slice #1056

EdJoPaTo opened this issue Apr 22, 2024 · 2 comments · May be fixed by #1064
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@EdJoPaTo
Copy link
Member

Problem

Slicing of strings can panic on multi width characters. See for example #1032

Solution

Enable the following lint to find all cases where this might happen (now and in the future).

https://rust-lang.github.io/rust-clippy/master/index.html#string_slice

This lint does not catch #1032 for example. But it catches other places. It is probably a good idea to suggest false negatives like #1032 to clippy to be integrated in this lint.

$ cargo clippy -- -W clippy::string_slice
warning: indexing into a string may panic if the index is within a UTF-8 character
   --> src/style/color.rs:237:49
    |
237 | ...                   u8::from_str_radix(&s[1..3], 16),
    |                                           ^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#string_slice
    = note: requested on the command line with `-W clippy::string-slice`

warning: indexing into a string may panic if the index is within a UTF-8 character
   --> src/style/color.rs:238:49
    |
238 | ...                   u8::from_str_radix(&s[3..5], 16),
    |                                           ^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#string_slice

warning: indexing into a string may panic if the index is within a UTF-8 character
   --> src/style/color.rs:239:49
    |
239 | ...                   u8::from_str_radix(&s[5..7], 16),
    |                                           ^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#string_slice

warning: indexing into a string may panic if the index is within a UTF-8 character
   --> src/widgets/reflow.rs:338:6
    |
338 |     &src[start..]
    |      ^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#string_slice

warning: `ratatui` (lib) generated 4 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 6.70s
@EdJoPaTo EdJoPaTo added the enhancement New feature or request label Apr 22, 2024
@joshka
Copy link
Member

joshka commented Apr 24, 2024

Sounds like a good plan. Just to clarify, this entails actually fixing those places identified by the lint, right?

@joshka joshka added the good first issue Good for newcomers label Apr 24, 2024
@EdJoPaTo
Copy link
Member Author

Enabling the lint and fixing the places yeah. But also finding cases where this lint is not noticing this bad behavior to report this and improve the lint (for us and everyone else).

Created rust-lang/rust-clippy#12708 for the missing hint on Cow, which ratatui uses heavily. Not sure whether there are other false negatives of the lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants