Skip to content

Commit

Permalink
Preserve and format type aliases in extern blocks
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ayazhafiz authored and bradleypmartin committed May 25, 2020
1 parent 445f71e commit 5524752
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 48 deletions.
95 changes: 54 additions & 41 deletions rustfmt-core/rustfmt-lib/src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand All @@ -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);
Expand All @@ -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() {
Expand All @@ -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,
Expand All @@ -1655,6 +1660,7 @@ pub(crate) fn rewrite_opaque_type(
ident,
&opaque_type_bounds,
generics,
Some(generic_bounds),
vis,
)
}
Expand Down Expand Up @@ -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))
}
}

Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down
16 changes: 9 additions & 7 deletions rustfmt-core/rustfmt-lib/src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions rustfmt-core/rustfmt-lib/tests/target/issue-4159.rs
Original file line number Diff line number Diff line change
@@ -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>;
}

0 comments on commit 5524752

Please sign in to comment.