New lints: Breaking changes in non-sealed trait items
obi1kenobi opened this issue ยท 6 comments
We can now query whether lints are sealed or not: obi1kenobi/trustfall-rustdoc-adapter#343
{
Crate {
item {
... on Trait {
name @output
sealed @output
}
}
}
}
Many changes in non-sealed traits are breaking downstream, since any downstream implementations of that trait will need to be adjusted to match the trait item changes.
Querying the sealed status of traits was a prerequisite for a variety of SemVer major lints, which can now be implemented:
- non-sealed trait added associated constant without default value: #875
- non-sealed trait added associated type without default value: #890
- non-sealed trait removed the default value for an associated constant: #888
- non-sealed trait removed the default value for an associated type: #889
- trait newly became sealed, breaking any external implementors: #876
- non-sealed trait added new supertraits (also tracked by #441): #892
- non-sealed trait added method without default impl: #891
- non-sealed trait removed a default impl for a trait method (also tracked by #294, claimed by @dmatos2012): #880
For guidance on how to write these lints, look at existing lints that check trait-related breakage. For example, here's the lint for "a non-sealed trait's method stopped being unsafe
, so implementations must remove the unsafe
keyword in their declarations": https://github.com/obi1kenobi/cargo-semver-checks/blob/661fcedaf8b1700959a61306433807f08edf457f/src/lints/trait_method_unsafe_removed.ron
Also make sure to check out our contributing guide, and take a look at what prior lints' merged PRs looked like so you know what to expect. It's easier than you think!
Hi! I have checked the linked PRs
and followed along and I would like start working on the item:
- trait newly became sealed (breaking any external implementors)
Thanks for the extremely well written out explanation, it really helps for newer contributors to get context of why/hows.
I would like to claim the following to start off
- non-sealed trait added associated constant without default value
Is there a written convention on lints name? non_sealed_trait_added_assoc_const_without_default
seems right to me yet quite lengthy.
Let's go with simpler names like trait_associated_const_added
, and mention the sealed-ness of the trait in the description field and in the error message as needed. We wouldn't lint this on sealed traits, or if there were a default value, so I don't see a reason to disambiguate from those cases.
There's no written convention, we're just aiming for consistency and relative brevity for now. Eventually I'd like to do a pass where we improve all the lint names, but that's not that high-priority right now.
From the PR
it looks like the has_body: Boolean!
property on Method
was merged recently, so if I am not mistaken that means we can also add the lints that need it like:
- non-sealed trait removed a default impl for a trait method
If thats the case, Id like to claim this one :)
Updated the list, go for it!
I can claim this one
- non-sealed trait added associated type without default value