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

Incorrect suggestion of string_lit_as_bytes lint #4494

Open
ordovicia opened this issue Sep 3, 2019 · 2 comments
Open

Incorrect suggestion of string_lit_as_bytes lint #4494

ordovicia opened this issue Sep 3, 2019 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@ordovicia
Copy link
Contributor

ordovicia commented Sep 3, 2019

Clippy version: clippy 0.0.212 (e3cb40e 2019-06-25)

Clippy suggests using a byte string literal b"string literal" instead of "string literal".as_bytes().
The suggestion does not compile in the following case because the type of byte string literal &[u8; N] does not implement std::io::Read.

fn f(_: impl std::io::Read) {
}

fn main() {
    f("string literal".as_bytes());
//    ^^^^^^^^^^^^^ help: consider using a byte string literal instead: `b"string literal"`

    // f(b"string literal");    // does not compile
    // ^ the trait `std::io::Read` is not implemented for `&[u8; 14]`
    
    f(&b"string literal"[..]);  // compiles
}

Playground

Clippy should allow the above case or suggest &b"string literal"[..].

@flip1995 flip1995 added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels Sep 4, 2019
@ghost
Copy link

ghost commented Sep 6, 2019

We should always suggest &b"string literal"[..]. Changing the type of the expression is asking for trouble and &b"string literal"[..] -> b"string literal" could be a separate lint anyway.

@mahkoh
Copy link

mahkoh commented Jul 9, 2020

The lint should instead be removed. The only justification for the lint given is that using byte strings is shorter. &b"string literal"[..] is perl-tier.

bors added a commit that referenced this issue Oct 8, 2020
Downgrade string_lit_as_bytes to nursery

Between #1402 (regarding `to_owned`) and #4494 (regarding `impl Read`), as well as other confusion I've seen hit in my work codebase involving string_lit_as_bytes (`"...".as_bytes().into()`), I don't think this lint is at a quality to be enabled by default.

I would consider re-enabling this lint after it is updated to understand when the surrounding type information is sufficient to unsize `b"..."` to &\[u8\] without causing a type error.

As currently implemented, this lint is pushing people to write `&b"_"[..]` which is not an improvement over `"_".as_bytes()` as far as I am concerned.

---

changelog: Remove string_lit_as_bytes from default set of enabled lints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing E-medium Call for participation: Medium difficulty level problem and requires some initial experience. I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

No branches or pull requests

3 participants