bors [Mon, 9 May 2022 19:55:37 +0000 (19:55 +0000)]
Auto merge of #8809 - Alexendoo:metadata-collection-expect, r=xFrednet
Ignore unfulfilled_lint_expectations in metadata collection
r? `@xFrednet`
When metadata collection is enabled the regular early/late pass registrations are skipped, which the `#[expect()]`s were no fan of: https://github.com/rust-lang/rust-clippy/runs/6356675328?check_suite_focus=true
bors [Mon, 9 May 2022 17:14:43 +0000 (17:14 +0000)]
Auto merge of #8797 - xFrednet:0000-expect-a-playground, r=flip1995
Replace `#[allow]` with `#[expect]` in Clippy
Hey `@rust-lang/clippy,` `@Alexendoo,` `@dswij,` I'm currently working on the expect attribute as defined in [Rust RFC 2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html). With that, an `#[allow]` attribute can be replaced with a `#[expect]` attribute that suppresses the lint, but also emits a warning, if the lint isn't emitted in the expected scope.
With this PR I would like to test the attribute on a project scale and Clippy obviously came to mind. This PR replaces (almost) all `#[allow]` attributes in `clippy_utils` and `clippy_lints` with the `#[expect]` attribute. I was also able to remove some allows since, the related FPs have been fixed :tada:.
My question is now, are there any concerns regarding this? It's still okay to add normal `#[allow]` attributes, I see the need to nit-pick about that in new PRs, unless it's actually a FP. Also, I would not recommend using `#[expect]` in tests, as changes to a lint could the trigger the expect attribute in other files.
Additionally, I've noticed that Clippy has a bunch of `#[allow(clippy::too_many_lines)]` attributes. Should we maybe allow the lint all together or increase the threshold setting? To me, it seems like we mostly just ignore it in our code. :sweat_smile: :upside_down_face:
---
changelog: none
r? `@flip1995` (I've requested you for now, since you're also helping with reviewing the expect implementation. You are welcome to delegate this PR, even if it should be a simple review :upside_down_face: )
bors [Mon, 9 May 2022 13:56:53 +0000 (13:56 +0000)]
Auto merge of #8805 - Alexendoo:let-chain-docs, r=flip1995
Recommend let chains over if_chain in docs
Switches over bit_mask.rs to let chains in order to create a nice example
While the rustfmt thing isn't resolved yet, my rust-analyzer isn't a fan of large `if_chains!`s, it stops giving me hover info and such after some number of if statements
bors [Sun, 8 May 2022 15:00:30 +0000 (15:00 +0000)]
Auto merge of #8802 - smoelius:allow-expect-unwrap-in-tests, r=llogiq
Optionally allow `expect` and `unwrap` in tests
This addresses #1015, except it makes the new behavior optional.
The reason for the msrv-related changes is as follows.
Rather than expand `check_methods` list of arguments, it seemed easier to make `check_methods` a method of `Methods`, so that `check_methods` could access `Methods`' fields.
`check_methods` had an `msrv` parameter, which I consequently made a field of `Methods`. But, to avoid adding a lifetime parameter to `Methods`, I made the field type `Option<RustcVersion>` instead of the parameter's existing type, `Option<&RustcVersion>`. This seemed sensible since `RustcVersion` implements `Copy`. But this broke a lot of code that expected an `Option<&RustcVersion>` or `&Option<RustcVersion>`. I changed all of those occurrences to `Option<RustcVersion>`. IMHO, the code is better as a result of these changes, though.
The msrv-related changes are in their own commit to (hopefully) ease review.
Closes #1015
changelog: optionally allow `expect` and `unwrap` in tests
bors [Sat, 7 May 2022 12:31:47 +0000 (12:31 +0000)]
Auto merge of #8794 - smoelius:fix-8759, r=llogiq
Address `unnecessary_to_owned` false positive
My proposed fix for #8759 is to revise the conditions that delineate `redundant_clone` and `unnecessary_to_owned`:
```rust
// Only flag cases satisfying at least one of the following three conditions:
// * the referent and receiver types are distinct
// * the referent/receiver type is a copyable array
// * the method is `Cow::into_owned`
// This restriction is to ensure there is no overlap between `redundant_clone` and this
// lint. It also avoids the following false positive:
// https://github.com/rust-lang/rust-clippy/issues/8759
// Arrays are a bit of a corner case. Non-copyable arrays are handled by
// `redundant_clone`, but copyable arrays are not.
```
This change causes a few cases that were previously flagged by `unnecessary_to_owned` to no longer be flagged. But one could argue those cases would be better handled by `redundant_clone`.
bors [Fri, 6 May 2022 09:44:19 +0000 (09:44 +0000)]
Auto merge of #8792 - jyn514:remove-ast-json, r=flip1995
Suggest -Zunpretty=ast-tree instead of -Zast-json
-Zast-json is being removed shortly: https://github.com/rust-lang/rust/pull/85993.
ast-tree does essentially the same thing, and still works today even before that PR lands.
Joshua Nelson [Fri, 6 May 2022 01:11:41 +0000 (20:11 -0500)]
Suggest -Zunpretty=ast-tree instead of -Zast-json
-Zast-json is being removed shortly: https://github.com/rust-lang/rust/pull/85993.
ast-tree does essentially the same thing, and still works today even before that PR lands.
bors [Fri, 6 May 2022 00:26:18 +0000 (00:26 +0000)]
Auto merge of #8778 - sunfishcode:main, r=giraffate
Fix `cast_lossless` to avoid warning on `usize` to `f64` conversion.
Previously, the `cast_lossless` lint would issue a warning on code that
converted a `usize` value to `f64`, on 32-bit targets.
`usize` to `f64` is a lossless cast on 32-bit targets, however there is
no corresponding `f64::from` that takes a `usize`, so `cast_lossless`'s
suggested replacement does not compile.
This PR disables the lint in the case of casting from `usize` or `isize`.
Fixes #3689.
changelog: [`cast_lossless`] no longer gives wrong suggestion on usize,isize->f64
bors [Thu, 5 May 2022 13:12:09 +0000 (13:12 +0000)]
Auto merge of #8788 - flip1995:rustup, r=xFrednet,flip1995
Rustup
r? `@ghost`
changelog: move trait_duplication_in_bounds and type_repetition_in_bounds to nursery temporarily. This could already be reverted before the release. Check the Clippy in the Rust repo beta branch when writing this changelog.
flip1995 [Thu, 5 May 2022 12:32:31 +0000 (13:32 +0100)]
HACK: Move buggy lints to nursery
Those lints are trait_duplication_in_bounds and
type_repetition_in_bounds. I don't think those can be fixed on the
Clippy side alone, but need changes in the compiler. So let's move them
to nursery to get the sync through and then fix them on the rustc side.
Also adds a regression test that has to be fixed before they can be
moved back to pedantic.
bors [Wed, 4 May 2022 21:16:28 +0000 (21:16 +0000)]
Auto merge of #96546 - nnethercote:overhaul-MacArgs, r=petrochenkov
Overhaul `MacArgs`
Motivation:
- Clarify some code that I found hard to understand.
- Eliminate one use of three places where `TokenKind::Interpolated` values are created.
The value in `MacArgs::Eq` is currently represented as a `Token`.
Because of `TokenKind::Interpolated`, `Token` can be either a token or
an arbitrary AST fragment. In practice, a `MacArgs::Eq` starts out as a
literal or macro call AST fragment, and then is later lowered to a
literal token. But this is very non-obvious. `Token` is a much more
general type than what is needed.
This commit restricts things, by introducing a new type `MacArgsEqKind`
that is either an AST expression (pre-lowering) or an AST literal
(post-lowering). The downside is that the code is a bit more verbose in
a few places. The benefit is that makes it much clearer what the
possibilities are (though also shorter in some other places). Also, it
removes one use of `TokenKind::Interpolated`, taking us a step closer to
removing that variant, which will let us make `Token` impl `Copy` and
remove many "handle Interpolated" code paths in the parser.
Things to note:
- Error messages have improved. Messages like this:
```
unexpected token: `"bug" + "found"`
```
now say "unexpected expression", which makes more sense. Although
arbitrary expressions can exist within tokens thanks to
`TokenKind::Interpolated`, that's not obvious to anyone who doesn't
know compiler internals.
- In `parse_mac_args_common`, we no longer need to collect tokens for
the value expression.
bors [Tue, 3 May 2022 20:42:35 +0000 (20:42 +0000)]
Auto merge of #8779 - binggh:easier-needless-late-init, r=llogiq
Easier readability for `needless_late_init` message
Closes #8530
Updated the lint to use a `MultiSpan`, showing where the `let` statement was first used and where the initialisation statement was done, as in the format described, for easier readability.
Was wondering why, when pushing the span label for the initialisation statement, that sometimes the prior statement above the initialisation statement gets pulled into the output as well - any insight is appreciated!
---
changelog: [`needless_late_init`]: Now shows the `let` statement where it was first initialized
bors [Tue, 3 May 2022 07:05:51 +0000 (07:05 +0000)]
Auto merge of #8730 - tamaroning:fix8724, r=Alexendoo
[FP] identity_op in front of if
fix #8724
changelog: FP: [`identity_op`]: is now allowed in front of if statements, blocks and other expressions where the suggestion would be invalid.
Resolved simular problems with blocks, mathces, and loops.
identity_op always does NOT suggest reducing `0 + if b { 1 } else { 2 } + 3` into `if b { 1 } else { 2 } + 3` even in the case that the expression is in `f(expr)` or `let x = expr;` for now.
Dan Gohman [Mon, 2 May 2022 20:46:29 +0000 (13:46 -0700)]
Fix `cast_lossless` to avoid warning on `usize` to `f64` conversion.
Previously, the `cast_lossless` lint would issue a warning on code that
converted a `usize` value to `f64`, on 32-bit targets.
`usize` to `f64` is a lossless cast on 32-bit targets, however there is
no corresponding `f64::from` that takes a `usize`, so `cast_lossless`'s
suggested replacement does not compile.
This PR disables the lint in the case of casting from `usize` or `isize`.
Fixes #3689.
changelog: [`cast_lossless`] no longer gives wrong suggestion on usize->f64
Only crate root def-ids don't have a parent, and in majority of cases the argument of `DefIdTree::parent` cannot be a crate root.
So we now panic by default in `parent` and introduce a new non-panicing function `opt_parent` for cases where the argument can be a crate root.
Same applies to `local_parent`/`opt_local_parent`.
Auto merge of #8764 - dtolnay-contrib:checkoutv3, r=llogiq
Update GitHub Actions actions/checkout@v2 to v3
The v2 implementation uses Node 12, which is end-of-life on April 30, 2022. See https://nodejs.org/en/about/releases/. Update to v3, which is based on Node 16 whose support lasts until April 30, 2024.
David Tolnay [Sat, 30 Apr 2022 02:28:03 +0000 (19:28 -0700)]
Update GitHub Actions actions/checkout@v2 to v3
The v2 implementation uses Node 12, which is end-of-life on April 30, 2022.
See https://nodejs.org/en/about/releases/. Update to v3, which is based on
Node 16 whose support lasts until April 30, 2024.
David Wood [Tue, 26 Apr 2022 05:17:33 +0000 (06:17 +0100)]
errors: `span_suggestion` takes `impl ToString`
Change `span_suggestion` (and variants) to take `impl ToString` rather
than `String` for the suggested code, as this simplifies the
requirements on the diagnostic derive.
Auto merge of #8743 - Alexendoo:useless-attribute-redundant-pub-crate, r=llogiq
ignore `redundant_pub_crate` in `useless_attribute`
changelog: [`useless_attribute`] no longer lints [`redundant_pub_crate`]
As mentioned in https://github.com/rust-lang/rust-clippy/issues/8732#issuecomment-1106489634
> And it turns out I can't even explicitly allow it at the usage site, because then `clippy::useless_attribute` fires (which would also be a FP?), which is deny-by-default.
>
> Though it does work if I then allow `clippy::useless_attribute`. 😂
>
> ```rust
> #[allow(clippy::useless_attribute)]
> #[allow(clippy::redundant_pub_crate)]
> pub(crate) use bit;
> ```
>
> The originally-reported warning now no longer occurs.
Avoids changing the drop order in an observable way, where the type of `x` has a drop with side effects and something between `x` and the first use also does, e.g.
The implementation of `type_needs_ordered_drop_inner` was changed a bit, it now uses `Ty::has_significant_drop` and reordered the ifs to check diagnostic name before checking the implicit drop impl
changelog: [`needless_late_init`]: No longer lints `if let` statements, `let mut` bindings and no longer significantly changes drop order
This PR fixes 2 things:
- The known problem that integer types are always suggested as signed, by suggesting an unsigned suffix for literals that wouldnt fit in the signed type, and ignores any literals too big for the corresponding unsigned type too.
- The lint would only look at the integer part of any floating point literals without an exponent, this causing #6129. This just ignores those literals.
Examples:
```rust
let _ = 2_32; // still 2_i32
let _ = 234_8; // would now suggest 234_u8
// these are now ignored
let _ = 500_8;
let _ = 123_32.123;
```
changelog: suggest correct integer types in [`mistyped_literal_suffix`], ignore float literals without an exponent
fixes #6129
mistyped_literal_suffixes: improve suggestions for integer types
Instead of just always suggesting signed suffixes regardless of size
of the value, it now suggests an unsigned suffix when the value wouldn't
fit into the corresponding signed type, and ignores the literal entirely
if it is too big for the unsigned type as well.
It's only a warning so this wasn't causing the test to fail, but if you had another error somewhere or used `--nocapture` the extra warnings would be shown
Auto merge of #8716 - binggh:stable-sort-message-update, r=giraffate
Less authoritative stable_sort_primitive message
fixes #8241
Hey all - first contribution here so I'm deciding to start with something small.
Updated the linked message to be less authoritative as well as moved the lint grouping from `perf` to `pedantic` as suggested by `@camsteffen` under the issue.
changelog: [`stable_sort_primitive`]: emit less authoritative message and move to `pedantic`