Improve diagnostic of `no_mangle_with_rust_abi`
fixes#10409
Pending rust-lang/rustfmt#5701
This rewords the message to focus on the error being an implicit ABI, rather than the `Rust` ABI. Also downgrades the suggestion to `MaybeIncorrect` and changes the suggestion span to better highlight the change.
---
changelog: None
<!-- changelog_checked -->
Migrate `write.rs` to `rustc_ast::FormatArgs`
changelog: none
Part 1 of #10233
The additions to `clippy_utils` are the main novelty of this PR, there's no removals yet since other parts still rely on `FormatArgsExpn`
The changes to `write.rs` itself are relatively straightforward this time around, as there's no lints in it that rely on type checking format params
r? `@flip1995`
Include async functions in the len_without_is_empty
fixes#7232
Changes done to the functionality:
Allowing different error types for the functions was disallowed. So the following was linted before but is not after this change
```
impl Foo {
pub len(&self) -> Result<usize, Error1> { todo!(); }
pub is_empty(&self) -> Result<bool, Error2> { todo!(); }
}
```
---
changelog: Enhancement: [`len_without_is_empty`]: Now also detects `async` functions
[#10359](https://github.com/rust-lang/rust-clippy/pull/10359)
<!-- changelog_checked -->
[arithmetic_side_effects] Fix#10252Fix#10252
At least for integers, shifts are already handled by the compiler.
----
changelog: [`arithmetic_side_effects`]: No longer lints on right or left shifts with constant integers, as the compiler warns about them.
[#10309](https://github.com/rust-lang/rust-clippy/pull/10309)
<!-- changelog_checked-->
This was previously needed because the indirection used to hide some unexplained lifetime errors, which it turned out were related to the `min_choice` algorithm.
Removing the indirection also solves a couple of cycle errors, large moves and makes async blocks support the `#[track_caller]` annotation.
Add `collection_is_never_read`
Fixes#9267
`@flip1995` and `@llogiq,` I talked with you about this one at Rust Nation in London last week. :-)
This is my first contribution to Clippy, so lots of feedback would be greatly appreciated.
- \[ ] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
`dogfood` found one true positive (see #9509) and no false positives.
`lintcheck` found no (true or false) positives, even when running on an extended set of crates.
---
changelog: new lint [`collection_is_never_read`]
[#10415](https://github.com/rust-lang/rust-clippy/pull/10415)
<!-- changelog_checked -->
Scope `missing_docs_in_private_items` to only private items
`missing_docs_in_private_items` currently detects missing docs for public items as well as private. Since `missing_docs`already covers public items, this PR updates `missing_docs_in_private_items` to only cover private items.
Fixes#1895
changelog: [`missing_docs_in_private_items`]: Apply lint only to private items (used to be public and private)
Run dogfood to completion
Run dogfood on all packages before failing the test. Failing early is painful on lints which trigger on multiple crates.
changelog: None
Fix ICE in `multiple_unsafe_ops_per_block`
fixes#10367
changelog: [`multiple_unsafe_ops_per_block`]: Fix ICE when calling a function-like object in an unsafe block
Do not suggest to derive `Default` on generics with implicit arguments
Fixes#10396
changelog: FP: [`derivable_impls`]: do not suggest to derive `Default` on generics with implicit arguments
chore: remove unneeded rustfmt skip
---
The associated rustfmt bug that originally necessitated these skips was resolved a while back, so these are no longer necessary
changelog: none
Fix test function checker in `unwrap_used`, `expect_used`
After #9686 , `unwrap` and `expect` in integration tests and raw test functions won't be allowed.
fixes#10011fixes#10238fixes#10264
---
changelog: Fix: [`expect_used`], [`unwrap_used`], [`dbg_macro`], [`print_stdout`], [`print_stderr`]: No longer lint in test functions, if the related configuration is set
[#10391](https://github.com/rust-lang/rust-clippy/pull/10391)
<!-- changelog_checked -->
Normalize projections types when checking `explicit_auto_deref`
fixes#10384
changelog: [`explicit_auto_deref`]: Better consider projection types when checking if auto deref is applicable
Ignore lifetimes from differing contexts in `needless_lifetimes`
Fixes#10379
changelog: [`needless_lifetimes`]: Don't lint signatures in macros if the lifetime is a metavariable
Add `impl_trait_in_params` lint
As this is a lint about style, and using `impl Trait` is purely cosmetical (even with downsides), a lot of unrelated files needed to allow this lint.
---
Resolves#10030
changelog: New lint: [`impl_trait_in_params`]
[10197](https://github.com/rust-lang/rust-clippy/pull/10197)
<!-- changelog_checked -->
Add configuration to lint missing docs of `pub(crate)` items
Fixes this: https://github.com/rust-lang/rust-clippy/issues/5736#issuecomment-1412442404
TODO:
- [x] Needs docs
- [x] Needs better names
- [x] Should `pub` items be checked to when this new option is enabled? I'm saying no because `missing_docs` already exists
`@flip1995` I'd like to get some input from you :)
---
changelog: Enhancement: [`missing_docs_in_private_items`]: Added new configuration `missing-docs-in-crate-items` to lint on items visible within the current crate. For example, `pub(crate)` items.
[#10303](https://github.com/rust-lang/rust-clippy/pull/10303)
<!-- changelog_checked -->
Within a larger expression, when the type of `Box::new(T::default())` is
`Box<dyn Trait>`, the concrete type `T` cannot be omitted in the
proposed replacement `Box::<T>::default()`.
[significant_drop_tightening] Ignore inexpensive statements
Not all statements that follow the last use of a lock guard are expensive and can therefore be ignored by the lint.
```rust
pub fn foo() -> i32 {
let mutex = Mutex::new(1);
let lock = mutex.lock().unwrap();
let rslt = *lock;
let another = rslt;
another
}
```
---
changelog: [`significant_drop_tightening`]: No longer lints for inexpensive statements after the lock guard
[#10363](https://github.com/rust-lang/rust-clippy/pull/10363)
<!-- changelog_checked -->
manual_let_else: do not suggest semantically different replacements
The problem is that this lint does not consider the possibility that the divergent branch can come first and that the patterns may overlap. This led to incorrect suggestions, previously registered as correct in the tests themselves:
```rust
let v = match build_enum() {
_ => continue,
Variant::Bar(v) | Variant::Baz(v) => v,
};
```
had a `let Variant::Bar(v) | Variant::Baz(v) = v else { continue; }` suggestion, which is obviously wrong as the original code `continue`s in any case. Issue #10241 gives another example.
The code now checks that the divergent branch comes second. It could be extended later (I've added a TODO) to check for non-overlapping patterns.
Fixes#10241.
changelog: [`manual_let_else`] do not suggest non equivalent replacements in `match`
Stop bytes_nth from suggesting code that does not compile
Fixes#10151
As discussed in the issue, this PR changes the lint in 2 ways
1. Replace `bytes().nth(n).unwrap()` with `as_bytes()[n]`
2. Replace other `bytes().nth(n)` with `as_bytes().get(n).copied()`
---
changelog: Stop bytes_nth from suggesting code that does not compile in some cases
Stop doc_markdown requiring backticks on links to external websites
Fixes#10302
This lint currently checks that any link should be enclosed with `backticks` if the title looks like a lang item. This PR changes the lint to only run on internal links. External links, indicated by `http` or `https`, are skipped.
This PR also reorganises `pulldown_cmark` imports to bypass the clippy lint enforcing 100 line functions.
---
changelog: Stop doc_markdown requiring backticks on links to external websites
Add question-mark-used lint
This lint complains when the question mark operator (try operator) is used. This is a restriction lint that can be useful on local scopes where a custom error handling macro is supposed to be used to augment the error based on local scope data before returning.
Fixes#10340
---
changelog: New lint [`question_mark_used`]
[#10342](https://github.com/rust-lang/rust-clippy/pull/10342)
<!-- changelog_checked -->
Add `let_underscore_untyped`
Fixes#6842
This adds a new pedantic `let_underscore_untyped` lint which checks for `let _ = <expr>`, and suggests to either provide a type annotation, or to remove the `let` keyword. That way the author is forced to specify the type they intended to ignore, and thus get forced to re-visit the decision should the type of `<expr>` change. Alternatively, they can drop the `let` keyword to truly just ignore the value no matter what.
r? `@llogiq`
changelog: New lint: [let_underscore_untyped]
[significant_drop_tightening] Add MVP
cc #9399
Creates the lint with minimum functionalities, which is a good start IMO.
---
changelog: new lint: [`significant_drop_tightening`]
[#10163](https://github.com/rust-lang/rust-clippy/pull/10163)
<!-- changelog_checked -->
As this is a lint about "style", and a purely cosmetical choice (using `<A: Trait>` over `impl Trait`), a lot of other files needed to be allowed this lint.
Fix false positives for `extra_unused_type_parameters`
Don't lint external macros. Also, if the function body is empty, any type parameters with bounds on them are not linted. Note that only the body needs be empty - this rule still applies if the function takes any arguments.
fixes#10318fixes#10319
changelog: none
<!-- changelog_checked -->
uninlined_format_args: do not inline argument with generic parameters
Fix#10339
---
changelog: FP: [`uninlined_format_args`]: No longer lints for arguments with generic parameters
[#10343](https://github.com/rust-lang/rust-clippy/pull/10343)
<!-- changelog_checked -->
fix [`needless_return`] incorrect suggestion when returning if sequence
fixes: #10049
---
changelog: [`needless_return`]: fix incorrect suggestion on if sequence
Liberate late-bound regions rather than erasing them in `needless_pass_by_value`
changelog: [`needless_pass_by_value`]: fixes an ICE when there are late-bound regions in function arguments that are needlessly passed by value
Fixesrust-lang/rust#107147
r? `@matthiaskrgr`
It is not sufficient to ignore break from a block inside the loop.
Instructions after the break must be ignored, as they are unreachable.
This is also true for all instructions in outer blocks and loops
until the right block is reached.
This lint complains when the question mark operator (try operator)
is used. This is a restriction lint that can be useful on local
scopes where a custom error handling macro is supposed to be used
to augment the error based on local scope data before returning.
Negate suggestions when needed in `bool_assert_comparison`
changelog: none assuming this gets into the same release as #10218Fixes#10291
r? `@dswij`
Thanks to `@black-puppydog` for spotting it early
wildcard_enum_match_arm lint takes the enum origin into account
fixes#7419
---
changelog: Enhancement: [`wildcard_enum_match_arm`]: Now lints missing private variants, for local enums
[#10250](https://github.com/rust-lang/rust-clippy/pull/10250)
<!-- changelog_checked -->
[`unused_io_amount`]: Lint with `is_ok` and `is_err`
Fixes#10132
changelog: Apply [`unused_io_amount`] lint to `is_ok` and `is_err` without checking read/write amount
prevents `len_without_is_empty` from yielding positive when `len` takes arguments besides `&self`
Fixes#9520
---
changelog: FP [`len_without_is_empty`]: No longer lints, if `len` as a non-default signature
[#10255](https://github.com/rust-lang/rust-clippy/pull/10255)
<!-- changelog_checked -->
more than just `&self` in non-standard implementations.
changelog: Fix [`len_without_is_empty`] false positive when len has a
non-standard method signature
Fixes#9520
`invalid_regex`: Show full error when string value doesn't match source
changelog: [`invalid_regex`]: Show full error when parsing non-literals or regular strings containing escape sequences
Fixes#4170, the escape sequence there causes the span to be incorrect which will have caused most of the confusion
Add `multiple_unsafe_ops_per_block` lint
Adds a lint, which restricts an `unsafe` block to only one unsafe operation.
Closes#10064
---
changelog: New lint: [`multiple_unsafe_ops_per_block`]
[#10206](https://github.com/rust-lang/rust-clippy/pull/10206)
<!-- changelog_checked -->
Fix suggestion in `transmutes_expressible_as_ptr_casts` when the source type is a borrow.
fixes#9894
changelog: `transmutes_expressible_as_ptr_casts`: Fix suggestion when the source type is a borrow.
[needless_return]: Remove all semicolons on suggestion
Closes#10182
Multiple semicolons currently breaks autofix for `needless_return` suggestions. Any semicolons left after removing return means that the return type will always be `()`, and thus fail to compile.
This PR allows `needless_return` to remove multiple semicolons.
The change won't cover the case where there is multiple line yet.
i.e.
```rust
fn needless_return() -> bool {
return true;
;;
}
```
---
changelog: Sugg: [`needless_return`]: Now removes all semicolons on the same line
[#10187](https://github.com/rust-lang/rust-clippy/pull/10187)
<!-- changelog_checked -->
`cast_possible_truncation` Suggest TryFrom when truncation possible
This fixes the last issues from https://github.com/rust-lang/rust-clippy/pull/9664 as the author seems to be inactive. The PR author was sadly kept during the rebase, due to the conflict resolution.
IDK if it's worth it do to a full review, I only added the last commit, everything else remained the same, besides a rebase.
---
changelog: Sugg: [`cast_possible_truncation`]: Now suggests using `try_from` or allowing the lint
[#10038](https://github.com/rust-lang/rust-clippy/pull/10038)
<!-- changelog_checked -->
closes: https://github.com/rust-lang/rust-clippy/issues/9231
Render missing generics suggestion verbosely
It's a bit easier to read like this, especially ones that are appending new generics onto an existing list, like ": `, T`" which render somewhat poorly inline.
Also don't suggest `dyn` as a type parameter to add, even if technically that's valid in edition 2015.
Whne SYSROOT is defined, clippy-driver will insert a --sysroot argument
when calling rustc. However, when a sysroot argument is already defined,
e.g. through RUSTFLAGS=--sysroot=... the `cargo clippy` call would
error. This tests that the sysroot argument is only passed once and that
SYSROOT is ignored in this case.
This is useful for rust-lang/rust to allow setting a sysroot that's
*only* for build scripts, different from the regular sysroot passed in
RUSTFLAGS (since cargo doesn't apply RUSTFLAGS to build scripts or
proc-macros).
That said, the exact motivation is not particularly important: this
fixes a regression from
5907e9155e (r1060215684).
Note that only RUSTFLAGS is tested in the new integration test; passing
--sysroot through `clippy-driver` never worked as far as I can tell, and
no one is using it, so I didn't fix it here.
Allow implementing `Hash` with derived `PartialEq` (`derive_hash_xor_eq`
This is a common pattern and is totally allowed by the `Hash` trait.
Fixes#2627
changelog: Move: Renamed `derive_hash_xor_eq` to [`derived_hash_with_manual_eq`]
[#10184](https://github.com/rust-lang/rust-clippy/pull/10184)
changelog: Enhancement: [`derived_hash_with_manual_eq`]: Now allows `#[derive(PartialEq)]` with custom `Hash` implementations
[#10184](https://github.com/rust-lang/rust-clippy/pull/10184)
<!-- changelog_checked -->
trim paths in `suspicious_to_owned`
This continues my path trimming spree. I'm not going to add yet another changelog entry, we should have one "trim paths in some applicable lints" entry instead.
---
changelog: none
unused_self: Don't trigger if the method body contains todo!()
If the author is using todo!(), presumably they intend to use self at some point later, so we don't have a good basis to recommend factoring out to an associated function.
Fixes#10117.
---
changelog: Enhancement: [`unused_self`]: No longer lints, if the method body contains a `todo!()` call
[#10166](https://github.com/rust-lang/rust-clippy/pull/10166)
<!-- changelog_checked -->
[arithmetic_side_effects] Add more tests related to custom types
Add tests to ensure that custom types are triggered with any type of arithmetic operation as well as combinations with or without references.
---
changelog: none
<!-- changelog_checked -->
trim paths in `box_default`
This might help with #10089, though I have not tested that yet. In any event, it keeps the suggestion short and to the point.
---
changelog: Trim paths in [`box_default`] suggestion
trim paths in `default_trait_access`/`clone_on_copy` suggestions
This should help making the suggestions more palatable. Similar to #10153.
---
changelog: trim paths in [`default_trait_access`]/[`clone_on_copy`] suggestions
If the author is using todo!(), presumably they intend to use self at
some point later, so we don't have a good basis to recommend factoring
out to an associated function.
Fixes#10117.
changelog: Don't trigger [`unused_self`] if the method body contains a `todo!()` call
[`drop_ref`]: don't lint idiomatic in match arm
fixes#10122
As established in issue #9482, it is idiomatic to use a single `drop()` expression in a match arm to achieve a side-effect of a function while discarding its output. This should also apply to cases where the function returns a reference.
The change to the lint's code was less than 1 line, because all the heavy lifting was done in PR #9491.
---
changelog: FP: [`drop_ref`]: No longer lints idiomatic expression in `match` arms
[#10142](https://github.com/rust-lang/rust-clippy/pull/10142)
<!-- changelog_checked -->
Make the iter_kv_map lint handle ref/mut annotations.
For the degenerate (`map(|(k, _)| k)`/`map(|(_, v)| v)`) cases a mut annotation is superfluous and a ref annotation won't compile, so no additional handling is required. For cases where the `map` call must be preserved ref/mut annotations should also be presereved so that the map body continues to work as expected.
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`iter_kv_map`]: handle ref/mut annotations
For the degenerate (`map(|(k, _)| k)`/`map(|(_, v)| v)`) cases a mut annotation is superfluous and a ref annotation won't compile, so no additional handling is required. For cases where the `map` call must be preserved ref/mut annotations should also be presereved so that the map body continues to work as expected.
don't lint field_reassign when field in closure
fixes#10136
This change makes the ContainsName struct visit all interior expressions, which means that ContainsName will return true even if `name` is used in a closure within `expr`.
---
changelog: FP: [`field_reassign_with_default`]: No longer lints cases, where values are initializes from closures capturing struct values
[#10143](https://github.com/rust-lang/rust-clippy/pull/10143)
<!-- changelog_checked -->
This commit makes the ContainsName struct visit all interior
expressions, which means that ContainsName will return true
even if `name` is used in a closure within `expr`.
fix incorrect suggestion in `suboptimal_flops`
fixes#10003
There was an error when trying to negate an expression like `x - 1.0`. We used to format it as `-x - 1.0` whereas a proper negation would be `-(x - 1.0)`.
Therefore, we add parentheses around the expression when it is `ExprKind::Binary`.
We also add parentheses around multiply and divide expressions, even though this is not strictly necessary.
changelog: [`suboptimal_flops`]: fix incorrect suggestion caused by an incorrect negation of floating point expressions.
There was an error when trying to negate an expression
like `x - 1.0`. We used to format it as `-x - 1.0` whereas
a proper negation would be `-(x - 1.0)`.
Therefore, we add parentheses around the expression when it is a
Binary ExprKind.
We also add parentheses around multiply and divide expressions,
even though this is not strictly necessary.
Add size_of_ref lint
This addresses #9995, which is likely raising a valid point about `std::mem::size_of_val()`: It's [very easy to use double-references as the argument](https://github.com/apache/arrow-datafusion/pull/4371#discussion_r1032385224), which the function will happily accept and give back the size of _the reference_, not the size of the value _behind_ the reference. In the worst case, if the value matches the programmer's expectation, this seems to work, while in fact, everything will go horribly wrong e.g. on a different platform.
The size of a `&T` is independent of what `T` is, and people might want to use `std::mem::size_of_val()` to actually get the size of _any_ reference (e.g. via `&&()`). I would rather suggest that this is always bad behavior, though ([instead](https://doc.rust-lang.org/reference/type-layout.html#pointers-and-references-layout), [and](https://doc.rust-lang.org/stable/std/primitive.usize.html#associatedconstant.BITS)). I, therefore, put this lint into `correctness`.
Since the problem is usually easily fixed by removing extra `&`, I went light on suggesting code.
---
changelog: New lint: [`size_of_ref`]
[#10098](https://github.com/rust-lang/rust-clippy/pull/10098)
<!-- changelog_checked -->
Improve `possible_borrower`
This PR makes several improvements to `clippy_uitls::mir::possible_borrower`. These changes benefit both `needless_borrow` and `redundant clone`.
1. **Use the compiler's `MaybeStorageLive` analysis**
I could spot not functional differences between the one in the compiler and the one in Clippy's repository. So, I removed the latter in favor of the the former.
2. **Make `PossibleBorrower` a dataflow analysis instead of a visitor**
The main benefit of this change is that allows `possible_borrower` to take advantage of statements' relative locations, which is easier to do in an analysis than in a visitor.
This is easier to illustrate with an example, so consider this one:
```rust
fn foo(cx: &LateContext<'_>, lint: &'static Lint) {
cx.struct_span_lint(lint, rustc_span::Span::default(), "", |diag| diag.note(&String::new()));
// ^
}
```
We would like to flag the `&` pointed to by the `^` for removal. `foo`'s MIR begins like this:
```rust
fn span_lint::foo::{closure#0}(_1: [closure@$DIR/needless_borrow.rs:396:68: 396:74], _2: &mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()>) -> &mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()> {
debug diag => _2; // in scope 0 at $DIR/needless_borrow.rs:396:69: 396:73
let mut _0: &mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()>; // return place in scope 0 at $DIR/needless_borrow.rs:396:75: 396:75
let mut _3: &mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()>; // in scope 0 at $DIR/needless_borrow.rs:396:75: 396:100
let mut _4: &mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()>; // in scope 0 at $DIR/needless_borrow.rs:396:75: 396:100
let mut _5: &std::string::String; // in scope 0 at $DIR/needless_borrow.rs:396:85: 396:99
let _6: std::string::String; // in scope 0 at $DIR/needless_borrow.rs:396:86: 396:99
bb0: {
StorageLive(_3); // scope 0 at $DIR/needless_borrow.rs:396:75: 396:100
StorageLive(_4); // scope 0 at $DIR/needless_borrow.rs:396:75: 396:100
_4 = &mut (*_2); // scope 0 at $DIR/needless_borrow.rs:396:75: 396:100
StorageLive(_5); // scope 0 at $DIR/needless_borrow.rs:396:85: 396:99
StorageLive(_6); // scope 0 at $DIR/needless_borrow.rs:396:86: 396:99
_6 = std::string::String::new() -> bb1; // scope 0 at $DIR/needless_borrow.rs:396:86: 396:99
// mir::Constant
// + span: $DIR/needless_borrow.rs:396:86: 396:97
// + literal: Const { ty: fn() -> std::string::String {std::string::String::new}, val: Value(<ZST>) }
}
bb1: {
_5 = &_6; // scope 0 at $DIR/needless_borrow.rs:396:85: 396:99
_3 = rustc_errors::diagnostic_builder::DiagnosticBuilder::<'_, ()>::note::<&std::string::String>(move _4, move _5) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/needless_borrow.rs:396:75: 396:100
// mir::Constant
// + span: $DIR/needless_borrow.rs:396:80: 396:84
// + literal: Const { ty: for<'a> fn(&'a mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()>, &std::string::String) -> &'a mut rustc_errors::diagnostic_builder::DiagnosticBuilder<'_, ()> {rustc_errors::diagnostic_builder::DiagnosticBuilder::<'_, ()>::note::<&std::string::String>}, val: Value(<ZST>) }
}
```
The call to `diag.note` appears in `bb1` on the line beginning with `_3 =`. The `String` is owned by `_6`. So, in the call to `diag.note`, we would like to know whether there are any references to `_6` besides `_5`.
The old, visitor approach did not consider the relative locations of statements. So all borrows were treated the same, *even if they occurred after the location of interest*.
For example, before the `_3 = ...` call, the possible borrowers of `_6` would be just `_5`. But after the call, the possible borrowers would include `_2`, `_3`, and `_4`.
So, in a sense, the call from which we are try to remove the needless borrow is trying to prevent us from removing the needless borrow(!).
With an analysis, things do not get so muddled. We can determine the set of possible borrowers at any specific location, e.g., using a `ResultsCursor`.
3. **Change `only_borrowers` to `at_most_borrowers`**
`possible_borrowers` exposed a function `only_borrowers` that determined whether the borrowers of some local were *exactly* some set `S`. But, from what I can tell, this was overkill. For the lints that currently use `possible_borrower` (`needless_borrow` and `redundant_clone`), all we really want to know is whether there are borrowers *other than* those in `S`. (Put another way, we only care about the subset relation in one direction.) The new function `at_most_borrowers` takes this more tailored approach.
4. **Compute relations "on the fly" rather than using `transitive_relation`**
The visitor would compute and store the transitive closure of the possible borrower relation for an entire MIR body.
But with an analysis, there is effectively a different possible borrower relation at each location in the body. Computing and storing a transitive closure at each location would not be practical.
So the new approach is to compute the transitive closure on the fly, as needed. But the new approach might actually be more efficient, as I now explain.
In all current uses of `at_most_borrowers` (previously `only_borrowers`), the size of the set of borrowers `S` is at most 2. So you need only check at most three borrowers to determine whether the subset relation holds. That is, once you have found a third borrower, you can stop, since you know the relation cannot hold.
Note that `transitive_relation` is still used by `clippy_uitls::mir::possible_origin` (a kind of "subroutine" of `possible_borrower`).
cc: `@Jarcho`
---
changelog: [`needless_borrow`], [`redundant_clone`]: Now track references better and detect more cases
[#9701](https://github.com/rust-lang/rust-clippy/pull/9701)
<!-- changelog_checked -->
Avoid `match_wildcard_for_single_variants` on guarded wild matches
fix#9993
changelog: FP: [`match_wildcard_for_single_variants`]: No longer lints on wildcards with a guard
[#10056](https://github.com/rust-lang/rust-clippy/pull/10056)
<!-- changelog_checked -->
r? `@Jarcho`
Null fn lints
Adds lints to check for code, that assumes nullable `fn()`.
### Lint examples:
`transmute_null_to_fn`:
```rust
error: transmuting a known null pointer into a function pointer
--> $DIR/transmute_null_to_fn.rs:9:23
|
LL | let _: fn() = std::mem::transmute(std::ptr::null::<()>());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this transmute results in undefined behavior
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `None` as a null pointer value
```
`fn_null_check`:
```rust
error: function pointer assumed to be nullable, even though it isn't
--> $DIR/fn_null_check.rs:13:8
|
LL | if (fn_ptr as *mut ()).is_null() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: try wrapping your function pointer type in `Option<T>` instead, and using `is_none` to check for null pointer value
```
Closes#1644
---
changelog: Improvement: [`transmuting_null`]: Now detects `const` pointers to all types
[#10099](https://github.com/rust-lang/rust-clippy/pull/10099)
changelog: New lint: [`transmute_null_to_fn`]
[#10099](https://github.com/rust-lang/rust-clippy/pull/10099)
changelog: New lint: [`fn_null_check`]
[#10099](https://github.com/rust-lang/rust-clippy/pull/10099)
<!-- changelog_checked (This is just a flag for me, please don't add it manually) -->
Identify more cases of useless `into_iter()` calls
changelog: Sugg: [`useless_conversion`]: Now suggests removing calls to `into_iter()` on an expression implementing `Iterator`
[#10020](https://github.com/rust-lang/rust-clippy/pull/10020)
<!-- changelog_checked -->
If the type of the result of a call to `IntoIterator::into_iter()`
and the type of the receiver are the same, then the receiver
implements `Iterator` and `into_iter()` is the identity function.
The call to `into_iter()` may be removed in all but two cases:
- If the receiver implements `Copy`, `into_iter()` will produce
a copy of the receiver and cannot be removed. For example,
`x.into_iter().next()` will not advance `x` while `x.next()` will.
- If the receiver is an immutable local variable and the call to
`into_iter()` appears in a larger expression, removing the call to
`into_iter()` might cause mutability issues. For example, if `x`
is an immutable local variable, `x.into_iter().next()` will
compile while `x.next()` will not as `next()` receives
`&mut self`.
Rustup
r? `@ghost`
I'm on the train and my internet is too bad to download the necessary toolchain, so I have to use CI to find sync fallout.
changelog: none
<!-- changelog_checked -->
fix: not suggest seek_to_start_instead_of_rewind when expr is used
changelog: [`seek_to_start_instead_of_rewind`]: No longer lints, if the return of `seek` is used.
[#10096](https://github.com/rust-lang/rust-clippy/pull/10096)
<!-- changelog_checked -->
Fixes#10065
There used to be a logical bug where IncrementVisitor would
completely stop checking an expression/block after seeing a continue
statement. This led to issue #10058 where a variable incremented
(or otherwise modified) after any continue statement would still be
considered incremented only once.
The solution is to continue scanning the expression after seeing a
`continue` statement, but increment self.depth so that the Visitor
thinks that the rest of the loop is within a conditional.
Cleanup `rustc_tool_util` and add a convenient macro for `build.rs`
changelog: none
<!-- changelog_checked -->
If possible, I'd like to have a new release for this, maybe `v0.3.0` to use the changes in another project. Then we can also remove the `path = "./rustc_tools_util"` from `Cargo.toml`. I'd be happy to help with the release on crates.io if you'd like the help :)
r? `@matthiaskrgr`
improve `manual_is_ascii_check ` check
Sorry, not familiar the api, i can only check the method name of expression `<expr-1>.contains(<expr-2>)` after read clippy book and hints from #9933 . i dont know how to check
1. if <expr-1> is a specific range
2. <expr-2> is a character
r? `@xFrednet` could you please provide some more hints? 😝️
---
changelog: Enhancement: [`manual_is_ascii_check`]: Now detects ranges with `.contains()` calls
[#10053](https://github.com/rust-lang/rust-clippy/pull/10053)
<!-- changelog_checked -->
Add 1.58 MSRV for `collapsible_str_replace`
The `Pattern` impl for `[char; N]` was added in 1.58
changelog: Enhancement: [`collapsible_str_replace`]: Now takes MSRV into consideration. The minimal version is 1.58
[#10047](https://github.com/rust-lang/rust-clippy/pull/10047)
add `suppress_restriction_lint_in_const` config
According to #9808 , add a new lint `suppress_lint_in_const` to report even in const context. BTW, i am not good at naming either, if anyone have a better idea, i am happy to change it.
This PR is still in progress, so i keep it draft.
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
changelog: Enhancement: [`indexing_slicing`]: add new config `suppress-restriction-lint-in-const` to enable restriction lints, even if the suggestion might not be applicable
r? `@xFrendet`
Fix 10021
This PR proposes a fix for #10021.
The problem is similar to the one that `@mikerite` described in #9505. The compiler is generating an empty substitution for a call, even though the type of `Self` seems to be needed for a predicate. In `@mikerite's` case, the call was to [`IntoFuture::into_future`](https://doc.rust-lang.org/std/future/trait.IntoFuture.html#tymethod.into_future). In this case, the call is to [`Try::branch`](https://doc.rust-lang.org/std/ops/trait.Try.html#tymethod.branch).
The proposed fix is to verify that the parameter whose type is changing has an index within the substitution. The strikes me as a reasonable approach, since if the check were to fail, the following code would be a no-op:
4c123a06ba/clippy_lints/src/methods/unnecessary_to_owned.rs (L420-L428)
Like `@mikerite's` original solution, this solution turns ICEs into false negatives.
changelog: fix `unnecessary_to_owned` false positive involving `Try::branch`
Don't lint `implicit_clone` when the type doesn't implement clone
fixes#10019
changelog: `implicit_clone`: Don't lint when the type doesn't implement clone
Fix#9958
This PR fixes#9958. In order to fix the issue, the lint will now peel reference operators and enclose the expression with parentheses when necessary.
changelog: [`comparison_to_empty`]: Peel deref operators in suggestions when necessary
Don't lint `from_over_into` for opaque types
fixes#9935
This is stalled until the next sync. The impl in question can't be written on the pinned nightly.
changelog: Don't lint `from_over_into` for opaque types
rustc_ast_lowering: Stop lowering imports into multiple items
Lower them into a single item with multiple resolutions instead.
This also allows to remove additional `NodId`s and `DefId`s related to those additional items.
Treat custom enum discriminant values as constants
fixes#9882
changelog: All lints: Don't lint in enum discriminant values when the suggestion won't work in a const context
Lower them into a single item with multiple resolutions instead.
This also allows to remove additional `NodId`s and `DefId`s related to those additional items.
Don't lint `explicit_auto_deref` when the initial type is neither a reference, nor a receiver
fixes#9901fixes#9777
changelog: `explicit_auto_deref`: Don't lint when the initial value is neither a reference, nor a receiver
Don't cross contexts while building the suggestion for `redundant_closure_call`
fixes#9957
changelog: `redundant_closure_call`: Don't cross macro contexts while building the suggestion
Move `unnecessary_unsafety_doc` to `pedantic`
This lint was added in #9822. I like the idea, but also agree with #9986 as well. I think it should at least not be warn-by-default. This is one of these cases, where I'd like a group between pedantic and restriction. But I believe that users using `#![warn(clippy::pedantic)]` will know how to enable the lint if they disagree with it.
---
Since the lint is new:
changelog: none
r? `@flip1995` since I'd suggest back porting this, the original PR was merged 16 days ago.
Closes: #9986 (While it doesn't address everything, I believe that this is the best compromise)
Add allow-mixed-uninlined-format-args config
Implement `allow-mixed-uninlined-format-args` config param to change the behavior of the `uninlined_format_args` lint. Now it is a part of `style` per [Zulip chat](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.60uninlined_format_args.60.20category), and won't propose inlining in case of a mixed usage, e.g. `print!("{} {}", var, 1+2)`. If the user sets `allow-mixed-uninlined-format-args` config param to `false`, the lint would behave like it did before -- proposing to inline args even in the mixed case.
---
changelog: [`uninlined_format_args`]: Added a new config `allow-mixed-uninlined-format-args` to allow the lint, if only some arguments can be inlined
[#9865](https://github.com/rust-lang/rust-clippy/pull/9865)
changelog: Moved [`uninlined_format_args`] to `style` (Now warn-by-default)
[#9865](https://github.com/rust-lang/rust-clippy/pull/9865)
Implement `allow-mixed-uninlined-format-args` config param to change the behavior of the `uninlined_format_args` lint. Now it is a part of `style`, and won't propose inlining in case of a mixed usage, e.g. `print!("{} {}", var, 1+2)`. If the user sets allow-mixed-uninlined-format-args config param to `false`, then it would behave like before, proposing to inline args even in the mixed case.
Previously, async constructs would be lowered to "normal" generators,
with an additional `from_generator` / `GenFuture` shim in between to
convert from `Generator` to `Future`.
The compiler will now special-case these generators internally so that
async constructs will *directly* implement `Future` without the need
to go through the `from_generator` / `GenFuture` shim.
The primary motivation for this change was hiding this implementation
detail in stack traces and debuginfo, but it can in theory also help
the optimizer as there is less abstractions to see through.
Add `clippy_utils::msrv::Msrv` to keep track of the current MSRV
changelog: Fix the scoping of the `#![clippy::msrv]` attribute
Fixes#6920
r? `@Jarcho`
Fix [`unnecessary_lazy_eval`] when type has significant drop
fix for https://github.com/rust-lang/rust-clippy/issues/9427#issuecomment-1295742590
However current implementation gives too many false positive, rending the lint almost useless.
I don't know what's the best way to check if a type has a "significant" drop (in the common meaning, not the internal rustc one, for example Option<(u8, u8)> should not be considered significant)
changelog: Fix [`unnecessary_lazy_eval`] when type has significant drop
Fix#9771 (`unnecessary_to_owned` false positive)
Fixes#9771
In that issue's example(s), the lint tried to add a `&` to a value, which implicitly changed the type of a field to a reference. The fix is to add the reference to `receiver_ty` (the type of the receiver of the `to_owned`-like method), before passing `receiver_ty` to `can_change_type`. `can_change_type` properly rejects the modified `receiver_ty`.
cc: `@mikerite` just because I think he was the author of `can_change_type`.
changelog: fix `unnecessary_to_owned` false positive which implicitly tried to change the type of a field to a reference
Fix `redundant_closure_for_method_calls` suggestion
Fixes#7746. The issue turns out to be more general than raw pointers. The `redundant_closure_for_method_calls` lint produces incorrect suggestions when the method is associated with a type that must be enclosed in angle brackets or must be written with generic arguments substituted. For example:
```rust
fn main() {
// Clippy's suggestion: [T; N]::as_slice
// Correct suggestion: <[u8; 3]>::as_slice
let array_opt: Option<&[u8; 3]> = Some(&[4, 8, 7]);
array_opt.map(|a| a.as_slice());
// Clippy's suggestion: [T]::len
// Correct suggestion: <[u8]>::len
let slice_opt: Option<&[u8]> = Some(b"slice");
slice_opt.map(|s| s.len());
// Clippy's suggestion: *const T::is_null
// Correct suggestion: <*const usize>::is_null
let ptr_opt: Option<*const usize> = Some(&487);
ptr_opt.map(|p| p.is_null());
// Clippy's suggestion: dyn TestTrait::method_on_dyn
// Correct suggestion: <dyn TestTrait>::method_on_dyn
let test_struct = TestStruct {};
let dyn_opt: Option<&dyn TestTrait> = Some(&test_struct);
dyn_opt.map(|d| d.method_on_dyn());
}
// For the trait object example:
trait TestTrait {}
struct TestStruct {}
impl TestTrait for TestStruct {}
impl dyn TestTrait + '_ {
fn method_on_dyn(&self) -> bool {
false
}
}
```
The issue also affects references and tuples, though I had to patch the standard library with non-trait methods for those types to test that. Just in case, I also included handling for `!`, since it appeared to be possible to call methods on it with angle brackets. I just couldn't verify the resulting suggestion, since dead-code analysis eliminates the code first.
This is my first exposure to Rust compiler internals, so please let me know if I'm taking the wrong approach here!
changelog: [`redundant_closure_for_method_calls`]: add angle brackets and substitute generic arguments in suggestion when needed
Add new lint [`misnamed-getters`]
```
changelog: Add new lint [`misnamed-getters`]
```
Closes#9769
The current lint matches all methods with a body of just one expression under the form `(&mut?)? <expr>.field` where field doesn't match the name of the method but there is a field of the same type in `<expr>` that matches the name. This allows matching nested structs, for example for newtype wrappers. This may cast the net a bit too wide and cause false positives. I'll run [clippy_lint_tester](https://github.com/mikerite/clippy_lint_tester) on the top crates to see how frequently false positives happen.
There also may be room for improvement by checking that the replacement field would work taking into account implementations of `Deref` and `DerefMut` even if the types don't exactly match but I don't know yet how this could be done.
[arithmetic-side-effects] Detect overflowing associated constants of integers
Triggers the negation of maximum unsigned integers using associated constants. Rustc already handles `-128i8` but doesn't handle `-i8::MAX`.
At the same time, allows stuff like `-1234`.
changelog: FP: [arithmetic-side-effects] Detect overflowing associated constants of integers
Keep original literal notation in suggestion
While I did some investigation of https://github.com/rust-lang/rust-clippy/issues/9866 (I couldn't reproduce it though) I found that `unused_rounding` formats as follows:
```rust
3.0_f64.round() // => 3.0f64
```
This PR makes them preserve as the original notation.
```rust
3.0_f64.round() // => 3.0_f64
```
changelog: Suggestion Enhancement: [`unused_rounding`]: The suggestion now preserves the original float literal notation
Return multiple resolutions from `def_path_res`
Changes `def_path_res` to return all the resolutions matching the path rather than the first one (with a namespace hint that covered some cases). This would fix any issues that come up with multiple versions of the same crate being present as they all have the same crate name
It also adds resolution of `impl _ {}` items for local items, and removes struct field resolution as it didn't seem to be used anywhere
I tested it on a local crate and it worked for the multiple crate issue, but I couldn't come up with a test that worked well with `// aux-build`, maybe `// aux-crate` after https://github.com/rust-lang/rust/pull/103266 could work but I'm not sure on that either
changelog: [`disallowed_methods`], [`disallowed_types`], [`disallowed_macros`]: fix path resolution with multiple versions of the same crate
changelog: [`disallowed_methods`]: Resolve methods in `impl`s in the current crate
Extend `needless_borrowed_reference` to structs and tuples, ignore _
changelog: [`needless_borrowed_reference`]: Lint struct and tuple patterns, and patterns containing `_`
Now lints patterns like
```rust
&(ref a, ref b)
&Tuple(ref a, ref b)
&Struct { ref a, ref b }
&(ref a, _)
```
Fix typo in `expect_used` and `unwrap_used` warning messages
"\`an Option\`" -> "an \`Option\`" and "\`a Result\`" -> "a \`Result\`".
changelog: fix typo in `expect_used` and `unwrap_used` warning messages
`never_loop`: don't emit AlwaysBreaks if it targets a block
ref: https://github.com/rust-lang/rust-clippy/pull/9837#issuecomment-1312788194
The previous fix (#9837) was too simple and ignored all break commands inside a labelled block, regardless of whether their destination was a labelled block or a loop. This fix tracks all the labelled blocks in scope to ensure that only breaks targeting loops are considered.
changelog: [`never_loop`]: prevent false negatives from `breaks` nested in labelled blocks
Clippy has an internal lint that checks for the usage of hardcoded def
paths and suggests to replace them with a lang or diagnostic item, if
possible. This was implemented with a hack, by getting all the variants
of the `LangItem` enum and then index into it with the position of the
`LangItem` in the `items` list. This is no longer possible, because the
`items` list can't be accessed anymore.
Introduced an ignored_ids parameter.
Takes O(n^2) time in the worst case.
Can be changed to collect block ids in first phase,
and then filter with binary search in second.
feat: lint unchecked subtraction of a 'Duration' from an 'Instant'
Hello all, I tried to tackle the open issue #9371 and this is what I came up with.
I have a difficulty currently - some tests are failing:
```
failures:
[ui] ui/manual_instant_elapsed.rs
```
The `manual_instant_elapsed` is failing because of `Instant::now() - duration` test, this now gets also picked by `unchecked_duration_subtraction` lint.
What is the correct way to proceed in this case? Simply update the `.stderr` file for `manual_instant_elapsed` lint?
changelog: [`unchecked_duration_subtraction`]: Add lint for unchecked subtraction of a `Duration` from an `Instant`.
fixes#9371
Make it clear that `or_fun_call` can be a false-positive
Also move it to nursery so that the false-positives can be dealt with.
CC #8574
changelog: [`or_fun_call`]: Mention false-positives, move to nursery.
Add `unnecessary_safety_doc` lint
changelog: [`unnecessary_safety_doc`]: Add `unnecessary_safety_doc` lint
fixes https://github.com/rust-lang/rust-clippy/issues/6880
This lint does not trigger for private functions, just like `missing_safety_docs`. Reason for that was implementation simplicity and because I figured asking first would make more sense, so if it should trigger for private functions as well let me know and I'll fix that up as well.
[`fn_params_excessive_bools`] Make it possible to allow the lint at the method level
changelog: FP: [`fn_params_excessive_bools`]: `#[allow]` now works on methods
fix https://github.com/rust-lang/rust-clippy/issues/9687
Tested without committing but `#[allow]`ing now works. Also rewrote the lint to be a late lint while at it :)
r? `@xFrednet`
Fix `explicit_auto_deref` fp
fixes#9763fixes#9811
changelog: `explicit_auto_deref`: Don't lint when the target type is a projection with generic arguments
Address issues 9739 and 9782
This PR fixes#9739 in the manner I suggested in https://github.com/rust-lang/rust-clippy/issues/9739#issuecomment-1296802376.
This PR also fixes the compilation failures in #9782 (but doesn't address `@e00E's` other objections).
Fixes#9739
r? `@Jarcho`
changelog: Fix two `needless_borrow` false positives, one involving borrows in `if`-`else`s, the other involving qualified function calls
Add `manual_is_ascii_check` lint
Addresses https://github.com/rust-lang/rust-clippy/issues/9290
This PR adds new lint `manual_is_ascii_check`, which detects comparison with ascii ranges using `matches!` macros.
As I mentioned as following in the Issue;
> Yes, that's true. we'll start small and then grow it.
> So I'll try to handle matches! macro with single range as suggested above.
However during writing first version, I was thinking that the changes to support alphabetic and digits will be small patch, so I made a single PR in hope review cost can be reduced.
changelog: add new lint [`manual_is_ascii_check`]
r? `@xFrednet`
Move needless_collect to nursery
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`needless_collect`]: Move `needless_collect` to nursery (Now allow-by-default)
After chatting with a few folks, it seems like `needless_collect` is giving false positives pretty regularly (https://github.com/rust-lang/rust-clippy/issues?q=is%3Aissue+is%3Aopen+needless_collect). We're big supporters of clippy in Nushell, and it's one of the passes we require for CI, but we've had to disable this particular lint. Perhaps it should be moved to the nursery until it's improved?
(apologies if this isn't the right approach to disabling a lint by default. I tried to follow the idea I saw other PRs doing in the past)
Track where diagnostics were created.
This implements the `-Ztrack-diagnostics` flag, which uses `#[track_caller]` to track where diagnostics are created. It is meant as a debugging tool much like `-Ztreat-err-as-bug`.
For example, the following code...
```rust
struct A;
struct B;
fn main(){
let _: A = B;
}
```
...now emits the following error message:
```
error[E0308]: mismatched types
--> src\main.rs:5:16
|
5 | let _: A = B;
| - ^ expected struct `A`, found struct `B`
| |
| expected due to this
-Ztrack-diagnostics: created at compiler\rustc_infer\src\infer\error_reporting\mod.rs:2275:31
```
Improve `needless_lifetimes`
This PR makes the following improvements to `needless_lifetimes`.
* It fixes the following false negative, where `foo` is flagged but `bar` is not:
```rust
fn foo<'a>(x: &'a u8, y: &'_ u8) {}
fn bar<'a>(x: &'a u8, y: &'_ u8, z: &'_ u8) {}
```
* It flags more cases, generally. Previously, `needless_borrow` required *all* lifetimes to be used only once. With the changes, individual lifetimes are flagged for being used only once, even if not all lifetimes are.
* Finally, it tries to produce more clear error messages.
changelog: fix `needless_lifetimes` false negative involving functions with multiple unnamed lifetimes
changelog: in `needless_lifetimes`, flag individual lifetimes used only once, rather than require all lifetimes to be used only once
changelog: in `needless_lifetimes`, emit "replace with `'_`" warnings only when applicable, and point to a generic argument
Add lint for confusing use of `^` instead of `.pow`
fixes#4205
Adds a lint named [`confusing_xor_and_pow`], it warns the user when `a ^ b` is used as the `.pow()` function, it doesn't warn for Hex, Binary... etc.
---
changelog: New lint: [`confusing_xor_and_pow`]
Warn when `clippy::restriction` is enabled via the command line
Currently it catches `#![warn(clippy::restriction)]`, it'll now catch `-W clippy::restriction` from the CLI. Also tweaks the message slightly
changelog: [`blanket_clippy_restriction_lints`]: Warn when `clippy::restriction` is enabled via the command line
fix `undocumented-unsafe-blocks` false positive
This fixes#9142 by iterating over the parent nodes as long as within a block, expression, statement, local, const or static.
---
changelog: none
Certain types must be enclosed in angle brackets and must have generic
arguments substituted to create a working suggestion. For example, if
`s` has type `&[u8]`, then `|s| s.len()` may be replaced with
`<[u8]>::len`. Previously, Clippy erroneously suggested `[T]::len`.
Ensure new_ret_no_self is not fired if impl Trait<Self> is returned.
Fix#7344: ensure new_ret_no_self is not fired if `impl Trait<Self>` is returned.
changelog: [`new_ret_no_self`]: No longer lints when `impl Trait<Self>` is returned
[`use_self`] fix suggestion when full path to struct was given
Previously the following wrong suggestion was given
```rust
impl Error for std::fmt::Error {
fn custom<T: std::fmt::Display>(_msg: T) -> Self {
- std::fmt::Error // Should lint
+ Self::Error // Should lint
}
}
```
Also remove known problem line related to #4140 since it's been closed, and refactor the lint
changelog: [`use_self`] fix suggestion when full path to struct was given
`question_mark` don't lint on `if let Err` with `else`
cc #9518
AFAICT the only time this would be a valid suggestion is the rather esoteric
```rust
let _ = if let Err(e) = x {
return Err(e);
} else {
// no side effects
x.unwrap()
}
```
which doesn't seem worth checking to me. Please correct me if I'm missing something.
changelog: [`question_mark`] don't lint on `if let Err` with `else`
Previously the following wrong suggestion was given
```rust
impl Error for std::fmt::Error {
fn custom<T: std::fmt::Display>(_msg: T) -> Self {
- std::fmt::Error // Should lint
+ Self::Error // Should lint
}
}
```
Also remove known problem line related to #4140 since it's been closed, and refactor the lint
make ignored internally mutable types for `mutable-key` configurable
We had some false positives where people would create their own types that had interior mutability unrelated to hash/eq. This addition lets you configure this as e.g. `arc-like-types=["bytes::Bytes"]`
This fixes#5325 by allowing users to specify the types whose innards like `Arc` should be ignored (the generic types are still checked) for the sake of detecting inner mutability.
r? `@Alexendoo`
---
changelog: Allow configuring types to ignore internal mutability in `mutable-key`
Update `from_raw_with_void_ptr` to support types other than `Box`
This PR updates the `from_raw_with_void_ptr` lint, which covered
`Box::from_raw`, to also cover the `from_raw` static method of the
`Rc`, `Arc`, `alloc::rc::Weak` and `alloc::sync::Weak` types.
It also improves the description and error messages of this lint.
---
changelog: [`from_raw_with_void_ptr`]: Now works with the `Rc`, `Arc`, `alloc::rc::Weak` and `alloc::sync::Weak` types.
add new lint `seek_to_start_instead_of_rewind `
changelog: `seek_to_start_instead_of_rewind`: new lint to suggest using `rewind` instead of `seek` to start
Resolve#8600
We had some false positives where people would create their own types
that had interior mutability unrelated to hash/eq. This addition lets
you configure this as e.g. `arc-like-types=["bytes::Bytes"]`
Sometimes type annotations are needed for type inferrence to work,
or because of coercions. We don't know this, and we also don't
want users to possibly repeat the entire pattern.
This PR updates the `from_raw_with_void_ptr` lint, which covered
`Box::from_raw`, to also cover the `from_raw` static method of the
`Rc`, `Arc`, `alloc::rc::Weak` and `alloc::sync::Weak` types.
It also improves the description and error messages of this lint.
---
changelog: [`from_raw_with_void_ptr`]: Now works with the `Rc`, `Arc`, `alloc::rc::Weak` and `alloc::sync::Weak` types.
Improvement for `equatable_if_let`
fixes#9221
This PR makes sure that enums or structs not implementing `PartialEq` trait but still using the `if let` patterns can be linted to be rewritten with `matches!`.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[ ] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[ ] Added lint documentation
- \[x] Run `cargo dev fmt`
---
changelog: Improve [`equatable_if_let`] with additional `matches!` suggestions.
fix: support `map_or` for `or_fun_call` lint
fixes https://github.com/rust-lang/rust-clippy/issues/8993
The methods defined in `KNOW_TYPES`, except for `map_or`, accepts only one argument, so the matching `if let [arg] = args` works only for these methods.
This PR adds `rest_arg` argument to `check_general_case` method and handling of cases with two arguments to support `map_or`.
changelog: `or_fun_call` support `map_or`
* add `rest_arg` to pass second argument from `map_or(U, F)`
* extract some procedures into closure
* add `rest_arg` to pass second argument from `map_or(U, F)`
* extract some procedures into closure
refac: rename rest_arg/second_arg
refac: organize function argument order
* put `second_arg` next to `arg` argument
Mark `let_underscore_lock` and `let_underscore_drop` as uplifted
Here I've renamed both the uplifted lints, however rustc's `let_underscore_lock` is slightly less capable than the clippy lint as it doesn't catch `parking_lot` types or `Result<Guard, ..>`, should we still remove it? The `Result` change looks like it was unintentional to me so that could probably be fixed upstream
changelog: Uplift [`let_underscore_drop`] to rustc https://github.com/rust-lang/rust/pull/97739
changelog: Remove overlap between rustc's `let_underscore_lock` and Clippy's [`let_underscore_lock`]
r? `@flip1995`
[`unwrap_used`], [`expect_used`] do not lint in `test` cfg
changelog: [`unwrap_used`], [`expect_used`] do not lint in `test` cfg
fix https://github.com/rust-lang/rust-clippy/issues/9612
I've updated the doc and used `cfg` acronym, not sure if `conditional compiler flag` would have been better
fix `box-default` ignoring trait objects' types
This avoids removing the turbofish when the `Box` type is a `dyn` or `impl _`.
This fixes#9621.
---
changelog: none
Move MSRV tests into the lint specific test files
There are currently two ways MSRV tests are done in the ui test suite, adding a case to the `#![clippy::msrv = "1.0"]` `tests/ui/min_rust_version_attr.rs` or adding the two `msrv_1_xx` functions to the test file of the lint in question
This updates the clippy book to suggest the `msrv_1_xx` style, and replaces the tests in `tests/ui/min_rust_version_attr.rs` with ones of that style
Almost the entire diff is just moving stuff around as a result, I made sure to check the line numbers the lints are emitted at correspond with the right `msrv` case, so feel free to only skim that part
changelog: none
Add `unused_format_specs` lint
Currently catches two cases:
An empty precision specifier:
```rust
// the same as {}
println!("{:.}", x);
```
And using formatting specs on `format_args!()`:
```rust
// prints `x.`, not `x .`
println("{:5}.", format_args!("x"));
```
changelog: new lint: [`unused_format_specs`]
[`unnecessary_cast`] Do not lint negative hexadecimal literals when cast as floats
fix https://github.com/rust-lang/rust-clippy/issues/9603
changelog: [`unnecessary_cast`] Do not lint negative hexadecimal literals when cast as floats
[`zero_prefixed_literal`] Do not advise to use octal form if not possible
fix https://github.com/rust-lang/rust-clippy/issues/9651
changelog: [`zero_prefixed_literal`] Do not advise to use octal form if not possible
Add new lint `partial_pub_fields`
Signed-off-by: TennyZhuang <zty0826@gmail.com>
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: `partial_pub_fields`: new lint to disallow partial fields of a struct be pub
Resolve#9604
Expand internal lint `unnecessary_def_path`
This PR does essentially two things:
* Separates the internal lints into modules by pass. (`internal_lints.rs` was over 1400 lines, which is a little unruly IMHO.)
* ~Adds a new~ Expands the `unnecessary_def_path` internal lint to flag hardcoded paths to diagnostic and language items.
My understanding is that the latter is currently done by reviewers. Automating this process should make things easier for both reviewers and contributors.
I could make the first bullet a separate PR, or remove it entirely, if desired.
changelog: Add internal lint `diagnostic_item_path`
Add a suggestion and a note about orphan rules for `from_over_into`
Adds a machine applicable suggestion to convert the `Into` impl into a `From` one to `from_over_into`
Also adds a note explaining that `impl From<Local> for Foreign` is fine if the `Into` type is foreign
Closes#7444
Addresses half of #9638
changelog: [`from_over_into`] Add a suggestion and a note about orphan rules
Fix edition revision ui tests
#9605 had me wondering how the edition revision tests were working for `manual_assert` but not for `@nyurik,` but it turns out `manual_assert`'s tests weren't working either. I checked how `rust-lang/rust` does it and apparently it comes down to whitespace, `//[rev] edition:X` works 😬
Removes the revisions from `match_wild_err_arm` as I couldn't find any edition dependant behaviour there
r? `@llogiq`
changelog: none
add tests in `implicit_saturating_sub` lint
This adds more tests to the `implicit_saturating_sub` lint to rule out certain false positives that have appeared in the past.
Now with those false positives out of the equation, we can move the lint to `style`.
---
changelog: promote [`implicit-saturating-sub`] to the `style` category
Fix to_string_in_format_args in parens
Fix suggestions like
```
print!("error: something failed at {}", (Location::caller().to_string()));
```
where the parenthesis enclose some portion of the value.
Fixes#9540
changelog: [`to_string_in_format_args`]: fix incorrect fix when value is enclosed in parenthesis
Fix suggestions like
```
print!("error: something failed at {}", (Location::caller().to_string()));
```
where the parenthesis enclose some portion of the value.
Don't suggest moving tuple structs with a significant drop to late evaluation
fixes#9608
changelog: Don't suggest moving tuple structs with a significant drop to late evaluation
Uplift `clippy::for_loops_over_fallibles` lint into rustc
This PR, as the title suggests, uplifts [`clippy::for_loops_over_fallibles`] lint into rustc. This lint warns for code like this:
```rust
for _ in Some(1) {}
for _ in Ok::<_, ()>(1) {}
```
i.e. directly iterating over `Option` and `Result` using `for` loop.
There are a number of suggestions that this PR adds (on top of what clippy suggested):
1. If the argument (? is there a better name for that expression) of a `for` loop is a `.next()` call, then we can suggest removing it (or rather replacing with `.by_ref()` to allow iterator being used later)
```rust
for _ in iter.next() {}
// turns into
for _ in iter.by_ref() {}
```
2. (otherwise) We can suggest using `while let`, this is useful for non-iterator, iterator-like things like [async] channels
```rust
for _ in rx.recv() {}
// turns into
while let Some(_) = rx.recv() {}
```
3. If the argument type is `Result<impl IntoIterator, _>` and the body has a `Result<_, _>` type, we can suggest using `?`
```rust
for _ in f() {}
// turns into
for _ in f()? {}
```
4. To preserve the original behavior and clear intent, we can suggest using `if let`
```rust
for _ in f() {}
// turns into
if let Some(_) = f() {}
```
(P.S. `Some` and `Ok` are interchangeable depending on the type)
I still feel that the lint wording/look is somewhat off, so I'll be happy to hear suggestions (on how to improve suggestions :D)!
Resolves#99272
[`clippy::for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
Further enhance `needless_borrow`, mildly refactor `redundant_clone`
This PR does the following:
* Moves some code from `redundant_clone` into a new `clippy_utils` module called `mir`, and wraps that code in a function called `dropped_without_further_use`.
* Relaxes the "is copyable" condition condition from #9136 by also suggesting to remove borrows from values dropped without further use. The changes involve the just mentioned function.
* Separates `redundant_clone` into modules.
Strictly speaking, the last bullet is independent of the others. `redundant_clone` is somewhat hairy, IMO. Separating it into modules makes it slightly less so, by helping to delineate what depends upon what.
I've tried to break everything up into digestible commits.
r? `@Jarcho`
(`@Jarcho` I hope you don't mind.)
changelog: continuation of #9136
Add `manual_filter` lint for `Option`
Share much of its implementation with `manual_map` and should greatly benefit from its previous feedback.
I'm sure it's possible to even more refactor both and would gladly take input on that as well as any clippy idiomatic usage, since this is my first lint addition.
I've added the lint to the complexity section for now, I don't know if every new lint needs to go in nursery first.
The matching could be expanded to more than `Some(<value>)` to lint on arbitrary struct matching inside the `Some` but I've left it like it was for `manual_map` for now. `needless_match::pat_same_as_expr` provides a more generic match example.
close https://github.com/rust-lang/rust-clippy/issues/8822
changelog: Add lint [`manual_filter`] for `Option`
extend `box-default` lint, add suggestion
This extends the recently added `box-default` lint to also cover `Box::new(vec![])`, `Box::new(String::from(""))` and `Box::new(Vec::from([]))`. Also the lint now suggests a suitable replacement. I did not find a simple way to check whether the type is fully determined by the outside, so I at least checked for some variations to remove the turbofish in those cases.
---
changelog: none
lint::unsafe_removed_from_name: fix false positive result when allowed
changelog: [`unsafe_removed_from_name`] Fix allowing on imports produces a false positive on `useless_attribute`.
Fixes: #9197
Signed-off-by: Andy-Python-Programmer <andypythonappdeveloper@gmail.com>
FormatArgsExpn: Find comma spans and ignore weird proc macro spans
Fixes the following cases:
A missing `, 1` from the `expect_fun_call` suggestion:
```rust
Some(()).expect(&format!("{x} {}", 1));
```
```
warning: use of `expect` followed by a function call
--> t.rs:7:14
|
7 | Some(()).expect(&format!("{x} {}", 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{x} {}"))`
```
The suggestion removing from the comma in the comment rather than the one after the format string:
```rust
println!(
"{}",
// a comment, with a comma in it
x
);
```
```
warning: variables can be used directly in the `format!` string
--> t.rs:9:5
|
9 | / println!(
10 | | "{}",
11 | | // a comment, with a comma in it
12 | | x
13 | | );
| |_____^
|
help: change this to
|
10 ~ "{x}",
11 ~ // a comment
|
```
It also no longer accepts expansions where a format string or argument has a "weird" proc macro span, that is one where the literal/expression it outputs has the span of one of its inputs. Kind of like a `format_args` specific `clippy_utils::is_from_proc_macro`, e.g. `format!(indoc! {" ... "})`
changelog: [`expect_fun_call`]: Fix suggestion for `format!` using captured variables
changelog: [`print_literal`], [`write_literal`], [`uninlined_format_args`]: Fix suggestion when following a comment including a comma
[arithmetic-side-effects] Do not ignore literal references
To my utter surprise, `rustc` does does not warn stuff like `let n: u8 = &255 + &1` or `let n: u8 = 255 + &1`.
changelog: [arithmetic-side-effects] Do not ignore literal references
fallout: fix tests to allow uninlined_format_args
In order to switch `clippy::uninlined_format_args` from pedantic to style, all existing tests must not raise a warning. I did not want to change the actual tests, so this is a relatively minor change that:
* add `#![allow(clippy::uninlined_format_args)]` where needed
* normalizes all allow/deny/warn attributes
* all allow attributes are grouped together
* sorted alphabetically
* the `clippy::*` attributes are listed separate from the other ones.
* deny and warn attributes are listed before the allowed ones
See also https://github.com/rust-lang/rust-clippy/pull/9233, https://github.com/rust-lang/rust-clippy/pull/9525, https://github.com/rust-lang/rust-clippy/pull/9527
cc: `@llogiq` `@Alexendoo`
changelog: none
Fix and improve `match_type_on_diagnostic_item`
This extracts the fix for the lint out of #7647. There's still a couple of other functions to check, but at least this will get lint working again.
The two added util functions (`is_diagnostic_item` and `is_lang_item`) are needed to handle `DefId` for unit and tuple struct/variant constructors. The `rustc_diagnostic_item` and `lang` attributes are attached to the struct/variant `DefId`, but most of the time they are used through their constructors which have a different `DefId`. The two utility functions will check if the `DefId` is for a constructor and switch to the associated struct/variant `DefId`.
There does seem to be a bug on rustc's side where constructor `DefId`s from external crates seem to be returning `DefKind::Variant` instead of `DefKind::Ctor()`. There's a workaround put in right.
changelog: None
In order to switch `clippy::uninlined_format_args` from pedantic to
style, all existing tests must not raise a warning. I did not want to
change the actual tests, so this is a relatively minor change that:
* add `#![allow(clippy::uninlined_format_args)]` where needed
* normalizes all allow/deny/warn attributes
* all allow attributes are grouped together
* sorted alphabetically
* the `clippy::*` attributes are listed separate from the other ones.
* deny and warn attributes are listed before the allowed ones
changelog: none
Remove unused `.fixed` files, only run asm_syntax doctests on x86
Two small changes, removes some unused `.fixed` and makes `clippy_lints` doctests pass on non x86 arches
changelog: none
* Check for `const`s and `static`s from external crates
* Check for `LangItem`s
* Handle inherent functions which have the same name as a field
* Also check the following functions:
* `match_trait_method`
* `match_def_path`
* `is_expr_path_def_path`
* `is_qpath_def_path`
* Handle checking for a constructor to a diagnostic item or `LangItem`
let unnecessary_cast work for trivial non_literal expressions
Signed-off-by: TennyZhuang <zty0826@gmail.com>
---
changelog: [`unnecessary_cast`]: fix for trivial non_literal expressions
Fixes#9562
[`unnecessary_cast`] add parenthesis when negative number uses a method
fix#9563
The issue was probably introduced by 90fe3bea52
changelog: [`unnecessary_cast`] add parenthesis when negative number uses a method
r? llogiq
Implement `manual_clamp` lint
Fixes#9477Fixes#6751
Identifies common patterns where usage of the `clamp` function would be more succinct and clear, and suggests using the `clamp` function instead.
changelog: [`manual_clamp`]: Implement manual_clamp lint
This lint detects calls to a `&self`-taking `as_ptr` method, where
the result is then immediately cast to a `*mut T`. Code like this
is probably invalid, as that pointer will not have write permissions,
and `*mut T` is usually used to write through.
fix [`needless_borrow`], [`explicit_auto_deref`] FPs on unions
fix https://github.com/rust-lang/rust-clippy/issues/9383
changelog: fix [`needless_borrow`] false positive on unions
changelog: fix [`explicit_auto_deref`] false positive on unions
Left a couple debug derived impls on purpose I needed to debug as I don't think it's noise
Don't lint unstable moves in `std_instead_of_core`
Fixes#9515
changelog: [`std_instead_of_core`]: No longer suggests unstable modules such as `core::error`
add `box-default` lint
This adds a `box-default` lint to suggest using `Box::default()` instead of `Box::new(Default::default())`, which offers less moving parts and potentially better performance according to [the perf book](https://nnethercote.github.io/perf-book/standard-library-types.html#box).
---
changelog: add [`box_default`] lint
[`needless_return`] Recursively remove unneeded semicolons
fix#8336,
fix#8156,
fix https://github.com/rust-lang/rust-clippy/issues/7358,
fix#9192,
fix https://github.com/rust-lang/rust-clippy/issues/9503
changelog: [`needless_return`] Recursively remove unneeded semicolons
For now the suggestion about removing the semicolons are hidden because they would be very noisy and should be obvious if the user wants to apply the lint manually instead of using `--fix`. This could be an issue for beginner, but haven't found better way to display it.
[arithmetic-side-effects] Consider references
Takes into consideration integer references like `&i32::MAX` because currently things like `let _ = &1 + 0` trigger the lint.
changelog: FP: [`arithmetic_side_effects`]: Now ignores references
[9507](https://github.com/rust-lang/rust-clippy/pull/9507)
Don't lint `*_interior_mutable_const` on unions due to potential ICE.
fixes#9445
cc rust-lang/rust#101113
This started ICE'ing sometime last month due to stricter UB checks. I'm not sure how we could check the value of a union as MIRI doesn't seem to store which field is currently active.
changelog: Don't ICE on const unions containing a `!Freeze` type.
Silence [`question_mark`] in const context
fix https://github.com/rust-lang/rust-clippy/issues/9175
When `const_try` is stabilised can be turned into a MSRV
changelog: Silence [`question_mark`] in const context
Stabilize const `BTree{Map,Set}::new`
The FCP was completed in #71835.
Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
Implement https://github.com/rust-lang/rust-clippy/issues/8368 - a new
lint to inline format arguments such as `print!("{}", var)` into
`print!("{var}")`.
code | suggestion | comment
---|---|---
`print!("{}", var)` | `print!("{var}")` | simple variables
`print!("{0}", var)` | `print!("{var}")` | positional variables
`print!("{v}", v=var)` | `print!("{var}")` | named variables
`print!("{0} {0}", var)` | `print!("{var} {var}")` | aliased variables
`print!("{0:1$}", var, width)` | `print!("{var:width$}")` | width
support
`print!("{0:.1$}", var, prec)` | `print!("{var:.prec$}")` | precision
support
`print!("{:.*}", prec, var)` | `print!("{var:.prec$}")` | asterisk
support
code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined. Supporting this
case requires re-indexing of the format string.
changelog: [`uninlined_format_args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
Since `len` and `is_empty` are not const stable yet, this also
creates a new feature for them since they previously used the same
`const_btree_new` feature.
[`never_loop`]: Fix FP with let..else statements.
Fixes#9356
This has been bugging me for a while, so I thought I'd take a stab at it! I'm completely uncertain about the quality of my code, but I think it's an alright start, so opening this PR to get some feedback from more experienced clippy people :)
changelog: [`never_loop`]: Fix FP with let..else statements
Fixes#9504
Compiler generated call `into_iter` nodes return empty substs
which we need when checking it's predicates. Handle this by
simply exitting when we encounter one. This change introduces
false negatives in place of the ICEs.
[arithmetic-side-effects] Finish non-overflowing ops
Extends https://github.com/rust-lang/rust-clippy/pull/9474 to also take into consideration "raw" binary operations. For example, `let a = b / 2` and `let a = 1 * b` won't trigger the lint.
changelog: [arithmetic-side-effects] Finish non-overflowing ops
Make module-style lints resilient to --remap-path-prefix
changelog: [`self_named_module_files`], [`mod_module_files`]: Make module-style lints resilient to `--remap-path-prefix`
Without this if a user has configured `--remap-path-prefix` to be used for a prefix containing the current source directory the lints would silently fail to generate a warning.
Migrate write.rs to a late pass
changelog: Migrates write.rs from a pre expansion pass to a late pass
changelog: [`positional_named_format_parameters`] is renamed in favour of the rustc lint `named_arguments_used_positionally`
- Macros are now identified by diagnostic items, so will no longer lint user defined macros named, e.g. a custom `print!`
- `print_literal`/`write_literal` no longer lint no longer lint literals that come from macro expansions, e.g. `env!("FOO")`
- `print_with_newline`/`write_with_newline` no longer lint strings with any internal `\r` or `\n`s
~~A false negative, `print_literal`/`write_literal` don't lint format strings that produce `FormatSpec`s, e.g. ones containing pretty print/width/align specifiers~~
Suggestion changes:
- ~~`print_literal`/`write_literal` no longer have suggestions, as the spans for the `{}`s were not easily obtainable~~
- `print_with_newline`/`write_with_newline` has a better suggestion for a sole literal newline, but no longer has suggestions for len > 1 strings that end in a literal newline
- ~~`use_debug` spans are less precise, now point to the whole format string~~
The diff for write.rs is pretty unwieldy, other than for the `declare_clippy_lint!`s I think you'd be better off viewing it as a brand new file rather than looking at the diff, as it's mostly written from scratch
cc #6610, fixes#5721, fixes#7195, fixes#8615
Fix `unused_peekable` closure and `f(&mut peekable)` false positives
changelog: Fix [`unused_peekable`] false positive when peeked in a closure or called as `f(&mut peekable)`
The `return`/`break` changes aren't part of the fix, they allow an earlier return in some cases. `break` is replaced with `return` for style purposes as they do the same thing in this case
Fixes#9456Fixes#9462
Don't panic on invalid shift while constfolding
Instead of panicking on invalid shifts while folding constants we simply give up. Fixes#9463
Notice the "attempt to shift right by `1316134912_u32`", which seems weird. AFAICS it comes from rustc itself.
changelog: none
Don't lint `large_stack_array` inside static items
We now check if the linted `Expr` is inside an `ItemKind::Static`, which can't take the suggested `Box<[...]`. I _think_ this is the correct fix for #9460
I removed `if_chain` while I was at it.
changelog: Don't lint `large_stack_array` inside static items
Use macro callsite when creating `Sugg` helper
Closes#9375
changelog: Improvement: [`collapsible_if`]: Suggestions now work with macros, by taking the call site into account.
Replace u128 with u64 in large_enum_variant uitest
A u128 has [an 8 byte alignment on x86](https://github.com/rust-lang/rust/issues/54341), but a 16 byte alignment on aarch64 which changes the size of the enums due to extra padding. This means the test fails on aarch64
changelog: none
Fix `range_{plus,minus}_one` bad suggestions
Fixes#9431.
The current `range_plus_one` and `range_minus_one` suggestions are completely incorrect when macros are involved.
This commit resolves this by disabling the lints for any range expression that is expanded from a macro. The reasons for this are that it is very difficult to create a correct suggestion in this case and that false negatives are less important for pedantic lints.
changelog: Fix `range_{plus,minus}_one` bad suggestions
Fixes#9431.
The current `range_plus_one` and `range_minus_one` suggestions
are completely incorrect when macros are involved.
This commit resolves this by disabling the lints for any range
expression that is expanded from a macro. The reasons for this
are that it is very difficult to create a correct suggestion in
this case and that false negatives are less important for
pedantic lints.
[Arithmetic] Consider literals
Fixes https://github.com/rust-lang/rust-clippy/issues/9307 and makes the `arithmetic` lint behave like `integer_arithmetic`.
It is worth noting that literal integers of a binary operation (`1 + 1`, `i32::MAX + 1`), **regardless if they are in a constant environment**, won't trigger the lint. Assign operations also have similar reasoning.
changelog: Consider literals in the arithmetic lint
Suggest `unwrap_or_default` when closure returns `"".to_string`
Closes https://github.com/rust-lang/rust-clippy/issues/9420
changelog: [`unwrap_or_else_default`]: suggest `unwrap_or_default()` instead of `unwrap_or_else` with a closure that returns an empty `to_string`.
`BindingAnnotation` refactor
* `ast::BindingMode` is deleted and replaced with `hir::BindingAnnotation` (which is moved to `ast`)
* `BindingAnnotation` is changed from an enum to a tuple struct e.g. `BindingAnnotation(ByRef::No, Mutability::Mut)`
* Associated constants added for convenience `BindingAnnotation::{NONE, REF, MUT, REF_MUT}`
One goal is to make it more clear that `BindingAnnotation` merely represents syntax `ref mut` and not the actual binding mode. This was especially confusing since we had `ast::BindingMode`->`hir::BindingAnnotation`->`thir::BindingMode`.
I wish there were more symmetry between `ByRef` and `Mutability` (variant) naming (maybe `Mutable::Yes`?), and I also don't love how long the name `BindingAnnotation` is, but this seems like the best compromise. Ideas welcome.
Unlike past similar work done in #6228, expand the existing `or_fun_call`
lint to detect `or_insert` calls with a `T::new()` or `T::default()`
argument, much like currently done for `unwrap_or` calls. In that case,
suggest the use of `or_default`, which is more idiomatic.
Note that even with this change, `or_insert_with(T::default)` calls
aren't detected as candidates for `or_default()`, in the same manner
that currently `unwrap_or_else(T::default)` calls aren't detected as
candidates for `unwrap_or_default()`.
Also, as a nearby cleanup, change `KNOW_TYPES` from `static` to `const`,
since as far as I understand it's preferred (should Clippy have a lint
for that?).
Fixes#3812.
Fixes#9351.
Note that this commit reworks that fix for #9317. The change
is to check that the type implements `AsRef<str>` before regarding
`to_string` as an equivalent of `to_owned`. This was suggested
by Jarcho in the #9317 issue comments.
The benefit of this is that it moves some complexity out of
`check_other_call_arg` and simplifies the module as a whole.
Fix `mut_mutex_lock` when Mutex is behind immutable deref
I *think* the problem here is the `if let ty::Ref(_, _, Mutability::Mut) = cx.typeck_results().expr_ty(recv).kind()` line tries to check if the `Mutex` can be mutably borrowed (there already is a test for `Arc<Mutex<_>>`), but gets bamboozled by the `&mut Arc` indirection. And I *think* checking the deref-adjustment to filter immutable-adjust (the deref through the `Arc`, starting from `&mut Arc`) is the correct fix.
Fixes#9415
changelog: Fix `mut_mutex_lock` when Mutex is behind immutable deref
Don't use `hir_ty_to_ty` in `result_large_err`
fixes#9414
This occurs starting with 2022-09-01. I checked that this does fix the ICE on rust-lang/rust@9353538. Not sure which pr caused the late-bound region to leak through `hir_ty_to_ty`.
changelog: None
Fix `suboptimal_float` not linting on `{const}.powf({const})`
There used to be an early return if the receiver was an effective const but the method was not linted, not taking into account later cases where the receiver and the arguments are both effective consts for different methods. Removed the early return.
Fixes#9402Fixes#9201
changelog: Fix `suboptimal_flops`, `imprecise_flops` not linting on `{const}.powf({const})` et al
Fix the emission order of `trait_duplication_in_bounds`
Makes the lint emit in source order rather than whatever order the hash map happens to be in. This is currently blocking the sync into rustc.
changelog: None
Strengthen invalid_value lint to forbid uninit primitives, adjust docs to say that's UB
For context: https://github.com/rust-lang/rust/issues/66151#issuecomment-1174477404=
This does not make it a FCW, but it does explicitly state in the docs that uninit integers are UB.
This also doesn't affect any runtime behavior, uninit u32's will still successfully be created through mem::uninitialized.
Fix missing parens in `suboptimal_flops` suggestion
Fixes#9391. The problem is simple enough, I didn't check if the same problem occurs elsewhere, though.
changelog: fix missing parenthesis in `suboptimal_flops` suggestion
Ignore `match_like_matches_macro` when there is comment
Closes#9164
changelog: [`match_like_matches_macro`] is ignored when there is some comment inside the match block.
Also add `span_contains_comment` util to check if given span contains comments.
Implemented `suspicious_to_owned` lint to check if `to_owned` is called on a `Cow`
changelog: Add lint ``[`suspicious_to_owned`]``
-----------------
Hi,
posting this unsolicited PR as I've been burned by this issue :)
Being unsolicited, feel free to reject it or reassign a different lint level etc.
This lint checks whether `to_owned` is called on `Cow<'_, _>`. This is done because `to_owned` is very similarly named to `into_owned`, but the effect of calling those two methods is completely different (one makes the `Cow::Borrowed` into a `Cow::Owned`, the other just clones the `Cow`). If the cow is then passed to code for which the type is not checked (e.g. generics, closures, etc.) it might slip through and if the cow data is coming from an unsafe context there is the potential for accidentally cause undefined behavior.
Even if not falling into this painful case, there's really no reason to call `to_owned` on a `Cow` other than confusing people reading the code: either `into_owned` or `clone` should be called.
Note that this overlaps perfectly with `implicit_clone` as a warning, but `implicit_clone` is classified pedantic (while the consequences for `Cow` might be of a wider blast radius than just pedantry); given the overlap, I set-up the lint so that if `suspicious_to_owned` triggers `implicit_clone` will not trigger. I'm not 100% sure this is done in the correct way (I tried to copy what other lints were doing) so please provide feedback on it if it isn't.
### Checklist
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
This is done because `to_owned` is very similarly named to `into_owned`, but the
effect of calling those two methods is completely different. This creates
confusion (stemming from the ambiguity of the 'owned' term in the context of
`Cow`s) and might not be what the writer intended.
new lint
This fixes#6576
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
---
changelog: add [`multi_assignments`] lint
feat(fix): Do not lint if the target code is inside a loop
close#8753
we consider the following code.
```rust
fn main() {
let vec = vec![1];
let w: Vec<usize> = vec.iter().map(|i| i * i).collect(); // <- once.
for i in 0..2 {
let _ = w.contains(&i);
}
}
```
and the clippy will issue the following warning.
```rust
warning: avoid using `collect()` when not needed
--> src/main.rs:3:51
|
3 | let w: Vec<usize> = vec.iter().map(|i| i * i).collect();
| ^^^^^^^
...
6 | let _ = w.contains(&i);
| -------------- the iterator could be used here instead
|
= note: `#[warn(clippy::needless_collect)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
help: check if the original Iterator contains an element instead of collecting then checking
|
3 ~
4 |
5 | for i in 0..2 {
6 ~ let _ = vec.iter().map(|i| i * i).any(|x| x == i);
```
Rewrite the code as indicated.
```rust
fn main() {
let vec = vec![1];
for i in 0..2 {
let _ = vec.iter().map(|i| i * i).any(|x| x == i); // <- execute `map` every loop.
}
}
```
this code is valid in the compiler, but, it is different from the code before the rewrite.
So, we should not lint, If `collect` is outside of a loop.
Thank you in advance.
---
changelog: Do not lint if the target code is inside a loop in `needless_collect`
Lint `collapsible_str_replace`
fixes#6651
```
changelog: [`collapsible_str_replace`]: create new lint `collapsible_str_replace`
```
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
Rework `only_used_in_recursion`
fixes#8782fixes#8629fixes#8560fixes#8556
This is a complete rewrite of the lint. This loses some capabilities of the old implementation. Namely the ability to track through tuple and slice patterns, as well as the ability to trace through assignments.
The two reported bugs are fixed with this. One was caused by using the name of the method rather than resolving to the `DefId` of the called method. The second was cause by using the existence of a cycle in the dependency graph to determine whether the parameter was used in recursion even though there were other ways to create a cycle in the graph.
Implementation wise this switches from using a visitor to walking up the tree from every use of each parameter until it has been determined the parameter is used for something other than recursion. This is likely to perform better as it avoids walking the entire function a second time, and it is unlikely to walk up the HIR tree very much. Some cases would perform worse though.
cc `@buttercrab`
changelog: Scale back `only_used_in_recursion` to fix false positives
changelog: Move `only_used_in_recursion` back to `complexity`
Enhance `needless_borrow` to consider trait implementations
The proposed enhancement causes `needless_borrow` to suggest removing `&` from `&e` when `&e` is an argument position requiring trait implementations, and `e` implements the required traits. Example:
```
error: the borrowed expression implements the required traits
--> $DIR/needless_borrow.rs:131:51
|
LL | let _ = std::process::Command::new("ls").args(&["-a", "-l"]).status().unwrap();
| ^^^^^^^^^^^^^ help: change this to: `["-a", "-l"]`
```
r? `@Jarcho`
changelog: Enhance `needless_borrow` to consider trait implementations
unwrap_used and expect_used: trigger on uses of their _err variants
changelog: [`unwrap_used`]: lint uses of `unwrap_err`
changelog: [`expect_used`]: lint uses of `expect_err`
fixes#9331
`transmute_undefined_repr` fix
changelog: Don't lint `transmute_undefined_repr` when the the first field of a `repr(C)` type is compatible with the other type
Use `CARGO_TARGET_DIR` in compile-test
changelog: none
I have a global `CARGO_TARGET_DIR` set, but forgot to delete the old target dir. `compile-test` was getting tripped up on an outdated `rustfix_missing_coverage.txt` I had in there, keeping me from running tests 😄.
Fix [`non_ascii_literal`] in tests
changelog: Don't lint [`non_ascii_literal`] when using non-ascii comments in tests
changelog: Don't lint [`non_ascii_literal`] when `allow`ed on tests
closes: #7739closes: #8263
Add new lint [`positional_named_format_parameters`]
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Add new lint [`positional_named_format_parameters`] to warn when named parameters in format strings are used as positional arguments.
Fix if_let_mutex not checking Mutexes behind refs
Fixes#9193
We can always peel references because we are looking for a method-call, for which autoderef applies.
---
changelog: [`if_let_mutex`]: detect calls to `Mutex::lock()` if mutex is behind a ref
changelog: [`if_let_mutex`]: Add labels to the two instances of the same Mutex that will deadlock
Add lint recommending using `std::iter::once` and `std::iter::empty`
```
changelog: [`iter_once`]: add new lint
changelog: [`iter_empty`]: add new lint
```
fixes#9186
- \[ ] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
The lint doesn't really follow the naming conventions. I don't have any better idea so I'm open to suggestions.
Extend `if_then_some_else_none` to also suggest `bool::then_some`
Closes#9094.
changelog: Extend `if_then_some_else_none` to also suggest `bool::then_some`
Add partialeq_to_none lint
Initial implementation of #9275, adding lint `partialeq_to_none`. This is my first time working on `clippy`, so please review carefully.
I'm unsure especially about the `Sugg`, as it covers the entire `BinOp`, instead of just covering one of the sides and the operator (see the multi-line example). I was unsure if pinpointing the suggestion wouldn't be brittle...
changelog: [`PARTIALEQ_TO_NONE`]: Initial commit
`explicit_auto_deref` changes
fixes#9123fixes#9109fixes#9143fixes#9101
This avoid suggesting code which hits a rustc bug. Basically `&{x}` won't use auto-deref if the target type is `Sized`.
changelog: Don't suggest using auto deref for block expressions when the target type is `Sized`
changelog: Include the borrow in the suggestion for `explicit_auto_deref`
changelog: Don't lint `explicit_auto_deref` on `dyn Trait` return
changelog: Don't lint `explicit_auto_deref` when other adjustments are required
changelog: Lint `explicit_auto_deref` in implicit return positions for closures
Fix suggestions for `async` closures in redundant_closure_call
Fixes#9052
changelog: Fix suggestions given by [`redundant_closure_call`] for async closures
- only compare where predicates to trait bounds when generating where
clause specific message to fix#9151
- use comparable_trait_ref to account for trait bound generics to fix#8757
Move [`assertions_on_result_states`] to restriction
Close#9263
This lint causes regression on readability of code and log output. And printing runtime values is not particularly useful for majority of tests which should be reproducible.
changelog: Move [`assertions_on_result_states`] to restriction and don't lint it for unit type
Signed-off-by: tabokie <xy.tao@outlook.com>
unwrap_used: Don't recommend using `expect` when the `expect_used` lint is not allowed
Fixes#9222
```
changelog: [`unwrap_used`]: Don't recommend using `expect` when the `expect_used` lint is not allowed
```
Read and use deprecated configuration (as well as emitting a warning)
Original change written by `@flip1995` I've simply rebased to master and fixed up the formatting/tests. This change teaches the configuration parser which config key replaced a deprecated key and attempts to populate the latter from the former. If both keys are provided this fails with a duplicate key error (rather than attempting to guess which the user intended).
Currently this on affects `cyclomatic-complexity-threshold` -> `cognitive-complexity-threshold` but will also be used in #8974 to handle `blacklisted-names` -> `disallowed-names`.
```
changelog: deprecated configuration keys are still applied as if they were provided as their non-deprecated name.
```
- [x] `cargo test` passes locally
- [x] Run `cargo dev fmt`
Add `ui_cargo_toml_metadata` test
This PR adds a test to check the metadata of packages in the `ui_cargo` directory.
A recent change to Cargo causes it to warn when it finds multiple packages with the same name in a git dependency (the issue is described [here](https://github.com/rust-lang/cargo/issues/10752)).
Many (if not all) Dylint libraries depend upon `clippy_utils`. As a result of the change, one now sees the following when building a Dylint library:
```
warning: skipping duplicate package `fail` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/module_style/pass_mod`
warning: skipping duplicate package `fail` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/module_style/fail_no_mod`
warning: skipping duplicate package `cargo_common_metadata` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/cargo_common_metadata/fail_publish_true`
warning: skipping duplicate package `fail-cargo` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/cargo_rust_version/pass_cargo`
warning: skipping duplicate package `fail-clippy` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/cargo_rust_version/fail_clippy`
warning: skipping duplicate package `fail-both-same` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/cargo_rust_version/fail_both_same`
warning: skipping duplicate package `fail-file-attr` found at `/home/smoelius/.cargo/git/checkouts/rust-clippy-4b72815e96774b3d/0cb0f76/tests/ui-cargo/cargo_rust_version/fail_file_attr`
```
There appear to be two contributing factors:
- Some packages in `ui_cargo` could have a `publish = false` added to them.
- Some packages in `ui_cargo` seem to be inconsistently named.
The new test checks that each package in the `ui_cargo` directory has a name matching one of its parent directories, and `publish = false` in its metadata (with a few exceptions).
Note that the packages in `cargo_common_metadata` require special care because `publish` is the subject of some of the `cargo_common_metadata` tests.
Also note that this PR adds `walkdir` as a dev dependency to the `clippy` package. However, it was already a dependency of `clippy_dev` and `lintcheck`. So hopefully this is acceptable.
Our continued thanks for making `clippy_utils` available, BTW. :)
r? `@flip1995`
changelog: none
Add new lint `obfuscated_if_else`
part of #9100, additional commits could make it work with `then` and `unwrap_or_else` as well
changelog: Add new lint `obfuscated_if_else`
unused_self: respect avoid-breaking-exported-api
```
changelog: [`unused_self`]: Now respects the `avoid-breaking-exported-api` config option
```
Fixes#9195.
I mostly copied the implementation from `unnecessary_wraps`, since I don't have much understanding of rustc internals.
[`box_collection`]: raise warn for all std collections
So far, only [`Vec`, `String`, `HashMap`] were considered.
Extend collection checklist for this lint with:
- `HashSet`
- `VecDeque`
- `LinkedList`
- `BTreeMap`
- `BTreeSet`
- `BinaryHeap`
changelog: [`box_collection`]: raise warn for all std collections
fix [`manual_flatten`] help texts order
fixes #8948
Whenever suggestion for this lint does not fit in one line,
legacy solution has some unexpected/unhandled behavior:
lint will then generate two help messages which seem to be shown in the wrong order.
The second help message in that case will contain the suggestion.
The first help message always refers to a suggestion message,
and **it should adapt** depending on the location of the suggestion:
- inline suggestion within the error/warning message
- suggestion separated into a second help text
This is my first contribution here, so I hope I didn't miss anything for creating this PR.
changelog: fix [`manual_flatten`] help texts order
Whenever suggestion for this lint does not fit in one line,
lint will generate two help messages. The second help message
will always contain the suggestion.
The first help message refers to suggestion message,
and it should adapt depending on the location of the suggestion:
- inline suggestion within the error/warning message
- suggestion separated into second help text
Add `repeated_where_clause_or_trait_bound` lint
I thought I would try and scratch my own itch for #8674.
1. Is comparing the `Res` the correct way for ensuring we have the same trait?
2. Is there a way to get the spans for the bounds and clauses for suggestions?
I tried to use `GenericParam::bounds_span_for_suggestions` but it only gave me an empty span at the end of the spans.
I tried `WhereClause::span_for_predicates_or_empty_place` and it included the comma.
3. Is there a simpler way to get the trait names? I have used the spans of the traits because I didn't see a way to get it off the `Res` or `Def`.
changelog: Add ``[`repeated_where_clause_or_trait_bound`]`` lint.
Fixes for `branches_sharing_code`
fixes#7198fixes#7452fixes#7555fixes#7589
changelog: Don't suggest moving modifications to locals used in any of the condition expressions in `branches_sharing_code`
changelog: Don't suggest moving anything after a local with a significant drop in `branches_sharing_code`
Fix span for or_fun_call
Closes#9033
changelog: [`or_fun_call`]: span points to the `unwrap_or` only instead of through the entire method chain expression
* Don't suggest moving modifications to locals used in any of the condition expressions
* Don't suggest moving anything after a local with a significant drop
Simplify if let statements
fixes: #8288
---
changelog: Allowing [`qustion_mark`] lint to check `if let` expressions that immediatly return unwrapped value
Add test for and fix rust-lang/rust-clippy#9131
This lint seems to have been broken by #98446 -- but of course, there was no clippy test for this case at the time.
`expr.span.ctxt().outer_expn_data()` now has `MacroKind::Derive` instead of `MacroKind::Attr` for something like:
```
#[derive(Clone, Debug)]
pub struct UnderscoreInStruct {
_foo: u32,
}
```
---
changelog: none
closes: https://github.com/rust-lang/rust-clippy/issues/9131
