internal: fix most Clippy lints #16398
Conversation
rustbot
added
the
S-waiting-on-review
|
The tests fail in CI, but not locally 🤔 |
|
|
|
I also see some new warnings:
|
I am on it |
| let Some(expn) = db.lookup_intern_syntax_context(span.ctx).outer_expn else { | ||
| return false; | ||
| }; | ||
| let expn = db.lookup_intern_macro_call(expn); | ||
| // FIXME: Record allow_internal_unstable in the macro def (not been done yet because it | ||
| // would consume quite a bit extra memory for all call locs...) | ||
| // if let Some(features) = expn.def.allow_internal_unstable { | ||
| // if features.iter().any(|&f| f == sym::edition_panic) { | ||
| // span = expn.call_site; | ||
| // continue; | ||
| // } | ||
| // } | ||
| expn.def.edition >= Edition::Edition2021 |
There was a problem hiding this comment.
Please undo this again, this is a loop because of the commented out code and so has meaning once thats fixed again
There was a problem hiding this comment.
I've reset the branch to not include the manual changes. They will come in a separate PR.
crates/stdx/src/thread/pool.rs
Outdated
| @@ -86,6 +86,7 @@ impl Pool { | |||
| self.job_sender.send(job).unwrap(); | |||
| } | |||
|
|
|||
| #[allow(clippy::len_without_is_empty)] | |||
There was a problem hiding this comment.
And this is why we don't really run clippy on the code base 😅 the defaults are sometimes questionable
There was a problem hiding this comment.
It's not Clippy's fault, I'd expect len to return either the thread count or the pending queue size, not the number of running jobs.
There was a problem hiding this comment.
And this is why we don't really run clippy on the code base 😅 the defaults are sometimes questionable
We can configure it. But I feel it is useful to catch a lot of small cleanups, which I believe is useful.
There was a problem hiding this comment.
Overall yes, I do want us to run clippy. I just don't like some of the default set of lints it has. Related #15918
crates/stdx/src/non_empty_vec.rs
Outdated
| @@ -28,6 +28,7 @@ impl<T> NonEmptyVec<T> { | |||
| } | |||
|
|
|||
| #[inline] | |||
| #[allow(clippy::len_without_is_empty)] | |||
There was a problem hiding this comment.
This could arguably return NonZeroUsize, but I don't know if it silences the Clippy warning. Anyway, it's not even used, so..
Urhengulas
force-pushed
the
satisfy-clippy
branch
from
df35dc2
to
255cde7
Compare
|
@bors r+ |
|
☀️ Test successful - checks-actions |
|
https://github.com/rust-lang/rust-analyzer/pull/16398/files#diff-fa039ab572a8591c57be2cea529bb1e287b071b8f30cd5d495da3df98d2a31d1L191-L193 |
|
Looks like some of the other tests in there might be affected too. |
|
Oh, that should definitely be reverted. Forgot we had these weird tests there |
|
@ivan thank you for spotting, created an issue for it |
This PR is the result of running
cargo clippy --fix && cargo fmtin the root of the repository. I did not manually review all the changes, but just skimmed through a few of them. The tests still pass, so it seems fine.