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

False positive when replacing a struct with a pub type alias #609

Open
jdisanti opened this issue Dec 14, 2023 · 1 comment
Open

False positive when replacing a struct with a pub type alias #609

jdisanti opened this issue Dec 14, 2023 · 1 comment
Labels
C-bug Category: doesn't meet expectations

Comments

@jdisanti
Copy link

Steps to reproduce the bug with the above code

We've found that we can't move things around in a backwards compatible way using pub use since the deprecated attribute doesn't work on use statements, so we've been using pub type to move them around instead. When we do this, cargo-semver-checks reports it as a breaking change:

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.24.1/src/lints/struct_missing.ron

To reproduce, have crates A and B, with B depending on A. Leave A empty for now. In B, add a struct:

pub struct Foo;

Then, as part of a patch release for both crates, move Foo into A, and re-export it using a pub type in B.

A:

pub struct Foo;

B:

#[deprecated(note = "Use A::Foo")]
pub type Foo = A::Foo;

There's a real-world repro in smithy-lang/smithy-rs#3318

Actual Behaviour

False positive saying the struct was removed and can't be referenced anymore.

Expected Behaviour

It should recognize that the type alias has the same name and points to the struct. (Actually not sure rustdoc has enough information to make this possible.)

Generated System Information

Software version

cargo-semver-checks 0.26.0

Operating system

macOS 13.6.2 (Darwin 22.6.0)

Command-line

/Users/jdisanti/.cargo/bin/cargo-semver-checks semver-checks --bugreport

cargo version

> cargo -V
cargo 1.70.0 (ec8a8a0ca 2023-04-25)

Compile time information

  • Profile: release
  • Target triple: aarch64-apple-darwin
  • Family: unix
  • OS: macos
  • Architecture: aarch64
  • Pointer width: 64
  • Endian: little
  • CPU features: aes,crc,dit,dotprod,dpb,dpb2,fcma,fhm,flagm,fp16,frintts,jsconv,lor,lse,neon,paca,pacg,pan,pmuv3,ras,rcpc,rcpc2,rdm,sb,sha2,sha3,ssbs,vh
  • Host: aarch64-apple-darwin

Build Configuration

No response

Additional Context

No response

@jdisanti jdisanti added the C-bug Category: doesn't meet expectations label Dec 14, 2023
@obi1kenobi
Copy link
Owner

The root cause here is the definition and the type alias being in different crates. For cases when they are in the same crate, we have test cases (one, two, there are more) that make sure we get this right.

Much of the below might already be familiar to you and others — I'm writing it down so I have an easy thing to reference in the future.

Cargo-semver-checks can't currently do cross-crate analysis, so all items from other crates look like they are missing. This is because:

  • Rustdoc doesn't inline re-exported items across crates. This used to be the case, but was removed because it was causing lots of bugs and other kinds of unpleasantness.
  • There's currently no reliable way to determine which dependency (including crate and exact version, to distinguish between multiple major versions) a cross-crate item came from. There has been interest in adding such a mechanism, though progress seems to have slowed recently: Add a new --build-id flag to rustc rust-lang/compiler-team#635
  • There will still be probably 3 person-months of work needed on the cargo-semver-checks side after such a mechanism is added. Such a large investment of time would require finding a more permanent source of funding for the project — sponsoring me on GitHub is the way to do that. When I can cover rent with cargo-semver-checks, I'll be much more able to work on time-consuming improvements like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: doesn't meet expectations
Projects
None yet
Development

No branches or pull requests

2 participants