From 104985b8275180ebe8811f8957a93740855995e0 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Wed, 27 Jun 2018 21:23:18 -0700 Subject: [PATCH] unreachable_pub lint: grab `pub` span from HIR rather than inferring it This is a true fix for #50455, superior to the mere bandage offered in #50476. --- src/librustc_lint/builtin.rs | 67 +++++++++---------- .../ui/lint/unreachable_pub-pub_crate.stderr | 13 ++-- src/test/ui/lint/unreachable_pub.stderr | 13 ++-- 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c5bde8af9cf..a9ee1c8f176 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1386,31 +1386,32 @@ fn get_lints(&self) -> LintArray { 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(); + }, + _ => {} } } } @@ -1418,28 +1419,20 @@ fn perform_lint(&self, cx: &LateContext, what: &str, id: ast::NodeId, 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); } } diff --git a/src/test/ui/lint/unreachable_pub-pub_crate.stderr b/src/test/ui/lint/unreachable_pub-pub_crate.stderr index 2948deb2300..1cbfbd21125 100644 --- a/src/test/ui/lint/unreachable_pub-pub_crate.stderr +++ b/src/test/ui/lint/unreachable_pub-pub_crate.stderr @@ -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 @@ -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 diff --git a/src/test/ui/lint/unreachable_pub.stderr b/src/test/ui/lint/unreachable_pub.stderr index ad88c55d540..25046055aa0 100644 --- a/src/test/ui/lint/unreachable_pub.stderr +++ b/src/test/ui/lint/unreachable_pub.stderr @@ -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 @@ -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 -- 2.44.0