Checks for `Read::bytes()` on an unbuffered Read type.
The default implementation calls `read` for each byte, which can be very
inefficient for data that’s not in memory, such as `File`.
Considerations which I'd like to have feedback on:
* Currently this lint triggers when `.bytes()` is called on any type
that implements `std::io::Read` but not `std::io::BufRead`. This is
quite aggressive and in and may result in false positives. Alternatives:
* Only trigger on concrete types, not generic types. This does mean that
cases where a function is generic over a `R: Read` and calls `.bytes()`
are not caught by the lint, which could be quite a nasty case of this
bug.
* Only trigger on an allowlist of stdlib types
* Compromise: Is it possible to make this lint `pedantic` on types that
are not on a allowlist?
* Theoretically, a trait implementation of `Read` could override
`.bytes()` with an efficient implementation. I'm not sure how to add
this check to the lint, and I can't find any cases of this being done in
practice.
* I don't think an automatic fix for this lint is possible, but I'd love
to be proven wrong
* This is my first time contributing to clippy, please let me know if I
did anything wrong
Fixes#14087
```
changelog: [`unbuffered_bytes`]: new lint
```
By default, do not lint `.unwrap()` and `.expect(…)` in always const
contexts, as a failure would be detected at compile time anyway.
New options `allow_expect_in_consts` and `allow_unwrap_in_consts`,
defaulting to `true`, can be turned unset to still lint in always const
contexts.
changelog: [`declare_interior_mutable_const`,
`borrow_interior_mutable_const`]: resolve `<T as Trait>::AssocT`
projections
---
This came up during https://github.com/rust-lang/rust/pull/130543 where
we have `<T as AtomicPrimitive>::Assoc = AtomicT` instead of just
`AtomicT` and clippy failed to resolve that properly.
This really needs a review, because
- I don't know if `try_normalize_erasing_regions` is the right thing to
call here.
- I'm not sure if I peel off the correct amount of `ValTree::Branch`
layers (I think I do).
Also, shouldn't this lint's infrastructure rely on `Freeze` trait
(https://github.com/rust-lang/rust/issues/121675) instead of hardcoding
a list of known-to-be-interior-mutable types?
---
Previously filed this in the main rust repo
(https://github.com/rust-lang/rust/pull/136369), was asked to do it here
instead
(https://github.com/rust-lang/rust/pull/136369#issuecomment-2628527774).
When looking for `Default` impls that could be derived, we look at the
body of their `fn default()` and if it is an fn call or literal we check
if they are equivalent to what `#[derive(Default)]` would have used.
Now, when checking those fn calls in the `fn default()` body, we also
compare against the corresponding type's `Default::default` body to see
if our call is equivalent to that one.
For example, given
```rust
struct S;
impl S {
fn new() -> S { S }
}
impl Default for S {
fn default() -> S { S::new() }
}
```
`<S as Default>::default()` and `S::new()` are considered equivalent.
Given that, if the user also writes
```rust
struct R {
s: S,
}
impl Default for R {
fn default() -> R {
R { s: S::new() }
}
}
```
the `derivable_impls` lint will now trigger.
The first commit fixes#14164 by making sure that temporaries with
non-static references are also looked for in expressions coming from
expansion. The shortcut that was done skipped those parts and reported
an absence of short-lived temporaries, which was incorrect.
The second commit distinguishes between edition 2024 and earlier ones.
Starting from edition 2024, the problematic drop order has been fixed,
and block variables, which might be referenced in a block expression,
are freed after the block expression itself. This allows more
`let_and_return` cases to be reported starting with edition 2024,
whereas in earlier editions an intermediary variable was necessary to
reorder the drops.
Incidentally, since Clippy is compiled in edition 2024 mode, the second
commit has led to a fix in
`clippy_lints/src/matches/significant_drop_in_scrutinee.rs`.
changelog: [`let_and_return`]: lint more cases in edition 2024, and fix
a false positive involving short-lived block temporary variables in
earlier editions.
Checks for functions with method calls to `.map(_)` on an arg of type
`Option` as the outermost expression.
Fixes#774
```
changelog: [`single_option_map`]: Checks for functions with method calls to `.map(_)` on an arg of type `Option` as the outermost expression.
```
One cannot avoid descending into expansion results when looking for
non-static references, or there is a risk of false negative which would
then trigger the `let_and_return` lint.
fixes: #12659
[`manual_map`] and [`manual_filter`] shares the same check logic, but
this issue doesn't seems like it could affect `manual_filter` (?)
---
changelog: make [`manual_map`] ignore types that contain `dyn`
An `if … { … } else { … }` used as the left operand of a binary
expression requires parentheses to be parsed as an expression.
Fix#11141
changelog: [`obfuscated_if_else`]: fix bug in suggestion by issuing
required parentheses around the left side of a binary expression
While extending the `option_map_or_err_ok` lint (warn by default,
"style") to recognize η-expanded forms of `Ok`, as in
```rust
// Should suggest `opt.ok_or("foobar")`
let _ = opt.map_or(Err("foobar"), |x| Ok(x));
```
I discovered that the `manual_ok_or` lint (allow by default, "pedantic")
already covered exactly the cases handled by `option_map_or_err_ok`,
including the one I was adding. Apparently, `option_map_or_err_ok` was
added without realizing that the lint already existed under the
`manual_ok_or` name. As a matter of fact, artifacts of this second lint
were even present in the first lint `stderr` file and went unnoticed for
more than a year.
This PR:
- deprecates `option_map_or_err_ok` with a message saying to use
`manual_ok_or`
- moves `manual_ok_or` from "pedantic" to "style" (the category in which
`option_map_or_err_ok` was)
In addition, I think that this lint, which is short, machine applicable,
and leads to shorter and clearer code with less arguments (`Ok`
disappears) and the removal of one level of call (`Err(x)` is replaced
by `x`), is a reason by itself to be in "style".
changelog: [`option_map_or_err_ok` and `manual_ok_or`]: move
`manual_ok_or` from "pedantic" to "style", and deprecate the redundant
style lint `option_map_or_err_ok`.
Removing the `.as_ref()` or `.as_mut()` as the top-level expression in a
closure may change the type of the result. In this case, it may be
better not to lint rather than proposing a fix that would not work.
changelog: [`useless_asref`]: do not remove the `.as_ref()` or
`.as_mut()` call if this would change the type of the enclosing closure
Fix#14088
Proposing to replace
```rust
let mut x = PathBuf::from("/foo");
x.push("/bar");
```
by
```rust
let mut x = PathBuf::from("/foo");
x.push("bar");
```
changes the content of `x` (`/bar` ⇒ `/foo/bar`). This is not equivalent
and should not be `MachineApplicable`, even if the original code is
suspicious.
changelog: none
Fix the version of `doc_overindented_list_items`. It actually will be in
1.86.0, not 1.80.0.
(https://github.com/rust-lang/rust/pull/136209 has a milestone of
1.86.0)
changelog: none
- `reindent_multiline()` always returns the result of
`reindent_multiline_inner()` which returns a `String`. Make
`reindent_multiline()` return a `String` as well, instead of a
systematically owned `Cow<'_, str>`.
- There is no reason for `reindent_multiline()` to force a caller to
build a `Cow<'_, str>` instead of passing a `&str` directly, especially
considering that a `String` will always be returned.
Also, both the input parameter and return value (of type `Cow<'_, str>`)
shared the same (elided) lifetime for no reason: this worked only
because the result was always the `Cow::Owned` variant which is
compatible with any lifetime.
As a consequence, the signature changes from:
```rust
fn reindent_multiline(s: Cow<'_, str>, …) -> Cow<'_, str> { … }
```
to
```rust
fn reindent_multiline(s: &str, …) -> String { … }
```
changelog: none
`MachineApplicable` is appropriate for the applicability of this lint.
changelog: [`if_then_some_else_none`]: change the applicability to
`MachineApplicable`
fixes#12407
I think it is meaningful to emit a warning even if the span is in a
macro.
changelog: [`manual_async_fn`]: don't emit suggestion inside macro
Counting in bytes for a pointer to `u8` is legitimate and must not
trigger the lint. Also, this prevents linting the
`{std,core}::ptr::write_bytes` as it manipulates bytes.
Fix#6590
changelog: [`size_of_in_element_count`]: do not lint if the pointee type
is `u8`
