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-types: Inconsitant #[derive(s #96189

Closed
aDotInTheVoid opened this issue Apr 18, 2022 · 0 comments · Fixed by #98789
Closed

rustdoc-json-types: Inconsitant #[derive(s #96189

aDotInTheVoid opened this issue Apr 18, 2022 · 0 comments · Fixed by #98789
Labels
A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@aDotInTheVoid
Copy link
Member

  • #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]: Most types
  • #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]: Id
  • #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)]: Header and Abi
  • #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)]: Generics

These are mostly a consequence of how they are used internaly (eg Header and Abi have Hash just because they used to be stored in a HashSet, Generics has Default because we use it once in conversions. We should aim to be more consistant and user facing in these traits.

What Traits I thing we should have and why?

The super clear ones:

  • Everything obviously need's Serialize and Deserialize
  • Clone and Debug are both super usefull, and I think everything should also Implement these, although it isn't required 1
  • Id needs Hash, Eq and PartialEq to be used as a HashMap key

The medium ones

  • PartialEq for everything. It has clear semantics, and if we do rustdoc-json: replace jsondocck with jsondocckng #94140, it's essensial
  • Eq for everything. If we do PartialEq, we may as well do this,
  • Hash for almost everything. Currently Item and Crate cannot do this because they have HashMaps. If we realy wanted to, we could replace these with BTreeMap, but that would mean Ord for Id and possible slower perf. Theirs alot of things that make sense to put in a HashMap/HashSet key, and I thing just not having Item and Crate is fine. (I The fact that Type didn't implement Hash was the reason I looked into this).

Things to drop

  • I don't thing anything should implement Default, as I don't think it's usefull or clear.

cc @CraftSpider

@rustbot modify labels: +T-rustdoc +A-rustdoc-json

Footnotes

  1. In practice, we depend on Clone for Item, which means basicly everything else needs to be Clone, but I'm almost certain this can be removed

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 18, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 13, 2022
rustdoc-json-types: Clean up derives.

Closes rust-lang#96189

Everything is `Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize` except `Crate` and `Item` which arn't `Hash`, as they have `HashMap`'s. See linked issue for reasoning.

`@rustbot` modify labels: +T-rustdoc +A-rustdoc-json
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 13, 2022
rustdoc-json-types: Clean up derives.

Closes rust-lang#96189

Everything is `Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize` except `Crate` and `Item` which arn't `Hash`, as they have `HashMap`'s. See linked issue for reasoning.

``@rustbot`` modify labels: +T-rustdoc +A-rustdoc-json
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 13, 2022
rustdoc-json-types: Clean up derives.

Closes rust-lang#96189

Everything is `Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize` except `Crate` and `Item` which arn't `Hash`, as they have `HashMap`'s. See linked issue for reasoning.

```@rustbot``` modify labels: +T-rustdoc +A-rustdoc-json
@bors bors closed this as completed in 18aea45 Jul 13, 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 T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants