False negative: major bumping of dependency with exposed API is not detected

[dodomorandi]

Steps to reproduce the bug with the above code

• Create a new crate with cargo new inner --lib and chdir inside.
• Add a dependency for which we are going to expose a member and later perform a dep bump. This examples uses time:

  cargo add time@0.2

• Edit src/lib.rs so it contains the following:

  pub struct Wrapper {
    pub date: time::Date
  }

• Create a new commit:

  git add . && git commit -m "Initial commit"

• Bump time to 0.3:

  cargo add time@0.3

• Add another commit (useful for further checks):

  git commit -a -m "bump time"

• Run cargo-semver-checks:

  cargo semver-checks check-release --baseline-rev HEAD^

Further check to ensure it is a breaking change

• Checkout to previous commit:

  git checkout HEAD^

• Create a worktree with the latest commit in a different path:

  git worktree add ../inner-new main

  Note that if you are using an old version of git, you could need to use master instead of main

• Change back to the parent folder and run cargo new --lib outer, then chdir inside outer.

• Add the first version of inner as dependency:

  cargo add inner --path ../inner

• Also add time@0.2 as dependency: cargo add time@0.2

• Edit the src/lib.rs so it contains the following:

  use inner::Wrapper;
  use time::Date;
  
  pub fn get_wrapped(wrapped: Wrapper) -> Date {
      wrapped.date
  }

• Run cargo check, assess that everything is fine.

• Use the new version of inner:

  cargo add inner --path ../inner-new

• Run cargo check again. Now you should expect a mismatching time::Date type.

Actual Behaviour

No breaking changes are reported

Expected Behaviour

A breaking change should be detected: bumping the major version of a dependency when the crate exposes something from its API is breaking.

Generated System Information

Software version

cargo-semver-checks 0.20.0 (cf03e7f)

Operating system

Linux 6.2.13-arch1-1

Command-line

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

cargo version

> cargo -V
cargo 1.69.0 (6e9a83356 2023-04-12)

Compile time information

• Profile: release
• Target triple: x86_64-unknown-linux-gnu
• Family: unix
• OS: linux
• Architecture: x86_64
• Pointer width: 64
• Endian: little
• CPU features: fxsr,sse,sse2
• Host: x86_64-unknown-linux-gnu

Build Configuration

Should not be relevant, but here the ~/.cargo/config.toml:

[registries.crates-io]
protocol = "sparse"

[target.x86_64-unknown-linux-gnu]
rustflags = ["-Clink-arg=-fuse-ld=lld", "-Clink-arg=-Wl,--no-rosegment"]

[target.x86_64-unknown-linux-musl]
rustflags = ["-Clink-arg=-fuse-ld=lld", "-Clink-arg=-Wl,--no-rosegment"]

[target.arm-unknown-linux-musleabihf]
linker = "arm-linux-musleabihf-gcc"

Additional Context

The issue has been discovered while bumping the wot-td dependency of the crate wot-serve.

CC @lu-zero

[obi1kenobi]

Thanks for the thorough report!

Semver in Rust is extremely complex, and there are easily hundreds of rules that aren't implemented yet: /~https://github.com/obi1kenobi/cargo-semver-checks#will-cargo-semver-checks-catch-every-semver-violation

The tracking issue for not-yet-implemented lints is #5. We're treating these as unimplemented features, and saving the C-bug label for issues like false-positives, or situations where an existing lint fails to catch the issue it purportedly is designed for. We don't have a lint for this case, so I'm going to re-tag this as a feature request.

Also, if you might be interested in contributing to the project by writing new lints, I'd be happy to mentor!

[dodomorandi]

Thank you @obi1kenobi!

You are right, it totally make sense to track the missing rules in a single issue instead of spreading them all around without any central document. I just did not find anything specifically related to the missed detection, I should have search more carefully 😅

I think I can try to implement the rule for this on my spare time (and maybe others, who knows), it could be an interesting experience. Also, I definitely love this project, I would be very happy to give a small hand. I am gonna try to write down something, if I need help I will ask, thank you in advance! ❤️

[obi1kenobi]

Unfortunately I don't think this rule is straightforward to implement since currently cargo-semver-checks doesn't see manifest information — it isn't exposed in the Trustfall schema it queries and it isn't particularly easy to add. I wouldn't recommend it as a starting point.

A few easier starting points:

• Split up "enum removed" from "enum changed to struct/union" checking #302
• New schema and lints for const and static values #366
• any of the items in Tracking issue: Required schema additions to make new lints possible #241 though note that they are in order of increasing difficulty

These run the gamut from "how can I help in half an hour" to "I've got a few afternoons / a few weekends to put in." I'm happy to be an issue concierge if you let me know what sort of thing you'd be most excited about.