cargo-semver-checks icon indicating copy to clipboard operation
cargo-semver-checks copied to clipboard

New lint: non-sealed `pub trait` method default implementation removed

Open SmolSir opened this issue 2 years ago • 4 comments

When the pub trait method's default implementation is removed, the method is no more provided and turns into a declared method. This forces the new version to implement the now-declared method in all of the impl blocks which were deriving that trait with its previously-provided method in the old version.

Required new version: Major

This requires some schema additions:

  • [x] adding the edge from Trait to its Methods [linked PR]
  • [ ] enabling the has_body field for Method [linked PR]

SmolSir avatar Jan 17 '23 17:01 SmolSir

Should we rename this issue to #293 which was meant to be this but I accidentally duplicated when linking in #5? (sigh)

SmolSir avatar Jan 17 '23 17:01 SmolSir

Sure!

obi1kenobi avatar Jan 17 '23 17:01 obi1kenobi

The "non-sealed" part here is very important: removing default implementations for sealed traits is not a breaking change -- I think it isn't even semver-minor.

Removing the default implementation is breaking if it can force other crates to provide implementations whereas previously they relied on the existence of a default. Sealed traits cannot be implemented by other crates -- only the crate that defines the sealed trait can implement it. So no other crate could possibly have had implementations of the trait which might be broken.

Sealing a previously non-sealed trait, however, is a very breaking change, and I just added it to the list in #5.

obi1kenobi avatar Jan 17 '23 17:01 obi1kenobi