Rust-clippy: Lint string_lit_as_bytes should not be reported for string literal of length 33 and more

[antoyo]

Hello.
The lint string_lit_as_bytes reports a warning even if the string literal has a length of 33 or more.
However, the AsRef<[T]> for array is only defined for array of length up to 32.

A code example triggering a false positive is here.
More specifically, it is this code:

    let request = tls_handshake.and_then(|socket| {
        tokio_core::io::write_all(socket, "\
            GET / HTTP/1.0\r\n\
            Host: www.rust-lang.org\r\n\
            \r\n\
        ".as_bytes())
    });

Since the string has a length of 43, the the trait bound '[u8; 43]: std::convert::AsRef<[u8]>' is not satisfied (compiler error), so we cannot use as_bytes().

I believe this issue happens only when a trait like AsRef<[T]> is used.

Thanks to fix this issue.

[mcarton]

fn foo(a: &[u8]) {
    println!("{}", a.len());
}

fn main() {
    foo(b"azertyuiopqsdfghjklmwxcvbnazertyuiopqsdfghjklmwxcvbn");
}

prints 52.

[antoyo]

@mcarton While the following:

fn foo<A: AsRef<[u8]>>(a: A) {
    println!("{}", a.as_ref().len());
}

fn main() {
    foo(b"azertyuiopqsdfghjklmwxcvbnazertyuiopqsdfghjklmwxcvbn");
}

gives a the above compiler error.

And using:

    foo("azertyuiopqsdfghjklmwxcvbnazertyuiopqsdfghjklmwxcvbn".as_bytes());

triggers the clippy warning.

[mcarton]

Ah, I only looked at std::io::Write::write_all which takes &[u8].

[antoyo]

I am not sure it is related to the size of the string.
The following code produces the same issue:

let cstring = CString::new("Hello".as_bytes()).unwrap();

because using b"Hello" gives:

the trait bound `std::vec::Vec<u8>: std::convert::From<&[u8; 5]>` is not satisfied

[jens1o]

I'm also affected by this problem. Is there somewhere I could start contributing to fix this?

[oli-obk]

I believe all that is needed is an if lit_content.as_str().len() < 32 around https://github.com/rust-lang/rust-clippy/blob/15d1731ce8d3782ba93b0fd583307240fc814ef3/clippy_lints/src/strings.rs#L173-L211

Also a rustfix test ensuring that everything works correctly should be added so we don't regress this.

[jens1o]

I'd like to take this! :)

[jens1o]

@oli-obk Before opening a PR: I wonder what you exactly mean with a rustfix test? I've removed the test expectations and any error that should be added should change the stderr, shouldn't it?

[oli-obk]

Adding //run-rustfix to the top of the .rs file, should automatically generate a new file with all suggestions applied. If the resulting file does not compile successfully, the test fails.

This allows us to detect cases like this one, since the suggested change doesn't compile.

[phansch]

@jens1o I believe you forgot to open a PR with your fix?

[jens1o]

Yeah. I don't know whether there is missing something. If so, please, @phansch, take it over, as I'll be unavailable over the next week. :)

[thiagoarrais]

@jens1o: are you working on this? I'm interested in porting this to master.

[flip1995]

@thiagoarrais It seems you can take this issue. :)