suggest alternatives to iterate an array of ranges
works towards #7125
changelog: [`single_element_loop`]: suggest better syntax when iterating over an array of a single range
`@thinkerdreamer` and myself worked on this issue during a workshop by `@llogiq` at the RustLab 2023 conference. It is our first contribution to clippy.
When iterating over an array of only one element, _which is a range_, our change suggests to replace the array with the contained range itself. Additionally, a hint is printed stating that the user probably intended to iterate over the range and not the array. If the single element in the array is not a range, the previous suggestion in the form of `let {pat_snip} = {prefix}{arg_snip};{block_str}`is used.
This change lints the array with the single range directly, so any prefixes or suffixes are covered as well.
Nit re `matches!` formatting
I think formatting `matches!` with `if` guards is [still unsupported](https://github.com/rust-lang/rustfmt/issues/5547), which is probably why this was missed.
changelog: none
[`deprecated_semver`]: Allow `#[deprecated(since = "TBD")]`
"TBD" is allowed by rustdoc, saying that it will be deprecated in a future version. rustc will also not actually warn on it.
I found this while checking the rust-lang/rust with clippy.
changelog: [`deprecated_semver`]: allow using `since = "TBD"`
[`missing_asserts_for_indexing`]: work with bodies instead of blocks separately
Fixes#11856
Before this change, this lint would check blocks independently of each other, which means that it misses `assert!()`s from parent blocks.
```rs
// check_block
assert!(x.len() > 1);
{
// check_block
// no assert here
let _ = x[0] + x[1];
}
```
This PR changes it to work with bodies rather than individual blocks. That means that a function will be checked in one go and we can remember if an `assert!` occurred anywhere.
Eventually it would be nice to have a more control flow-aware analysis, possibly by rewriting it as a MIR lint, but that's more complicated and I wanted this fixed first.
changelog: [`missing_asserts_for_indexing`]: accept `assert!`s from parent blocks
Improve error messages format
Following review in https://github.com/rust-lang/rust-clippy/pull/11845, since there is already suggestions, no need to add the second part of the sentence every time.
r? `@flip1995`
changelog: Improve some error messages
Extend `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`
Fixes https://github.com/rust-lang/rust-clippy/issues/10365.
As indicated in the title, it extends the `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`.
changelog: extension of the `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`
r? `@blyxyas`
Add documentation update hint using `cargo collect-metadata`
This adds a little reminder to update the documentation in the book using `cargo collect-metadata` after changing the lint configuration since this can easily be missed (been there done that 🙈).
> Yeah a note would be good, would be good for us to see if we can make it automatic also
_Originally posted by `@Alexendoo` in https://github.com/rust-lang/rust-clippy/issues/11757#issuecomment-1823474156_
Regarding the automation Im not sure whats the best option here. I thought about a kind of a "semi-automated" way, e.g. a `cargo dev` command which runs the "is the documentation updated" check (and maybe other useful checks) locally and reminds the user of updating the documentation so this gets caught before pushing.
changelog: none
Fix iter_kv_map false positive into_keys and into_values suggestion
fixes: #11752
changelog: [`iter_kv_map`]: fix false positive: Don't suggest `into_keys()` and `into_values()` if the MSRV is to low
Add tests for issues #10285, #10286, #10289, #10287Fixes#10285.
Fixes#10286.
Fixes#10289.
Fixes#10287.
This PR simply adds tests for the listed issues as they're already implemented so we can close them.
r? `@blyxyas`
changelog:none
[`needless_return_with_question_mark`]: don't lint if never type is used for coercion
Fixes#11616
When we have something like
```rs
let _x: String = {
return Err(())?;
};
```
we shouldn't suggest removing the `return` because the `!`-ness of `return` is used to coerce the enclosing block to some other type. That will lead to a typeck error without a diverging expression like `return`.
changelog: [`needless_return_with_question_mark`]: don't lint if `return`s never typed-ness is used for coercion
Add new `check_private_items` config
Fixes https://github.com/rust-lang/rust-clippy/issues/11742.
It adds a new configuration which allows for `MISSING_SAFETY_DOC`, `UNNECESSARY_SAFETY_DOC`, `MISSING_PANICS_DOC` and `MISSING_ERRORS_DOC` lints to run on private items.
changelog: [`missing_safety_doc`], [`unnecessary_safety_doc`], [`missing_panics_doc`], [`missing_errors_doc`]: Added the [`check-private-items`] configuration to enable lints on private items.
[#11842](https://github.com/rust-lang/rust-clippy/pull/11842)
r? `@blyxyas`
Split `doc.rs` up into a subdirectory
So, first, sorry for the bad diff. 😅
In #11798, `@flip1995` suggested splitting `doc.rs` up, much like how we have the `methods/`, `matches/`, `types/` subdirectories.
I agree with this, the file is getting bigger as we add more and more doc lints that it makes sense to do this refactoring.
This is purely an internal change that moves things around a bit.
(**EDIT:** depending on the outcome of https://github.com/rust-lang/rust-clippy/pull/11801#issuecomment-1816715615 , this may change the lint group name from `doc_markdoc` to `doc`).
I tried to not change any of the actual logic of the lints and as such some things weren't as easy to move to a separate file. So we still have some `span_lint*` calls in the `doc/mod.rs` file, which I think is fine. This is also the case in `methods/mod.rs`.
Also worth mentioning that the lints missing_errors_doc, missing_panics_doc, missing_safety_doc and unnecessary_safety_doc have a lot of the same logic so it didn't make much sense for each of these to be in their own file. Instead I just put them all in `missing_headers.rs`
I also added a bit of documentation to the involved `check_{attrs,doc}` methods.
changelog: none
Improve maybe misused cfg
Follow-up of the improvements that were suggested to me in https://github.com/rust-lang/rust-clippy/pull/11821:
* I unified the output to use the same terms.
* I updated the code to prevent creating a new symbol.
r? `@blyxyas`
changelog: [`maybe_misued_cfg`]: Output and code improvements
Verify Borrow<T> semantics for types that implement Hash, Borrow<str> and Borrow<[u8]>.
Fixes#11710
The essence of the issue is that types that implement Borrow<T> provide a facet or a representation of the underlying type. Under these semantics `hash(a) == hash(a.borrow())`.
This is a problem when a type implements `Borrow<str>`, `Borrow<[u8]>` and Hash, it is expected that the hash of all three types is identical. The problem is that the hash of [u8] is not the same as that of a String, even when the byte reference ([u8]) is derived from `.as_bytes()`
- [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`
- [x] Added lint documentation
- [x] Run `cargo dev fmt`
---
- [x] Explanation of the issue in the code
- [x] Tests reproducing the issue
- [x] Lint rule and emission
---
changelog: New lint: [`impl_hash_borrow_with_str_and_bytes`]
[#11781](https://github.com/rust-lang/rust-clippy/pull/11781)
Implements a lint to prevent implementation of Hash, Borrow<str> and
Borrow<[u8]> as it breaks Borrow<T> "semantics". According to the book,
types that implement Borrow<A> and Borrow<B> must ensure equality of
borrow results under Eq,Ord and Hash.
> In particular Eq, Ord and Hash must be equivalent for borrowed and
owned values: x.borrow() == y.borrow() should give the same result as x == y.
In the same way, hash(x) == hash(x as Borrow<[u8]>) != hash(x as Borrow<str>).
changelog: newlint [`impl_hash_with_borrow_str_and_bytes`]
Fix typos in recent lint documentation.
Fixes typos and markup errors, and also makes the examples more realistic by hiding the `;`s so as not to visibly be discarding the computed value. Affected lints:
* `redundant_as_str`
* `unnecessary_map_on_constructor`
changelog: none
teach `eager_or_lazy` about panicky arithmetic operations
Fixes#9422Fixes#9814Fixes#11793
It's a bit sad that we have to do this because arithmetic operations seemed to me like the prime example where a closure would not be necessary, but this has "side effects" (changes behavior when going from lazy to eager) as some of these panic on overflow/underflow if compiled with `-Coverflow-checks` (which is the default in debug mode).
Given the number of backlinks in the mentioned issues, this seems to be a FP that is worth fixing, probably.
changelog: [`unnecessary_lazy_evaluations`]: don't lint if closure has panicky arithmetic operations
Extend `maybe_misused_cfg` lint over `cfg(test)`
Fixes#11240.
One thought I had is that we could use the levenshtein distance (of 1) to ensure this is indeed `test` that was targeted. But maybe it's overkill, not sure.
changelog: [`maybe_misused_cfg`]: Extend lint over `cfg(test)`
r? `@blyxyas`
