Do not propose to simplify a not expression coming from a macro
Fixes#10523
changelog: FP [`nonminimal_bool`]: do not propose to change code coming from a macro
Do not propose to remove `async move` if variables are captured by ref
Fixes#10482
changelog: FP [`redundant_async_block`] Do not propose to remove `async move` if variables are captured by ref
Really dogfood clippy
The dogfood success condition was inverted in `tests/dogfood.rs`:
```rust
assert!(!failed_packages.is_empty(), …);
```
while instead the `failed_packages` collection must be empty:
```rust
assert!(failed_packages.is_empty(), …);
```
And indeed, several clippy lint source files were not clean and had to be fixed in the process.
changelog: none
The dogfood success condition was inverted in `tests/dogfood.rs`:
```rust
assert!(!failed_packages.is_empty(), …);
```
while instead the `failed_packages` collection must be empty:
```rust
assert!(failed_packages.is_empty(), …);
```
And indeed, several clippy lint source files were not clean and had to be
fixed in the process.
rustc has proper heuristics for actually checking whether a type allows
being left uninitialized (by asking CTFE). We can now use this for our
helper instead of rolling our own bad version with false positives.
Issue function modifiers in the right order in manual_async_fn lint
Fixes#10450
changelog: [`manual_async_fn`] output function modifiers in correct order
fix `almost_swapped`: Ignore external macros
Fixes#10421 ; Related to #10499 (Fixing points *1* and *3* from #10421)
changelog: [`almost_swapped`]: Add a check to ignore external macros
Fix `almost_swapped` false positive (`let mut a = b; a = a`)
Fixes `2` in #10421
changelog: [`almost_swapped`]: Fix false positive when a variable is changed to itself. (`a = a`)
Remove `identity_future` indirection
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.
Fixes https://github.com/rust-lang/rust/issues/104826.
enhance [`ifs_same_cond`] to warn same immutable method calls as well
fixes: #10272
---
changelog: Enhancement: [`ifs_same_cond`]: Now also detects immutable method calls.
[#10350](https://github.com/rust-lang/rust-clippy/pull/10350)
<!-- changelog_checked -->
Remove `box_syntax`
r? `@Nilstrieb`
This removes the feature `box_syntax`, which allows the use of `box <expr>` to create a Box, and finalises removing use of the feature from the compiler. `box_patterns` (allowing the use of `box <pat>` in a pattern) is unaffected.
It also removes `ast::ExprKind::Box` - the only way to create a 'box' expression now is with the rustc-internal `#[rustc_box]` attribute.
As a temporary measure to help users move away, `box <expr>` now parses the inner expression, and emits a `MachineApplicable` lint to replace it with `Box::new`
Closes#49733
Strengthen state tracking in const-prop
Some/many of the changes are replicated between both the const-prop lint and the const-prop optimization.
Behaviour changes:
- const-prop opt does not give a span to propagated values. This was useless as that span's primary purpose is to diagnose evaluation failure in codegen.
- we remove the `OnlyPropagateInto` mode. It was only used for function arguments, which are better modeled by a write before entry.
- the tracking of assignments and discriminants make clearer that we do nothing in `NoPropagation` mode or on indirect places.
Add utility macros to help with writing tests.
Adds two utility macros to help with testing:
* `external` expands to it's argument tokens, but makes them appear to come from an external macro. Helps make tests for `in_external_macro` much more readable.
* `inline_macros` is an attribute macro which allows the use of a pseudo `inline!` macro which expands to it's argument tokens, but makes them appear to be from a crate-local macro expansion. This removes the need to write `macro_rules` boilerplate when testing how lints interact with macros.
---
`external`'s usage is simple. `external!(struct Foo { x: u32});` will make the struct appear as though it came from an external macro. Individual tokens can be escaped if needed. `external!($x + 0 / 10)` will make everything except `x` appear as though it came from an external macro. Can also use `$literal` and `$(tokens...)` as well.
---
`inline_macros` is more complicated due to compiler constraints. Given:
```rust
#[inline_macros]
fn foo() {
inline!(5 + 5 / 10);
}
```
`inline!(5 + 5 / 10)` will be replace with a call to a generated macro which expands to the contained tokens.
Tokens can be escaped by prefixing them with `$`:
```rust
#[inline_macros]
fn foo() {
let x = 5;
inline!($x + 5 / $10);
}
```
This will pass `x` as an `ident` argument and `10` as a `literal` argument.
Token sequences can also be passed with `$(...)`:
```rust
#[inline_macros]
fn foo() {
let mut x = 5;
inline!(if $(x >= 5) {
$x = 5;
});
}
```
This will pass `x >= 5` as `tt` arguments, and `x` as an `ident` argument.
---
Not 100% sure `inline_macros` is actually worth having. It does make the tests a little easier to read once you're used to it and it becomes more useful once there are multiple macro tests. The verbosity of declaring single use macros starts to hurt at that point.
changelog: None
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
