Preserve and format type aliases in extern blocks

Previously, non-trivial type aliases in extern blocks were dropped by
rustfmt because only the type alias name would be passed to a rewritter.
This commit fixes that by passing all type information (generics,
bounds, and assignments) to a type alias rewritter, and consolidates
`rewrite_type_alias` and `rewrite_associated_type` as one function.

Closes rust-lang#4159

Author: ayazhafiz

rustfmt-core/rustfmt-lib/src/items.rs

@@ -1562,6 +1562,7 @@ fn rewrite_type_prefix(
     prefix: &str,
     ident: ast::Ident,
     generics: &ast::Generics,
+    generic_bounds_opt: Option<&ast::GenericBounds>,
 ) -> Option<String> {
     let mut result = String::with_capacity(128);
     result.push_str(prefix);
@@ -1578,6 +1579,19 @@ fn rewrite_type_prefix(
         result.push_str(&generics_str);
     }
 
+    let type_bounds_str = if let Some(bounds) = generic_bounds_opt {
+        if bounds.is_empty() {
+            String::new()
+        } else {
+            // 2 = `: `
+            let shape = Shape::indented(indent, context.config).offset_left(result.len() + 2)?;
+            bounds.rewrite(context, shape).map(|s| format!(": {}", s))?
+        }
+    } else {
+        String::new()
+    };
+    result.push_str(&type_bounds_str);
+
     let where_budget = context.budget(last_line_width(&result));
     let option = WhereClauseOption::snuggled(&result);
     let where_clause_str = rewrite_where_clause(
@@ -1604,6 +1618,7 @@ fn rewrite_type_item<R: Rewrite>(
     ident: ast::Ident,
     rhs: &R,
     generics: &ast::Generics,
+    generic_bounds_opt: Option<&ast::GenericBounds>,
     vis: &ast::Visibility,
 ) -> Option<String> {
     let mut result = String::with_capacity(128);
@@ -1613,6 +1628,7 @@ fn rewrite_type_item<R: Rewrite>(
         &format!("{}{} ", format_visibility(context, vis), prefix),
         ident,
         generics,
+        generic_bounds_opt,
     )?);
 
     if generics.where_clause.predicates.is_empty() {
@@ -1627,17 +1643,6 @@ fn rewrite_type_item<R: Rewrite>(
     rewrite_assign_rhs(context, result, rhs, rhs_shape).map(|s| s + ";")
 }
 
-pub(crate) fn rewrite_type_alias(
-    context: &RewriteContext<'_>,
-    indent: Indent,
-    ident: ast::Ident,
-    ty: &ast::Ty,
-    generics: &ast::Generics,
-    vis: &ast::Visibility,
-) -> Option<String> {
-    rewrite_type_item(context, indent, "type", " =", ident, ty, generics, vis)
-}
-
 pub(crate) fn rewrite_opaque_type(
     context: &RewriteContext<'_>,
     indent: Indent,
@@ -1655,6 +1660,7 @@ pub(crate) fn rewrite_opaque_type(
         ident,
         &opaque_type_bounds,
         generics,
+        Some(generic_bounds),
         vis,
     )
 }
@@ -1897,39 +1903,39 @@ fn rewrite_static(
     }
 }
 
-pub(crate) fn rewrite_associated_type(
+pub(crate) fn rewrite_type_alias(
     ident: ast::Ident,
     ty_opt: Option<&ptr::P<ast::Ty>>,
     generics: &ast::Generics,
     generic_bounds_opt: Option<&ast::GenericBounds>,
     context: &RewriteContext<'_>,
     indent: Indent,
+    vis: &ast::Visibility,
 ) -> Option<String> {
-    let ident_str = rewrite_ident(context, ident);
-    // 5 = "type "
-    let generics_shape = Shape::indented(indent, context.config).offset_left(5)?;
-    let generics_str = rewrite_generics(context, ident_str, generics, generics_shape)?;
-    let prefix = format!("type {}", generics_str);
-
-    let type_bounds_str = if let Some(bounds) = generic_bounds_opt {
-        if bounds.is_empty() {
-            String::new()
-        } else {
-            // 2 = ": ".len()
-            let shape = Shape::indented(indent, context.config).offset_left(prefix.len() + 2)?;
-            bounds.rewrite(context, shape).map(|s| format!(": {}", s))?
-        }
-    } else {
-        String::new()
-    };
+    let mut prefix = rewrite_type_prefix(
+        context,
+        indent,
+        &format!("{}type ", format_visibility(context, vis)),
+        ident,
+        generics,
+        generic_bounds_opt,
+    )?;
 
     if let Some(ty) = ty_opt {
         // 1 = `;`
         let shape = Shape::indented(indent, context.config).sub_width(1)?;
-        let lhs = format!("{}{} =", prefix, type_bounds_str);
+
+        // If there's a where clause, add a newline before the assignment. Otherwise just add a
+        // space.
+        if !generics.where_clause.predicates.is_empty() {
+            prefix.push_str(&indent.to_string_with_newline(context.config));
+        } else {
+            prefix.push(' ');
+        }
+        let lhs = format!("{}=", prefix);
         rewrite_assign_rhs(context, lhs, &**ty, shape).map(|s| s + ";")
     } else {
-        Some(format!("{}{};", prefix, type_bounds_str))
+        Some(format!("{};", prefix))
     }
 }
 
@@ -1973,13 +1979,14 @@ pub(crate) fn rewrite_opaque_impl_type(
 
 pub(crate) fn rewrite_associated_impl_type(
     ident: ast::Ident,
+    vis: &ast::Visibility,
     defaultness: ast::Defaultness,
     ty_opt: Option<&ptr::P<ast::Ty>>,
     generics: &ast::Generics,
     context: &RewriteContext<'_>,
     indent: Indent,
 ) -> Option<String> {
-    let result = rewrite_associated_type(ident, ty_opt, generics, None, context, indent)?;
+    let result = rewrite_type_alias(ident, ty_opt, generics, None, context, indent, vis)?;
 
     match defaultness {
         ast::Defaultness::Default(..) => Some(format!("default {}", result)),
@@ -3132,7 +3139,7 @@ impl Rewrite for ast::ForeignItem {
         // FIXME: this may be a faulty span from libsyntax.
         let span = mk_sp(self.span.lo(), self.span.hi() - BytePos(1));
 
-        let item_str = match self.kind {
+        let item_str: String = match self.kind {
             ast::ForeignItemKind::Fn(_, ref fn_sig, ref generics, _) => rewrite_fn_base(
                 context,
                 shape.indent,
@@ -3156,14 +3163,20 @@ impl Rewrite for ast::ForeignItem {
                 // 1 = ;
                 rewrite_assign_rhs(context, prefix, &**ty, shape.sub_width(1)?).map(|s| s + ";")
             }
-            ast::ForeignItemKind::TyAlias(..) => {
-                let vis = format_visibility(context, &self.vis);
-                Some(format!(
-                    "{}type {};",
-                    vis,
-                    rewrite_ident(context, self.ident)
-                ))
-            }
+            ast::ForeignItemKind::TyAlias(
+                _,
+                ref generics,
+                ref generic_bounds,
+                ref type_default,
+            ) => rewrite_type_alias(
+                self.ident,
+                type_default.as_ref(),
+                generics,
+                Some(generic_bounds),
+                &context,
+                shape.indent,
+                &self.vis,
+            ),
             ast::ForeignItemKind::MacCall(ref mac) => {
                 rewrite_macro(mac, None, context, shape, MacroPosition::Item)
             }

rustfmt-core/rustfmt-lib/src/visitor.rs

@@ -9,9 +9,8 @@ use crate::comment::{rewrite_comment, CodeCharKind, CommentCodeSlices};
 use crate::config::{BraceStyle, Config};
 use crate::items::{
     format_impl, format_trait, format_trait_alias, is_mod_decl, is_use_item,
-    rewrite_associated_impl_type, rewrite_associated_type, rewrite_extern_crate,
-    rewrite_opaque_impl_type, rewrite_opaque_type, rewrite_type_alias, FnBraceStyle, FnSig,
-    StaticParts, StructParts,
+    rewrite_associated_impl_type, rewrite_extern_crate, rewrite_opaque_impl_type,
+    rewrite_opaque_type, rewrite_type_alias, FnBraceStyle, FnSig, StaticParts, StructParts,
 };
 use crate::macros::{macro_style, rewrite_macro, rewrite_macro_def, MacroPosition};
 use crate::rewrite::{Rewrite, RewriteContext};
@@ -538,11 +537,12 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 ast::ItemKind::TyAlias(_, ref generics, ref generic_bounds, ref ty) => match ty {
                     Some(ty) => {
                         let rewrite = rewrite_type_alias(
-                            &self.get_context(),
-                            self.block_indent,
                             item.ident,
-                            &*ty,
+                            Some(&*ty),
                             generics,
+                            Some(generic_bounds),
+                            &self.get_context(),
+                            self.block_indent,
                             &item.vis,
                         );
                         self.push_rewrite(item.span, rewrite);
@@ -609,13 +609,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 );
             }
             ast::AssocItemKind::TyAlias(_, ref generics, ref generic_bounds, ref type_default) => {
-                let rewrite = rewrite_associated_type(
+                let rewrite = rewrite_type_alias(
                     ti.ident,
                     type_default.as_ref(),
                     generics,
                     Some(generic_bounds),
                     &self.get_context(),
                     self.block_indent,
+                    &ti.vis,
                 );
                 self.push_rewrite(ti.span, rewrite);
             }
@@ -656,6 +657,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
                 let rewrite_associated = || {
                     rewrite_associated_impl_type(
                         ii.ident,
+                        &ii.vis,
                         defaultness,
                         ty.as_ref(),
                         generics,

rustfmt-core/rustfmt-lib/tests/target/issue-4159.rs

@@ -0,0 +1,18 @@
+extern "C" {
+    type A: Ord;
+
+    type A<'a>
+    where
+        'a: 'static,;
+
+    type A<T: Ord>
+    where
+        T: 'static,;
+
+    type A = u8;
+
+    type A<'a: 'static, T: Ord + 'static>: Eq + PartialEq
+    where
+        T: 'static + Copy,
+    = Vec<u8>;
+}