Possible false-negative for changed mutability in trait

[jaybosamiya-ms]

Is this about an existing lint, or proposing a new one?

trait-item-signature

Known issues that might be causing this

• Is the item that should have been flagged originating from another crate, or from a re-export of such an item? (#638)
• Is the issue related to a change in the type of a field, function parameter, generic, or trait bound? (#149)
• If you're proposing a new lint, is it already part of our lint wishlist? (#5)

Steps to reproduce the bug with the above code

Old code:

pub trait Foo {
    fn foo(&mut self);
}
pub trait Bar {
    fn bar(&self);
}

Updated code (note how the mutability switched for both):

pub trait Foo {
    fn foo(&self);
}
pub trait Bar {
    fn bar(&mut self);
}

Run cargo semver-checks --baseline-rev {oldrev}

Actual Behaviour

$ cargo semver-checks --baseline-rev ed53c34
     Cloning ed53c34
    Building temptemp v0.1.0 (current)
       Built [   0.508s] (current)
     Parsing temptemp v0.1.0 (current)
      Parsed [   0.003s] (current)
    Building temptemp v0.1.0 (baseline)
       Built [   0.454s] (baseline)
     Parsing temptemp v0.1.0 (baseline)
      Parsed [   0.003s] (baseline)
    Checking temptemp v0.1.0 -> v0.1.0 (no change)
     Checked [   0.005s] 127 checks: 127 pass, 0 skip
     Summary no semver update required
    Finished [   1.184s] temptemp

Notice it says Summary no semver update required even though it should report a breakage.

Expected Behaviour

Both the change from &self -> &mut self, and &mut self -> &self are breaking changes (downstream crates that impl the trait, or might be depending on the trait need to update their code). Thus it should be reported as breaking.

Generated System Information

Software version

cargo-semver-checks 0.37.0 (68711cc)

Operating system

Linux 5.15.167.4-microsoft-standard-WSL2

Command-line

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

cargo version

> cargo -V 
cargo 1.84.0 (66221abde 2024-11-19)

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

No response

Additional Context

I believe this is yet another instance of not yet catching type changes sufficiently. Nonetheless, I thought it might be helpful to report this as a separate issue since I didn't notice any issues specifically talking about the types in trait function signatures, and trait-item-signature seems to indicate that any such change should be reported. Sorry for the noise if you think this is already covered as part of #149.

[obi1kenobi]

Neat idea, thanks @jaybosamiya-ms!

Technically this is part of #149, but in practice it's enough of a special case that I think we should be able to tackle it separately, without needing to complete #149 first.

Is this relevant to a Microsoft project by any chance? Asking because I noticed this issue was opened via your work account.

[jaybosamiya-ms]

Thanks for the quick response @obi1kenobi!

I did come across the issue at work, but not on any Microsoft product/service (the work/personal account split is more of an access-separation thing, more than anything else)

Context, if curious: I build research prototypes. On one such (currently private) codebase, I recently experimentally added cargo-semver-checks as a GitHub action. Interestingly, not to remind us to bump semver, since this is very early-stage prototype code (not even "v0.0.1" ready 🙃), but as a comment-only bot---roughly speaking, to possibly induce extra thought about decisions while we are figuring out the right abstractions for the actual research project itself. It has worked great to point some changes out (especially noticing dyn-compatibility changes has been helpful!). However, when I recently introduced a change to one of our traits where I would've expected the bot to say "this changed", it didn't, thus me opening the issue. Not a blocker; I mostly opened the issue for easier search just in case anyone else hits the same, since my first few searches didn't locate related issues (and I noticed #149 only while actually making the new issue).

Again, please feel free to close this in preference for the more general #149 if it helps reduce your work.

[obi1kenobi]

Thanks, useful context! And this issue is useful because we should be able to tackle it before #149 as a whole, so I'll keep it.

In the meantime, I'd appreciate any help in getting Microsoft to fund cargo-semver-checks! It's used in a number of public Microsoft repositories (and I'd imagine in some non-public ones too), and it'd be lovely to have an official communication channel + a relationship of mutual support.

[jaybosamiya-ms]

While I'm not familiar with the processes regarding funding and/or official comms channels (I'm relatively new here) I will keep a look out if I find anything. A quick (internal) search seems to suggest product/service teams may have a clearer process, but I couldn't immediately find specifics, unfortunately. Will keep an eye out though.

[obi1kenobi]

Thank you, I appreciate it!