Clippy subtree update #128837
Merged
Clippy subtree update #128837
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856
TODO: Should we move `ty::peel_mid_ty_refs_is_mutable` to super module too?
Fixes rust-lang#12751 changelog: Fix [`redundant_slicing`] when the slice is behind a mutable reference
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).
Inlining format args prevents accidental `&` misuse.
Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>
…-errors Implement lint against ambiguous negative literals This PR implements a lint against ambiguous negative literals with a literal and method calls right after it. ## `ambiguous_negative_literals` (deny-by-default) The `ambiguous_negative_literals` lint checks for cases that are confusing between a negative literal and a negation that's not part of the literal. ### Example ```rust,compile_fail -1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1 ``` ### Explanation Method calls take precedence over unary precedence. Setting the precedence explicitly makes the code clearer and avoid potential bugs. <details> <summary>Old proposed lint</summary> ## `ambiguous_unary_precedence` (deny-by-default) The `ambiguous_unary_precedence` lint checks for use the negative unary operator with a literal and method calls. ### Example ```rust -1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1 ``` ### Explanation Unary operations take precedence on binary operations and method calls take precedence over unary precedence. Setting the precedence explicitly makes the code clearer and avoid potential bugs. </details> ----- Note: This is a strip down version of rust-lang#117161, without the binary op precedence. Fixes rust-lang#117155 `@rustbot` labels +I-lang-nominated cc `@scottmcm` r? compiler
Also get the receiver T in `T.method(|| f())`.
…=xFrednet Remove unnecessary `res` field in `for_each_expr` visitors Small refactor in the `for_each_expr*` visitors. This should not change anything functionally. Instead of storing the final value `Option<B>` in the visitor and setting it to `Some` when we get a `ControlFlow::Break(B)` from the closure, we can just directly return it from the visitor itself now that visitors support that. cc rust-lang#12829 and rust-lang/rust-clippy#12830 (comment) changelog: none
… r=xFrednet needless_borrows_for_generic_args: Fix for &mut This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings. Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary. However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it. To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references. Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work. Fixes rust-lang#12856 changelog: none
…endoo
Fix handling of `Deref` in `assigning_clones`
The `assigning_clones` lint had a special case for producing a bit nicer code for mutable references:
```rust
fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
*mut_thing = Clone::clone(ref_thing);
}
//v
fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
Clone::clone_from(mut_thing, ref_thing);
}
```
However, this could [break](rust-lang/rust-clippy#12437) when combined with `Deref`.
This PR removes the special case, so that the generated code should work more generally. Later we can improve the detection of `Deref` and put the special case back in a way that does not break code.
Fixes: rust-lang/rust-clippy#12437
r? `@blyxyas`
changelog: [`assigning_clones`]: change applicability to `Unspecified` and fix a problem with `Deref`.
…tthiaskrgr Clippy subtree update r? `@Manishearth` Updates Cargo.lock due to the Clippy version update and the ui_test bump to v0.24
…hat, r=y21 Make restriction lint's use `span_lint_and_then` (i -> p) This migrates a few restriction lints to use `span_lint_and_then`. This change is motivated by rust-lang/rust-clippy#7797. I've also cleaned up some lint message. Mostly minor stuff. For example: suggestions with a longer message than `"try"` now use `SuggestionStyle::ShowAlways` --- cc: rust-lang/rust-clippy#7797 brother PR of: rust-lang/rust-clippy#13136 changelog: none
Signed-off-by: riyueguang <rustruby@outlook.com>
Add path prefixes back when compiling `clippy_dev` and `lintcheck` `cargo dev` and `cargo lintcheck` use `--manifest-path` to select the package to compile, with this Cargo changes the CWD to the package's containing directory meaning paths in diagnostics start from e.g. `src/` instead of `clippy_dev/src/` Lintcheck uses a `--remap-path-prefix` trick when linting crates to re-add the directory name in diagnostics: https://github.com/rust-lang/rust-clippy/blob/5ead90f13ae3e03f0c346aea3198f18298960d83/lintcheck/src/main.rs#L93-L94 https://github.com/rust-lang/rust-clippy/blob/5ead90f13ae3e03f0c346aea3198f18298960d83/lintcheck/src/main.rs#L102-L103 It works well as far as I can tell, when absolute paths appear they stay absolute and do not have the prefix added [`profile-rustflags`](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#profile-rustflags-option) allows us to set per package `RUSTFLAGS` in order to use the same trick on the `clippy_dev` and `lintcheck` packages themselves Now when you run into warnings/errors the filename will be correctly clickable Before ``` error[E0425]: cannot find value `moo` in this scope --> src/lib.rs:41:5 | 41 | moo; | ^^^ not found in this scope ``` After ``` error[E0425]: cannot find value `moo` in this scope --> clippy_dev/src/lib.rs:41:5 | 41 | moo; | ^^^ not found in this scope ``` r? `@flip1995` changelog: none
…dnet lintcheck: disable doc links Removes the `help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#...` line to make the reports more concise r? `@xFrednet` changelog: none
`single_match`: fix checking of explicitly matched enums fixes rust-lang#11365 This approach has false-negatives, but fixing them will require a significant amount of additional state tracking. The comment in `add_and_pats` has the explanation. changelog: `single_match`: correct checking if the match explicitly matches all of an enum's variants.
…ndoo Don't walk the HIR tree when checking for a const context changelog: none
Don't use `LateContext` in the constant evaluator This also changes the interface to require explicitly creating the context. `constant` could be added back in, but the others are probably not worth it. A couple of bugs have been fixed. The wrong `TypeckResults` was used once when evaluating a constant, and the wrong `ParamEnv` was used by some callers (there wasn't a way to use the correct one). changelog: none
Rustup r? `@ghost` changelog: ICE fix [`uninit_vec`] through rust-lang#128720
rustbot
added
the
S-waiting-on-review
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
|
@bors r+ p=1 rollup=never |
bors
added
S-waiting-on-bors
|
@bors rollup=iffy |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 8, 2024
…Manishearth Clippy subtree update r? `@Manishearth` Updates Cargo.lock due to uitest bump
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 9, 2024
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#128640 (rwlock: disable 'frob' test in Miri on macOS) - rust-lang#128791 (Don't implement `AsyncFn` for `FnDef`/`FnPtr` that wouldnt implement `Fn`) - rust-lang#128806 (Split `ColorConfig` off of `HumanReadableErrorType`) - rust-lang#128818 (std float tests: special-case Miri in feature detection) - rust-lang#128834 (rustdoc: strip unreachable modules) - rust-lang#128836 (rustdoc-json: add a test for impls on private & hidden types) - rust-lang#128837 (Clippy subtree update) - rust-lang#128851 (Add comment that bors did not see pushed before it merged) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 9, 2024
Rollup merge of rust-lang#128837 - flip1995:clippy-subtree-update, r=Manishearth Clippy subtree update r? ``@Manishearth`` Updates Cargo.lock due to uitest bump
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @Manishearth
Updates Cargo.lock due to uitest bump