-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add new lint octal_escapes
#8007
Conversation
r? @xFrednet (rust-highfive has picked a reviewer for you, use r? to override) |
This checks for sequences in strings that would be octal character escapes in C, but are not supported in Rust. It suggests either to use the `\x00` escape, or an equivalent hex escape if the octal was intended.
b524984
to
982124a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR, the general structure is already good IMO :)
Ok, I think I got all the requested changes. However, I just found a pretty unfortunate problem: the format strings of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic looks good to me! I've marked to more test cases which would be good to have, then it's ready to be merged 🙃
clippy_lints/src/octal_escapes.rs
Outdated
if let Some((_, '0'..='7')) = iter.next() { | ||
to += 1; | ||
} | ||
emit(cx, &contents, from, to + 1, span, is_string); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past, Clippy had some errors with string indexing and Unicode characters. This should be safe, could you still add these test cases to ensure that is stays stable:
let _test_unicode = "锈\011锈"
let _test_unicode = "锈\01锈"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, that's a good point.
Interesting. @camsteffen I think you worked with the |
09a390c
to
850e7f5
Compare
This is a problem that has come up a few times. Format strings are just not available in their original form in post-expanded code. So I don't have a neat solution unfortunately. Getting a snippet and parsing it is indeed hacky and I think we should avoid re-parsing the string from scratch. |
Agreed, then that's just something we accept. It might be good to add this to the |
14ed409
to
0bc25d0
Compare
Looks good to me, thank you for the implementation and for the adjustments. I like this final version! @bors r+ |
📌 Commit 1210bb4 has been approved by |
Thanks! It was fun getting back to clippy after quite a few years :) |
Happy to hear that it's nice to have you back! I just looked back into the history, and you were a really early contributor. That's really awesome to see. The project and codebase probably changed quite a bit since back then ^^ |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
This checks for sequences in strings that would be octal character
escapes in C, but are not supported in Rust. It suggests either
to use the
\x00
escape, or an equivalent hex escape if the octalwas intended.
Fixes #7981
Please write a short comment explaining your change (or "none" for internal only changes)
changelog: Add new lint [
octal_escapes
], which checks for literals like"\033[0m"
.