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

[rustdoc-json] Free-floating struct_field items with enum tuple struct variants #92945

Closed
Enselic opened this issue Jan 15, 2022 · 2 comments · Fixed by #101462
Closed

[rustdoc-json] Free-floating struct_field items with enum tuple struct variants #92945

Enselic opened this issue Jan 15, 2022 · 2 comments · Fixed by #101462
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Enselic
Copy link
Member

Enselic commented Jan 15, 2022

There is currently a problematic inconsistency with regards to the generated rustdoc JSON for tuple structs. If the tuple struct is an enum variant, a "parentless" or "free-floating" item is generated. This does not happen with regular tuple structs.

Consider this code in src/lib.rs.

pub struct S1(pub usize);

pub enum E1 {
    S2(usize)
}

pub struct S3 {
    pub s3: usize,
}

pub enum E2 {
    S4 {
        s4: usize,
    }
}

Now we generate the rustdoc JSON (I used rustc 1.60.0-nightly (ad46af247 2022-01-14)):

RUSTDOCFLAGS='-Z unstable-options --output-format json' cargo doc

The rustdoc JSON output will contain 4 struct_field items, corresponding to S1.0, S2.0, S3.s3 and S4.s4 above.

The JSON item for S1 includes a reference to S1.0 (some parts of the JSON are omitted for brevity):

        "0:3": {
            "id": "0:3",
            "crate_id": 0,
            "name": "S1",
            "kind": "struct",
            "inner": {
                "struct_type": "tuple",
                "fields": [
                    "0:5"            <-- this is `S1.0`
                ],
        },

And the JSON item for S3 includes a reference to S3.s3:

        "0:10": {
            "id": "0:10",
            "crate_id": 0,
            "name": "S3",
            "kind": "struct",
            "inner": {
                "struct_type": "plain",
                "fields": [
                    "0:11"       <-- this is `S3.s3`
                ],
            }
        },

And the JSON item for S4 includes a reference to S4.s4:

        "0:13": {
            "id": "0:13",
            "crate_id": 0,
            "name": "S4",
            "kind": "variant",
            "inner": {
                "variant_kind": "struct",
                "variant_inner": [
                    "0:14"       <-- this is `S4.s4`
                ]
            }
        },

However, and this is the bug, the JSON item for S2 does not contain a reference to S2.0:

        "0:7": {
            "id": "0:7",
            "crate_id": 0,
            "name": "S2",
            "kind": "variant",
            "inner": {
                "variant_kind": "tuple",
                "variant_inner": [
                    {
                        "kind": "primitive",
                        "inner": "usize"
                    }
                ]
            }
        },

But an item for S2.0 is still included in the output. Here are all struct_field items in the JSON output:

% jq  '.index | .[] | select(.crate_id == 0) | select(.kind == "struct_field")' target/doc/repro.json 
{
  "id": "0:11",
  "crate_id": 0,
  "name": "s3",
  "kind": "struct_field",
  "inner": {
    "kind": "primitive",
    "inner": "usize"
  }
}
{
  "id": "0:14",
  "crate_id": 0,
  "name": "s4",
  "kind": "struct_field",
  "inner": {
    "kind": "primitive",
    "inner": "usize"
  }
}
{
  "id": "0:9",
  "crate_id": 0,
  "name": "0",
  "kind": "struct_field",
  "inner": {
    "kind": "primitive",
    "inner": "usize"
  }
}
{
  "id": "0:5",
  "crate_id": 0,
  "name": "0",
  "kind": "struct_field",
  "inner": {
    "kind": "primitive",
    "inner": "usize"
  }
}

but "0:9" is "free-floating" or "parentless", because it is not referenced by any other item. These "free-floating" rustdoc JSON items that are not referenced by any other items are problematic, because tools that processes the rustdoc JSON can't do anything sensible with them. They can't be ignored, because they describe items that are part of the public API of a crate. But they can't be used either, because they can not be put in a context since they are not referenced by anything.

Fortunately, I think fixing this is pretty straightforward. First, let us note that for a regular struct, the fields of it are referenced regardless of if it is a regular struct or a tuple struct (code from https://github.com/rust-lang/rust/blob/master/src/rustdoc-json-types/lib.rs):

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct Struct {
    pub struct_type: StructType,
    pub generics: Generics,
    pub fields_stripped: bool,
    pub fields: Vec<Id>,
    pub impls: Vec<Id>,
}

However, for enum variant structs, fields are only referenced for regular struct variants:

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(rename_all = "snake_case")]
#[serde(tag = "variant_kind", content = "variant_inner")]
pub enum Variant {
    Plain,
    Tuple(Vec<Type>),
    Struct(Vec<Id>),
}

Conceptually I think the fix should be to replace Tuple(Vec<Type>), with Tuple(Vec<Id>), in the snippet above. But I have not thought this through in depth, it just intuitively seems like the right way to tackle this.

@Enselic
Copy link
Member Author

Enselic commented Jan 16, 2022

Proof-of-concept fix to experiment with can be found at Enselic#1.

I have done some casual verification of that change and it appears to solve the problem in a good way. But the fix requires more thought and testing.

@Enselic Enselic changed the title [rustdoc-json] Dangling struct_field item with enum tuple struct [rustdoc-json] Free-floating struct_field items with enum tuple struct variants Jan 17, 2022
@CraftSpider CraftSpider added A-rustdoc-json Area: Rustdoc JSON backend C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 17, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 20, 2022
…-fields, r=CraftSpider

src/test/rustdoc-json: Check for `struct_field`s in `variant_tuple_struct.rs`

The presence of `struct_field`s is being checked for already in
`variant_struct.rs`. We should also check for them in `variant_tuple_struct.rs`.

This PR is one small step towards resolving rust-lang#92945.
@camelid camelid added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 25, 2022
@aDotInTheVoid
Copy link
Member

aDotInTheVoid commented Aug 19, 2022

This happens because we add all the inner items here

EnumItem(e) => e.variants.iter(),

but I think it's better to move to having fields as their own item

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Sep 6, 2022
…GuillaumeGomez

Rustdoc-Json: Store Variant Fields as their own item.

Closes rust-lang#100587
Closes rust-lang#92945

Successor to rust-lang#100762

Unlike that one, we don't have merge `StructType` and `Variant`, as after rust-lang#101386 `Variant` stores enum specific information (discriminant).

Resolves the naming discussion (rust-lang#100762 (comment)) by having seperate enums for struct and enum kinds

Resolves `#[doc(hidden)]` on tuple structs (rust-lang#100762 (comment)) by storing as a `Vec<Option<Id>>`

r? `@GuillaumeGomez`
@bors bors closed this as completed in 065e0b9 Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
4 participants