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
Retire the `unnamed_fields` feature for now
`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.
However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.
Fixes#117942Fixes#121161Fixes#121263Fixes#121299Fixes#121722Fixes#121799Fixes#126969Fixes#131041
Tracking:
* https://github.com/rust-lang/rust/issues/49804
[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: https://github.com/rust-lang/rust/issues/49804#issuecomment-1972619108
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
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).
Reduce default 'large array' threshold
As-is this threshold is `512kb`, but as #9449 points out this is way too high for most people to consider sensible (why would you want to copy `256kb` of data around on the stack or duplicate it via `const`) and didn't get any discussion when originally added. This PR reduces it the threshold to `1kb`, which is higher than the issue says ("a few cpu words") but helps out for actual codebases.
While reducing this, I found that `large_stack_arrays` was triggering for statically promoted arrays in constants/statics, so I also fixed that up as seen in the difference to [array_size_threshold.stderr](https://github.com/rust-lang/rust-clippy/compare/master...GnomedDev:rust-clippy:reduce-large-threshold?expand=1#diff-4c2a2a855d9ff7777f1d385be0c1bede2a3fc8aaab94837cde27a35235233fc7).
Closes#9449.
changelog: [`large_stack_arrays`]: No longer triggers in `static`/`const` context
changelog: [`large_const_arrays`]: Changed the default of [`array-size-threshold`] from `512kb` to `16kb`
Implement lint for regex::Regex compilation inside a loop
Closes#598.
Seems like a pretty simple one, I'm not sure if I sorted out all the lint plumbing correctly because I was adding it to the existing regex pass, but seems to work. The name is a bit jank and I'm super open to suggestions for changing it.
changelog: [`regex_creation_in_loops`]: Added lint for Regex compilation inside loops.
