Rename thread_local_initializer_can_be_made_const to missing_const_for_thread_local
Close#12934
As discussed at #12934 name `thread_local_initializer_can_be_made_const` sounds against convention for other lints which describe the issue/wrong code but not suggestion and it is quite long. The new name take example from existing lint `missing_const_for_fn`
changelog: `thread_local_initializer_can_be_made_const` : Rename to [`missing_const_for_thread_local`]
feat: add cfg_not_test lint
<!--
- \[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`
- \[ ] Added lint documentation
- \[x] Run `cargo dev fmt`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
-->
Fixes#11234
changelog: new lint: [`cfg_not_test`]
I don't know whether to lint only the `attr` or also the item associated to it. I guess this would mean putting the check in another place than `check_attribute` but I can't find a way to get the associated item to the attribute.
Also, I'm not sure how to document this lint, I feel like my explications are bad.
Remove `is_in_test_module_or_function`
Uses are replaced with `is_in_test` for consistency with other lints and to simplify the implementation of the lints. This means the module name is no longer checked, but that was a horrible hack from a time when late passes couldn't see `#[cfg(..)]` attributes.
changelog: none
[`unnecessary_to_owned`]: catch `to_owned` on byte slice to create temporary `&str`
Closes#11648
Detects the pattern `&String::from_utf8(bytes.to_vec()).unwrap()` and suggests `core::str::from_utf8(bytes).unwrap()`, which avoids the unnecessary intermediate allocation.
I decided to put this in the existing `unnecessary_to_owned` lint (rather than creating a new lint) for a few reasons:
- we get to use some of its logic (for example, recognizing any of the functions in the `to_owned` family, e.g. `to_vec`)
- the actual inefficient operation that can be avoided here is the call to `.to_vec()`, so this is in a way similar to the other cases caught by `unnecessary_to_owned`, just through a bunch of type conversions
- we can make this more "generic" later and catch other cases, so imo it's best not to tie this lint specifically to the `String` type
changelog: [`unnecessary_to_owned`]: catch `&String::from_utf8(bytes.to_vec()).unwrap()` and suggest `core::str::from_utf8(bytes).unwrap()`
[`missing_const_for_fn`]: fix suggestions for fn with abi that requires `const_extern_fn` feature
closes: #13008
---
changelog: [`missing_const_for_fn`]: fix suggestions for fn with abi that requires `const_extern_fn` feature.
This patch adds a new lint that checks for potentially harder to read
byte char slices: `&[b'a', b'b']` and suggests to replace them with the
easier to read `b"ab"` form.
Signed-Off-By: Marcel Müller <m.mueller@ifm.com>
Co-authored-by: Matthias Beyer <matthias.beyer@ifm.com>
Use iterator to skip validation
Signed-off-by: Marcel Müller <m.mueller@ifm.com>
Suggested-by: Alex Macleod <alex@macleod.io>
Convert quote escapes to proper form
Signed-off-by: Marcel Müller <m.mueller@ifm.com>
Add more convertable test cases
Signed-off-by: Marcel Müller <m.mueller@ifm.com>
Add new lint `hashset_insert_after_contains`
This PR closes https://github.com/rust-lang/rust-clippy/issues/11103.
This is my first PR creating a new lint (and the second attempt of creating this PR, the first one I was not able to continue because of personal reasons). Thanks for the patience :)
The idea of the lint is to find insert in hashmanps inside if staments that are checking if the hashmap contains the same value that is being inserted. This is not necessary since you could simply call the insert and check for the bool returned if you still need the if statement.
changelog: new lint: [hashset_insert_after_contains]
Fix some false-positive cases of `explicit_auto_deref`
changelog: [`explicit_auto_deref`] Fix some false-positive cases
Fix part of #9841
Fix #12969
r? xFrednet
Honor `avoid-breaking-exported-api` in `needless_pass_by_ref_mut`
Until now, the lint only emitted a warning, when breaking public API. Now it doesn't lint at all when the config value is not set to `false`, bringing it in line with the other lints using this config value.
Also ensures that this config value is documented in the lint.
changelog: none
(I don't think a changelog is necessary, since this lint is in `nursery`)
---
Fixes https://github.com/rust-lang/rust-clippy/issues/11374
cc `@GuillaumeGomez`
Marking as draft: Does this lint even break public API? If I change a function signature from `fn foo(x: &mut T)` to `fn foo(x: &T)`, I can still call it with `foo(&mut x)`. The only "breaking" thing is that the `clippy::unnecessary_mut_passed` lint will complain that `&mut` at the callsite is not necessary, possibly trickling down to the crate user having to remote a `mut` from a variable. [Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=058165a7663902e84af1d23e35c10d66).
Are there examples where this actually breaks public API, that I'm missing?
Until now, the lint only emitted a warning, when breaking public API. Now it
doesn't lint at all when the config value is not set to `false`, bringing it in
line with the other lints using this config value.
Also ensures that this config value is documented in the lint.
Don't lint `assertions_on_constants` on any const assertions
close#12816close#12847
cc #12817
----
changelog: Fix false positives in consts for `assertions_on_constants` and `unnecessary_operation`.
This change addresses cases where doc comments are separated
by blank lines, comments, or non-doc-comment attributes,
like this:
```rust
/// - first line
// not part of doc comment
/// second line
```
Before this commit, Clippy gave a pedantically-correct
warning about how you needed to indent the second line.
This is unlikely to be what the user intends, and has
been described as a "false positive" (since Clippy is
warning you about a highly unintuitive behavior that
Rustdoc actually has, we definitely want it to output
*something*, but the suggestion to indent was poor).
https://github.com/rust-lang/rust-clippy/issues/12917
Fix `...` in multline code-skips in suggestions
When we have long code skips, we write `...` in the line number gutter.
For suggestions, we were "centering" the `...` with the line, but that was inconsistent with what we do in every other case *and* off-center.
Fix incorrect suggestion for `manual_unwrap_or_default`
Fixes#12928.
If this not a "simple" pattern, better not emit the lint.
changelog: Fix incorrect suggestion for `manual_unwrap_or_default`
When we have long code skips, we write `...` in the line number gutter.
For suggestions, we were "centering" the `...` with the line, but that was consistent with what we do in every other case.
Lint `manual_unwrap_or` for it let cases
This PR modifies `manual_unwrap_or` to lint for `if let` cases as well. This effort is part of the fixes desired by https://github.com/rust-lang/rust-clippy/issues/12618
changelog:[`manual_unwrap_or`]: Lint for `if let` cases.
Rework `octal_escapes`
Main changes are not doing UTF-8 decoding, noting each occurrence as an individual lint emission, and narrowing the span to point to the escape itself.
changelog: none
Fix ICE in `upper_case_acronyms`
fixes#12284
The logic has been rewritten to avoid allocations. The old version allocated multiple vecs and strings for each identifier. The new logic allocates a single string only when the lint triggers.
This also no longer lints on strings which don't start with an uppercase letter (e.g. `something_FOO`).
changelog: none
Avoid emitting `assigning_clones` when cloned data borrows from the place to clone into
Fixes#12444Fixes#12460Fixes#12749Fixes#12757Fixes#12929
I think the documentation for the function should describe what- and how this is fixing the issues well.
It avoids emitting a warning when the data being cloned borrows from the place to clone into, which is information that we can get from `PossibleBorrowerMap`. Unfortunately, it is a tiny bit tedious to match on the MIR like that and I'm not sure if this is possibly relying a bit too much on the exact MIR lowering for assignments.
Things left to do:
- [x] Handle place projections (or verify that they work as expected)
- [x] Handle non-`Drop` types
changelog: [`assigning_clones`]: avoid warning when the suggestion would lead to a borrow-check error
When both `std::` and `core::` items are available, only suggest the
`std::` ones. We ensure that in `no_std` crates we suggest `core::`
items.
Ensure that the list of items suggested to be imported are always in the
order of local crate items, `std`/`core` items and finally foreign crate
items.
Tweak wording of import suggestion: if there are multiple items but they
are all of the same kind, we use the kind name and not the generic "items".
Fix#83564.
Let `qualify_min_const_fn` deal with drop terminators
Fixes#12677
The `method_accepts_droppable` check that was there seemed overly conservative.
> Returns true if any of the method parameters is a type that implements `Drop`.
> The method can't be made const then, because `drop` can't be const-evaluated.
Accepting parameters that implement `Drop` should still be fine as long as the parameter isn't actually dropped, as is the case in the linked issue where the droppable is moved into the return place. This more accurate analysis ("is there a `drop` terminator") is already done by `qualify_min_const_fn` [here](f5e250180c/clippy_utils/src/qualify_min_const_fn.rs (L298)), so I don't think this additional check is really necessary?
Fixing the other, second case in the linked issue was only slightly more involved, since `Vec::new()` is a function call that has the ability to panic, so there must be a `drop()` terminator for cleanup, however we should be able to freely ignore that. [Const checking ignores cleanup blocks](https://github.com/rust-lang/rust/blob/master/compiler/rustc_const_eval/src/transform/check_consts/check.rs#L382-L388), so we should, too?
r? `@Jarcho`
----
changelog: [`missing_const_for_fn`]: continue linting on fns with parameters implementing `Drop` if they're not actually dropped
Don't lint indexing_slicing lints on proc macros
This pr fixes https://github.com/rust-lang/rust-clippy/issues/12824
Even though the issue mentions the indexing case only, it was easy to apply the fix to the slicing case as well.
changelog: [`out_of_bounds_indexing`, `indexing_slicing`]: Don't lint on procedural macros.
Handle single chars with `to_string()` for `single_char_add_str`
Add support for single chars / literals with `to_string()` call for `push_str()` and `insert_str()`.
changelog: [`single_char_add_str`]: handle single chars with `to_string()` call
Closes#12775
- remove now dead code in ASSERTIONS_ON_CONSTANTS
cc #11966
- Partially revert "ignore `assertions-on-constants` in const contexts"
This reverts commit c7074de420a2192fb40d3f2194a20dd0d1b65cc6.
Don't lint blocks in closures for blocks_in_conditions
Seemed like an outlier for the lint which generally caught only the syntactically confusing cases, it lints blocks in closures but excludes closures passed to iterator methods, this changes it to ignore closures in general
changelog: none
Remove `lazy_static` mention
I planned to replace any mention with `LazyLock` but I think `thread_local` is more appropriate here - `const`s that aren't `Sync` wouldn't be able to go in a `lazy_static`/`static LazyLock` either
Also removed a test file that was mostly commented out so wasn't testing anything
changelog: none
Make `for_each_expr` visit closures by default, rename the old version `for_each_expr_without_closures`
A lot of the time `for_each_expr` is picked when closures should be visited so I think it makes sense for this to be the default with the alternative available for when you don't need to visit them.
The first commit renames `for_each_expr` to `for_each_expr_without_closures` and `for_each_expr_with_closures` to `for_each_expr`
The second commit switches a few uses that I caught over to include closures to fix a few bugs
changelog: none
Handle const effects inherited from parent correctly in `type_certainty`
This fixes a (debug) ICE in `type_certainty` that happened in the [k256 crate]. (I'm sure you can also specifically construct an edge test case that will run into type_certainty false positives visible outside of debug builds from this bug)
<details>
<summary>Minimal ICE repro</summary>
```rs
use std::ops::Add;
Add::add(1_i32, 1).add(i32::MIN);
```
</details>
The subtraction here overflowed:
436675b477/clippy_utils/src/ty/type_certainty/mod.rs (L209)
... when we have something like `Add::add` where `add` fn has 0 generic params but the `host_effect_index` is `Some(2)` (inherited from the parent generics, the const trait `Add`), and we end up executing `0 - 1`.
(Even if the own generics weren't empty and we didn't overflow, this would still be wrong because it would assume that a trait method with 1 generic parameter didn't have any generics).
So, *only* exclude the "host" generic parameter if it's actually bound by the own generics
changelog: none
[k256 crate]: https://github.com/RustCrypto/elliptic-curves/tree/master/k256
Revert: create const block bodies in typeck via query feeding
as per the discussion in https://github.com/rust-lang/rust/pull/125806#discussion_r1622563948
It was a mistake to try to shoehorn const blocks and some specific anon consts into the same box and feed them during typeck. It turned out not simplifying anything (my hope was that we could feed `type_of` to start avoiding the huge HIR matcher, but that didn't work out), but instead making a few things more fragile.
reverts the const-block-specific parts of https://github.com/rust-lang/rust/pull/124650
`@bors` rollup=never had a small perf impact previously
fixes https://github.com/rust-lang/rust/issues/125846
r? `@compiler-errors`
