Error on private items with stability attributes

It doesn't make sense for a private item to have a stability attribute,
and it is likely a mistake. Report an error when we see a case of this.

Author: camelid

compiler/rustc_passes/src/stability.rs

@@ -219,7 +219,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
             if kind == AnnotationKind::Prohibited
                 || (kind == AnnotationKind::Container && stab.level.is_stable() && is_deprecated)
             {
-                self.tcx.sess.struct_span_err(span,"this stability annotation is useless")
+                self.tcx.sess.struct_span_err(span, "this stability annotation is useless")
                     .span_label(span, "useless stability annotation")
                     .span_label(item_sp, "the stability attribute annotates this item")
                     .emit();
@@ -605,6 +605,24 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> {
             }
         }
     }
+
+    fn check_private_stability(&self, hir_id: HirId, span: Span) {
+        let stab = self.tcx.stability().local_stability(hir_id);
+        let is_error = stab.is_some() && !self.access_levels.is_reachable(hir_id);
+        if is_error {
+            let def_id = self.tcx.hir().local_def_id(hir_id);
+            let descr = self.tcx.def_kind(def_id).descr(def_id.to_def_id());
+            self.tcx
+                .sess
+                .struct_span_err(
+                    span,
+                    &format!("{} is private but has a stability attribute", descr),
+                )
+                .help("if it is meant to be private, remove the stability attribute")
+                .help("or, if it is meant to be public, make it public")
+                .emit();
+        }
+    }
 }
 
 impl<'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'tcx> {
@@ -635,11 +653,14 @@ impl<'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'tcx> {
             self.check_missing_const_stability(i.hir_id(), i.span);
         }
 
+        self.check_private_stability(i.hir_id(), i.span);
+
         intravisit::walk_item(self, i)
     }
 
     fn visit_trait_item(&mut self, ti: &'tcx hir::TraitItem<'tcx>) {
         self.check_missing_stability(ti.hir_id(), ti.span);
+        self.check_private_stability(ti.hir_id(), ti.span);
         intravisit::walk_trait_item(self, ti);
     }
 
@@ -648,26 +669,31 @@ impl<'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'tcx> {
         if self.tcx.impl_trait_ref(impl_def_id).is_none() {
             self.check_missing_stability(ii.hir_id(), ii.span);
         }
+        self.check_private_stability(ii.hir_id(), ii.span);
         intravisit::walk_impl_item(self, ii);
     }
 
     fn visit_variant(&mut self, var: &'tcx Variant<'tcx>, g: &'tcx Generics<'tcx>, item_id: HirId) {
         self.check_missing_stability(var.id, var.span);
+        self.check_private_stability(var.id, var.span);
         intravisit::walk_variant(self, var, g, item_id);
     }
 
     fn visit_field_def(&mut self, s: &'tcx FieldDef<'tcx>) {
         self.check_missing_stability(s.hir_id, s.span);
+        self.check_private_stability(s.hir_id, s.span);
         intravisit::walk_field_def(self, s);
     }
 
     fn visit_foreign_item(&mut self, i: &'tcx hir::ForeignItem<'tcx>) {
         self.check_missing_stability(i.hir_id(), i.span);
+        self.check_private_stability(i.hir_id(), i.span);
         intravisit::walk_foreign_item(self, i);
     }
 
     fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
         self.check_missing_stability(md.hir_id(), md.span);
+        self.check_private_stability(md.hir_id(), md.span);
     }
 
     // Note that we don't need to `check_missing_stability` for default generic parameters,
@@ -930,6 +956,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
         let krate = tcx.hir().krate();
         let mut missing = MissingStabilityAnnotations { tcx, access_levels };
         missing.check_missing_stability(hir::CRATE_HIR_ID, krate.item.inner);
+        missing.check_private_stability(hir::CRATE_HIR_ID, krate.item.inner);
         intravisit::walk_crate(&mut missing, krate);
         krate.visit_all_item_likes(&mut missing.as_deep_visitor());
     }

src/test/ui/stability-attribute/stability-without-publicity.rs

@@ -0,0 +1,29 @@
+#![feature(staged_api)]
+#![stable(feature = "rust1", since = "1.0.0")]
+#![crate_type = "lib"]
+
+#[stable(feature = "rust1", since = "1.0.0")]
+mod mod_stable {}
+//~^ ERROR module is private but has a stability attribute
+
+#[unstable(feature = "foo", issue = "none")]
+mod mod_unstable {}
+//~^ ERROR module is private but has a stability attribute
+
+#[stable(feature = "rust1", since = "1.0.0")]
+fn fn_stable() {}
+//~^ ERROR function is private but has a stability attribute
+
+#[unstable(feature = "foo", issue = "none")]
+fn fn_unstable() {}
+//~^ ERROR function is private but has a stability attribute
+
+#[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_const_stable(feature = "rust1", since = "1.0.0")]
+const fn const_fn_stable() {}
+//~^ ERROR function is private but has a stability attribute
+
+#[unstable(feature = "foo", issue = "none")]
+#[rustc_const_unstable(feature = "foo", issue = "none")]
+const fn const_fn_unstable() {}
+//~^ ERROR function is private but has a stability attribute

src/test/ui/stability-attribute/stability-without-publicity.stderr

@@ -0,0 +1,56 @@
+error: module is private but has a stability attribute
+  --> $DIR/stability-without-publicity.rs:6:1
+   |
+LL | mod mod_stable {}
+   | ^^^^^^^^^^^^^^^^^
+   |
+   = help: if it is meant to be private, remove the stability attribute
+   = help: or, if it is meant to be public, make it public
+
+error: module is private but has a stability attribute
+  --> $DIR/stability-without-publicity.rs:10:1
+   |
+LL | mod mod_unstable {}
+   | ^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if it is meant to be private, remove the stability attribute
+   = help: or, if it is meant to be public, make it public
+
+error: function is private but has a stability attribute
+  --> $DIR/stability-without-publicity.rs:14:1
+   |
+LL | fn fn_stable() {}
+   | ^^^^^^^^^^^^^^^^^
+   |
+   = help: if it is meant to be private, remove the stability attribute
+   = help: or, if it is meant to be public, make it public
+
+error: function is private but has a stability attribute
+  --> $DIR/stability-without-publicity.rs:18:1
+   |
+LL | fn fn_unstable() {}
+   | ^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if it is meant to be private, remove the stability attribute
+   = help: or, if it is meant to be public, make it public
+
+error: function is private but has a stability attribute
+  --> $DIR/stability-without-publicity.rs:23:1
+   |
+LL | const fn const_fn_stable() {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if it is meant to be private, remove the stability attribute
+   = help: or, if it is meant to be public, make it public
+
+error: function is private but has a stability attribute
+  --> $DIR/stability-without-publicity.rs:28:1
+   |
+LL | const fn const_fn_unstable() {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: if it is meant to be private, remove the stability attribute
+   = help: or, if it is meant to be public, make it public
+
+error: aborting due to 6 previous errors
+