Don't show no_effect warning on unit structs implementing fn_once
Fixes#7792
changelog: Don't show [`no_effect`] or [`unecessary_operation`] warning for unit struct implementing FnOnce
Fix for #7889 and add new lint needless_splitn
fixes: #7889
1. Fix the problem of manual_split_once changing the original behavior.
2. Add a new lint needless_splitn.
changelog: Fix the problem of manual_split_once changing the original behavior and add a new lint needless_splitn.
1. Fix the problem of manual_split_once changing the original behavior.
2. Add a new lint needless_splitn.
changelog: Fix the problem of manual_split_once changing the original behavior and add a new lint needless_splitn.
Fix `needless_collect`'s tendency to suggest code requiring multiple mutable borrows of the same value.
Fixes error specified in #7975.
changelog: [`needless_collect`] no longer suggests removal of `collect` when removal would create code requiring mutably borrowing a value multiple times.
* Lint when a borrow is auto dereferenced more than once
* Lint when the expression is used as the expression of a block for a match arm
Moves `needless_borrow` and `ref_binding_to_reference` to `dereference`
lint pass in preperation for `explicit_auto_deref` lint.
Support suggestion for #7854
I think the detection of parking_lot's mutex and rwlock is valuable, so submit this pr, please help judge and review, thank you.
Make let_underscore_lock support parking_lot.(Fixes#7854)
changelog: Make let_underscore_lock support parking_lot
I think the detection of parking_lot's mutex and rwlock is valuable, so submit this pr, please help judge and review, thank you.
Make let_underscore_lock support parking_lot.
changelog: Make let_underscore_lock support parking_lot
Lint for bool to integer casts in `cast_lossless`
The lint description says
> Checks for casts between *numerical* types that may be replaced by safe conversion functions.
Which is strictly speaking being violated here, but it seems within the spirit of the lint. I think it is still a useful lint to have, and having a different lint for just this feels excessive. Thoughts?
Fixes#7947
changelog: Lint for bool to integer casts in [`cast_lossless`]
Author improvements
changelog: none
Various aspects of the author implementation are re-imagined to be much less repetitive. Also fixes some bugs. I hope this makes author more fun to work on for future contributors.
The last commit is pretty heavy but I tried to at least separate some changes so that the test file diffs per commit are simple.
* Finding pattern slices for `avoidable_slice_indexing`
* `avoidable_slice_indexing` analysing slice usage
* Add configuration to `avoidable_slice_indexing`
* Emitting `avoidable_slice_indexing` with suggestions
* Dogfooding and fixing bugs
* Add ui-toml test for `avoidable_slice_indexing`
* Correctly suggest `ref` keywords for `avoidable_slice_indexing`
* Test and document `mut` for `avoid_slice_indexing`
* Handle macros with `avoidable_slice_indexing` lint
* Ignore slices with sub patterns in `avoidable_slice_indexing`
* Update lint description for `avoidable_slice_indexing`
* Move `avoidable_slice_indexing` to nursery
* Added more tests for `avoidable_slice_indexing`
* Update documentation and message for `avoidable_slice_indexing`
* Teach `avoidable_slice_indexing` about `HirId`s and `Visitors`
* Rename lint to `index_refutable_slice` and connected config
Add Clippy version to Clippy's lint list
Hey, hey, the semester is finally over, and I wanted to get back into hacking on Clippy. It has also been some time since our metadata collection monster has been feed. So, this PR adds a new attribute `clippy::version` to document which version a lint was stabilized. I considered using `git blame` but that would be very hacky and probably not accurate.
I'm also thinking that this attribute can be used to have a `clippy::nightly` lint group which is allow-by-default that delays setting the actual lint group until the defined version is reached. Just something to consider regarding #6623🙃
This PR only adds the version to 4 lints to keep it reviewable. I'll do a followup PR to add the version to other lints if the implementation is accepted 🙃
Also, mobile approved xD
---
r? `@flip1995`
cc: #7172closes: #6492
changelog: [Clippy's lint list](https://rust-lang.github.io/rust-clippy/master/index.html) now displays the version a lint was added. 🎉
---
Example lint declaration after this update:
```rs
declare_clippy_lint! {
/// [...]
///
/// ### Example
/// ```rust
/// // Bad
/// let x = 3.14;
/// // Good
/// let x = std::f32::consts::PI;
/// ```
#[clippy::version = "pre 1.29.0"]
pub APPROX_CONSTANT,
correctness,
"the approximate of a known float constant (in `std::fXX::consts`)"
}
```
Fix `explicit_counter_loop` suggestion for non-usize types
changelog: Add a new suggestion for non-usize types in [`explicit_counter_loop`]
closes: #7920
Fix suggestion for deref expressions in redundant_pattern_matching
changelog: Fix suggestion for deref expressions in [`redundant_pattern_matching`]
closes: #7921
The only reason to use `abort_if_errors` is when the program is so broken that either:
1. later passes get confused and ICE
2. any diagnostics from later passes would be noise
This is never the case for lints, because the compiler has to be able to deal with `allow`-ed lints.
So it can continue to lint and compile even if there are lint errors.
Replace `in_macro` usage with `from_expansion`
changelog: none
Generally replace `in_macro(span)` with `span.from_expansion()`. If we're just trying to avoid expanded code, this seems more appropriate because any kind of expanded code is prone to false positives. One place I did not touch is `macro_use.rs`. I think this lint could use a rewrite so I moved `in_macro` there, the only place it is still used.
Prevent clippy::needless_lifetimes false positive in async function definition
Scan `OpaqueDef` bounds for lifetimes as well. Those `OpaqueDef` instances are generated while desugaring an `async` function definition.
This fixes#7893
changelog: Prevent [`clippy::needless_lifetimes`] false positive in `async` function definition
Fix manual_assert and match_wild_err_arm for `#![no_std]` and Rust 2021
Rust 2015 `std::panic!` has a wrapping block while `core::panic!` and Rust 2021 `std::panic!` does not. See rust-lang/rust#88919 for details.
Note that the test won't pass until clippy changes in rust-lang/rust#88860 is synced.
---
changelog: Fix [`manual_assert`] and [`match_wild_err_arm`] for `#![no_std]` and Rust 2021.
Fixes#7723
Unseparated literal suffix
Closes#7658
Since `literal_suffix` style is opinionated, we should disable by default and only enforce if it's stated as so.
changelog: [`unseparated_literal_suffix`] is renamed to `literal_suffix`, adds a new configuration `literal-suffix-style` to enforce a certain style writing literal_suffix. Possible values for `literal-suffix-style`: `"separated"`, `"unseparated"`
avoid linting `possible_truncation` on bit-reducing operations
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: avoid linting `possible_truncation` on bit-reducing operations
update most tests to 2021 edition
Some tests would no longer work at all, so I added `edition:2015` or `edition:2018` to them.
Notably 2021 panics are not yet detected correctly. Once ready, this closes#7842.
---
changelog: none
`unseparated_literal_suffix`
This commit adds a configuration `literal-suffix-style` to enforce a
specific style for unseparated_literal_suffix. The configuration accepts
two values:
- "separated"
enforce all literals to be written separately (e.g. `123_i32`)
- "unseparated"
enforce all literals to be written as unseparated (e.g. `123i32`)
Not specifying a value means that there is no preference on style and
any style should not be warned.
Disable "if_not_else" lints from firing on else-ifs
Fixes#7892
1. Convert `['if_not_else']` to `LateLintPass` and use `clippy_utils::is_else_clause` for checking.
2. Update tests.
changelog: [`if_not_else`] now ignores else if statements.
1. Convert IfNotElse to LateLintPass and use clippy_utils::is_else_clause for checking.
2. Handle the case where the span comes from desugaring.
3. Update tests.
Ignore references to type aliases in ptr_arg
Works using the fact that the hir path will point to a TyAlias, rather than being resolved to the underlying type
Fixes#7699
changelog: [`ptr_arg`] No longer lints references to type aliases
Add unit-hash lint
changelog: [`unit_hash`] Add lint for hashing unit values
This will lint for situations where the end user is attempting to hash a unit value (`()`), as the implementation in `std` simply [does nothing][impl]. Closes#7159 .
Example:
```rust
().hash(&mut state);
// Should (probably) be replaced with:
0_u8.hash(&mut state);
```
[impl]: a5f164faad/library/core/src/hash/mod.rs (L656)
Fix `question_mark` FP on custom error type
Closes#7859#7840 aims to ignore `question_mark` when the return type is custom, which is [covered here](df65291edd/tests/ui/question_mark.rs (L144-L149)). But this fails when there is a call in conditional predicate
changelog: [`question_mark`] Fix false positive when there is call in conditional predicate
Clean up tests/ui/rename.rs
Part one of #7057, cleaning up `tests/ui/rename.rs`. `tests/ui/deprecated.rs` will be updated in a subsequent PR.
changelog: none
new lint: string-slice
This is a restriction lint to highlight code that should have tests containing non-ascii characters. See #6623.
changelog: new lint: [`string-slice`]
Fix `match_str_case_mismatch` on uncased chars
False positives would result because `char::is_lowercase` and friends will return `false` for non-alphabetic chars and alphabetic chars lacking case (such as CJK scripts). Care also has to be taken for handling titlecase characters (`Dz`) and lowercased chars with no uppercase equivalent (`ʁ`).
For example, when verifying lowercase:
* Check `!any(char::is_ascii_uppercase)` instead of `all(char::is_ascii_lowercase)` for ASCII.
* Check that `all(|c| c.to_lowercase() == c)` instead of `all(char::is_lowercase)` for non-ASCII
Fixes#7863.
changelog: Fix false positives in [`match_str_case_mismatch`] on uncased characters
missing_safety_doc: Handle 'implementation safety' headers as well
We hit some FPs on this in `yoke`, it's somewhat normal to mark trait impl safety with "implementation safety". We could also broaden the check for headers which contain the word "safety" somehow, or split out impl safety stuff to only apply to traits.
changelog: handle 'implementation safety' headers in `missing_safety_doc`
Fix FP: no lint when cast is coming from `signum` method call for `cast_possible_truncation` lint
Fixes a FP when cast is coming from `signum` method call
fixes: #5395
changelog: [`cast_possible_truncation`] Fix FP when cast is coming from `signum` method call
Warn on structs with a trailing zero-sized array but no `repr` attribute
Closes#2868
changelog: Implement ``[`trailing_empty_array`]``, which warns if a struct is defined where the last field is a zero-sized array but there are no `repr` attributes. Zero-sized arrays aren't very useful in Rust itself, so such a struct is likely being created to pass to C code or in some other situation where control over memory layout matters. Either way, a `repr` attribute is needed.
FIx FP in `missing_safety_doc` lint
Fix FP where lint souldn't fire if any parent has `#[doc(hidden)]` attribute
fixes: #7347
changelog: [`missing_safety_doc`] Fix FP if any parent has `#[doc(hidden)]` attribute
avoid `eq_op` in test code
Add a check to `eq_op` that will avoid linting in functions annotated with `#[test]`
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: avoid `eq_op` in test functions
rustc_span: `Ident::invalid` -> `Ident::empty`
The equivalent for `Symbol`s was renamed some time ago (`kw::Invalid` -> `kw::Empty`), and it makes sense to do the same thing for `Ident`s as well.
Some "parenthesis" and "parentheses" fixes
"Parenthesis" is the singular (e.g. one `(` or one `)`) and "parentheses" is the plural (multiple `(` or `)`s) and this is not hard to mix up so here are some fixes for that.
Inspired by #89958
Add `format_in_format_args` and `to_string_in_format_args` lints
Fixes#7667 and #7729
I put these in `perf` since that was one of `@jplatte's` suggestions, and `redundant_clone` (which I consider to be similar) lives there as well.
However, I am open to changing the category or anything else.
r? `@camsteffen`
changelog: Add `format_in_format_args` and `to_string_in_format_args` lints
Do not expand macros in equatable_if_let suggestion
Fixes#7781
Let's use Hacktoberfest as a motivation to start contributing PRs myself again :)
changelog: [`equatable_if_let`]: No longer expands macros in the suggestion
Add #[must_use] to From::from and Into::into
Risk of churn: **High**
Magic 8-Ball says: **Outlook not so good**
I figured I'd put this out there. If we don't do it now maybe we save it for a rainy day.
Parent issue: #89692
r? `@joshtriplett`
Allow giving reasons for `disallowed_types`
Similar to #7609 but for the `disallowed_type` lint. The permitted form of configuration is the same as for `disallowed_methods`.
changelog: Allow giving reasons for [`disallowed_type`]
Fix FP in external macros for `mut_mut` lint
Fix FP in `mut_mut` lint when type is defined in external macros.
fixes: #6922
changelog: [`mut_mut`] Fix FP when type is defined in external macros
Refactor `clippy::match_ref_pats` to check for multiple reference patterns
fixes#7740
When there is only one pattern, to begin with, i.e. a single deref(`&`), then in such cases we suppress `clippy::match_ref_pats`.
This is done by checking the count of the reference pattern and emitting `clippy::match_ref_pats` when more than one pattern is present.
Removed certain errors in the `stderr` tests as they would not be triggered.
changelog: Refactor `clippy::match_ref_pats` to check for multiple reference patterns
Before this lint didn't trigger on macros. With rust-lang/rust#88175
this isn't enough anymore. In this PR a `WhileLoop` desugaring kind was
introduced. This overrides the span of expanded expressions when
lowering the while loop. So if a while loop is in a macro, the
expressions that it expands to are no longer marked with
`ExpnKind::Macro`, but with `ExpnKind::Desugaring`. In general, this is
the correct behavior and the same that is done for `ForLoop`s. It just
tripped up this lint.
Restriction lint for function pointer casts
The existing lints for function pointer casts cover the cases where a cast is non-portable or would result in truncation, however there's currently no way to forbid numeric function pointer casts entirely.
I've added a new lint `fn_to_numeric_cast_any`, which allows one to ban _all_ numeric function pointer casts, including to `usize`. This is useful if you're writing high-level Rust and want to opt-out of potentially surprising behaviour, avoiding silent bugs from forgotten parentheses, e.g.
```rust
fn foo() -> u32 {
10
}
fn main() {
let _ = foo as usize; // oops, forgot to call foo and got a random address instead!
}
```
~~I'm open to suggestions for the naming of the lint, because `fn_to_numeric_cast_any` is a bit clunky. Ideally I'd call this lint `fn_to_numeric_cast`, but that name is already taken for the more specific lint~~. We've stuck with `fn_to_numeric_cast_any` to avoid renaming the existing lint, or choosing a different name that's too generic (like `fn_cast`).
I'm also open to changing the suggestion behaviour, as adding parentheses is only one of many possible ways to fix the lint.
changelog: add ``[`fn_to_numeric_cast_any`]`` restriction lint
Make `shadow_reuse` suggestion less verbose
Closes#7764
Make `shadow_reuse` suggestion less verbose.
changelog: [`shadow_reuse`] does not warn shadowing statement
make test module detection more strict
I started with some small improvements to clippy_utils/src/lib.rs, but then found that our "test" module detection would also catch words containing "test" like e.g. "attestation". So I made this a bit more strict (splitting by `'_'` and checking for `test` or `tests`), adding a test case as I went.
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: none
Add expansion to while desugar spans
In the same vein as #88163, this reverts a change in Clippy behavior as a result of #80357 (and reverts some `#[allow]`s): This changes `clippy::blocks_in_if_conditions` to not fire on `while` loops. Though we might actually want Clippy to lint those cases, we should introduce the change purposefully, with tests, and possibly under a different lint name.
The actual change here is to add a desugaring expansion to the spans when lowering a `while` loop.
r? `@Manishearth`
Fix ICE when `start` lang item has wrong generics
In my previous pr #87875 I missed the requirements on the `start` lang item due to its relative difficulty to test and opting for more conservative estimates. This fixes that by updating the requirement to be exactly one generic type.
The `start` lang item should have exactly one generic type for the return type of the `main` fn ptr passed to it. I believe having zero would previously *sometimes* compile (often with the use of `fn() -> ()` as the fn ptr but it was likely UB to call if the return type of `main` was not `()` as far as I know) however it also sometimes would not for various errors including ICEs and LLVM errors depending on exact situations. Having more than 1 generic has always failed with an ICE because only the one generic type is expected and provided.
Fixes#79559, fixes#73584, fixes#83117 (all duplicates)
Relevant to #9307
r? ````@cjgillot````
fix bug for large_enum_variants
Fix the discussion problem in the issue of https://github.com/rust-lang/rust-clippy/issues/7666#issuecomment-919654291
About the false positive problem of case:
```rust
enum LargeEnum6 {
A,
B([u8;255]),
C([u8;200]),
}
```
changelog: Fix largest_enum_variant wrongly identifying the second largest variant.
Rustup
This needs a review this time. Especially 521bf8f0fa cc `@camsteffen` I think this is necessary now, because `itertools` is no longer a dependency of `clippy_utils` and therefore this path can't be found 🤔
( I forgot about the sync last week. I should get to document this process better, so other people can do it when I'm not around )
changelog: none
Don't lint `suspicious_else_formatting` inside proc-macros
fixes: #7650
I'll add a test for this one soon.
changelog: Don't lint `suspicious_else_formatting` inside proc-macros
Expand box_vec lint to box_collection
fixed#7451
changelog: Expand `box_vec` into [`box_collection`], and have it error on all sorts of boxed collections
Change `while_let_on_iterator` suggestion to use `by_ref()`
It came up in the discussion #7659 that suggesting `iter.by_ref()` is a clearer suggestion than `&mut iter`. I personally think they're equivalent, but if `by_ref()` is clearer to people then that should be the suggestion.
changelog: Change `while_let_on_iterator` suggestion when using `&mut` to use `by_ref()`
