This is a very large commit since a lot needs to be changed in order to
make the tests pass. The salient changes are:
- `ConstArgKind` gets a new `Path` variant, and all const params are now
represented using it. Non-param paths still use `ConstArgKind::Anon`
to prevent this change from getting too large, but they will soon use
the `Path` variant too.
- `ConstArg` gets a distinct `hir_id` field and its own variant in
`hir::Node`. This affected many parts of the compiler that expected
the parent of an `AnonConst` to be the containing context (e.g., an
array repeat expression). They have been changed to check the
"grandparent" where necessary.
- Some `ast::AnonConst`s now have their `DefId`s created in
rustc_ast_lowering rather than `DefCollector`. This is because in some
cases they will end up becoming a `ConstArgKind::Path` instead, which
has no `DefId`. We have to solve this in a hacky way where we guess
whether the `AnonConst` could end up as a path const since we can't
know for sure until after name resolution (`N` could refer to a free
const or a nullary struct). If it has no chance as being a const
param, then we create a `DefId` in `DefCollector` -- otherwise we
decide during ast_lowering. This will have to be updated once all path
consts use `ConstArgKind::Path`.
- We explicitly use `ConstArgHasType` for array lengths, rather than
implicitly relying on anon const type feeding -- this is due to the
addition of `ConstArgKind::Path`.
- Some tests have their outputs changed, but the changes are for the
most part minor (including removing duplicate or almost-duplicate
errors). One test now ICEs, but it is for an incomplete, unstable
feature and is now tracked at #127009.
Add more doc-valid-idents
* "AccessKit" is a commonly used accessibility toolkit used in Rust GUIs.
* "CoreFoundation", "CoreGraphics", "CoreText" are frameworks on Apple OSes.
* "Direct2D", "Direct3D", "DirectWrite" are frameworks on Windows
* "PostScript" is a programming language and is mentioned when talking about text and vector graphics.
* "OpenAL" is an audio framework / API.
* "OpenType" is a font format (TrueType is already mentioned).
* "WebRTC", "WebSocket", "WebTransport" are web networking technologies.
* "NetBSD" and "OpenBSD" are like the already included FreeBSD.
changelog: [`doc_markdown`]: Add AccessKit, CoreFoundation, CoreGraphics, CoreText, Direct2D, Direct3D, DirectWrite, PostScript, OpenAL, OpenType, WebRTC, WebSocket, WebTransport, NetBSD, and OpenBSD to `doc-valid-idents`.
* "AccessKit" is a commonly used accessibility toolkit used in Rust GUIs.
* "CoreFoundation", "CoreGraphics", "CoreText" are frameworks on Apple OSes.
* "Direct2D", "Direct3D", "DirectWrite" are frameworks on Windows
* "PostScript" is a programming language and is mentioned when talking about
text and vector graphics.
* "OpenAL" is an audio framework / API.
* "OpenType" is a font format (TrueType is already mentioned).
* "WebRTC", "WebSocket", "WebTransport" are web networking technologies.
* "NetBSD" and "OpenBSD" are like the already included FreeBSD.
Fix and rename `overflow_check_conditional`
fixes#2457
Other changes:
* Limit the lint to unsigned types.
* Actually check if the operands are the same rather than using only the first part of the path.
* Allow the repeated expression to be anything as long as there are no side effects.
changelog: Rename `overflow_check_conditional` to `panicking_overflow_check` and move to `correctness`
Fix guidance of [`float_cmp`] and [`float_cmp_const`] to not incorrectly recommend `f__::EPSILON` as the error margin.
Using `f32::EPSILON` or `f64::EPSILON` as the floating-point equality comparison error margin is incorrect, yet `float_cmp` has until now recommended this be done. This change fixes the given guidance (both in docs and compiler hints) to not reference these unsuitable constants.
Instead, the guidance now clarifies that the scenarios in which an absolute error margin is usable, provides a sample implementation for using a user-defined absolute error margin (as an absolute error margin can only be used-defined and may be different for different comparisons) and references the floating point guide for a reference implementation of relative error based equality comparison for cases where absolute error margins cannot be identified.
changelog: [`float_cmp`] Fix guidance to not incorrectly recommend `f__::EPSILON` as the error margin.
changelog: [`float_cmp_const`] Fix guidance to not incorrectly recommend `f__::EPSILON` as the error margin.
Fixes#6816
This lint makes Clippy warn about situations where an owned
struct is essentially recreated by moving all its fields into a
new instance of the struct. Until now this lint only triggered
for structs recreated from a base struct.
NB: The new functionality too will cause false positives for the
situation where a non-copy struct consisting of all copy members
is touched again in subsequent code.
Using `f32::EPSILON` or `f64::EPSILON` as the floating-point equality comparison error margin is incorrect, yet `float_cmp` has until now recommended this be done. This change fixes the given guidance (both in docs and compiler hints) to not reference these unsuitable constants.
Instead, the guidance now clarifies that the scenarios in which an absolute error margin is usable, provides a reference implementation of using a user-defined absolute error margin (as an absolute error margin can only be used-defined and may be different for different comparisons) and references the floating point guide for a reference implementation of relative error based equaltiy comparison for when absolute error margin cannot be used.
changelog: Fix guidance of [`float_cmp`] and [`float_cmp_const`] to not incorrectly recommend `f64::EPSILON` as the error margin.
Fixes#6816
`significant_drop_in_scrutinee`: Trigger lint also for scrutinees in `while let` and `if let`
This lint should also work for `if let` and `while let`, so this PR makes it actually work.
For `while let`, I can't think of any reason why this lint shouldn't be enabled. The only problem is that the lint suggests moving the significant drop above the `while let`, which is clearly invalid in the case of `while let`. I don't know if this is fixable, but this PR simply disables the wrong suggestions.
For `if let`, it seems that another lint called `if_let_mutex` has some overlapping functionality. But `significant_drop_in_scrutinee` is a bit stricter, as it will trigger even if the `else` branch does not try to lock the same mutex.
changelog: [`significant_drop_in_scrutinee`]: Trigger lint also for scrutinees in `while let` and `if let`.
r? `@blyxyas` (the third PR as promised in https://github.com/rust-lang/rust-clippy/pull/12740#issuecomment-2094876350, thanks for your review!)
[`missing_const_for_fn`]: fix FP when arg ty is impl trait alias ty
closes: #13009
---
changelog: [`missing_const_for_fn`]: fix FP when arg ty is impl trait alias ty
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.
