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 negative: changing tuple with public members is not detected as breaking #448

Open
dodomorandi opened this issue May 4, 2023 · 1 comment
Labels
A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations

Comments

@dodomorandi
Copy link

Steps to reproduce the bug with the above code

  • Create a new testing lib crate:

    cargo new --lib testing
  • Change to the created testing folder and edit src/lib.rs so it contains the following:

    pub struct Wrapper(pub i32);
  • Create a commit:

    git add . && git commit -m "Initial commit"
  • Change the type from i32 to f32

  • Run semver checks:

    cargo semver-checks check-release --baseline-rev HEAD
  • (Bonus) Transform Wrapper into a unit type and perform semver checks again.

Actual Behaviour

A breaking change should be detected, but the the semver check passes successfully. It should be a breaking change because the field is pub. It is worth noting that using a named struct correctly triggers the semver checks.

Needless to say, removing the inner field from the tuple is breaking, and tests pass as well.

Expected Behaviour

cargo-semver-checks should raise a breaking issue.

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 exploring issue #447.

CC @lu-zero

@dodomorandi dodomorandi added the C-bug Category: doesn't meet expectations label May 4, 2023
@obi1kenobi
Copy link
Owner

Same reasoning as here, we don't yet have a lint for this yet but it's already part of the tracking issue: #5

It's additionally blocked on #149, which is possible to fix but requires probably a month or two of focused work to put all the prerequisites in.

This issue isn't a great one to get started with, but if you're interested in contributing to the project by writing lints, adding test cases for missing lints, etc. I'd be happy to point you to something that'd be a good on-ramp!

@obi1kenobi obi1kenobi added A-lint Area: new or existing lint C-enhancement Category: raise the bar on expectations and removed C-bug Category: doesn't meet expectations labels May 4, 2023
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