Fixes#14488
Context: the `macro_metavars_in_unsafe` lint looks for unsafe blocks
with a macro span that then contain expressions with a root context span
(which means that it is a macro with an unsafe block expanding a
metavariable inside). In order to avoid emitting a warning for every
single macro invocation, it will deduplicate the unsafe blocks by the
span in the macro.
This leads to the linked issue where because of the deduplicating and
removing unsafe blocks that all belong to the same unsafe block in the
macro, only one of the unsafe blocks will actually have its lint
expectation fulfilled. This PR fixes that by manually fulfilling all of
the unsafe blocks from all expansions before deduplicating them.
changelog: [`macro_metavars_in_unsafe`]: fix unfulfilled `#[expect]` if
macro is invoked multiple times
A discrepancy between the `cmake` version available on the runners and
the one required by the `aws-lc-sys` dependency prevents the crate from
building.
changelog: none
r? @flip1995
cc @Alexendoo
A discrepancy between the `cmake` version available on the runners and
the one required by the `aws-lc-sys` dependency prevents the crate from
buiding.
Found this while reviewing PR
https://github.com/rust-lang/rust/pull/138384: See
https://github.com/rust-lang/rust/pull/138384#discussion_r1996111087 in
which I suggested a FIXME to be added that I'm now fixing in this PR.
---
Before this PR, `redundant_pub_crate` computed a broken primary Span
when flagging items (`hir::Item`s) that never bear a name (`ForeignMod`,
`GlobalAsm`, `Impl`, `Use(UseKind::Glob)`, `Use(UseKind::ListStem)`).
Namely, it created a span whose high byte index is `DUMMY_SP.hi()` which
is quite broken (`DUMMY_SP` is synonymous with `0..0` wrt. the entire
`SourceMap` meaning it points at the start of the very first source file
in the `SourceMap`).
Why did this happen? Before PR
https://github.com/rust-lang/rust/pull/138384, the offending line looked
like `let span = item.span.with_hi(item.ident.span.hi());`. For nameless
items, `item.ident` used to be set to `Ident(sym::empty, DUMMY_SP)`.
This is where the `DUMMY_SP` came from.
The code means to compute a "shorter item span" that doesn't include the
"body" of items, only the "head" (similar to `TyCtxt::def_span`).
<details><summary>Examples of Clippy's butchered output on
master</summary>
```rs
#![deny(clippy::redundant_pub_crate)]
mod m5 {
pub mod m5_1 {}
pub(crate) use m5_1::*;
}
```
```
error: pub(crate) import inside private module
--> file.rs:1:1
|
1 | / #![deny(clippy::redundant_pub_crate)]
2 | |
3 | | mod m5 {
4 | | pub mod m5_1 {}
5 | | pub(crate) use m5_1::*;
| | ^---------- help: consider using: `pub`
| |____|
|
```
Or if the `SourceMap` contains multiple files (notice how it leaks
`clippy.toml`!):
```
error: pub(crate) import inside private module
--> /home/fmease/programming/rust/clippy/clippy.toml:1:1
|
1 | / avoid-breaking-exported-api = false
2 | |
3 | | check-inconsistent-struct-field-initializers = true
... |
6 | |
| |_^
|
::: file.rs:6:5
|
6 | pub(crate) use m5_1::{*}; // Glob
| ---------- help: consider using: `pub`
|
```
</details>
---
**Note**: Currently, the only nameless item kind that can also have a
visibility is `Use(UseKind::{Glob,ListStem})`. Thus I'm just falling
back to the entire item's Span which wouldn't be that verbose. However,
in the future Rust will feature impl restrictions (like `pub(crate) impl
Trait for Type {}`, see [RFC
3323](https://rust-lang.github.io/rfcs/3323-restrictions.html) and
https://github.com/rust-lang/rust/pull/106074). Using `item.span` for
these would be quite bad (it would include all associated items). Should
I add a FIXME?
---
changelog: [`redundant_pub_crate`]: Fix the code highlighting for
nameless items.
`tokio-rustls` requires `aws-lc-sys` which requires `cmake 3.x` to be
built. This installs `cmake 3.x` from the repository, overriding the
`cmake 4.x` installation.
The situation will be reexamined in a few weeks, as `aws-lc-sys` will
get a new release in a few days with `cmake 4.x` support. We will then
be able to remove this temporary fix.
changelog: none
r? @ghost
Having a macro call as the scrutinee is supported. However, the proposed
suggestion must use the macro call itself, not its expansion.
When the scrutinee is a macro call, do not complain about an irrefutable
match, as the user may not be aware of the result of the macro. A
comparaison will be suggested instead, as if we couldn't see the outcome
of the macro.
Similarly, do not accept macro calls as arm patterns.
changelog: [`single_match`]: proper suggestions in presence of macros
Fixes#14493
These method chains can be expressed concisely with `if` / `else`.
changelog: [`obfuscated_if_else`]: support `then().unwrap_or_default()`
and `then_some().unwrap_or_default()`
Implements
https://github.com/rust-lang/rust-clippy/issues/11436#issuecomment-2719199421
> [...] I'd really like to be able to (reusing some above examples),
>
> ``` rust
> /// Do something
> pub fn string_from_byte_stream() -> String {
> let bytes = get_valid_utf8();
> #[expect(clippy::missing_panics_doc_ok, reason="caller can't do
anything about this")]
> String::from_utf8(bytes).expect("`get_valid_utf8()` always returns
valid UTF-8")
> }
> ```
Also fixes an issue where if the first panic is in a `const` context
disables the lint for the rest of the function
The first commit is just moving code around
changelog: [`missing_panics_doc`]: `#[allow]` and `#[expect]` can now be
used within the function body to ignore individual panics
This is a small refactor of `ConfVisitor`'s `visit_map` method.
It adds comments and reduces `match` nesting by adding `continue`
statements.
IMHO, the code is easier to read in this form. No one asked for this, so
I hope the maintainers agree it is an improvement.
changelog: none
Closes#10202.
This adds a new lint that checks for uses of the `.chars().enumerate()`
position in a context where a byte index is required and suggests
changing it to use `.char_indices()` instead.
I'm planning to extend this lint to also detect uses of the position in
iterator chains, e.g. `s.chars().enumerate().for_each(|(i, _)|
s.split_at(i));`, but that's for another time
----------------
changelog: new lint: `chars_enumerate_for_byte_indices`
I sometimes run lintcheck/CI experiments by opening a PR on my own
clippy fork. This has recently "stopped" working (as in, fails CI) when
the changelog CI was added because the repository is hardcoded as
"rust-lang/rust-clippy", where the PR number won't match with the one on
my repo.
changelog: none
r? flip1995
Having a macro call as the scrutinee is supported. However, the proposed
suggestion must use the macro call itself, not its expansion.
When the scrutinee is a macro call, do not complain about an irrefutable
match, as the user may not be aware of the result of the macro. A
comparaison will be suggested instead, as if we couldn't see the outcome
of the macro.
Similarly, do not accept macro calls as arm patterns.
The main reason for this is that we might transform MIR in the
optimization passes in a way that doesn't work with const-eval, but it
is irrelevant since const-eval uses another MIR (`mir_for_ctfe`).
Specifically this came up when adding a new check in debug builds
(https://github.com/rust-lang/rust/pull/134424), which is added as part
of an optimization pass.
changelog: none
This has two reasons: First of all we don't need the optimized MIR for
evaluating the checks of this lint, as const-eval anyways operates on
`mir_for_ctf` which is derived from `mir_drops_elaborated_and_const_checked`.
Second of all we might transform MIR in the optimization passes in a way
that doesn't work with const-eval, but it is irrelevant since const-eval
uses another MIR.
Specifically this came up when adding a new check in debug builds
(https://github.com/rust-lang/rust/pull/134424), which is added as part
of an optimization pass.
Until `if let` chains are stabilized, we do not collapse them together
or with other `if` expressions unless the `let_chains` feature is
enabled. This is the case for example in Clippy sources.
This was made possible by converting the `collapsible_if` to a late lint
to get access to the set of enabled features. This allows this PR to
supersede #14455 and no longer require an additional configuration
option.
The three commits are, in order:
- a conversion of the existing early lint to a late lint, with no new
feature or tests changes
- the addition of the `let_chains` feature detection and action, and
tests
- the application of the enhanced lint to Clippy sources (136 files
modified)
changelog: [`collapsible_if`]: recognize the rust compiler `let_chains`
feature
r? @flip1995
The `unnecessary_filter_map` and `unnecessary_find_map` lints were
making partial suggestions, proposing to replace the whole expression by
only the method name, or a subexpression which contained explicit
placeholders.
Since even `MaybeIncorrect` suggestions must generate code that
compiles, this changes those lints to recommandation lints with no code
suggestion.
Fixes#14486
changelog: [`unnecessary_find_map`, `unnecessary_filter_map`]: do not
make suggestions that will not compile
The `unnecessary_filter_map` and `unnecessary_find_map` lints were
making partial suggestions, proposing to replace the whole expression by
only the method name, or a subexpression which contained explicit
placeholders.
Since even `MaybeIncorrect` suggestions must generate code that
compiles, this changes those lints to recommandation lints with no
code suggestion.
Until `if let` chains are stabilized, we do not collapse them together
or with other `if` expressions unless the `let_chains` feature is
enabled. This is the case for example in Clippy sources.
When lowering a destructuring assignment from AST to HIR, the compiler
will reuse the same identifier name (namely `sym::lhs`) for all the
fields. The desugaring must be checked for to avoid a false positive of
the `shadow_unrelated` lint.
Fix#10279Fix#14377
changelog: [`shadow_unrelated`]: prevent false positive in destructuring
assignments
When lowering a destructuring assignment from AST to HIR, the compiler
will reuse the same identifier name (namely `sym::lhs`) for all the
fields. The desugaring must be checked for to avoid a false positive
of the `shadow_unrelated` lint.
Dereferencing string literals in suggestions is redundant.
changelog: [`cmp_owned`]: more natural suggestions are provided for
string literals now
fixes#8103
This extra condition prevents a problem when removing the '}' in:
```rust
( // There was an opening bracket after the parenthesis, which has been removed
// This is a comment
})
```
Removing the whitespaces, including the linefeed, before the '}', would
put the closing parenthesis at the end of the `// This is a comment`
line, which would make it part of the comment as well. In this case, it
is best to keep the span on the '}' alone.
changelog: none
Note to reviewer: `manual_inspect` and `collapsible_if` were two lints
exhibiting this problem in cooked up test cases, which have been
included as non-regression tests.
This extra condition prevents a problem when removing the '}' in:
```rust
( // There was an opening bracket after the parenthesis, which has been removed
// This is a comment
})
```
Removing the whitespaces, including the linefeed, before the '}', would put the
closing parenthesis at the end of the `// This is a comment` line, which would
make it part of the comment as well. In this case, it is best to keep the span
on the '}' alone.
Alejandra González