[`map_entry`]: Check insert expression for map use
The lint makes sure that the map is not used (borrowed) before the call to `insert`. Since the lint creates a mutable borrow on the map with the `Entry`, it wouldn't be possible to replace such code with `Entry`. However, expressions up to the `insert` call are checked, but not expressions for the arguments of the `insert` call itself. This commit fixes that.
Fixes#11935
----
changelog: [`map_entry`]: Fix false positive when borrowing the map in the `insert` call
If the whole cast expression is a unary expression (`(*x as T)`) or an
addressof expression (`(&x as T)`), then not surrounding the suggestion
into a block risks us changing the precedence of operators if the cast
expression is followed by an operation with higher precedence than the
unary operator (`(*x as T).foo()` would become `*x.foo()`, which changes
what the `*` applies on).
The same is true if the expression encompassing the cast expression is a
unary expression or an addressof expression.
The lint supports the latter case, but missed the former one. This PR
fixes that.
Fixes#11968
The lint makes sure that the map is not used (borrowed) before the call
to `insert`. Since the lint creates a mutable borrow on the map with the
`Entry`, it wouldn't be possible to replace such code with `Entry`.
However, expressions up to the `insert` call are checked, but not
expressions for the arguments of the `insert` call itself. This commit
fixes that.
Fixes#11935
Fix sign-handling bugs and false negatives in `cast_sign_loss`
**Note: anyone should feel free to move this PR forward, I might not see notifications from reviewers.**
changelog: [`cast_sign_loss`]: Fix sign-handling bugs and false negatives
This PR fixes some arithmetic bugs and false negatives in PR #11883 (and maybe earlier PRs).
Cc `@J-ZhengLi`
I haven't updated the tests yet. I was hoping for some initial feedback before adding tests to cover the cases listed below.
Here are the issues I've attempted to fix:
#### `abs()` can return a negative value in release builds
Example:
```rust
i32::MIN.abs()
```
https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=022d200f9ef6ee72f629c0c9c1af11b8
Docs: https://doc.rust-lang.org/std/primitive.i32.html#method.abs
Other overflows that produce negative values could cause false negatives (and underflows could produce false positives), but they're harder to detect.
#### Values with uncertain signs can be positive or negative
Any number of values with uncertain signs cause the whole expression to have an uncertain sign, because an uncertain sign can be positive or negative.
Example (from UI tests):
```rust
fn main() {
foo(a: i32, b: i32, c: i32) -> u32 {
(a * b * c * c) as u32
//~^ ERROR: casting `i32` to `u32` may lose the sign of the value
}
println!("{}", foo(1, -1, 1));
}
```
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=165d2e2676ee8343b1b9fe60db32aadd
#### Handle `expect()` the same way as `unwrap()`
Since we're ignoring `unwrap()` we might as well do the same with `expect()`.
This doesn't seem to have tests but I'm happy to add some like `Some(existing_test).unwrap() as u32`.
#### A negative base to an odd exponent is guaranteed to be negative
An integer `pow()`'s sign is only uncertain when its operants are uncertain. (Ignoring overflow.)
Example:
```rust
((-2_i32).pow(3) * -2) as u32
```
This offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. (Rather than just an odd number of uncertain signs.)
#### Both sides of a multiply or divide should be peeled recursively
I'm not sure why the lhs was peeled recursively, and the rhs was left intact. But the sign of any sequence of multiplies and divides is determined by the signs of its operands. (Ignoring overflow.)
I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.
But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, these should all lint:
```rust
fn peel_all(x: i32) {
(-p(x) * -p(x) * -p(x)) as u32;
((-p(x) * -p(x)) * -p(x)) as u32;
(-p(x) * (-p(x) * -p(x))) as u32;
}
```
#### The right hand side of a Rem doesn't change the sign
Unlike Mul and Div,
> Given remainder = dividend % divisor, the remainder will have the same sign as the dividend.
https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators
I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.
But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, only the first six expressions should lint.
The expressions that start with a constant should lint (or not lint) regardless of whether the lint supports `p()` or unary negation, because only the dividend's sign matters.
Example:
```rust
fn rem_lhs(x: i32) {
(-p(x) % -1) as u32;
(-p(x) % 1) as u32;
(-1 % -p(x)) as u32;
(-1 % p(x)) as u32;
(-1 % -x) as u32;
(-1 % x) as u32;
// These shouldn't lint:
(p(x) % -1) as u32;
(p(x) % 1) as u32;
(1 % -p(x)) as u32;
(1 % p(x)) as u32;
(1 % -x) as u32;
(1 % x) as u32;
}
```
#### There's no need to bail on other expressions
When peeling, any other operators or expressions can be left intact and sent to the constant evaluator.
If these expressions can be evaluated, this offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. If not, they end up marked as having uncertain sign.
[`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls
This lint now also realizes that a comparison like `s == "foo"` and calls such as `s.ends_with("foo")` will fail if `s` was initialized by a call to `Stdin::read_line` (because of the trailing newline).
changelog: [`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls
r? `@giraffate` assigning you because you reviewed #10970 that added this lint, so this is kinda a followup PR ^^
fix suggestion error in [`useless_vec`]
fixes: #12101
---
changelog: fix suggestion error in [`useless_vec`]
r+ `@matthiaskrgr` since they opened the issue?
Empty docs
Fixes https://github.com/rust-lang/rust-clippy/issues/9931
changelog: [`empty_doc`]: Detects documentation that is empty.
changelog: Doc comment lints now trigger for struct field and enum variant documentation
When encountering code such as:
```
Box::new(outer::Inner::default())
```
clippy would suggest replacing with `Box::<Inner>::default()`, dropping
the `outer::` segment. This behavior is incorrect and that commit fixes
it.
What it does is it checks the contents of the `Box::new` and, if it is
of the form `A::B::default`, does a text replacement, inserting `A::B`
in the `Box`'s quickfix generic list.
If the source does not match that pattern (including `Vec::from(..)`
or other `T::new()` calls), we then fallback to the original code.
Fixes#11927
Look for `implied_bounds_in_impls` in more positions
With this, we lint `impl Trait` implied bounds in more positions:
- Type alias impl trait
- Associated type position impl trait
- Argument position impl trait
- these are not opaque types, but instead are desugared to `where` clauses, so we need extra logic for finding them (`check_generics`), however the rest of the logic is the same
Before this, we'd only lint RPIT `impl Trait`s.
"Hide whitespaces" and reviewing commits individually might make this easier
changelog: [`implied_bounds_in_impls`]: start linting implied bounds in APIT, ATPIT, TAIT
FIX(12243): redundant_guards
Fixed#12243
changelog: Fix[`redundant_guards`]
I have made a correction so that no warning does appear when y.is_empty() is used within a constant function as follows.
```rust
pub const fn const_fn(x: &str) {
match x {
// Shouldn't lint.
y if y.is_empty() => {},
_ => {},
}
}
```
A warning is now suppressed when "<str_va> if <str_var>.is_empty" is used in a constant function.
FIX: instead of clippy_util::in_const
FIX: Merged `redundant_guards_const_fn.rs` into `redundant_guards.rs`.
Extend `unnecessary_to_owned` to handle `Borrow` trait in map types
Fixes https://github.com/rust-lang/rust-clippy/issues/8088.
Alternative to #12315.
r? `@y21`
changelog: Extend `unnecessary_to_owned` to handle `Borrow` trait in map types
----
UPDATE: add async block into test.
FIX: no_effect
Fixed asynchronous function parameter names with underscores so that warnings are not displayed when underscores are added to parameter names
ADD: test case
Always evaluate free constants and statics, even if previous errors occurred
work towards https://github.com/rust-lang/rust/issues/79738
We will need to evaluate static items before the `definitions.freeze()` below, as we will start creating new `DefId`s (for nested allocations) within the `eval_static_initializer` query.
But even without that motivation, this is a good change. Hard errors should always be reported and not silenced if other errors happened earlier.
Default test output conflict handling to error
https://github.com/oli-obk/ui_test/pull/175 got rid of the `bool` that controlled the default handling so we need to specify it ourselves
r? `@flip1995`
changelog: none
Remove `$DIR` replacement
This won't cause problems because the old `$DIR` replacement was based on the parent of the test path, which for us is relative: 5471e0645a/tests/compile-test.rs (L122)
The new pattern being `"tests/{test_dir}"` is more clearly relative
That's why we have custom filters applied to the toml/cargo tests where absolute paths do appear in the output 5471e0645a/tests/compile-test.rs (L198-L202)
Removing it allows clicking the paths in the terminal
changelog: none
r? `@flip1995`
Ensure ASM syntax detect `global_asm!` and `asm!` only on x86 architectures
The ASM syntax lint is only relevant on x86 architectures, so this PR ensures it doesn't trigger on other architectures. This PR also makes the lints check `global_asm!` items as well as `asm!` expressions.
changelog: Check `global_asm!` items in the ASM syntax lints, and fix false positives on non-x86 architectures.
When encountering a verbose/multipart suggestion that has changes
that are only caused by different capitalization of ASCII letters that have
little differenciation, expand the message to highlight that fact (like we
already do for inline suggestions).
The logic to do this was already present, but implemented incorrectly.
Ignore imported items in `min_ident_chars`
Suppress the `min_ident_chars` warning for items whose name we cannot control. Do not warn for `use a::b`, but warn for `use a::b as c`, since `c` is a local identifier.
Fixes#12232
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`min_ident_chars`]: Do not warn on non-local identifiers
[`incompatible_msrv`]: allow expressions that come from desugaring
Fixes#12273
changelog: [`incompatible_msrv`]: don't lint on the `IntoFuture::into_future` call desugared by `.await`
Suppress the `min_ident_chars` warning for items whose name we cannot
control. Do not warn for `use a::b`, but warn for `use a::b as c`, since
`c` is a local identifier.
Fixes#12232
[`implied_bounds_in_impls`]: avoid linting on overlapping associated tys
Fixes#11880
Before this change, we were simply ignoring associated types (except for suggestion purposes), because of an incorrect assumption (see the comment that I also removed).
For something like
```rs
trait X { type T; }
trait Y: X { type T; }
// Can't constrain `X::T` through `Y`
fn f() -> impl X<T = i32> + Y<T = u32> { ... }
```
We now avoid linting if the implied bound (`X<T = i32>`) "names" associated types that also exists in the implying trait (`trait Y`). Here that would be the case.
But if we only wrote `impl X + Y<T = u32>` then that's ok because `X::T` was never constrained in the first place.
I haven't really thought about how this interacts with GATs, but I think it's fine. Fine as in, it might create false negatives, but hopefully no false positives.
(The diff is slightly annoying because of formatting things. Really the only thing that changed in the if chain is extracting the `implied_by_def_id` which is needed for getting associated types from the trait, and of course actually checking for overlap)
cc `@Jarcho` ? idk if you want to review this or not. I assume you looked into this code a bit to find this bug.
changelog: [`implied_bounds_in_impls`]: avoid linting when associated type from supertrait can't be constrained through the implying trait bound
[`mem_replace_with_default`] No longer triggers on unused expression
changelog:[`mem_replace_with_default`]: No longer triggers on unused expression
Change [`mem_replace_with_default`] to not trigger on unused expression because the lint from `#[must_use]` handle this case better.
fixes: #5586
Warn on references casting to bigger memory layout
This PR extends the [`invalid_reference_casting`](https://doc.rust-lang.org/rustc/lints/listing/deny-by-default.html#invalid-reference-casting) lint (*deny-by-default*) which currently lint on `&T -> &mut T` casting to also lint on `&(mut) A -> &(mut) B` where `size_of::<B>() > size_of::<A>()` (bigger memory layout requirement).
The goal is to detect such cases:
```rust
let u8_ref: &u8 = &0u8;
let u64_ref: &u64 = unsafe { &*(u8_ref as *const u8 as *const u64) };
//~^ ERROR casting references to a bigger memory layout is undefined behavior
let mat3 = Mat3 { a: Vec3(0i32, 0, 0), b: Vec3(0, 0, 0), c: Vec3(0, 0, 0) };
let mat3 = unsafe { &*(&mat3 as *const _ as *const [[i64; 3]; 3]) };
//~^ ERROR casting references to a bigger memory layout is undefined behavior
```
This is added to help people who write unsafe code, especially when people have matrix struct that they cast to simple array of arrays.
EDIT: One caveat, due to the [`&Header`](https://github.com/rust-lang/unsafe-code-guidelines/issues/256) uncertainty the lint only fires when it can find the underline allocation.
~~I have manually tested all the new expressions that warn against Miri, and they all report immediate UB.~~
r? ``@est31``
fix: ICE when array index exceeds usize
fixes#12253
This PR fixes ICE in `indexing_slicing` as it panics when the index of the array exceeds `usize`.
changelog: none
Don't allow derive macros to silence `disallowed_macros`
fixes#12254
The implementation is a bit of a hack, but "works". A derive expanding to another derive won't work properly, but we shouldn't be linting those anyways.
changelog: `disallowed_macros`: Don't allow derive macros to silence their own expansion
stop linting [`blocks_in_conditions`] on `match` with weird attr macro case
should fixes: #12016
---
changelog: [`blocks_in_conditions`] - fix FP on `match` with weird attr macro
This might not be the best solution, as the root cause (i think?) is the `span` of block was incorrectly given by the compiler?
I'm open to better solutions
