New lint needless_as_bytes

[samueltardieu]

changelog: [needless_as_bytes]: new lint

Fix #13434

[rustbot]

r? @y21

rustbot has assigned @y21.
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

[LikeLakers2]

LGTM, at least documentation-wise. Thank you for making the lint I suggested. :)

[bors]

☔ The latest upstream changes (presumably #13322) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

Apart from the missing ui test annotations, looks good to me, nice work!

[samueltardieu]

I've added the missing test annotations, thanks for the review!

[samueltardieu]

@rustbot ready

[GuillaumeGomez]

Looks all good to me, thanks! Let's just wait someone from the team to approve it too. :)

[y21]

Looks pretty good to me already, most of the comments I initially had were mentioned by others and are now resolved :)

[samueltardieu]

@y21 Do you see something that needs to be changed?

[y21]

I'll try to take another look later today

[y21]

Looks good to me!

[LikeLakers2]

Not a team member, but figured I'd do one final approval of my own, to let you know that I don't see anything else I think needs fixing.

[y21]

FCP¹ thread: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/FCP.3A.20needless_as_bytes

Footnotes

1. final comment period, a process that all new lints need to go through where others can share "last minute" concerns ↩

[samueltardieu]

@y21 Any news?

[y21]

Sorry, have not had much time the last few days. There are some comments on the Zulip thread suggesting that this is something that some people do intentionally write to make it clear that it's the byte length (and this argument is also in the linked issue's drawbacks), so I'd want to see a poll on the Zulip for the category. I'll try to do it later today.

I'm personally in favor of this lint being warn-by-default, but we shouldn't have warn-by-default lints that people find hurts readability.

[LikeLakers2]

@y21 I think it's also worth weighing the possibility of people simply not knowing string.len() == string.as_bytes().len(). Though, whether you'd be weighing it for or against warn-by-default, is up to you.

That said, as long as the lint exists in any capacity, I would be happy with the results.

[y21]

It's been about a week since I've posted the poll on zulip and the majority agrees with the current category (complexity, warn-by-default), so I'd say we're good to go.

Can you update the #[clippy::version] attribute? Also, you'll probably have to rebase and run cargo dev update_lints since this is going to be the 750th lint

[y21]

Suggested change

	#[clippy::version = "1.83.0"]
	#[clippy::version = "1.84.0"]

[samueltardieu]

It's been about a week since I've posted the poll on zulip and the majority agrees with the current category (complexity, warn-by-default), so I'd say we're good to go.

Can you update the #[clippy::version] attribute? Also, you'll probably have to rebase and run cargo dev update_lints since this is going to be the 750th lint

Updated

[y21]

Thanks (also to @GuillaumeGomez and @LikeLakers2 for helping with reviewing)!

@bors r+

[bors]

📌 Commit f152bcb has been approved by y21

It is now in the queue for this repository.

[bors]

⌛ Testing commit f152bcb with merge 625d391...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: y21
Pushing 625d391 to master...