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 "enum variant added on exhaustive enum" #710

Closed
1 task
adamchalmers opened this issue Mar 19, 2024 · 2 comments
Closed
1 task

False-positive "enum variant added on exhaustive enum" #710

adamchalmers opened this issue Mar 19, 2024 · 2 comments
Labels
A-lint Area: new or existing lint C-bug Category: doesn't meet expectations

Comments

@adamchalmers
Copy link

adamchalmers commented Mar 19, 2024

Which lint or lints are the issue

enum_marked_non_exhaustive

Known issues that might be causing this

  • Is the flagged item defined in another crate, or a re-export of such an item? (#638)

Steps to reproduce the bug with the above code

git clone and checkout out this PR: KittyCAD/modeling-api#234
Run cargo semver-checks check-release -p kittycad-modeling-cmds

Actual Behaviour

I get two semver check failures:
--- failure enum_marked_non_exhaustive: enum marked #[non_exhaustive] ---
--- failure enum_variant_added: enum variant added on exhaustive enum ---

Expected Behaviour

I should not get any semver failures, as:

  1. The enum was marked #[non_exhaustive] many commits ago (back in the 0.1.x of the crate)
  2. The enum is only conditionally marked #[non_exhaustive], if the unstable_exhaustive crate feature is not used. This feature should be used -- it has the prefix "unstable_" and it's also not a default feature. So, by default the enum should be marked as non_exhaustive.
  3. Because the enum is marked N.E. there should be no warning about enum variant added on exhaustive enum.

Verbose Lint Output

 Parsing kittycad-modeling-cmds v0.2.0 (current)
  Parsed [   2.130s] (current)
 Parsing kittycad-modeling-cmds v0.2.0 (baseline, cached)
  Parsed [   0.089s] (baseline)
Checking kittycad-modeling-cmds v0.2.0 -> v0.2.0 (no change)
Starting 67 checks, 0 unnecessary on 10 threads
    PASS [   0.052s]       major        auto_trait_impl_removed
    PASS [   0.017s]       major        constructible_struct_adds_field
    PASS [   0.003s]       major        constructible_struct_adds_private_field
    PASS [   0.002s]       major        constructible_struct_changed_type
    PASS [   0.059s]       major        derive_trait_impl_removed
    FAIL [   0.002s]       major        enum_marked_non_exhaustive
    PASS [   0.002s]       major        enum_missing
    PASS [   0.002s]       minor        enum_must_use_added
    PASS [   0.003s]       major        enum_now_doc_hidden
    PASS [   0.003s]       major        enum_repr_int_changed
    PASS [   0.005s]       major        enum_repr_int_removed
    PASS [   0.002s]       major        enum_repr_transparent_removed
    PASS [   0.004s]       major        enum_struct_variant_field_added
    PASS [   0.004s]       major        enum_struct_variant_field_missing
    PASS [   0.016s]       major        enum_struct_variant_field_now_doc_hidden
    PASS [   0.013s]       major        enum_tuple_variant_field_added
    PASS [   0.012s]       major        enum_tuple_variant_field_missing
    PASS [   0.010s]       major        enum_tuple_variant_field_now_doc_hidden
    FAIL [   0.007s]       major        enum_variant_added
    PASS [   0.014s]       major        enum_variant_missing
    PASS [   0.073s]       major        function_abi_no_longer_unwind
    PASS [   0.009s]       major        function_changed_abi
    PASS [   0.002s]       major        function_changed_export_name
    PASS [   0.002s]       major        function_const_removed
    PASS [   0.003s]       major        function_missing
    PASS [   0.004s]       minor        function_must_use_added
    PASS [   0.002s]       major        function_now_doc_hidden
    PASS [   0.002s]       major        function_parameter_count_changed
    PASS [   0.003s]       major        function_unsafe_added
    PASS [   0.003s]       major        inherent_method_const_removed
    PASS [   0.003s]       major        inherent_method_missing
    PASS [   0.004s]       minor        inherent_method_must_use_added
    PASS [   0.007s]       major        inherent_method_unsafe_added
    PASS [   0.007s]       major        method_parameter_count_changed
    PASS [   0.002s]       major        module_missing
    PASS [   0.003s]       major        pub_module_level_const_missing
    PASS [   0.001s]       major        pub_module_level_const_now_doc_hidden
    PASS [   0.001s]       major        pub_static_missing
    PASS [   0.002s]       major        repr_c_removed
    PASS [   0.001s]       major        repr_packed_added
    PASS [   0.002s]       major        repr_packed_removed
    PASS [   0.015s]       major        sized_impl_removed
    PASS [   0.002s]       major        struct_marked_non_exhaustive
    PASS [   0.007s]       major        struct_missing
    PASS [   0.003s]       minor        struct_must_use_added
    PASS [   0.013s]       major        struct_now_doc_hidden
    PASS [   0.018s]       major        struct_pub_field_missing
    PASS [   0.013s]       major        struct_pub_field_now_doc_hidden
    PASS [   0.002s]       major        struct_repr_transparent_removed
    PASS [   0.007s]       major        struct_with_pub_fields_changed_type
    PASS [   0.002s]       major        trait_method_missing
    PASS [   0.002s]       major        trait_method_now_doc_hidden
    PASS [   0.001s]       major        trait_method_unsafe_added
    PASS [   0.002s]       major        trait_method_unsafe_removed
    PASS [   0.001s]       major        trait_missing
    PASS [   0.002s]       minor        trait_must_use_added
    PASS [   0.001s]       major        trait_now_doc_hidden
    PASS [   0.001s]       major        trait_removed_associated_constant
    PASS [   0.001s]       major        trait_removed_associated_type
    PASS [   0.001s]       major        trait_removed_supertrait
    PASS [   0.001s]       major        trait_unsafe_added
    PASS [   0.001s]       major        trait_unsafe_removed
    PASS [   0.001s]       major        tuple_struct_to_plain_struct
    PASS [   0.003s]       minor        type_marked_deprecated
    PASS [   0.002s]       major        union_now_doc_hidden
    PASS [   0.001s]       major        unit_struct_changed_kind
    PASS [   0.020s]       major        variant_marked_non_exhaustive
 Checked [   0.084s] 67 checks; 65 passed, 2 failed, 0 unnecessary

--- failure enum_marked_non_exhaustive: enum marked #[non_exhaustive] ---

Description:
A public enum has been marked #[non_exhaustive]. Pattern-matching on it outside of its crate must now include a wildcard pattern like _, or it will fail to compile.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#attr-adding-non-exhaustive
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.30.0/src/lints/enum_marked_non_exhaustive.ron

Failed in:
enum ModelingCmd in /Users/adamchalmers/kc-repos/modeling-api/modeling-cmds/src/def_enum.rs:9

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.30.0/src/lints/enum_variant_added.ron

Failed in:
variant ModelingCmdEndpoint:TestDoNotMerge in /Users/adamchalmers/kc-repos/modeling-api/modeling-cmds/src/def_enum.rs:810
Summary semver requires new major version: 2 major and 0 minor checks failed
Finished [ 2.387s] kittycad-modeling-cmds

Generated System Information

Software version

cargo-semver-checks 0.30.0

Operating system

macOS 13.4.1 (Darwin 22.5.0)

Command-line

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

cargo version

> cargo -V 
cargo 1.76.0 (c84b36747 2024-01-18)

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

None

Additional Context

I want to mark the enum as #[non_exhaustive] for most consumers, but for my own internal projects, I am OK opting into breaking my code every time I add a new enum variant.

@adamchalmers adamchalmers added A-lint Area: new or existing lint C-bug Category: doesn't meet expectations labels Mar 19, 2024
@adamchalmers adamchalmers changed the title False-positive "public enum has been marked #[non_exhaustive]" False-positive "enum variant added on exhaustive enum" Mar 19, 2024
@adamchalmers
Copy link
Author

OK I figured out one of the lints is my fault, still think the other is legit but will open a new issue

@obi1kenobi
Copy link
Owner

I'm having trouble reproducing this:

$ git checkout achalmers/test-semver
branch 'achalmers/test-semver' set up to track 'origin/achalmers/test-semver'.
Switched to a new branch 'achalmers/test-semver'

$ cargo semver-checks -p kittycad-modeling-cmds
     Parsing kittycad-modeling-cmds v0.2.2 (current)
      Parsed [   4.063s] (current)
     Parsing kittycad-modeling-cmds v0.2.2 (baseline, cached)
      Parsed [   0.145s] (baseline)
    Checking kittycad-modeling-cmds v0.2.2 -> v0.2.2 (no change)
     Checked [   0.197s] 67 checks; 67 passed, 0 unnecessary
    Finished [   4.642s] kittycad-modeling-cmds

Though I do notice that in my checkout, the library seems to be at v0.2.2 and in the issue's repro that was at v0.2.0. I re-ran the branch's code against v0.2.0 as well (cargo semver-checks -p kittycad-modeling-cmds --baseline-version 0.2.0), and that didn't flag any issues either.

The repro branch was force-pushed a few times, so maybe something about it changed that broke the repro? Or maybe there's some non-standard configuration somewhere in your local checkout, or in your CI? If the latter, I'd love to get to the bottom of that so I can make cargo-semver-checks robust against that.

Regarding the unstable feature, it won't be enabled by default during semver checking. By default we use a heuristic that enables more than just the default features, but de-selects features that are unstable or nightly, and the unstable_exhaustive feature name would be excluded by default. More info on the heuristic here: https://github.com/obi1kenobi/cargo-semver-checks#what-features-does-cargo-semver-checks-enable-in-the-tested-crates

So I agree that your expectation of what the tool should do here seems accurate based on the info you gave here. As far as I can tell at the moment, that seems to be happening. If you still feel there's an issue, I'd love to work it out with you.

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-bug Category: doesn't meet expectations
Projects
None yet
Development

No branches or pull requests

2 participants