Repository: rust-lang/rustfmt (rust)

Issue:
Rustfmt loses visibility on some trait items and impl items
As of current master, rustfmt applies the following incorrect diff to this valid Rust code.

```diff
 #[cfg(any())]
 trait Visible {
-    pub const C: i32;
+    const C: i32;
     pub type T;
-    pub fn f();
+    fn f();
-    pub fn g() {}
+    fn g() {}
 }

 #[cfg(any())]
 impl Visible for Bar {
     pub const C: i32;
     pub type T;
-    pub fn f();
+    fn f();
     pub fn g() {}
 }
```

To reproduce: `cargo run --bin rustfmt repro.rs`

Pull request:
Preserve visibility on trait items
Fixes #4960.

Patch:
diff --git a/src/items.rs b/src/items.rs
index 1a56acc7..4fa7190c 100644
--- a/src/items.rs
+++ b/src/items.rs
@@ -168,14 +168,14 @@ pub(crate) struct FnSig<'a> {
     constness: ast::Const,
     defaultness: ast::Defaultness,
     unsafety: ast::Unsafe,
-    visibility: ast::Visibility,
+    visibility: &'a ast::Visibility,
 }
 
 impl<'a> FnSig<'a> {
     pub(crate) fn from_method_sig(
         method_sig: &'a ast::FnSig,
         generics: &'a ast::Generics,
-        visibility: ast::Visibility,
+        visibility: &'a ast::Visibility,
     ) -> FnSig<'a> {
         FnSig {
             unsafety: method_sig.header.unsafety,
@@ -198,7 +198,7 @@ impl<'a> FnSig<'a> {
         match *fn_kind {
             visit::FnKind::Fn(fn_ctxt, _, fn_sig, vis, _) => match fn_ctxt {
                 visit::FnCtxt::Assoc(..) => {
-                    let mut fn_sig = FnSig::from_method_sig(fn_sig, generics, vis.clone());
+                    let mut fn_sig = FnSig::from_method_sig(fn_sig, generics, vis);
                     fn_sig.defaultness = defaultness;
                     fn_sig
                 }
@@ -210,7 +210,7 @@ impl<'a> FnSig<'a> {
                     is_async: Cow::Borrowed(&fn_sig.header.asyncness),
                     defaultness,
                     unsafety: fn_sig.header.unsafety,
-                    visibility: vis.clone(),
+                    visibility: vis,
                 },
             },
             _ => unreachable!(),
@@ -317,6 +317,7 @@ impl<'a> FmtVisitor<'a> {
         indent: Indent,
         ident: symbol::Ident,
         sig: &ast::FnSig,
+        vis: &ast::Visibility,
         generics: &ast::Generics,
         span: Span,
     ) -> Option<String> {
@@ -328,7 +329,7 @@ impl<'a> FmtVisitor<'a> {
             &context,
             indent,
             ident,
-            &FnSig::from_method_sig(sig, generics, DEFAULT_VISIBILITY),
+            &FnSig::from_method_sig(sig, generics, vis),
             span,
             FnBraceStyle::None,
         )?;
@@ -1754,7 +1755,7 @@ impl<'a> StaticParts<'a> {
         };
         StaticParts {
             prefix: "const",
-            vis: &DEFAULT_VISIBILITY,
+            vis: &ti.vis,
             ident: ti.ident,
             ty,
             mutability: ast::Mutability::Not,
@@ -3110,7 +3111,7 @@ impl Rewrite for ast::ForeignItem {
                         context,
                         shape.indent,
                         self.ident,
-                        &FnSig::from_method_sig(&fn_sig, generics, self.vis.clone()),
+                        &FnSig::from_method_sig(&fn_sig, generics, &self.vis),
                         span,
                         FnBraceStyle::None,
                     )
diff --git a/src/visitor.rs b/src/visitor.rs
index 12a3281e..c67ebe79 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -3,7 +3,7 @@ use std::rc::Rc;
 
 use rustc_ast::{ast, token::DelimToken, visit, AstLike};
 use rustc_data_structures::sync::Lrc;
-use rustc_span::{symbol, BytePos, Pos, Span, DUMMY_SP};
+use rustc_span::{symbol, BytePos, Pos, Span};
 
 use crate::attr::*;
 use crate::comment::{contains_comment, rewrite_comment, CodeCharKind, CommentCodeSlices};
@@ -568,6 +568,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                             indent,
                             item.ident,
                             &fn_signature,
+                            &item.vis,
                             generics,
                             item.span,
                         );
@@ -641,14 +642,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 let ast::FnKind(defaultness, ref sig, ref generics, ref block) = **fn_kind;
                 if let Some(ref body) = block {
                     let inner_attrs = inner_attributes(&ti.attrs);
-                    let vis = ast::Visibility {
-                        kind: ast::VisibilityKind::Inherited,
-                        span: DUMMY_SP,
-                        tokens: None,
-                    };
                     let fn_ctxt = visit::FnCtxt::Assoc(visit::AssocCtxt::Trait);
                     self.visit_fn(
-                        visit::FnKind::Fn(fn_ctxt, ti.ident, sig, &vis, Some(body)),
+                        visit::FnKind::Fn(fn_ctxt, ti.ident, sig, &ti.vis, Some(body)),
                         generics,
                         &sig.decl,
                         ti.span,
@@ -658,7 +654,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 } else {
                     let indent = self.block_indent;
                     let rewrite =
-                        self.rewrite_required_fn(indent, ti.ident, sig, generics, ti.span);
+                        self.rewrite_required_fn(indent, ti.ident, sig, &ti.vis, generics, ti.span);
                     self.push_rewrite(ti.span, rewrite);
                 }
             }
@@ -708,7 +704,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 } else {
                     let indent = self.block_indent;
                     let rewrite =
-                        self.rewrite_required_fn(indent, ii.ident, sig, generics, ii.span);
+                        self.rewrite_required_fn(indent, ii.ident, sig, &ii.vis, generics, ii.span);
                     self.push_rewrite(ii.span, rewrite);
                 }
             }


Test patch:
diff --git a/tests/source/impls.rs b/tests/source/impls.rs
index fb870198..dcd1f0cd 100644
--- a/tests/source/impls.rs
+++ b/tests/source/impls.rs
@@ -13,6 +13,14 @@ pub impl Foo for Bar {
     // Comment 3
 }
 
+#[inherent]
+impl Visible for Bar {
+    pub const C: i32;
+    pub type T;
+    pub fn f();
+    pub fn g() {}
+}
+
 pub unsafe impl<'a, 'b, X, Y: Foo<Bar>> !Foo<'a, X> for Bar<'b, Y> where X: Foo<'a, Z> {
     fn foo() { "hi" }    
 }
diff --git a/tests/source/trait.rs b/tests/source/trait.rs
index 80ee0188..b6db9e15 100644
--- a/tests/source/trait.rs
+++ b/tests/source/trait.rs
@@ -174,3 +174,10 @@ Send
     + DDDDDDDD
     + DDDDDDDDD
     + EEEEEEE;
+
+trait Visible {
+    pub const C: i32;
+    pub type T;
+    pub fn f();
+    pub fn g() {}
+}
diff --git a/tests/target/impls.rs b/tests/target/impls.rs
index bf63f924..99e02990 100644
--- a/tests/target/impls.rs
+++ b/tests/target/impls.rs
@@ -21,6 +21,14 @@ pub impl Foo for Bar {
     // Comment 3
 }
 
+#[inherent]
+impl Visible for Bar {
+    pub const C: i32;
+    pub type T;
+    pub fn f();
+    pub fn g() {}
+}
+
 pub unsafe impl<'a, 'b, X, Y: Foo<Bar>> !Foo<'a, X> for Bar<'b, Y>
 where
     X: Foo<'a, Z>,
diff --git a/tests/target/trait.rs b/tests/target/trait.rs
index 620046a7..7f067991 100644
--- a/tests/target/trait.rs
+++ b/tests/target/trait.rs
@@ -211,3 +211,10 @@ where
         + DDDDDDDD
         + DDDDDDDDD
         + EEEEEEE;
+
+trait Visible {
+    pub const C: i32;
+    pub type T;
+    pub fn f();
+    pub fn g() {}
+}