Commit 6ca6b09
authored
Fix the primary span of
Found this while reviewing PR
rust-lang#138384: See
rust-lang#138384 (comment) 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
rust-lang#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
rust-lang#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.redundant_pub_crate when flagging nameless items (rust-lang#14516)File tree
4 files changed
+37
-8
lines changed- clippy_lints/src
- tests/ui
4 files changed
+37
-8
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
62 | 59 | | |
63 | 60 | | |
64 | 61 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
131 | 131 | | |
132 | 132 | | |
133 | 133 | | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
134 | 142 | | |
135 | 143 | | |
136 | 144 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
131 | 131 | | |
132 | 132 | | |
133 | 133 | | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
134 | 142 | | |
135 | 143 | | |
136 | 144 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
129 | 129 | | |
130 | 130 | | |
131 | 131 | | |
132 | | - | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
133 | 149 | | |
0 commit comments