obi1kenobi / cargo-semver-checks

Scan your Rust crate for semver violations.
Apache License 2.0
1.18k stars 75 forks source link

Changing `?Sized` related trait bounds is not detected #532

Open weiznich opened 1 year ago

weiznich commented 1 year ago

Steps to reproduce the bug with the above code

git clone https://github.com/diesel-rs/diesel
cd diesel/diesel
cargo semver-checks --baseline-rev v2.0.4 --only-explicit-features --features postgres --features sqlite --features mysql --features extras --features with-deprecated --manifest-path .

Actual Behaviour

No breaking change is reported:

     Cloning v2.0.4
     Parsing diesel v2.1.1 (current)
     Parsing diesel v2.0.4 (baseline)
    Checking diesel v2.0.4 -> v2.1.1 (minor change)
   Completed [   0.150s] 39 checks; 39 passed, 6 unnecessary

Expected Behaviour

Cargo semver-checks should report a major breaking change. A trait function added a ?Sized bound to it's where clause.

Documentation:

Relevant function signature: 2.0.4:

    fn push_bound_value<T, U>(
        &mut self,
        bind: &'a U,
        metadata_lookup: &mut DB::MetadataLookup,
    ) -> QueryResult<()>
    where
        DB: Backend + HasSqlType<T>,
        U: ToSql<T, DB> + 'a;

2.1.1:

    fn push_bound_value<T, U>(
        &mut self,
        bind: &'a U,
        metadata_lookup: &mut DB::MetadataLookup,
    ) -> QueryResult<()>
    where
        DB: Backend + HasSqlType<T>,
        U: ToSql<T, DB> + ?Sized + 'a;

Note the added ?Sized bound in the newer version. From caller side this is a non-breaking change as it only extends the set of allowed values. For existing implementations outside of diesel of the given trait this is a breaking change as it requires updating the function signature. It also might break an implementation that assumes that U is Sized.

Generated System Information

Software version

cargo-semver-checks 0.22.1

Operating system

Linux 6.4.7-200.fc38.x86_64

Command-line

/home/weiznich/.cargo/bin/cargo-semver-checks semver-checks --bugreport 

cargo version

> cargo -V 
cargo 1.68.0 (115f34552 2023-02-26)

Compile time information

Build Configuration

No response

Additional Context

No response

obi1kenobi commented 1 year ago

Thanks, this is awesome! Especially appreciate the details on how this might be breaking and for whom, which will be super helpful writing the diagnostic message this should show.