Out of cycle Clippy update
I want to do an out-of-cycle sync for rust-lang/rust-clippy#8295, and possibly backport this to stable together with https://github.com/rust-lang/rust/issues/92938. If this doesn't get backported to stable, then I at least want to backport it to beta.
r? `@Manishearth`
fix op_ref false positive
fixes#7572
changelog: `op_ref` don't lint for unnecessary reference in BinOp impl if removing the reference will lead to unconditional recursion
Replace use of `ty()` on term and use it in more places. This will allow more flexibility in the
future, but slightly worried it allows items which are consts which only accept types.
ProjectionPredicate should be able to handle both associated types and consts so this adds the
first step of that. It mainly just pipes types all the way down, not entirely sure how to handle
consts, but hopefully that'll come with time.
Replace `NestedVisitorMap` with generic `NestedFilter`
This is an attempt to make the `intravisit::Visitor` API simpler and "more const" with regard to nested visiting.
With this change, `intravisit::Visitor` does not visit nested things by default, unless you specify `type NestedFilter = nested_filter::OnlyBodies` (or `All`). `nested_visit_map` returns `Self::Map` instead of `NestedVisitorMap<Self::Map>`. It panics by default (unreachable if `type NestedFilter` is omitted).
One somewhat trixty thing here is that `nested_filter::{OnlyBodies, All}` live in `rustc_middle` so that they may have `type Map = map::Map` and so that `impl Visitor`s never need to specify `type Map` - it has a default of `Self::NestedFilter::Map`.
issue #8239: Printed hint for lint or_fun_call is cropped and does no…
fixesrust-lang/rust-clippy#8239
changelog: [`or_fun_call`]: if suggestion contains more lines than MAX_SUGGESTION_HIGHLIGHT_LINES it is stripped to one line
Remove deprecated LLVM-style inline assembly
The `llvm_asm!` was deprecated back in #87590 1.56.0, with intention to remove
it once `asm!` was stabilized, which already happened in #91728 1.59.0. Now it
is time to remove `llvm_asm!` to avoid continued maintenance cost.
Closes#70173.
Closes#92794.
Closes#87612.
Closes#82065.
cc `@rust-lang/wg-inline-asm`
r? `@Amanieu`
Handle implicit named arguments in `useless_format`
fixes#8290
Ideally this would fix the macro parsing code to handle this, but this is a smaller change and easier to back port.
changelog: Handle implicit named arguments in `useless_format`
* Track the argument when used to initialize simple `let` bindings
* Check if the argument is passed to a function requiring the original type
* Use `multipart_suggestion` rather than multiple suggestions
* Check if the name given in the source code matches the name of the actual type
`manual_memcpy` fix
fixes#8160
Ideally this would work with `VecDeque`, but the current interface is unsuitable for it. At a minimum something like `range_as_slices` would be needed.
changelog: Don't lint `manual_memcpy` on `VecDeque`
changelog: Suggest `copy_from_slice` for `manual_memcpy` when applicable
Improve documentation for `borrowed-box` lint
fixes#8161
Updates documentation to elaborate more on how removing Box from a function parameter can generalize the function.
changelog: none
The field is also renamed from `ident` to `name. In most cases,
we don't actually need the `Span`. A new `ident` method is added
to `VariantDef` and `FieldDef`, which constructs the full `Ident`
using `tcx.def_ident_span()`. This method is used in the cases
where we actually need an `Ident`.
This makes incremental compilation properly track changes
to the `Span`, without all of the invalidations caused by storing
a `Span` directly via an `Ident`.
Downgrade mutex_atomic to nursery
See #1516 and #4295.
There are suggestions about removing this lint from the default warned lints in both issues.
Also, [`mutex_integer`](https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer) lint that has the same problems as this lint is in `nursery` group.
changelog: Moved [`mutex_atomic`] to `nursery`
new lint: `single_char_lifetime_names`
This pull request adds a lint against single character lifetime names, as they might not divulge enough information about the purpose of the lifetime. This can make code harder to understand. I placed this in `restriction` rather than `pedantic` (as suggested in #8233) since most of the Rust ecosystem already uses single character lifetime names (to my knowledge, at least) and since single character lifetime names aren't incorrect. I'd be happy to change this upon request, however. Fixes#8233.
- [x] Followed lint naming conventions
- [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: new lint: [`single_char_lifetime_names`]
This pull request adds a lint against single character lifetime names, as they might not divulge enough information about the purpose of the lifetime. This can make code harder to understand. I placed this in `restriction` rather than `pedantic` (as suggested in #8233) since most of the Rust ecosystem already uses single character lifetime names (to my knowledge, at least) and since single character lifetime names aren't incorrect. I'd be happy to change this upon request, however. Fixes#8233.
- [x] Followed lint naming conventions
- [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: new lint: [`single_char_lifetime_names`]
Change `unnecessary_to_owned` `into_iter` suggestions to `MaybeIncorrect`
I am having a hard time finding a good solution for #8148, so I am wondering if is enough to just change the suggestion's applicability to `MaybeIncorrect`?
I apologize, as I realize this is a bit of a cop out.
changelog: none
Better detect when a field can be moved from in `while_let_on_iterator`
fixes#8113
changelog: Better detect when a field can be moved from in `while_let_on_iterator`
Fix `type_repetition_in_bounds`
fixes#7360fixes#8162fixes#8056
changelog: Check for full equality in `type_repetition_in_bounds` rather than just equal hashes
Remove in_macro from clippy_utils
changelog: none
Previously done in #7897 but reverted in #8170. I'd like to keep `in_macro` out of utils because if a span is from expansion in any way (desugaring or macro), we should not proceed without understanding the nature of the expansion IMO.
r? `@llogiq`
changelog: none
Sorry, this is a big one. A lot of interrelated changes and I wanted to put the new utils to use to make sure they are somewhat battle-tested. We may want to divide some of the lint-specific refactoring commits into batches for smaller reviewing tasks. I could also split into more PRs.
Introduces a bunch of new utils at `clippy_utils::macros::...`. Please read through the docs and give any feedback! I'm happy to introduce `MacroCall` and various functions to retrieve an instance. It feels like the missing puzzle piece. I'm also introducing `ExpnId` from rustc as "useful for Clippy too". `@rust-lang/clippy`
Fixes#7843 by not parsing every node of macro implementations, at least the major offenders.
I probably want to get rid of `is_expn_of` at some point.
wrong_self_convention: Match `SelfKind::No` more restrictively
The `wrong_self_convention` lint uses a `SelfKind` type to decide
whether a method has the right kind of "self" for its name, or whether
the kind of "self" it has makes its name confusable for a method in
a common trait. One possibility is `SelfKind::No`, which is supposed
to mean "No `self`".
Previously, SelfKind::No matched everything _except_ Self, including
references to Self. This patch changes it to match Self, &Self, &mut
Self, Box<Self>, and so on.
For example, this kind of method was allowed before:
```
impl S {
// Should trigger the lint, because
// "methods called `is_*` usually take `self` by reference or no `self`"
fn is_foo(&mut self) -> bool { todo!() }
}
```
But since SelfKind::No matched "&mut self", no lint was triggered
(see #8142).
With this patch, the code above now gives a lint as expected.
fixes#8142
changelog: [`wrong_self_convention`] rejects `self` references in more cases
Inspired by a discussion in rust-lang/rust-clippy#8197
---
r? `@llogiq`
changelog: none
The lint is this on nightly, therefore no changelog entry for you xD
The `wrong_self_convention` lint uses a `SelfKind` type to decide
whether a method has the right kind of "self" for its name, or whether
the kind of "self" it has makes its name confusable for a method in
a common trait. One possibility is `SelfKind::No`, which is supposed
to mean "No `self`".
Previously, SelfKind::No matched everything _except_ Self, including
references to Self. This patch changes it to match Self, &Self, &mut
Self, Box<Self>, and so on.
For example, this kind of method was allowed before:
```
impl S {
// Should trigger the lint, because
// "methods called `is_*` usually take `self` by reference or no `self`"
fn is_foo(&mut self) -> bool { todo!() }
}
```
But since SelfKind::No matched "&mut self", no lint was triggered
(see #8142).
With this patch, the code above now gives a lint as expected.
Fixes#8142
changelog: [`wrong_self_convention`] rejects `self` references in more cases
This improves the quality of the genrated output and makes it
more in line with other lint messages.
changelog: [`unused_io_amount`]: Improve help text
Clippy helpfully warns about code like this, telling you that you
probably meant "write_all":
fn say_hi<W:Write>(w: &mut W) {
w.write(b"hello").unwrap();
}
This patch attempts to extend the lint so it also covers this
case:
async fn say_hi<W:AsyncWrite>(w: &mut W) {
w.write(b"hello").await.unwrap();
}
(I've run into this second case several times in my own programming,
and so have my coworkers, so unless we're especially accident-prone
in this area, it's probably worth addressing?)
This patch covers the Async{Read,Write}Ext traits in futures-rs,
and in tokio, since both are quite widely used.
changelog: [`unused_io_amount`] now supports AsyncReadExt and AsyncWriteExt.
Limit the ``[`identity_op`]`` lint to integral operands.
changelog: limit ``[`identity_op`]`` to integral operands
In the ``[`identity_op`]`` lint, if the operands are non-integers, then the lint is likely
wrong.
Fixed issues with to_radians and to_degrees lints
fixes#7651
I fixed the original problem as described in the issue, but the bug remains for complex expressions (the commented out TC I added is an example). I would also love some feedback on how to cleanup my code and reduce duplication. I hope it's not a problem that the issue has been claimed by someone else - that was over two months ago.
changelog: ``[`suboptimal_flops`]`` no longer proposes broken code with `to_radians` and `to_degrees`
Fix `enum_variants` FP on prefixes that are not camel-case
closes#8090
Fix FP on `enum_variants` when prefixes are only a substring of a camel-case word. Also adds some util helpers on `str_utils` to help parsing camel-case strings.
This changes how the lint behaves:
1. previously if the Prefix is only a length of 1, it's going to get ignored, i.e. these were previously ignored and now is warned
```rust
enum Foo {
cFoo,
cBar,
cBaz,
}
enum Something {
CCall,
CCreate,
CCryogenize,
}
```
2. non-ascii characters that doesn't have casing will not be split,
```rust
enum NonCaps {
PrefixXXX,
PrefixTea,
PrefixCake,
}
```
will be considered as `PrefixXXX`, `Prefix`, `Prefix`, so this won't lint as opposed to fired previously.
changelog: [`enum_variant_names`] Fix FP when first prefix are only a substring of a camel-case word.
---
(Edited by `@xFrednet` removed some non ascii characters)
closes#8177
previously, `needless_return` suggests an empty block `{}` to replace void `return` on match arms, this PR improve the suggestion by suggesting a unit instead.
changelog: `needless_return` suggests `()` instead of `{}` on match arms
`enum_variant_names` will consider characters with no case to be a part
of prefixes/suffixes substring that are compared. This means `Foo1` and
`Foo2` has different prefixes (`Foo1` and `Foo2` prefix respeectively).
This applies to all non-ascii characters with no casing.
fix an ICE on unwrapping a None
This very likely fixes#8166 though I wasn't able to meaningfully reduce a test case. This line is the only call to `unwrap` within that function, which was the one in the stack trace that triggered the ICE, so I think we'll be OK.
`@hackmad` can you pull and build this branch and check if it indeed fixes your problem?
---
changelog: Fixed ICE in [`unnecessary_cast`]
Fix `SAFETY` comment tag casing in undocumented_unsafe_blocks
This changes the lint introduced in #7748 to suggest adding a `SAFETY` comment instead of a `Safety` comment.
Searching for `// Safety:` in rust-lang/rust yields 67 results while `// SAFETY:` yields 1072.
I think it's safe to say that this comment tag is written in upper case, just like `TODO`, `FIXME` and so on are. As such I would expect this lint to follow the official convention as well.
Note that I intentionally introduced some casing diversity in `tests/ui/undocumented_unsafe_blocks.rs` to test more cases than just `Safety:`.
changelog: Capitalize `SAFETY` comment in [`undocumented_unsafe_blocks`]
Don't emit RETURN_SELF_NOT_MUST_USE lint if `Self` already is marked as `#[must_use]`
New bug discovered with this lint. Hopefully, this is the last one.
---
changelog: none
Remove `SymbolStr`
This was originally proposed in https://github.com/rust-lang/rust/pull/74554#discussion_r466203544. As well as removing the icky `SymbolStr` type, it allows the removal of a lot of `&` and `*` occurrences.
Best reviewed one commit at a time.
r? `@oli-obk`
Ensure that RETURN_SELF_NOT_MUST_USE is not emitted if the method already has `#[must_use]`
Fixes https://github.com/rust-lang/rust-clippy/issues/8140.
---
Edit:
changelog: none
(The lint is not in beta yet, this should therefore not be included inside the changelog :) )
Implement let-else type annotations natively
Tracking issue: #87335Fixes#89688, fixes#89807, edit: fixes #89960 as well
As explained in https://github.com/rust-lang/rust/issues/89688#issuecomment-940405082, the previous desugaring moved the let-else scrutinee into a dummy variable, which meant if you wanted to refer to it again in the else block, it had moved.
This introduces a new hir type, ~~`hir::LetExpr`~~ `hir::Let`, which takes over all the fields of `hir::ExprKind::Let(...)` and adds an optional type annotation. The `hir::Let` is then treated like a `hir::Local` when type checking a function body, specifically:
* `GatherLocalsVisitor` overrides a new `Visitor::visit_let_expr` and does pretty much exactly what it does for `visit_local`, assigning a local type to the `hir::Let` ~~(they could be deduplicated but they are right next to each other, so at least we know they're the same)~~
* It reuses the code in `check_decl_local` to typecheck the `hir::Let`, simply returning 'bool' for the expression type after doing that.
* ~~`FnCtxt::check_expr_let` passes this local type in to `demand_scrutinee_type`, and then imitates check_decl_local's pattern checking~~
* ~~`demand_scrutinee_type` (the blindest change for me, please give this extra scrutiny) uses this local type instead of of creating a new one~~
* ~~Just realised the `check_expr_with_needs` was passing NoExpectation further down, need to pass the type there too. And apparently this Expectation API already exists.~~
Some other misc notes:
* ~~Is the clippy code supposed to be autoformatted? I tried not to give huge diffs but maybe some rustfmt changes simply haven't hit it yet.~~
* in `rustc_ast_lowering/src/block.rs`, I noticed some existing `self.alias_attrs()` calls in `LoweringContext::lower_stmts` seem to be copying attributes from the lowered locals/etc to the statements. Is that right? I'm new at this, I don't know.
By changing `as_str()` to take `&self` instead of `self`, we can just
return `&str`. We're still lying about lifetimes, but it's a smaller lie
than before, where `SymbolStr` contained a (fake) `&'static str`!
Stabilize `iter::zip`
Hello all!
As the tracking issue (#83574) for `iter::zip` completed the final commenting period without any concerns being raised, I hereby submit this stabilization PR on the issue.
As the pull request that introduced the feature (#82917) states, the `iter::zip` function is a shorter way to zip two iterators. As it's generally a quality-of-life/ergonomic improvement, it has been integrated into the codebase without any trouble, and has been
used in many places across the rust compiler and standard library since March without any issues.
For more details, I would refer to `@cuviper's` original PR, or the [function's documentation](https://doc.rust-lang.org/std/iter/fn.zip.html).
fix clippy format using `cargo fmt -p clippy_{lints,utils}`
manually revert rustfmt line truncations
rename to hir::Let in clippy
Undo the shadowing of various `expr` variables after renaming `scrutinee`
reduce destructuring of hir::Let to avoid `expr` collisions
cargo fmt -p clippy_{lints,utils}
bless new clippy::author output
Add new lint to warn when #[must_use] attribute should be used on a method
This lint is somewhat similar to https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate but also different: it emits a warning by default and only targets methods (so not functions nor associated functions).
Someone suggested it to me after this tweet: https://twitter.com/m_ou_se/status/1466439813230477312
I think it would reduce the number of cases of API misuses quite a lot.
What do you think?
---
changelog: Added new [`return_self_not_must_use`] lint
Ignore associated types in traits when considering type complexity
changelog: Ignore associated types in traits when checking ``[`type_complexity`]`` lint.
fixes#1013
Fix bad suggestion on `option_if_let_else` when there is complex subpat
closes#7991
Prefer not warning any complex subpat in `option_if_let_else` rather than suggesting obscure suggestions.
changelog: [`option_if_let_else`] does not warn when complex subpat is present
Parenthesize blocks in `needless_bool` suggestion
Because the `if .. {}` statement already puts the condition in expression scope, contained blocks would be parsed as complete
statements, so any `&` binary expression whose left operand ended in a block would lead to a non-compiling suggestion.
We identify such expressions and add parentheses. Note that we don't make a difference between normal and unsafe blocks because the parsing problems are the same for both.
This fixes#8052.
---
changelog: none
Because the `if .. {}` statement already puts the condition in
expression scope, contained blocks would be parsed as complete
statements, so any `&` binary expression whose left operand ended in a
block would lead to a non-compiling suggestion.
This adds a visitor to identify such expressions and add parentheses.
This fixes#8052.
Consider NonNull as a pointer type
PR 1/2 for issue #8045. Add `NonNull` as a pointer class to suppress false positives like `UnsafeCell<NonNull<()>>`. However, this change is not sufficient to handle the cases shared in gtk-rs and Rug in the issue.
changelog: none
r? `@xFrednet`
Fix `any()` not taking reference in `search_is_some` lint
`find` gives reference to the item, but `any` does not, so suggestion is broken in some specific cases.
Fixes: #7392
changelog: [`search_is_some`] Fix suggestion for `any()` not taking item by reference
Cleanup: Eliminate ConstnessAnd
This is almost a behaviour-free change and purely a refactoring. "almost" because we appear to be using the wrong ParamEnv somewhere already, and this is now exposed by failing a test using the unstable `~const` feature.
We most definitely need to review all `without_const` and at some point should probably get rid of many of them by using `TraitPredicate` instead of `TraitRef`.
This is a continuation of https://github.com/rust-lang/rust/pull/90274.
r? `@oli-obk`
cc `@spastorino` `@ecstatic-morse`
Improve `strlen_on_c_string`
fixes: #7436
changelog: lint `strlen_on_c_string` when used without a fully-qualified path
changelog: suggest removing the surrounding unsafe block for `strlen_on_c_string` when possible
Add `needless_late_init` lint
examples:
```rust
let a;
a = 1;
// to
let a = 1;
```
```rust
let b;
match 3 {
0 => b = "zero",
1 => b = "one",
_ => b = "many",
}
// to
let b = match 3 {
0 => "zero",
1 => "one",
_ => "many",
};
```
```rust
let c;
if true {
c = 1;
} else {
c = -1;
}
// to
let c = if true {
1
} else {
-1
};
```
changelog: Add [`needless_late_init`]
Add new lint `octal_escapes`
This checks for sequences in strings that would be octal character
escapes in C, but are not supported in Rust. It suggests either
to use the `\x00` escape, or an equivalent hex escape if the octal
was intended.
Fixes#7981
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Add new lint [`octal_escapes`], which checks for literals like `"\033[0m"`.
Allow `suboptimal_flops` in const functions
This PR allows `clippy::suboptimal_flops` in constant functions. The check also effects the `clippy::imprecise_flops` lint logic. However, this doesn't have any effects as all functions checked for are not const and can therefore not be found in such functions.
---
changelog: [`suboptimal_flops`]: No longer triggers in constant functions
Closes: rust-lang/rust-clippy#8004
This checks for sequences in strings that would be octal character
escapes in C, but are not supported in Rust. It suggests either
to use the `\x00` escape, or an equivalent hex escape if the octal
was intended.
Pluralize `disallowed_type` lint
This was brought up in [Zulip] and is also mentioned in the lint naming
conventions. Since this is still a nursery lint, I think there shouldn't
be any problem in renaming it.
[Zulip]: rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/disallow_type.20vs.20disallowed-types
changelog: Rename nursery lint [`disallowed_type`] to [`disallowed_types`].
Improve `needless_borrow` lint
fixes: #5327fixes: #1726fixes: #1212
This is merging `needless_borrow` into the `dereference` pass in preparation for `explicit_auto_deref`. `explicit_auto_deref` needs to implement most of what `needless_borrow` implements in order to work.
There is a minor regression here where `let x: &str = &x.deref()` will trigger `needless_borrow` without triggering `explicit_deref_methods`. Removing the redundant borrow will cause `explicit_deref_methods` to trigger. This will be fixed when `explicit_auto_deref` is implemented.
changelog: Lint `needless_borrow` when a borrow is auto-derefed more than once
changelog: Lint `needless_borrow` in the trailing expression of a block for a match arm
This function parameter attribute was introduced in https://github.com/rust-lang/rust/pull/44866 as an intermediate step in implementing `impl Trait`, it's not necessary or used anywhere by itself.
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.
1. Make the lifetime contained in LateContext `'tcx`.
2. Fix `'txc` to `'tcx` because it was a typo.
3. Refactor `IterFunctionVisitor`'s `visit_block` method to be more readable.
4. Replace uses of `rustc_middle::ty::TyKind` with `rustc::middle::ty`, and remove the `#[allow(...)]`.
(Thank you llogiq for all these suggestions!)
* 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`)"
}
```
This commit adds a `no_std` and `no_core` check on `swap` lint and additionally suggest `core::mem::swap` whenever possible.
Remove warning if both `std` and `core` is not present.
So, some context for this, well, more a story. I'm not used to scripting, I've never really scripted anything, even if it's a valuable skill. I just never really needed it. Now, `@flip1995` correctly suggested using a script for this in `rust-clippy#7813`...
And I decided to write a script using nushell because why not? This was a mistake... I spend way more time on this than I would like to admit. It has definitely been more than 4 hours. It shouldn't take that long, but me being new to scripting and nushell just wasn't a good mixture... Anyway, here is the script that creates another script which adds the versions. Fun...
Just execute this on the `gh-pages` branch and the resulting `replacer.sh` in `clippy_lints` and it should all work.
```nu
mv v0.0.212 rust-1.00.0;
mv beta rust-1.57.0;
mv master rust-1.58.0;
let paths = (open ./rust-1.58.0/lints.json | select id id_span | flatten | select id path);
let versions = (
ls | where name =~ "rust-" | select name | format {name}/lints.json |
each { open $it | select id | insert version $it | str substring "5,11" version} |
group-by id | rotate counter-clockwise id version |
update version {get version | first 1} | flatten | select id version);
$paths | each { |row|
let version = ($versions | where id == ($row.id) | format {version})
let idu = ($row.id | str upcase)
$"sed -i '0,/($idu),/{s/pub ($idu),/#[clippy::version = "($version)"]\n pub ($idu),/}' ($row.path)"
} | str collect ";" | str find-replace --all '1.00.0' 'pre 1.29.0' | save "replacer.sh";
```
And this still has some problems, but at this point I just want to be done -.-
`match_overlapping_arm` refactoring
The main purpose of this pull request is to remove the unneeded and scary `unimplented!()` in the `match_arm_overlapping` code.
The rest is gratuitous refactoring.
changelog: none
Fix `explicit_counter_loop` suggestion for non-usize types
changelog: Add a new suggestion for non-usize types in [`explicit_counter_loop`]
closes: #7920
Type inference for inline consts
Fixes#78132Fixes#78174Fixes#81857Fixes#89964
Perform type checking/inference of inline consts in the same context as the outer def, similar to what is currently done to closure.
Doing so would require `closure_base_def_id` of the inline const to return the outer def, and since `closure_base_def_id` can be called on non-local crate (and thus have no HIR available), a new `DefKind` is created for inline consts.
The type of the generated anon const can capture lifetime of outer def, so we couldn't just use the typeck result as the type of the inline const's def. Closure has a similar issue, and it uses extra type params `CK, CS, U` to capture closure kind, input/output signature and upvars. I use a similar approach for inline consts, letting it have an extra type param `R`, and then `typeof(InlineConst<[paremt generics], R>)` would just be `R`. In borrowck region requirements are also propagated to the outer MIR body just like it's currently done for closure.
With this PR, inline consts in expression position are quitely usable now; however the usage in pattern position is still incomplete -- since those does not remain in the MIR borrowck couldn't verify the lifetime there. I have left an ignored test as a FIXME.
Some disucssions can be found on [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/260443-project-const-generics/topic/inline.20consts.20typeck).
cc `````@spastorino````` `````@lcnr`````
r? `````@nikomatsakis`````
`````@rustbot````` label A-inference F-inline_const T-compiler
Fix suggestion for deref expressions in redundant_pattern_matching
changelog: Fix suggestion for deref expressions in [`redundant_pattern_matching`]
closes: #7921
Only the end bounds of ranges can actually be included or excluded. This
commit changes the SpannedRange type to reflect that. Update `Kind::value`
to and `Kind::cmp` for this change. `Kind::cmp` gets flipped to check value
first and then the bound details and is much shorter.
This unbounded case never actually happens because `all_ranges(..)` uses
the scrutinee type bounds for open ranges. Switch to our own `Bound`
enum so that we don't have this case.
Introduce `expr_visitor` and `expr_visitor_no_bodies`
changelog: none
A couple utils that satisfy a *lot* of visitor use cases. Factoring in every possible usage would be really big so I just focused on cleaning clippy_utils.
TraitKind -> Trait
TyAliasKind -> TyAlias
ImplKind -> Impl
FnKind -> Fn
All `*Kind`s in AST are supposed to be enums.
Tuple structs are converted to braced structs for the types above, and fields are reordered in syntactic order.
Also, mutable AST visitor now correctly visit spans in defaultness, unsafety, impl polarity and constness.
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.
Move non_ascii_literal to restriction
It feels like the more apt category, since cases where you'd want it enabled would be pretty specific
changelog: Move [`non_ascii_literal`] to `restriction`
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
`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)
Register the generated lints from `cargo dev new_lint`
How to register a lint was something that took me a couple reads to figure out, this will hopefully make that easier. It appends the created lint to the end of the list when running `cargo dev new_lint`
changelog: none
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
Update `str` utils to prevent ICEs and FNs
This PR reworks some string handling for lints regarding enum naming. I hope the refactoring will prevent future ICEs and help with new bug free implementations.
It might be better to review this PR by going through the commits, as `clippy_utils::camel_case` was renamed to `clippy_utils::str_utils` and then changed further. GH sadly doesn't really make the changes that obvious 🙃
Not too much more to say. Have a nice day 🌞
---
Fixes: rust-lang/rust-clippy#7869
changelog: ICE Fix: [`enum_variant_names`] #7869
Don't mark for loop iter expression as desugared
We typically don't mark spans of lowered things as desugared. This helps Clippy rightly discern when code is (not) from expansion. This was discovered by ``@flip1995`` at https://github.com/rust-lang/rust-clippy/pull/7789#issuecomment-939289501.
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
The bug was dues to the constant bytes being compared instead of their
values. This meant that negative values were being treated as larger
than some positive values.
Fixes#7829
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
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
