Forbid pub/priv where it has no effect

Closes #5495

Author: alexcrichton

src/libextra/crypto/sha1.rs

@@ -159,7 +159,7 @@ impl Sha1 {
 }
 
 impl Digest for Sha1 {
-    pub fn reset(&mut self) {
+    fn reset(&mut self) {
         self.length_bits = 0;
         self.h[0] = 0x67452301u32;
         self.h[1] = 0xEFCDAB89u32;
@@ -169,9 +169,9 @@ impl Digest for Sha1 {
         self.buffer.reset();
         self.computed = false;
     }
-    pub fn input(&mut self, msg: &[u8]) { add_input(self, msg); }
-    pub fn result(&mut self, out: &mut [u8]) { return mk_result(self, out); }
-    pub fn output_bits(&self) -> uint { 160 }
+    fn input(&mut self, msg: &[u8]) { add_input(self, msg); }
+    fn result(&mut self, out: &mut [u8]) { return mk_result(self, out); }
+    fn output_bits(&self) -> uint { 160 }
 }
 
 #[cfg(test)]

src/libextra/enum_set.rs

@@ -21,9 +21,9 @@ pub struct EnumSet<E> {
 /// An iterface for casting C-like enum to uint and back.
 pub trait CLike {
     /// Converts C-like enum to uint.
-    pub fn to_uint(&self) -> uint;
+    fn to_uint(&self) -> uint;
     /// Converts uint to C-like enum.
-    pub fn from_uint(uint) -> Self;
+    fn from_uint(uint) -> Self;
 }
 
 fn bit<E:CLike>(e: E) -> uint {
@@ -142,11 +142,11 @@ mod test {
     }
 
     impl CLike for Foo {
-        pub fn to_uint(&self) -> uint {
+        fn to_uint(&self) -> uint {
             *self as uint
         }
 
-        pub fn from_uint(v: uint) -> Foo {
+        fn from_uint(v: uint) -> Foo {
             unsafe { cast::transmute(v) }
         }
     }

src/libextra/num/bigint.rs

@@ -537,7 +537,7 @@ impl ToStrRadix for BigUint {
 impl FromStrRadix for BigUint {
     /// Creates and initializes an BigUint.
 
-    pub fn from_str_radix(s: &str, radix: uint)
+    fn from_str_radix(s: &str, radix: uint)
         -> Option<BigUint> {
         BigUint::parse_bytes(s.as_bytes(), radix)
     }

src/libextra/test.rs

@@ -104,7 +104,7 @@ pub struct Metric {
 pub struct MetricMap(TreeMap<~str,Metric>);
 
 impl Clone for MetricMap {
-    pub fn clone(&self) -> MetricMap {
+    fn clone(&self) -> MetricMap {
         MetricMap((**self).clone())
     }
 }

src/libextra/treemap.rs

@@ -895,7 +895,7 @@ impl<K: TotalOrd, V, T: Iterator<(K, V)>> Extendable<(K, V), T> for TreeMap<K, V
 }
 
 impl<T: TotalOrd, Iter: Iterator<T>> FromIterator<T, Iter> for TreeSet<T> {
-    pub fn from_iterator(iter: &mut Iter) -> TreeSet<T> {
+    fn from_iterator(iter: &mut Iter) -> TreeSet<T> {
         let mut set = TreeSet::new();
         set.extend(iter);
         set

src/libextra/url.rs

@@ -701,7 +701,7 @@ pub fn to_str(url: &Url) -> ~str {
 }
 
 impl ToStr for Url {
-    pub fn to_str(&self) -> ~str {
+    fn to_str(&self) -> ~str {
         to_str(self)
     }
 }

src/librustc/middle/privacy.rs

@@ -351,6 +351,7 @@ pub fn check_crate<'mm>(tcx: ty::ctxt,
             // Do not check privacy inside items with the resolve_unexported
             // attribute. This is used for the test runner.
             if !attr::contains_name(item.attrs, "!resolve_unexported") {
+                check_sane_privacy(tcx, item);
                 oldvisit::visit_item(item, (method_map, visitor));
             }
         },
@@ -539,3 +540,81 @@ pub fn check_crate<'mm>(tcx: ty::ctxt,
     });
     oldvisit::visit_crate(crate, (method_map, visitor));
 }
+
+/// Validates all of the visibility qualifers placed on the item given. This
+/// ensures that there are no extraneous qualifiers that don't actually do
+/// anything. In theory these qualifiers wouldn't parse, but that may happen
+/// later on down the road...
+fn check_sane_privacy(tcx: ty::ctxt, item: @ast::item) {
+    match item.node {
+        // implementations of traits don't need visibility qualifiers because
+        // that's controlled by having the trait in scope.
+        ast::item_impl(_, Some(*), _, ref methods) => {
+            for m in methods.iter() {
+                match m.vis {
+                    ast::private | ast::public => {
+                        tcx.sess.span_err(m.span, "unnecessary visibility")
+                    }
+                    ast::inherited => {}
+                }
+            }
+        }
+
+        ast::item_enum(ref def, _) => {
+            for v in def.variants.iter() {
+                match v.node.vis {
+                    ast::public => {
+                        if item.vis == ast::public {
+                            tcx.sess.span_err(v.span, "unnecessary `pub` \
+                                                       visibility");
+                        }
+                    }
+                    ast::private => {
+                        if item.vis != ast::public {
+                            tcx.sess.span_err(v.span, "unnecessary `priv` \
+                                                       visibility");
+                        }
+                    }
+                    ast::inherited => {}
+                }
+            }
+        }
+
+        ast::item_struct(ref def, _) => {
+            for f in def.fields.iter() {
+                match f.node.kind {
+                    ast::named_field(_, ast::public) => {
+                        tcx.sess.span_err(f.span, "unnecessary `pub` \
+                                                   visibility");
+                    }
+                    ast::named_field(_, ast::private) => {
+                        // Fields should really be private by default...
+                    }
+                    ast::named_field(*) | ast::unnamed_field => {}
+                }
+            }
+        }
+
+        ast::item_trait(_, _, ref methods) => {
+            for m in methods.iter() {
+                match *m {
+                    ast::provided(ref m) => {
+                        match m.vis {
+                            ast::private | ast::public => {
+                                tcx.sess.span_err(m.span, "unnecessary \
+                                                           visibility");
+                            }
+                            ast::inherited => {}
+                        }
+                    }
+                    // this is warned about in the parser
+                    ast::required(*) => {}
+                }
+            }
+        }
+
+        ast::item_impl(*) | ast::item_static(*) | ast::item_foreign_mod(*) |
+        ast::item_fn(*) | ast::item_mod(*) | ast::item_ty(*) |
+        ast::item_mac(*) => {}
+    }
+}

src/librustc/middle/trans/base.rs

@@ -165,7 +165,7 @@ impl<'self> StatRecorder<'self> {
 
 #[unsafe_destructor]
 impl<'self> Drop for StatRecorder<'self> {
-    pub fn drop(&self) {
+    fn drop(&self) {
         if self.ccx.sess.trans_stats() {
             let end = time::precise_time_ns();
             let elapsed = ((end - self.start) / 1_000_000) as uint;

src/librustc/middle/ty.rs

@@ -706,10 +706,10 @@ pub fn AllBuiltinBounds() -> BuiltinBounds {
 }
 
 impl CLike for BuiltinBound {
-    pub fn to_uint(&self) -> uint {
+    fn to_uint(&self) -> uint {
         *self as uint
     }
-    pub fn from_uint(v: uint) -> BuiltinBound {
+    fn from_uint(v: uint) -> BuiltinBound {
         unsafe { cast::transmute(v) }
     }
 }
@@ -4315,16 +4315,16 @@ pub fn normalize_ty(cx: ctxt, t: t) -> t {
 }
 
 pub trait ExprTyProvider {
-    pub fn expr_ty(&self, ex: &ast::expr) -> t;
-    pub fn ty_ctxt(&self) -> ctxt;
+    fn expr_ty(&self, ex: &ast::expr) -> t;
+    fn ty_ctxt(&self) -> ctxt;
 }
 
 impl ExprTyProvider for ctxt {
-    pub fn expr_ty(&self, ex: &ast::expr) -> t {
+    fn expr_ty(&self, ex: &ast::expr) -> t {
         expr_ty(*self, ex)
     }
 
-    pub fn ty_ctxt(&self) -> ctxt {
+    fn ty_ctxt(&self) -> ctxt {
         *self
     }
 }

src/librustc/middle/typeck/check/mod.rs

@@ -287,11 +287,11 @@ pub fn blank_fn_ctxt(ccx: @mut CrateCtxt,
 }
 
 impl ExprTyProvider for FnCtxt {
-    pub fn expr_ty(&self, ex: &ast::expr) -> ty::t {
+    fn expr_ty(&self, ex: &ast::expr) -> ty::t {
         self.expr_ty(ex)
     }
 
-    pub fn ty_ctxt(&self) -> ty::ctxt {
+    fn ty_ctxt(&self) -> ty::ctxt {
         self.ccx.tcx
     }
 }

src/librustc/middle/typeck/infer/error_reporting.rs

@@ -76,12 +76,12 @@ use util::ppaux::UserString;
 use util::ppaux::note_and_explain_region;
 
 pub trait ErrorReporting {
-    pub fn report_region_errors(@mut self,
-                                errors: &OptVec<RegionResolutionError>);
+    fn report_region_errors(@mut self,
+                            errors: &OptVec<RegionResolutionError>);
 
-    pub fn report_and_explain_type_error(@mut self,
-                                         trace: TypeTrace,
-                                         terr: &ty::type_err);
+    fn report_and_explain_type_error(@mut self,
+                                     trace: TypeTrace,
+                                     terr: &ty::type_err);
 
     fn values_str(@mut self, values: &ValuePairs) -> Option<~str>;
 
@@ -112,8 +112,8 @@ pub trait ErrorReporting {
 
 
 impl ErrorReporting for InferCtxt {
-    pub fn report_region_errors(@mut self,
-                                errors: &OptVec<RegionResolutionError>) {
+    fn report_region_errors(@mut self,
+                            errors: &OptVec<RegionResolutionError>) {
         for error in errors.iter() {
             match *error {
                 ConcreteFailure(origin, sub, sup) => {
@@ -139,7 +139,7 @@ impl ErrorReporting for InferCtxt {
         }
     }
 
-    pub fn report_and_explain_type_error(@mut self,
+    fn report_and_explain_type_error(@mut self,
                                      trace: TypeTrace,
                                      terr: &ty::type_err) {
         let tcx = self.tcx;

src/librustc/middle/typeck/mod.rs

@@ -294,11 +294,11 @@ trait get_and_find_region {
 }
 
 impl get_and_find_region for isr_alist {
-    pub fn get(&self, br: ty::bound_region) -> ty::Region {
+    fn get(&self, br: ty::bound_region) -> ty::Region {
         self.find(br).unwrap()
     }
 
-    pub fn find(&self, br: ty::bound_region) -> Option<ty::Region> {
+    fn find(&self, br: ty::bound_region) -> Option<ty::Region> {
         let mut ret = None;
         do list::each(*self) |isr| {
             let (isr_br, isr_r) = *isr;

src/libstd/num/num.rs

@@ -413,11 +413,11 @@ impl_num_cast!(f64,   to_f64)
 impl_num_cast!(float, to_float)
 
 pub trait ToStrRadix {
-    pub fn to_str_radix(&self, radix: uint) -> ~str;
+    fn to_str_radix(&self, radix: uint) -> ~str;
 }
 
 pub trait FromStrRadix {
-    pub fn from_str_radix(str: &str, radix: uint) -> Option<Self>;
+    fn from_str_radix(str: &str, radix: uint) -> Option<Self>;
 }
 
 /// Calculates a power to a given radix, optimized for uint `pow` and `radix`.

src/libstd/ptr.rs

@@ -416,7 +416,7 @@ impl<T, I: Int> Add<I, *T> for *T {
     /// Add an integer value to a pointer to get an offset pointer.
     /// Is calculated according to the size of the type pointed to.
     #[inline]
-    pub fn add(&self, rhs: &I) -> *T {
+    fn add(&self, rhs: &I) -> *T {
         self.offset(rhs.to_int() as int)
     }
 }
@@ -426,7 +426,7 @@ impl<T, I: Int> Sub<I, *T> for *T {
     /// Subtract an integer value from a pointer to get an offset pointer.
     /// Is calculated according to the size of the type pointed to.
     #[inline]
-    pub fn sub(&self, rhs: &I) -> *T {
+    fn sub(&self, rhs: &I) -> *T {
         self.offset(-rhs.to_int() as int)
     }
 }
@@ -436,7 +436,7 @@ impl<T, I: Int> Add<I, *mut T> for *mut T {
     /// Add an integer value to a pointer to get an offset pointer.
     /// Is calculated according to the size of the type pointed to.
     #[inline]
-    pub fn add(&self, rhs: &I) -> *mut T {
+    fn add(&self, rhs: &I) -> *mut T {
         self.offset(rhs.to_int() as int)
     }
 }
@@ -446,7 +446,7 @@ impl<T, I: Int> Sub<I, *mut T> for *mut T {
     /// Subtract an integer value from a pointer to get an offset pointer.
     /// Is calculated according to the size of the type pointed to.
     #[inline]
-    pub fn sub(&self, rhs: &I) -> *mut T {
+    fn sub(&self, rhs: &I) -> *mut T {
         self.offset(-rhs.to_int() as int)
     }
 }

src/libstd/rand.rs

@@ -260,7 +260,7 @@ pub mod rustrt {
 /// A random number generator
 pub trait Rng {
     /// Return the next random integer
-    pub fn next(&mut self) -> u32;
+    fn next(&mut self) -> u32;
 }
 
 /// A value with a particular weight compared to other values
@@ -825,7 +825,7 @@ pub struct XorShiftRng {
 
 impl Rng for XorShiftRng {
     #[inline]
-    pub fn next(&mut self) -> u32 {
+    fn next(&mut self) -> u32 {
         let x = self.x;
         let t = x ^ (x << 11);
         self.x = self.y;

src/libstd/rt/io/comm_adapters.rs

@@ -31,9 +31,9 @@ impl<C: GenericChan<~[u8]>> ChanWriter<C> {
 }
 
 impl<C: GenericChan<~[u8]>> Writer for ChanWriter<C> {
-    pub fn write(&mut self, _buf: &[u8]) { fail!() }
+    fn write(&mut self, _buf: &[u8]) { fail!() }
 
-    pub fn flush(&mut self) { fail!() }
+    fn flush(&mut self) { fail!() }
 }
 
 struct ReaderPort<R>;