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

Lint: Changing the type of a pub struct's pub field

Open passcod opened this issue 3 years ago • 6 comments

Steps to reproduce the bug with the above code

  • Checkout this branch: https://github.com/cargo-bins/cargo-binstall/tree/semver-check-test
  • Run cargo semver-checks check-release -p binstalk

Actual Behaviour

All tests pass

Expected Behaviour

I've deliberately broken the public API in that branch. On this public type:

 pub struct BinFile {
-    pub base_name: CompactString,
+    pub base_name: String,
     pub source: PathBuf,
     pub dest: PathBuf,
     pub link: Option<PathBuf>,
 }

This should fail a semver check, obviously. (I tried asking cargo-semverver to bring me confidence but it didn't work, so I recruited the good people of Discord instead:)

image

Additional Context

Background: I'd added cargo-semver-checks as an optional step in cargo-bins/release-pr after reading this article.

Debug Output

Software version

cargo-semver-checks 0.12.0

Operating system

Linux 5.19.7-arch1-1

Command-line

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

cargo nightly version

> cargo +nightly -V 
cargo 1.66.0-nightly (b8f30cb23 2022-10-10)

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: adx,aes,avx,avx2,bmi1,bmi2,fma,fxsr,lzcnt,pclmulqdq,popcnt,rdrand,rdseed,sse,sse2,sse3,sse4.1,sse4.2,ssse3,xsave,xsavec,xsaveopt,xsaves
  • Host: x86_64-unknown-linux-gnu

passcod avatar Oct 15 '22 12:10 passcod

Hi! Thanks for checking out cargo-semver-checks and for opening this issue.

I'm sorry our inadequate README FAQ section didn't cover this situation better. If it did, it could have saved you some confusion and debugging time. That's my bad.

Unfortunately, this is a case of a missing lint rule. There are many ways to break semver, and cargo-semver-checks doesn't have lints for all of them yet — including this one. There's a long list of not-yet-implemented lints in #5 that we are working our way through, and we encourage contributing new lints and are happy to offer mentorship as well.

I just merged a PR that adds more information to the FAQ about this: https://github.com/obi1kenobi/cargo-semver-check#does-cargo-semver-checks-have-false-positives-or-false-negatives

obi1kenobi avatar Oct 15 '22 16:10 obi1kenobi

Currently blocked on #149.

obi1kenobi avatar Oct 15 '22 17:10 obi1kenobi

FWIW I'm using a release-plz which uses cargo-semver-checks for version bumps, and this missing lint is the most often occurring bug we have with using cargo-semver-checks for octocrab

XAMPPRocky avatar Sep 01 '23 07:09 XAMPPRocky

As painful as I know this issue is, unfortunately I suspect I won't manage to get to it in the rest of this year. I have an approach in mind that should work, but it's a very substantial amount of work that I think is unlikely to fit in my available time or take priority over other cargo-semver-checks needs, like #120.

Of course, that's all barring some kind of major change. For example, like large corporate donor(s) setting up a substantial monthly GitHub Sponsors contribution comparable to my consulting rates, which would let me reallocate my time away from consulting and toward more cargo-semver-checks work.

This is probably not what you wanted to hear, and I am sorry about that. I hope sharing more detail, instead of just keeping quiet, is not something anyone in the community would see as a negative.

obi1kenobi avatar Sep 01 '23 16:09 obi1kenobi

This is probably not what you wanted to hear, and I am sorry about that.

Oh not at all, as another open source maintainer I very much understand the time being a limited resource. I just wanted to report back on our experience so that you have some data to make decisions when prioritising work.

I hope sharing more detail, instead of just keeping quiet, is not something anyone in the community would see as a negative.

I can only speak for myself, but I definitely prefer a "no not this year" to silence, communication is always better, even when it's communicating limitations and not just features.

FWIW I would help contribute this lint myself if not for #149 that task seems large in design and requires a lot of project knowledge I don't have to be able to contribute.

XAMPPRocky avatar Sep 01 '23 17:09 XAMPPRocky

Thank you for the kind words, much appreciated!

I agree #149 is not the best issue for a community contribution, unless someone is willing to put in enough work to effectively become a co-maintainer (which I'd be very open to!). The full long-term-stable solution to #149 would end up touching practically ever part of cargo-semver-checks, the underlying Trustfall adapter, and likely even require a moderately-sized feature addition to Trustfall itself.

obi1kenobi avatar Sep 01 '23 17:09 obi1kenobi