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

Flip order of s[a..b].as_bytes() #10981

Open
A-Walrus opened this issue Jun 18, 2023 · 2 comments
Open

Flip order of s[a..b].as_bytes() #10981

A-Walrus opened this issue Jun 18, 2023 · 2 comments
Labels
A-lint Area: New lints

Comments

@A-Walrus
Copy link

What it does

Suggests replacing slicing a str followed by as_bytes with the opposite order

s[a..b].as_bytes()

with

&s.as_bytes()[a..b]

Advantage

  • Removes unnecessary utf8 indexing check
  • won't panic if a and b are not on utf8 boundaries

Drawbacks

Technically possible someone relied on it panicking, but find that unlikely.

Example

let s = "Hello World";
let bytes = s[1..3].as_bytes();

Could be written as:

let s = "Hello World";
let bytes = &s.as_bytes()[1..3];
@A-Walrus A-Walrus added the A-lint Area: New lints label Jun 18, 2023
@A-Walrus
Copy link
Author

I'd like to work on this but I'm new to the codebase and might need some guidance.
Want to make sure I'm on the right track... this lint should implement the LateLintPass check_expr method right?

@y21
Copy link
Member

y21 commented Jun 18, 2023

This seems like it would need a LateLintPass, since you need to know that the type of the expression being indexed is str (type information are only available in late lint passes), and check_expr sounds right to me. I would implement the check_expr method, check if the expression being checked is a method call to str::as_bytes and the receiver of the call is an index expression, with the indexed type being str. The clippy documentation (specifically "common tools") is probably helpful here. Good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants