Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add a lint for pub items that are not reachable from crate root #45521

Closed
nikomatsakis opened this issue Oct 25, 2017 · 1 comment
Closed

add a lint for pub items that are not reachable from crate root #45521

nikomatsakis opened this issue Oct 25, 2017 · 1 comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

As part of #44660, we need to create a lint for items that are declared as pub but are not reachable from the crate root (such items are advised to use crate visibility instead).

The lint should probably be allow-by-default for now, to start.

This issue needs precise mentoring instructions. There are some tips regarding adding "future compatibility" lints here in this forge article -- see the section "Issuing future compatibility warnings". We don't need a future compatibility lint here, so we can skip the stuff specific to that, but it does give some more general advice on how to make lints overall.
@nikomatsakis nikomatsakis added E-needs-mentor T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 25, 2017
@aturon aturon mentioned this issue Oct 25, 2017
Closed
11 tasks
@nikomatsakis
Copy link
Contributor Author

@zackmdavis mentioned that they were working on this

@zackmdavis zackmdavis mentioned this issue Oct 27, 2017
Merged
bors added a commit that referenced this issue Nov 3, 2017
@bors
Auto merge of #45569 - zackmdavis:unexported_pub_lint, r=petrochenkov
59d4845 
`unreachable-pub` lint (as authorized by RFC 2126)

To whom it may concern:

RFC 2126 commissions the creation of a lint for `pub` items that are not visible from crate root (#45521). We understand (but seek confirmation from more knowledgable compiler elders) that this can be implemented by linting HIR items that are _not_ ~~`cx.access_levels.is_exported`~~ `cx.access_levels.is_reachable` but have a `vis` (-ibility) field of `hir::Visibility::Public`.

The lint, tentatively called ~~`unexported-pub`~~ `unreachable-pub` (with the understanding that much could be written on the merits of various names, as it is said of the colors of bicycle-sheds), suggests `crate` as a replacement for `pub` if the `crate_visibility_modifier` feature is enabled (see #45388), and `pub(crate)` otherwise. We also use help messaging to suggest the other potential fix of exporting the item; feedback is desired as to whether this may be confusing or could be worded better.

As a preview of what respecting the proposed lint would look like (and to generate confirmatory evidence that this implementation doesn't issue false positives), ~~we take its suggestions for `libcore`~~ (save one, which is deferred to another pull request because it brings up an unrelated technical matter). I remain your obedient servant.

![unexported_pub](https://user-images.githubusercontent.com/1076988/32089794-fbd02420-baa0-11e7-87e5-3ec01f18924a.png)

r? @petrochenkov
@bors bors closed this as completed in 085ec6d Nov 3, 2017
@Manishearth Manishearth mentioned this issue Mar 4, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@nikomatsakis