edition lint: prefer `crate` to `pub(crate)`

[nrc]

[zackmdavis]

Mentoring hints: study the unreachable_pub lint for an example of a lint that operates on visibility modifiers, except in this case, we probably want an EarlyLintPass (which operates on the AST rather than the HIR). The Item, ForeignItem, StructField, and ImplItem AST nodes all have a vis field of type Visibility. Visibility has two fields: span (a Span, representing the place in the source code that the visibility modifier takes up) and node (which is a VisibilityKind). We want the lint to fire if node is the variant VisibilityKind::Crate(CrateSugar) and the value it wraps is a CrateSugar::PubCrate. We should suggest (using span_suggestion_with_applicability) replacing the visibility span with the text crate.

[seanmonstar]

What does this mean exactly? Will the lint warn if I use pub(crate)? Or this is more for rustfix?

I personally find pub(crate) much clearer as a visibility modifier than just crate. I'd probably even want a lint in the opposite direction (warn contributors to not change things to crate over pub(crate) in libraries I maintain.

[Manishearth]

This is an edition lint; post edition upgrade this will ask you to replace pub(crate) with crate, via rustfix.

[seanmonstar]

Is there a reason to push people from pub(crate) to crate? Why not allow (as in, leave alone) pub(crate) to stay?

[carllerche]

👍 I agree, I find pub(crate) much clearer and will most likely stick with that.

[Manishearth]

I feel like this issue is not the place to discuss this -- perhaps the tracking issue for the path changes? (I'm not sure where it should be discussed, I just find that implementation issues are not that place)

[seanmonstar]

I get that, though the path changes tracking issue is so noisy, it'd get easily lost. Besides, the path changes doesn't seem to suggest that pub(crate) is deprecated, just that people who want to can change to crate. If that's the case, then this implementation issue might be slightly over-zealous.