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_cratecomputed a broken primary Span when flagging items (hir::Items) 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_SPis synonymous with0..0wrt. the entireSourceMapmeaning 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.identused to be set toIdent(sym::empty, DUMMY_SP). This is where theDUMMY_SPcame 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
SourceMapcontains 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.spanfor 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.