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

verbose-file-reads false positive #5368

Open
MikailBag opened this issue Mar 24, 2020 · 4 comments
Open

verbose-file-reads false positive #5368

MikailBag opened this issue Mar 24, 2020 · 4 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@MikailBag
Copy link

Reproducer on playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a06d5c34e0ec3cd9eafbd42af458028f

std::fs::read only supports reading from path, not from raw handle.

@MikailBag
Copy link
Author

Second bug: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8e49e12c99d43fe69dbfdbe375f785bf
If this code will be rewritten using std::fs::read, it won't be able to reuse one allocation, because std::fs::read allocates and returns new buffer

@MikailBag
Copy link
Author

Third bug: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=9f18b7ce2a3758ff5d37cd4e08ca4afe
Current behavior: all file are read into same buffer, so their contents are concatenated easily.
With std::fs::read: I need manually append.
It is less efficient and it is not less verbose

@flip1995
Copy link
Member

I'd move this lint to nursery, but since these bugs are hard to fix, and therefore this lint probably won't get out of nursery any time soon. But since I think the lint idea is good, I'll move it to restriction, so it can be enabled if someone wants to check their code about this.

@flip1995 flip1995 added the C-bug Category: Clippy is not doing the correct thing label Mar 26, 2020
@vorner
Copy link
Contributor

vorner commented Mar 27, 2020

I would like to add another false positive to this lint. Here the provided hint doesn't really make sense at all:

use std::fs::File;
use std::io::Read;
fn main() {
    let mut f = File::open("example.txt").unwrap();
    let mut header = [0; 4];
    f.read_exact(&mut header).unwrap();
    let mut rest = Vec::new();
    f.read_to_end(&mut rest).unwrap();
}

Further similar ones would include seeking around the file and reading it multiple times, opening the file sooner, then deleting it on the FS but keeping it open and similar cases.

Clippy version:

clippy 0.0.212 (23549a8 2020-03-16)

bors added a commit that referenced this issue Mar 27, 2020
…hiaskrgr

Move verbose_file_reads to restriction

cc #5368

Using `File::read` instead of  `fs::read_to_end` does make sense in multiple cases, so this lint is rather restriction, than complexity

changelog: Move [`verbose_file_reads`] to restriction
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 19, 2020
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 I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

4 participants