Fix sliced_string_as_bytes FP with a RangeFull

[yotamofek]

changelog: [sliced_string_as_bytes]: don't fire on str[..].as_bytes()

So I ran into this in some codebase I was working on,
where the lint fired on this line:

let string: &str;
string[..].as_bytes()

So I was trying to understand the rationale behind this lint, and it says:

It involves doing an unnecessary UTF-8 alignment check which is less efficient, and can cause a panic.

This is obviously not true in the case where a RangeFull slice is being taken, since there is no UTF-8 boundary check, and no panic can be caused. So I created an exemption for RangeFulls.

Two other notes:

1. I'm not sure the word "alignment" in the lint's description (quoted above) is really correct, should probably say "char boundary" instead?
2. I might be missing something, but isn't there a lint for doing superfluous slice indexing, and then calling a slice method? e.g. str[..].len() or slice[..].len(), where str and slice are &str and &[T], respectively. If we had one, I would expect it to fire for the example code I quoted above.

[rustbot]

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[ada4a]

I'm not sure the word "alignment" in the lint's description (quoted above) is really correct, should probably say "char boundary" instead?

I mean this it's really a char boundary alignment check, so "UTF-8 alignment" isn't incorrect?

I might be missing something, but isn't there a lint for doing superfluous slice indexing, and then calling a slice method

The closest I could find is redundant_slicing? Though apparently it only lints on literally &str[..], which is why &string[..].as_bytes() wasn't linted, and (&string[..]).as_bytes() would be.

Adding support for that could be an improvement to the lint.

[llogiq]

Please add some comment as to why this is not linted.

[llogiq]

Just a very tiny nit, otherwise this looks good to merge.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[yotamofek]

@llogiq thanks!
Added the comment.
Also had to adapt for rust-lang/rust@ae9d202 , so ended up using higher::Range::hir instead of rustc_hir::is_range_literal, which I think is more correct? Not sure.

[ada4a]

I'm pretty sure higher::Range will also match Ranges that don't come from desugaring -- I think you'll need to use hir::Expr::range_span for this

[yotamofek]

I'm pretty sure higher::Range will also match Ranges that don't come from desugaring -- I think you'll need to use hir::Expr::range_span for this

it doesn't (I checked), but also, why not lint on e.g. Range { start: 1, end: 5 }?

[ada4a]

it doesn't (I checked)

Ah, that's because it calls range_span internally. Sorry for the noise!

why not lint on e.g. Range { start: 1, end: 5 }?

I'd say that generally, if someone is having a need to spell out Range explicitly, then they're probably doing something very specific, and so the lint could just be noise

[yotamofek]

it doesn't (I checked)

Ah, that's because it calls range_span internally. Sorry for the noise!

No worries, thanks for taking a look!

why not lint on e.g. Range { start: 1, end: 5 }?

I'd say that generally, if someone is having a need to spell out Range explicitly, then they're probably doing something very specific, and so the lint could just be noise

Good point.

[llogiq]

@yotamofek thank you.

I will merge this now, any other extension can be a followup PR.