Add lint for unnecessary lifetime bounded &str return
Closes#305.
Currently implemented with a pretty strong limitation that it can only see the most basic implicit return, but this should be fixable by something with more time and brain energy than me. Cavets from #13388 apply, as I have not had a review on my clippy lints yet so am pretty new to this.
```
changelog: [`unnecessary_literal_bound`]: Add lint for unnecessary lifetime bounded &str return.
```
Use correct std/core prefix in lint output
changelog: none
I was waiting for #13452 to be merged before sending this one. `std` is used instead of `core` when appropriate in messages.
Move `clippy::module_name_repetitions` to `restriction` (from `pedantic`)
Rational:
- Too pedantic IMO, I use `#[warn(pedantic)]` in my personal projects, but then always allow this lint. The fact that we had a few `#[expect(clippy::module_name_repetitions)]` also underlines this point IMO
- STD doesn't do this either. Examples:
- std::vec::Vec
- std::collections::vec_deque::VecDequeue
- #7666 commonly ignored
---
changelog: Move [`module_name_repetitions`] to `restriction` (from `pedantic`)
[#13541](https://github.com/rust-lang/rust-clippy/pull/13541)
Fix lint `manual_slice_size_calculation` when a slice is ref more than once
When a slice is ref more than once, current suggestion given by `manual_slice_size_calculation` is wrong. For example:
```rs
let s: &[i32] = &[1, 2][..];
let ss: &&[i32] = &s; // <-----
let _ = size_of::<i32>() * ss.len();
```
clippy now suggests:
```patch
- let _ = size_of::<i32>() * ss.len();
+ let _ = size_of_val(ss);
```
However, this can result in calculating the size of `&[i32]`, instead of `[i32]` (this wrong suggestion also leads to `size_of_ref` warning: https://rust-lang.github.io/rust-clippy/master/index.html#/size_of_ref )
Now I am sending this PR to fix this bug, so that clippy will suggest (some deref added):
```patch
- let _ = size_of::<i32>() * ss.len();
+ let _ = size_of_val(*ss);
```
As I am not familiar with current clippy code-base, please correct me if I am not doing well or I can do it better :)
changelog: [`manual_slice_size_calculation`]: fix a bug when a slice is ref more than once.
[`implicit_saturating_sub`] Fix suggestion with a less volatile approach
Related to #13533, such and obvious mistake got pass my watch, quite embarassing :/
Revert #13533 and implement a more robust solution.
Revert "Fix span issue on `implicit_saturating_sub`
This reverts commit 140a1275f2.
changelog: [`lint_name`]: Fix suggestion for `if {} else if {} else {}` cases
r? `@y21`
Check MethodCall/Call arg count earlier or at all
This gets rid of a bunch of possible panic spots, as well as bailing out earlier for optimisation reasons.
I started doing this because I saw that a significant amount of time was being spent in the `create_dir` restriction lint when running clippy with `perf`, but this also helps with robustness.
changelog: none
Rational:
- Too pedantic IMO, it's often better to have fine grained modules and
then rexport stuff instead of one gigantic file
- STD doesn't do this either. Examples:
- std::vec::Vec
- std::collections::vec_deque::VecDequeue
- rust-clippy#7666 commonly ignored
Mark unnecessary_first_then_check and byte_char_slices as Applicable
I don't really see situations where this isn't Applicable that aren't weird edge cases where the lint should be disabled.
changelog: none
Improved wording of or_fun_call lint
The current wording (e.g. ``use of `ok_or` followed by a function call``) is potentially confusing (at least it confused me) by suggesting that the function that follows the (in this case) `ok_or` is the problem and not the function that is an argument to it.
The code in my program that triggered the confusing message is the following:
```rust
let file_id = buf
.lines()
.next()
.ok_or((
InternalError::ProblemReadingFromInbox,
anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
))
.html_context(stream, lang)?;
```
I thought that `html_context` was the problem and that I should do something along the following lines:
```rust
let file_id = buf
.lines()
.next()
.ok_or_else(
(
InternalError::ProblemReadingFromInbox,
anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
),
html_context(stream, lang),
)?
```
This is of course wrong. My confusion was only cleared up through the help message indicating what I should try instead.
If someone has a better idea of a replacement wording (currently e.g. ``` function call inside of `ok_or` ```), I'm all ears.
changelog: none
Rewrite lints page
This PR has multiple goals:
* Make lints page to work without needing a web server by removing the json file.
* Prepare the field to also make the page work with JS (not done in this PR but should be straightforward).
* Remove angular dependency.
r? `@Alexendoo`
changelog: make lint page work without web server
Turn declare_clippy_lint into a declarative macro
Ease of development, and hopefully compile times (the dependencies are still there because of ui-test). The procedural macro was doing just some very basic processing (like assigning a lint level to each category), so it didn't have a reason to stay IMO
changelog: None
Fix large_stack_arrays triggering when nesting const items
Fixes#13529.
r? `@flip1995`
changelog: [`large_stack_arrays`]: No longer triggers in static/const context when using nested items
Back from burnout
This reverts commit 5ea7044d72. I needed some time free from reviewing to focus on the Project Goal and myself.
Now I'm much better, and we can continue reviewing!
I hope that I can approve this myself 😅
changelog: none
Don't warn on proc macro generated code in `needless_return`
Fixes#13458Fixes#13457Fixes#13467Fixes#13479Fixes#13481Fixes#13526Fixes#13486
The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does* fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.
The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.
"Hide whitespace" helps a bit for reviewing the proc macro detection change
changelog: none
Check for needless raw strings in `format_args!()` template as well
changelog: [`needless_raw_strings`, `needless_raw_string_hashes`]: check `format_args!()` template as well
Fix#13503
Show interior mutability chain in `mutable_key_type`
Fixes#10619
Just ran into this myself and I definitely agree it's not very nice to have to manually go through all the types involved to figure out why this happens and to evaluate if this is really a problem (knowing if the field of a struct is something that a hash impl relies on), so this changes the lint to emit notes for each step involved.
changelog: none
Style: do not defensively use `saturating_sub()`
Using `saturating_sub()` here in code which cannot fail brings a false sense of security. If for any reason a logic error was introduced and caused `self.loop_depth` to reach 0 before being decremented, using `saturating_sub(1)` would silently mask the programming error instead of panicking loudly as it should (at least in dev profile).
changelog: none
Using `saturating_sub()` here in code which cannot fail brings a false
sense of security. If for any reason a logic error was introduced and
caused `self.loop_depth` to reach 0 before being decremented, using
`saturating_sub(1)` would silently mask the programming error instead of
panicking loudly as it should (at least in dev profile).
