Skip to content

Commit

Permalink
unreachable_pub lint: grab pub span from HIR rather than inferring it
Browse files Browse the repository at this point in the history 
This is a true fix for #50455, superior to the mere bandage offered
in #50476.
  • Loading branch information
@zackmdavis
zackmdavis committed Jul 1, 2018
1 parent 4ae8912 commit 104985b
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 47 deletions.
67 changes: 30 additions & 37 deletions src/librustc_lint/builtin.rs
View file
Expand Up @@ -1386,60 +1386,53 @@ impl LintPass for UnreachablePub {

impl UnreachablePub {
fn perform_lint(&self, cx: &LateContext, what: &str, id: ast::NodeId,
vis: &hir::Visibility, span: Span, exportable: bool,
mut applicability: Applicability) {
if !cx.access_levels.is_reachable(id) && vis.node.is_pub() {
if span.ctxt().outer().expn_info().is_some() {
applicability = Applicability::MaybeIncorrect;
}
let def_span = cx.tcx.sess.codemap().def_span(span);
let mut err = cx.struct_span_lint(UNREACHABLE_PUB, def_span,
&format!("unreachable `pub` {}", what));
// We are presuming that visibility is token at start of
// declaration (can be macro variable rather than literal `pub`)
let pub_span = cx.tcx.sess.codemap().span_until_char(def_span, ' ');
let replacement = if cx.tcx.features().crate_visibility_modifier {
"crate"
} else {
"pub(crate)"
}.to_owned();
err.span_suggestion_with_applicability(pub_span,
"consider restricting its visibility",
replacement,
applicability);
if exportable {
err.help("or consider exporting it for use by other crates");
}
err.emit();
vis: &hir::Visibility, span: Span, exportable: bool) {
let mut applicability = Applicability::MachineApplicable;
match vis.node {
hir::VisibilityPublic if !cx.access_levels.is_reachable(id) => {
if span.ctxt().outer().expn_info().is_some() {
applicability = Applicability::MaybeIncorrect;
}
let def_span = cx.tcx.sess.codemap().def_span(span);
let mut err = cx.struct_span_lint(UNREACHABLE_PUB, def_span,
&format!("unreachable `pub` {}", what));
let replacement = if cx.tcx.features().crate_visibility_modifier {
"crate"
} else {
"pub(crate)"
}.to_owned();

err.span_suggestion_with_applicability(vis.span,
"consider restricting its visibility",
replacement,
applicability);
if exportable {
err.help("or consider exporting it for use by other crates");
}
err.emit();
},
_ => {}
}
}
}


impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
let applicability = match item.node {
// suggestion span-manipulation is inadequate for `pub use
// module::{item}` (Issue #50455)
hir::ItemUse(..) => Applicability::MaybeIncorrect,
_ => Applicability::MachineApplicable,
};
self.perform_lint(cx, "item", item.id, &item.vis, item.span, true, applicability);
self.perform_lint(cx, "item", item.id, &item.vis, item.span, true);
}

fn check_foreign_item(&mut self, cx: &LateContext, foreign_item: &hir::ForeignItem) {
self.perform_lint(cx, "item", foreign_item.id, &foreign_item.vis,
foreign_item.span, true, Applicability::MachineApplicable);
foreign_item.span, true);
}

fn check_struct_field(&mut self, cx: &LateContext, field: &hir::StructField) {
self.perform_lint(cx, "field", field.id, &field.vis, field.span, false,
Applicability::MachineApplicable);
self.perform_lint(cx, "field", field.id, &field.vis, field.span, false);
}

fn check_impl_item(&mut self, cx: &LateContext, impl_item: &hir::ImplItem) {
self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false,
Applicability::MachineApplicable);
self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false);
}
}

Expand Down
13 changes: 8 additions & 5 deletions src/test/ui/lint/unreachable_pub-pub_crate.stderr
View file
Expand Up @@ -17,7 +17,9 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:27:24
|
LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced
| ^^^^ help: consider restricting its visibility: `pub(crate)`
| --- ^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

Expand Down Expand Up @@ -121,12 +123,13 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:50:47
|
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
| -----------^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
| ^^^^^^^^^^^^^^^^^^^^^^^^
LL | }
LL | define_empty_struct_with_visibility!(pub, Fluorine);
| ---------------------------------------------------- in this macro invocation
| ----------------------------------------------------
| | |
| | help: consider restricting its visibility: `pub(crate)`
| in this macro invocation
|
= help: or consider exporting it for use by other crates

Expand Down
13 changes: 8 additions & 5 deletions src/test/ui/lint/unreachable_pub.stderr
View file
Expand Up @@ -17,7 +17,9 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub.rs:22:24
|
LL | pub use std::env::{Args}; // braced-use has different item spans than unbraced
| ^^^^ help: consider restricting its visibility: `crate`
| --- ^^^^
| |
| help: consider restricting its visibility: `crate`
|
= help: or consider exporting it for use by other crates

Expand Down Expand Up @@ -121,12 +123,13 @@ warning: unreachable `pub` item
--> $DIR/unreachable_pub.rs:45:47
|
LL | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
| -----------^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `crate`
| ^^^^^^^^^^^^^^^^^^^^^^^^
LL | }
LL | define_empty_struct_with_visibility!(pub, Fluorine);
| ---------------------------------------------------- in this macro invocation
| ----------------------------------------------------
| | |
| | help: consider restricting its visibility: `crate`
| in this macro invocation
|
= help: or consider exporting it for use by other crates

Expand Down

0 comments on commit 104985b

Please sign in to comment.