Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix json tuple struct enum variant #88391

Merged
merged 2 commits into from
Aug 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1703,12 +1703,28 @@ impl Clean<VariantStruct> for rustc_hir::VariantData<'_> {
}
}

impl Clean<Vec<Item>> for hir::VariantData<'_> {
fn clean(&self, cx: &mut DocContext<'_>) -> Vec<Item> {
self.fields().iter().map(|x| x.clean(cx)).collect()
}
}

impl Clean<Item> for ty::VariantDef {
fn clean(&self, cx: &mut DocContext<'_>) -> Item {
let kind = match self.ctor_kind {
CtorKind::Const => Variant::CLike,
CtorKind::Fn => Variant::Tuple(
self.fields.iter().map(|f| cx.tcx.type_of(f.did).clean(cx)).collect(),
self.fields
.iter()
.map(|field| {
let name = Some(field.ident.name);
let kind = StructFieldItem(cx.tcx.type_of(field.did).clean(cx));
let what_rustc_thinks =
Item::from_def_id_and_parts(field.did, name, kind, cx);
// don't show `pub` for fields, which are always public
Item { visibility: Visibility::Inherited, ..what_rustc_thinks }
})
.collect(),
),
CtorKind::Fictive => Variant::Struct(VariantStruct {
struct_type: CtorKind::Fictive,
Expand Down Expand Up @@ -1738,13 +1754,7 @@ impl Clean<Variant> for hir::VariantData<'_> {
fn clean(&self, cx: &mut DocContext<'_>) -> Variant {
match self {
hir::VariantData::Struct(..) => Variant::Struct(self.clean(cx)),
// Important note here: `Variant::Tuple` is used on tuple structs which are not in an
// enum (so where converting from `ty::VariantDef`). In case we are in an enum, the kind
// is provided by the `Variant` wrapper directly, and since we need the fields' name
// (even for a tuple struct variant!), it's simpler to just store it as a
// `Variant::Struct` instead of a `Variant::Tuple` (otherwise it would force us to make
// a lot of changes when rendering them to generate the name as well).
hir::VariantData::Tuple(..) => Variant::Struct(self.clean(cx)),
hir::VariantData::Tuple(..) => Variant::Tuple(self.clean(cx)),
hir::VariantData::Unit(..) => Variant::CLike,
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,7 @@ impl ItemKind {
StructItem(s) => s.fields.iter(),
UnionItem(u) => u.fields.iter(),
VariantItem(Variant::Struct(v)) => v.fields.iter(),
VariantItem(Variant::Tuple(v)) => v.iter(),
camelid marked this conversation as resolved.
Show resolved Hide resolved
EnumItem(e) => e.variants.iter(),
TraitItem(t) => t.items.iter(),
ImplItem(i) => i.items.iter(),
Expand Down Expand Up @@ -1937,7 +1938,7 @@ crate struct Enum {
#[derive(Clone, Debug)]
crate enum Variant {
CLike,
Tuple(Vec<Type>),
Tuple(Vec<Item>),
Struct(VariantStruct),
}

Expand Down
4 changes: 4 additions & 0 deletions src/librustdoc/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ crate trait DocFolder: Sized {
|| j.fields.iter().any(|f| f.is_stripped());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Self note (for future):] We can probably remove this part because variant fields always have inherited visibility.

VariantItem(Variant::Struct(j))
}
Variant::Tuple(fields) => {
let fields = fields.into_iter().filter_map(|x| self.fold_item(x)).collect();
VariantItem(Variant::Tuple(fields))
}
_ => VariantItem(i2),
}
}
Expand Down
41 changes: 24 additions & 17 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,19 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni
document_type_layout(w, cx, def_id);
}

fn print_tuple_struct_fields(w: &mut Buffer, cx: &Context<'_>, s: &[clean::Item]) {
for (i, ty) in s
.iter()
.map(|f| if let clean::StructFieldItem(ref ty) = *f.kind { ty } else { unreachable!() })
.enumerate()
{
if i > 0 {
w.write_str(",&nbsp;");
}
write!(w, "{}", ty.print(cx));
}
}

fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum) {
wrap_into_docblock(w, |w| {
wrap_item(w, "enum", |w| {
Expand Down Expand Up @@ -964,14 +977,9 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
match *v.kind {
clean::VariantItem(ref var) => match var {
clean::Variant::CLike => write!(w, "{}", name),
clean::Variant::Tuple(ref tys) => {
clean::Variant::Tuple(ref s) => {
write!(w, "{}(", name);
for (i, ty) in tys.iter().enumerate() {
if i > 0 {
w.write_str(",&nbsp;")
}
write!(w, "{}", ty.print(cx));
}
print_tuple_struct_fields(w, cx, s);
w.write_str(")");
}
clean::Variant::Struct(ref s) => {
Expand Down Expand Up @@ -1024,14 +1032,9 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
id = id,
name = variant.name.as_ref().unwrap()
);
if let clean::VariantItem(clean::Variant::Tuple(ref tys)) = *variant.kind {
if let clean::VariantItem(clean::Variant::Tuple(ref s)) = *variant.kind {
w.write_str("(");
for (i, ty) in tys.iter().enumerate() {
if i > 0 {
w.write_str(",&nbsp;");
}
write!(w, "{}", ty.print(cx));
}
print_tuple_struct_fields(w, cx, s);
w.write_str(")");
}
w.write_str("</code>");
Expand All @@ -1041,7 +1044,11 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
document_non_exhaustive(w, variant);

use crate::clean::Variant;
if let clean::VariantItem(Variant::Struct(ref s)) = *variant.kind {
if let Some((extra, fields)) = match *variant.kind {
clean::VariantItem(Variant::Struct(ref s)) => Some(("", &s.fields)),
clean::VariantItem(Variant::Tuple(ref fields)) => Some(("Tuple ", fields)),
_ => None,
} {
let variant_id = cx.derive_id(format!(
"{}.{}.fields",
ItemType::Variant,
Expand All @@ -1051,10 +1058,10 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum
write!(
w,
"<h3>{extra}Fields of <b>{name}</b></h3><div>",
extra = if s.struct_type == CtorKind::Fn { "Tuple " } else { "" },
extra = extra,
name = variant.name.as_ref().unwrap(),
);
for field in &s.fields {
for field in fields {
use crate::clean::StructFieldItem;
if let StructFieldItem(ref ty) = *field.kind {
let id = cx.derive_id(format!(
Expand Down
13 changes: 12 additions & 1 deletion src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,18 @@ impl FromWithTcx<clean::Variant> for Variant {
use clean::Variant::*;
match variant {
CLike => Variant::Plain,
Tuple(t) => Variant::Tuple(t.into_iter().map(|x| x.into_tcx(tcx)).collect()),
Tuple(fields) => Variant::Tuple(
fields
.into_iter()
.map(|f| {
if let clean::StructFieldItem(ty) = *f.kind {
ty.into_tcx(tcx)
} else {
unreachable!()
}
})
.collect(),
),
Struct(s) => Variant::Struct(ids(s.fields)),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/passes/stripper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ impl<'a> DocFolder for Stripper<'a> {

// implementations of traits are always public.
clean::ImplItem(ref imp) if imp.trait_.is_some() => true,
// Struct variant fields have inherited visibility
clean::VariantItem(clean::Variant::Struct(..)) => true,
// Variant fields have inherited visibility
clean::VariantItem(clean::Variant::Struct(..) | clean::Variant::Tuple(..)) => true,
Comment on lines +96 to +97
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like an unrelated—but still good—fix, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah indeed, didn't even pay attention. But yes, bug fix! :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually it shouldn't have any effect on behavior, but it is a performance fix :)

_ => false,
};

Expand Down
11 changes: 11 additions & 0 deletions src/test/rustdoc-json/enums/variant_struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// @has variant_struct.json "$.index[*][?(@.name=='EnumStruct')].visibility" \"public\"
// @has - "$.index[*][?(@.name=='EnumStruct')].kind" \"enum\"
pub enum EnumStruct {
// @has - "$.index[*][?(@.name=='VariantS')].inner.variant_kind" \"struct\"
// @has - "$.index[*][?(@.name=='x')]"
// @has - "$.index[*][?(@.name=='y')]"
VariantS {
x: u32,
y: String,
},
}
6 changes: 6 additions & 0 deletions src/test/rustdoc-json/enums/variant_tuple_struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @has variant_tuple_struct.json "$.index[*][?(@.name=='EnumTupleStruct')].visibility" \"public\"
// @has - "$.index[*][?(@.name=='EnumTupleStruct')].kind" \"enum\"
pub enum EnumTupleStruct {
// @has - "$.index[*][?(@.name=='VariantA')].inner.variant_kind" \"tuple\"
VariantA(u32, String),
}