Skip to content

Add new lint for byte char slices#10155

Merged
bors merged 1 commit into
rust-lang:masterfrom
TheNeikos:feature/add_byte_char_slice_lint
Jul 4, 2024
Merged

Add new lint for byte char slices#10155
bors merged 1 commit into
rust-lang:masterfrom
TheNeikos:feature/add_byte_char_slice_lint

Conversation

@TheNeikos

@TheNeikos TheNeikos commented Jan 4, 2023

Copy link
Copy Markdown
Contributor

This patch adds a new lint that checks for potentially harder to read byte char slices: &[b'a', b'b'] and suggests to replace them with the easier to read b"ab" form.

Fixes #10147


changelog: new lint: [byte_char_slices]
#10155

@rustbot

rustbot commented Jan 4, 2023

Copy link
Copy Markdown
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 4, 2023
@TheNeikos

Copy link
Copy Markdown
Contributor Author

Potentially also @Alexendoo ? (Since you 'approved' the idea)

@TheNeikos TheNeikos force-pushed the feature/add_byte_char_slice_lint branch 5 times, most recently from 4ca3bfb to 2e49b11 Compare January 4, 2023 10:05
Comment thread clippy_lints/src/byte_char_slice.rs Outdated
Comment thread clippy_lints/src/byte_char_slice.rs Outdated
Comment thread tests/ui/byte_char_slice.stderr Outdated
@Manishearth

Copy link
Copy Markdown
Member
---- src/byte_char_slice.rs - byte_char_slice::BYTE_CHAR_SLICE (line 21) stdout ----
error[E0308]: mismatched types
 --> src/byte_char_slice.rs:22:1
  |
2 | fn main() { #[allow(non_snake_case)] fn _doctest_main_src_byte_char_slice_rs_21_0() {
  |                                                                                     - help: try adding a return type: `-> &'static [u8; 5]`
3 | b"Hello"
  | ^^^^^^^^ expected `()`, found `&[u8; 5]`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
Couldn't compile the test.
---- src/byte_char_slice.rs - byte_char_slice::BYTE_CHAR_SLICE (line 17) stdout ----
error[E0308]: mismatched types
 --> src/byte_char_slice.rs:18:1
  |
2 | fn main() { #[allow(non_snake_case)] fn _doctest_main_src_byte_char_slice_rs_17_0() {
  |                                                                                     - help: try adding a return type: `-> &[u8; 5]`
3 | &[b'H', b'e', b'l', b'l', b'o']
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `&[u8; 5]`

error: aborting due to previous error

@bors

bors commented Jan 19, 2023

Copy link
Copy Markdown
Contributor

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

@llogiq

llogiq commented Mar 24, 2023

Copy link
Copy Markdown
Contributor

@TheNeikos I think we can merge this after a rebase.

@xFrednet

Copy link
Copy Markdown
Contributor

Hey @TheNeikos this is a ping from triage. Would you mind rebasing this PR?

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 21, 2024
@xFrednet xFrednet force-pushed the feature/add_byte_char_slice_lint branch 4 times, most recently from 27bfb1b to e252f22 Compare June 20, 2024 19:48
@xFrednet

Copy link
Copy Markdown
Contributor

I've just rebased, since that seamed to be the last thing holding this back. I'll create an FCP for this.

@xFrednet xFrednet added S-final-comment-period Status: final comment period it will be merged unless new objections are raised (~1 week) and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jun 20, 2024
@bors

bors commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

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

This patch adds a new lint that checks for potentially harder to read
byte char slices: `&[b'a', b'b']` and suggests to replace them with the
easier to read `b"ab"` form.

Signed-Off-By: Marcel Müller <m.mueller@ifm.com>
Co-authored-by: Matthias Beyer <matthias.beyer@ifm.com>

Use iterator to skip validation

Signed-off-by: Marcel Müller <m.mueller@ifm.com>
Suggested-by: Alex Macleod <alex@macleod.io>

Convert quote escapes to proper form

Signed-off-by: Marcel Müller <m.mueller@ifm.com>

Add more convertable test cases

Signed-off-by: Marcel Müller <m.mueller@ifm.com>
@xFrednet xFrednet force-pushed the feature/add_byte_char_slice_lint branch from e252f22 to 88c4a22 Compare July 4, 2024 12:31
@xFrednet

xFrednet commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

I'd say the FCP is done. The only feedback was to rename the lint, which I've done.

@Manishearth Can we r+ it, or do you want to check something else?

FCP: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/FCP.3A.20.60byte_char_slice.60.20rust-clippy.2310155/near/445939887

@xFrednet xFrednet added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-final-comment-period Status: final comment period it will be merged unless new objections are raised (~1 week) labels Jul 4, 2024
@Manishearth

Copy link
Copy Markdown
Member

Yeah that sounds good!

@bors r+

@bors

bors commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

📌 Commit 88c4a22 has been approved by Manishearth

It is now in the queue for this repository.

@bors

bors commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

⌛ Testing commit 88c4a22 with merge 2b01d69...

@bors

bors commented Jul 4, 2024

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 2b01d69 to master...

@bors bors merged commit 2b01d69 into rust-lang:master Jul 4, 2024
@TheNeikos TheNeikos deleted the feature/add_byte_char_slice_lint branch July 5, 2024 08:05
@danielocfb

Copy link
Copy Markdown

I think this lint has a false positive

error: can be more succinctly written as a byte str
   --> src/symbolize/perf_map.rs:381:58
    |
381 |         let () = child.stdin.as_ref().unwrap().write_all(&[b'\n']).unwrap();
    |                                                          ^^^^^^^^ help: try: `b"\n"`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#byte_char_slices
    = note: `-D clippy::byte-char-slices` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::byte_char_slices)]`

The suggestion seems blatantly wrong, as the code doesn't even compile if I follow it:

error[E0308]: mismatched types
   --> src/symbolize/perf_map.rs:382:60
    |
382 |         let () = child.stdin.as_ref().unwrap().write_all(&[b"\n"]).unwrap();
    |                                                            ^^^^^ expected `u8`, found `&[u8; 1]`

@y21

y21 commented Sep 5, 2024

Copy link
Copy Markdown
Member

@danielocfb The arrows in the diagnostic cover the whole function argument, so clippy is suggesting .write_all(b"\n"), which should compile.

@danielocfb

Copy link
Copy Markdown

Ah my bad. Sorry for the noise then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use byte string instead of slice of byte characters

9 participants