Rollup merge of #47479 - zackmdavis:and_the_case_of_the_suggested_double-pub, r=estebank

private no-mangle lints: only suggest `pub` if it doesn't already exist

Fixes #47383 (function or static can be `pub` but unreachable because it's in a private module; adding another `pub` is nonsensical).

r? @estebank

3 files changed, 42 insertions, 16 deletions

diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs
index c5c27c92ab4..de55710bdf3 100644
--- a/src/librustc_lint/builtin.rs
+++ b/src/librustc_lint/builtin.rs
@@ -1154,9 +1154,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
                         let msg = "function is marked #[no_mangle], but not exported";
                         let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
                         let insertion_span = it.span.with_hi(it.span.lo());
-                        err.span_suggestion(insertion_span,
-                                            "try making it public",
-                                            "pub ".to_owned());
+                        if it.vis == hir::Visibility::Inherited {
+                            err.span_suggestion(insertion_span,
+                                                "try making it public",
+                                                "pub ".to_owned());
+                        }
                         err.emit();
                     }
                     if generics.is_type_parameterized() {
@@ -1177,9 +1179,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
                        let msg = "static is marked #[no_mangle], but not exported";
                        let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
                        let insertion_span = it.span.with_hi(it.span.lo());
-                       err.span_suggestion(insertion_span,
-                                           "try making it public",
-                                           "pub ".to_owned());
+                       if it.vis == hir::Visibility::Inherited {
+                           err.span_suggestion(insertion_span,
+                                               "try making it public",
+                                               "pub ".to_owned());
+                       }
                        err.emit();
                 }
             }
diff --git a/src/test/ui/lint/suggestions.rs b/src/test/ui/lint/suggestions.rs
index 3789b6dfc8b..dfcaede1402 100644
--- a/src/test/ui/lint/suggestions.rs
+++ b/src/test/ui/lint/suggestions.rs
@@ -24,6 +24,16 @@ pub fn defiant<T>(_t: T) {}
 fn rio_grande() {} // should suggest `pub`
 //~^ WARN function is marked
 
+mod badlands {
+    // The private-no-mangle lints shouldn't suggest inserting `pub` when the
+    // item is already `pub` (but triggered the lint because, e.g., it's in a
+    // private module). (Issue #47383)
+    #[no_mangle] pub static DAUNTLESS: bool = true;
+    //~^ WARN static is marked
+    #[no_mangle] pub fn val_jean() {}
+    //~^ WARN function is marked
+}
+
 struct Equinox {
     warp_factor: f32,
 }
diff --git a/src/test/ui/lint/suggestions.stderr b/src/test/ui/lint/suggestions.stderr
index 701a9522218..8b30f552d37 100644
--- a/src/test/ui/lint/suggestions.stderr
+++ b/src/test/ui/lint/suggestions.stderr
@@ -1,7 +1,7 @@
 warning: unnecessary parentheses around assigned value
-  --> $DIR/suggestions.rs:36:21
+  --> $DIR/suggestions.rs:46:21
    |
-36 |         let mut a = (1); // should suggest no `mut`, no parens
+46 |         let mut a = (1); // should suggest no `mut`, no parens
    |                     ^^^ help: remove these parentheses
    |
 note: lint level defined here
@@ -11,17 +11,17 @@ note: lint level defined here
    |                     ^^^^^^^^^^^^^
 
 warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721
-  --> $DIR/suggestions.rs:31:1
+  --> $DIR/suggestions.rs:41:1
    |
-31 | #[no_debug] // should suggest removal of deprecated attribute
+41 | #[no_debug] // should suggest removal of deprecated attribute
    | ^^^^^^^^^^^ help: remove this attribute
    |
    = note: #[warn(deprecated)] on by default
 
 warning: variable does not need to be mutable
-  --> $DIR/suggestions.rs:36:13
+  --> $DIR/suggestions.rs:46:13
    |
-36 |         let mut a = (1); // should suggest no `mut`, no parens
+46 |         let mut a = (1); // should suggest no `mut`, no parens
    |             ---^^
    |             |
    |             help: remove this `mut`
@@ -72,18 +72,30 @@ warning: function is marked #[no_mangle], but not exported
    |
    = note: #[warn(private_no_mangle_fns)] on by default
 
+warning: static is marked #[no_mangle], but not exported
+  --> $DIR/suggestions.rs:31:18
+   |
+31 |     #[no_mangle] pub static DAUNTLESS: bool = true;
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: function is marked #[no_mangle], but not exported
+  --> $DIR/suggestions.rs:33:18
+   |
+33 |     #[no_mangle] pub fn val_jean() {}
+   |                  ^^^^^^^^^^^^^^^^^^^^
+
 warning: denote infinite loops with `loop { ... }`
-  --> $DIR/suggestions.rs:34:5
+  --> $DIR/suggestions.rs:44:5
    |
-34 |     while true { // should suggest `loop`
+44 |     while true { // should suggest `loop`
    |     ^^^^^^^^^^ help: use `loop`
    |
    = note: #[warn(while_true)] on by default
 
 warning: the `warp_factor:` in this pattern is redundant
-  --> $DIR/suggestions.rs:41:23
+  --> $DIR/suggestions.rs:51:23
    |
-41 |             Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
+51 |             Equinox { warp_factor: warp_factor } => {} // should suggest shorthand
    |                       ------------^^^^^^^^^^^^
    |                       |
    |                       help: remove this