new lint: bufreader_stdin#13682
Conversation
|
r? @Manishearth rustbot has assigned @Manishearth. Use |
11c3cce to
c266263
Compare
| /// let reader = stdin.lock(); | ||
| /// ``` | ||
| #[clippy::version = "1.84.0"] | ||
| pub BUFREADER_STDIN, |
There was a problem hiding this comment.
nit: I think Buf and Reader should be separated words? no?
There was a problem hiding this comment.
nah, this is BufReader + Stdin, so I'd keep BufReader as a single atomic unit in the name
| let reader = a.lock(); | ||
|
|
||
| let b = io::stdin().lock(); | ||
| let reader = b; |
There was a problem hiding this comment.
we should be very careful dealing with suggestions where type changes occur, this suggestion won't work if the resulting variable is used somewhere else, for example:
use std::io::{self, BufReader};
use std::io::StdinLock;
fn main() {
let b = io::stdin().lock();
let reader = BufReader::new(b);
// let reader = b; // does not work
i_want_buf_reader(reader);
}
fn i_want_buf_reader(reader: BufReader<StdinLock>) {
// do something with the reader
drop(reader);
}this will not compile, and another example would be
static READER_ONCE: OnceLock<BufReader<Stdin>> = OnceLock::new();
let a = READER_ONCE.get_or_init(|| BufReader::new(io::stdin()));although these are very unlikely to happen, but you can never be too careful~ So I'd say at least changing the applicability to MaybeIncorrect?
Also, it would be nice to run lintcheck to see if there are any false positive cases.
There was a problem hiding this comment.
Yes, these suggestions aren't MachineApplicable. MaybeIncorrect, unfortunately.
| error: unknown lint: `clippy::BUFREADER_STDIN` | ||
| --> tests/ui/bufreader_stdin.rs:1:9 | ||
| | | ||
| LL | #![warn(clippy::BUFREADER_STDIN)] | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^ help: did you mean: `clippy::bufreader_stdin` | ||
| | | ||
| = note: `-D unknown-lints` implied by `-D warnings` | ||
| = help: to override `-D warnings` add `#[allow(unknown_lints)]` |
There was a problem hiding this comment.
looks like an accident, make sure to change it to lowercase name
|
☔ The latest upstream changes (possibly d28d234) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Ping @yawara from triage. Do you plan to return to working on this? |
|
Closing since there's been no response. @yawara if you wish to return to this feel free to reopen or create a new PR. |
changelog: [
bufreader_stdin]: new lintfixes #6755