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

reversed_empty_ranges should suggest alternatives for desirably empty ranges #5628

Closed
djc opened this issue May 21, 2020 · 2 comments · Fixed by #5638
Closed

reversed_empty_ranges should suggest alternatives for desirably empty ranges #5628

djc opened this issue May 21, 2020 · 2 comments · Fixed by #5638

Comments

@djc
Copy link
Contributor

djc commented May 21, 2020

With clippy 1.45.0-nightly (0aa6751 2020-05-20), I'm getting the following lint:

error: this range is empty and using it to index a slice will always yield an empty slice
   --> askama_shared/src/parser.rs:143:16
    |
143 |         (s, &s[0..0], &s[0..0])
    |                ^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges

As ought to be obvious to the reader, these slices are empty on purpose. However, neither the lint itself nor the further information explains a good alternative for empty slices (and some googling also didn't turn up anything so far). In particular, &[] doesn't work.

djc added a commit to rinja-rs/askama that referenced this issue May 21, 2020
@ebroto
Copy link
Member

ebroto commented May 21, 2020

Hey @djc, TBH I thought the use cases for actually wanting an empty slice would be rare and that it would be more often a developer error (the lint works with constants so it's not necessarily obvious at first sight). So in this case I think an allow could make sense.

Otherwise the lint could suggest using &[] as &[T] which does not require a binding. Would that work for you?

A third option would be not linting in this case, as it's the less problematic of all the possible problems that the lint tries to warn against. I would go for this if the use case happens to be more common that we thought.

I will be glad to work on making the changes whatever the outcome is :)

@djc
Copy link
Contributor Author

djc commented May 21, 2020

Yeah, I feel like the case of two identical literals should probably just be allowed. Adding the suggestion about casting using as sounds like a good idea either way!

@bors bors closed this as completed in a00025a May 26, 2020
blyxyas added a commit to blyxyas/rust-clippy that referenced this issue Jan 6, 2023
…or` loop.

Reading the documentation for the lint, one could expect that the lint works in all cases that `X == Y`. This is false.

While the lint was updated, the documentation wasn't.

More information about the `N..N` problem in rust-lang#5689 and rust-lang#5628
bors added a commit that referenced this issue Jan 6, 2023
[#10167] Clarify that the lint only works if x eq. y in a `for` loop.

Reading the documentation for the lint, one could expect that the lint works in all cases that `X == Y`. This is false.

While the lint was updated, the documentation wasn't.

More information about the `N..N` problem in #5689 and #5628

---

Fixes #10167
changelog: [`reversed_empty_ranges`]: Update and clarify documentation
blyxyas added a commit to blyxyas/rust-clippy that referenced this issue Jan 12, 2023
…or` loop.

Reading the documentation for the lint, one could expect that the lint works in all cases that `X == Y`. This is false.

While the lint was updated, the documentation wasn't.

More information about the `N..N` problem in rust-lang#5689 and rust-lang#5628
busgaidw2 added a commit to busgaidw2/askama that referenced this issue Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants