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

Lint to prevent people from indexing a string via iterator.nth(i) #6391

Closed
sebschmi opened this issue Nov 27, 2020 · 4 comments · Fixed by #6695
Closed

Lint to prevent people from indexing a string via iterator.nth(i) #6391

sebschmi opened this issue Nov 27, 2020 · 4 comments · Fixed by #6695
Assignees
Labels
A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@sebschmi
Copy link

sebschmi commented Nov 27, 2020

What it does

It is possible to access the nth byte of a string using string.bytes().nth(i). But in case of i = 0 this collides with the lint iter-nth-zero.
Alternatively one could write string.as_bytes().get(1), which does not produce any clippy warnings. This code is equivalent to the code above in what it does.

Categories (optional)

  • Kind: maybe clippy::style and maybe clippy::pedantic. Maybe also clippy::perf.

Advantages

  • Does not wrongly warn about iter-nth-zero when accessing the first byte of a string
  • Does not rely on the compiler to optimise out the iteration when doing direct indexed access

Drawbacks

The iteration is very easy for the compiler to optimise out, so that argument is weak.
The argument about iter-nth-zero is valid but it might be better to patch iter-nth-zero instead of teaching people how to code around the drawbacks of iter-nth-zero.

Maybe iter-nth-zero should become pedantic instead?

Example

string.bytes().nth(i)

Could be written as:

string.as_bytes().get(i)
@sebschmi sebschmi added the A-lint Area: New lints label Nov 27, 2020
@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages L-suggestion Lint: Improving, adding or fixing lint suggestions labels Nov 27, 2020
@TaKO8Ki
Copy link
Member

TaKO8Ki commented Dec 25, 2020

Can I tackle this issue?

@giraffate
Copy link
Contributor

I think you can take this.

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Feb 2, 2021

I'm going to tackle this issue soon.

@TaKO8Ki
Copy link
Member

TaKO8Ki commented Feb 6, 2021

@rustbot claim

This was referenced Feb 7, 2021
@bors bors closed this as completed in b5e4389 Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants