Handle pub(restricted) (#1013)

* Handle pub(restricted)

This commit properly handles pub(restricted) as introduced in RFC 1422
[0]. The syntax support was added in #971, but they were not correctly
formatted.

[0] https://github.com/rust-lang/rfcs/blob/master/text/1422-pub-restricted.md

Fixes #970

* Drop #[inline] attribute on format_visibility

* Make newly non-failing functions return String

The change to `format_visibiilty` means that `format_header` and
`format_unit_struct` can no longer fail. Their return type is updated to
reflect that.

Cargo.lock

@@ -5,6 +5,7 @@ dependencies = [
  "diff 0.1.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.4.15 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex 0.1.71 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -47,6 +48,11 @@ name = "getopts"
 version = "0.2.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "itertools"
+version = "0.4.15"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "kernel32-sys"
 version = "0.2.2"

Cargo.toml

@@ -25,3 +25,4 @@ syntex_syntax = "0.32"
 log = "0.3"
 env_logger = "0.3"
 getopts = "0.2"
+itertools = "0.4.15"

src/items.rs

@@ -139,10 +139,7 @@ impl<'a> FmtVisitor<'a> {
             ast::ForeignItemKind::Static(ref ty, is_mutable) => {
                 // FIXME(#21): we're dropping potential comments in between the
                 // function keywords here.
-                let vis = match format_visibility(&item.vis) {
-                    Some(s) => s,
-                    None => return,
-                };
+                let vis = format_visibility(&item.vis);
                 let mut_str = if is_mutable {
                     "mut "
                 } else {
@@ -305,11 +302,7 @@ impl<'a> FmtVisitor<'a> {
                       enum_def: &ast::EnumDef,
                       generics: &ast::Generics,
                       span: Span) {
-        let header_str = match format_header("enum ", ident, vis) {
-            Some(s) => s,
-            None => return,
-        };
-        self.buffer.push_str(&header_str);
+        self.buffer.push_str(&format_header("enum ", ident, vis));
 
         let enum_snippet = self.snippet(span);
         let body_start = span.lo + BytePos(enum_snippet.find_uncommented("{").unwrap() as u32 + 1);
@@ -453,7 +446,7 @@ pub fn format_impl(context: &RewriteContext, item: &ast::Item, offset: Indent) -
                                ref self_ty,
                                ref items) = item.node {
         let mut result = String::new();
-        result.push_str(try_opt!(format_visibility(&item.vis)));
+        result.push_str(&*format_visibility(&item.vis));
         result.push_str(format_unsafety(unsafety));
         result.push_str("impl");
 
@@ -593,7 +586,7 @@ pub fn format_struct(context: &RewriteContext,
                      one_line_width: Option<usize>)
                      -> Option<String> {
     match *struct_def {
-        ast::VariantData::Unit(..) => format_unit_struct(item_name, ident, vis),
+        ast::VariantData::Unit(..) => Some(format_unit_struct(item_name, ident, vis)),
         ast::VariantData::Tuple(ref fields, _) => {
             format_tuple_struct(context,
                                 item_name,
@@ -623,7 +616,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent)
            item.node {
         let mut result = String::new();
         let header = format!("{}{}trait {}",
-                             try_opt!(format_visibility(&item.vis)),
+                             format_visibility(&item.vis),
                              format_unsafety(unsafety),
                              item.ident);
 
@@ -744,14 +737,8 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent)
     }
 }
 
-fn format_unit_struct(item_name: &str, ident: ast::Ident, vis: &ast::Visibility) -> Option<String> {
-    let mut result = String::with_capacity(1024);
-
-    let header_str = try_opt!(format_header(item_name, ident, vis));
-    result.push_str(&header_str);
-    result.push(';');
-
-    Some(result)
+fn format_unit_struct(item_name: &str, ident: ast::Ident, vis: &ast::Visibility) -> String {
+    format!("{};", format_header(item_name, ident, vis))
 }
 
 fn format_struct_struct(context: &RewriteContext,
@@ -766,7 +753,7 @@ fn format_struct_struct(context: &RewriteContext,
                         -> Option<String> {
     let mut result = String::with_capacity(1024);
 
-    let header_str = try_opt!(format_header(item_name, ident, vis));
+    let header_str = format_header(item_name, ident, vis);
     result.push_str(&header_str);
 
     let body_lo = context.codemap.span_after(span, "{");
@@ -859,7 +846,7 @@ fn format_tuple_struct(context: &RewriteContext,
                        -> Option<String> {
     let mut result = String::with_capacity(1024);
 
-    let header_str = try_opt!(format_header(item_name, ident, vis));
+    let header_str = format_header(item_name, ident, vis);
     result.push_str(&header_str);
 
     // FIXME(#919): don't lose comments on empty tuple structs.
@@ -945,7 +932,7 @@ pub fn rewrite_type_alias(context: &RewriteContext,
                           -> Option<String> {
     let mut result = String::new();
 
-    result.push_str(&try_opt!(format_visibility(&vis)));
+    result.push_str(&format_visibility(&vis));
     result.push_str("type ");
     result.push_str(&ident.to_string());
 
@@ -1013,7 +1000,7 @@ impl Rewrite for ast::StructField {
         }
 
         let name = self.ident;
-        let vis = try_opt!(format_visibility(&self.vis));
+        let vis = format_visibility(&self.vis);
         let mut attr_str = try_opt!(self.attrs
             .rewrite(context, context.config.max_width - offset.width(), offset));
         if !attr_str.is_empty() {
@@ -1042,7 +1029,7 @@ pub fn rewrite_static(prefix: &str,
                       context: &RewriteContext)
                       -> Option<String> {
     let prefix = format!("{}{} {}{}: ",
-                         try_opt!(format_visibility(vis)),
+                         format_visibility(vis),
                          prefix,
                          format_mutability(mutability),
                          ident);
@@ -1260,7 +1247,7 @@ fn rewrite_fn_base(context: &RewriteContext,
 
     let mut result = String::with_capacity(1024);
     // Vis unsafety abi.
-    result.push_str(try_opt!(format_visibility(vis)));
+    result.push_str(&*format_visibility(vis));
 
     if let ast::Constness::Const = constness {
         result.push_str("const ");
@@ -1816,8 +1803,8 @@ fn rewrite_where_clause(context: &RewriteContext,
     }
 }
 
-fn format_header(item_name: &str, ident: ast::Ident, vis: &ast::Visibility) -> Option<String> {
-    Some(format!("{}{}{}", try_opt!(format_visibility(vis)), item_name, ident))
+fn format_header(item_name: &str, ident: ast::Ident, vis: &ast::Visibility) -> String {
+    format!("{}{}{}", format_visibility(vis), item_name, ident)
 }
 
 fn format_generics(context: &RewriteContext,

src/lib.rs

@@ -24,6 +24,7 @@ extern crate unicode_segmentation;
 extern crate regex;
 extern crate diff;
 extern crate term;
+extern crate itertools;
 
 use syntax::ast;
 use syntax::codemap::{mk_sp, CodeMap, Span};

src/utils.rs

@@ -8,9 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use std::borrow::Cow;
 use std::cmp::Ordering;
 
-use syntax::ast::{self, Visibility, Attribute, MetaItem, MetaItemKind};
+use itertools::Itertools;
+
+use syntax::ast::{self, Visibility, Attribute, MetaItem, MetaItemKind, Path};
 use syntax::codemap::{CodeMap, Span, BytePos};
 use syntax::abi;
 
@@ -66,14 +69,23 @@ pub fn extra_offset(text: &str, offset: Indent) -> usize {
     }
 }
 
-#[inline]
-pub fn format_visibility(vis: &Visibility) -> Option<&'static str> {
+// Uses Cow to avoid allocating in the common cases.
+pub fn format_visibility(vis: &Visibility) -> Cow<'static, str> {
     match *vis {
-        Visibility::Public => Some("pub "),
-        Visibility::Inherited => Some(""),
-        // FIXME(#970): Handle new visibility types.
-        Visibility::Crate(_) => None,
-        Visibility::Restricted { .. } => None,
+        Visibility::Public => Cow::from("pub "),
+        Visibility::Inherited => Cow::from(""),
+        Visibility::Crate(_) => Cow::from("pub(crate) "),
+        Visibility::Restricted { ref path, .. } => {
+            let Path { global, ref segments, .. } = **path;
+            let prefix = if global {
+                "::"
+            } else {
+                ""
+            };
+            let mut segments_iter = segments.iter().map(|seg| seg.identifier.name.as_str());
+
+            Cow::from(format!("pub({}{}) ", prefix, segments_iter.join("::")))
+        }
     }
 }
 

src/visitor.rs

@@ -502,9 +502,7 @@ impl<'a> FmtVisitor<'a> {
         let local_file_name = self.codemap.span_to_filename(s);
         let is_internal = local_file_name == self.codemap.span_to_filename(source!(self, m.inner));
 
-        if let Some(vis) = utils::format_visibility(vis) {
-            self.buffer.push_str(vis);
-        }
+        self.buffer.push_str(&*utils::format_visibility(vis));
         self.buffer.push_str("mod ");
         self.buffer.push_str(&ident.to_string());
 
@@ -540,10 +538,7 @@ impl<'a> FmtVisitor<'a> {
     }
 
     fn format_import(&mut self, vis: &ast::Visibility, vp: &ast::ViewPath, span: Span) {
-        let vis = match utils::format_visibility(vis) {
-            Some(s) => s,
-            None => return,
-        };
+        let vis = utils::format_visibility(vis);
         let mut offset = self.block_indent;
         offset.alignment += vis.len() + "use ".len();
         // 1 = ";"

tests/source/pub-restricted.rs

@@ -0,0 +1,51 @@
+pub(   super   ) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}
+
+pub(  crate  ) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}
+
+pub(  ::global::  path :: to::some_mod  ) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}
+
+pub(  local::  path :: to::some_mod  ) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}

tests/target/pub-restricted.rs

@@ -0,0 +1,51 @@
+pub(super) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}
+
+pub(crate) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}
+
+pub(::global::path::to::some_mod) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}
+
+pub(local::path::to::some_mod) enum WriteState<D> {
+    WriteId {
+        id: U64Writer,
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteSize {
+        size: U64Writer,
+        payload: Option<Writer<D>>,
+    },
+    WriteData(Writer<D>),
+}