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.

diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index c5bde8af9cf54fce26f547a32eb86c68d602f582..a9ee1c8f17686003dcc19a3454ce9740934a9d66 100644 (file)
--- 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 2948deb23009c64b62b33b56fa7a380974aae3b3..1cbfbd211255dcef575ea683832093185fd28fe9 100644 (file)
--- 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 ad88c55d54013a92c97697454bdfcfcac7ef4fb4..25046055aa0247310a6d086251ab09a66d5fa5bc 100644 (file)
--- 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