Fix the primary span of redundant_pub_crate
when flagging nameless items
#14516
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Found this while reviewing PR rust-lang/rust#138384: See rust-lang/rust#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 isDUMMY_SP.hi()
which is quite broken (DUMMY_SP
is synonymous with0..0
wrt. the entireSourceMap
meaning it points at the start of the very first source file in theSourceMap
).Why did this happen? Before PR rust-lang/rust#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 toIdent(sym::empty, DUMMY_SP)
. This is where theDUMMY_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
).Examples of Clippy's butchered output on master
Or if the
SourceMap
contains multiple files (notice how it leaksclippy.toml
!):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 (likepub(crate) impl Trait for Type {}
, see RFC 3323 and rust-lang/rust#106074). Usingitem.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.