Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed receiver type and return lifetime #961

Open
1 of 3 tasks
cuviper opened this issue Oct 2, 2024 · 3 comments
Open
1 of 3 tasks

Changed receiver type and return lifetime #961

cuviper opened this issue Oct 2, 2024 · 3 comments
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations

Comments

@cuviper
Copy link

cuviper commented Oct 2, 2024

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

new

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

I was curious if semver-checks could detect this intentional breaking change:
indexmap-rs/indexmap#304

$ git clone https://github.com/indexmap-rs/indexmap
$ cd indexmap
$ git checkout 244bd81
$ cargo semver-checks --baseline-rev @^

Actual Behaviour

     Parsing indexmap v2.2.1 (current)
      Parsed [   2.700s] (current)
     Parsing indexmap v2.2.0 (baseline)
      Parsed [   2.695s] (baseline)
    Checking indexmap v2.2.0 -> v2.2.1 (patch change)
     Checked [   0.027s] 89 checks: 89 pass, 0 skip
     Summary no semver update required

Expected Behaviour

I hoped it would detect the same change that cargo public-api diff finds:

$ cargo public-api diff @^..@
[...]
Changed items in the public API
===============================
-pub fn indexmap::map::raw_entry_v1::RawOccupiedEntryMut<'a, K, V, S>::into_key(&mut self) -> &mut K
+pub fn indexmap::map::raw_entry_v1::RawOccupiedEntryMut<'a, K, V, S>::into_key(self) -> &'a mut K

This could probably be two different lints, one for the changed receiver and one for the return lifetime.

Generated System Information

Software version

cargo-semver-checks 0.35.0

Operating system

Linux 6.10.10-200.fc40.x86_64

Command-line

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

cargo version

> cargo -V
cargo 1.81.0 (2dbb1af80 2024-08-20)

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

No response

@cuviper cuviper added A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations labels Oct 2, 2024
@obi1kenobi
Copy link
Owner

Thanks! Agreed, probably two desirable lints here as you said: receiver changed + return lifetime change.

It's possible we might be able to catch these before we implement all of #149.

For the "receiver changed" lint, I'd like to make sure our approach is robust to the possible future changes from the "arbitrary receiver types" RFC. I'll need to think about the edge cases there.

@cuviper
Copy link
Author

cuviper commented Oct 9, 2024

FWIW, I remembered there was also an intentional receiver change way back in rust-lang/rust#39466, between Rust 1.15.0 and 1.15.1. Maybe these examples aren't particularly motivating since they were on purpose though. :)

@obi1kenobi
Copy link
Owner

obi1kenobi commented Oct 9, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants