False-positive for removed feature from Cargo.toml

[thomaseizinger]

Which lint or lints are the issue

feature_missing

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

See https://github.com/firezone/boringtun/actions/runs/12651147994/job/35251141325?pr=45.

Actual Behaviour

It appears that when removing an optional crate, cargo-semver-checks thinks that the entire feature is gone when in fact it is still there but doesn't activate any optional crates any more.

Expected Behaviour

Removing an optional crate but keeping the feature flag should be semver-compatible

Verbose Lint Output

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 8.44s
       Built [   8.487s] (baseline)
     Parsing boringtun v0.6.0 (baseline)
      Parsed [   0.015s] (baseline)
    Checking boringtun v0.6.0 -> v0.6.0 (no change)
    Starting 107 checks, 0 unnecessary on 16 threads
        PASS [   0.002s]       major        attribute_proc_macro_missing
        PASS [   0.006s]       major        auto_trait_impl_removed
        PASS [   0.001s]       major        constructible_struct_adds_field
        PASS [   0.003s]       major        constructible_struct_adds_private_field
        PASS [   0.001s]       major        constructible_struct_changed_type
        PASS [   0.001s]       major        declarative_macro_missing
        PASS [   0.001s]       major        derive_helper_attr_removed
        PASS [   0.001s]       major        derive_proc_macro_missing
        PASS [   0.004s]       major        derive_trait_impl_removed
        PASS [   0.001s]       major        enum_discriminants_undefined_non_exhaustive_variant
        PASS [   0.001s]       major        enum_discriminants_undefined_non_unit_variant
        PASS [   0.001s]       major        enum_marked_non_exhaustive
        PASS [   0.001s]       major        enum_missing
        PASS [   0.001s]       minor        enum_must_use_added
        PASS [   0.002s]       major        enum_no_repr_variant_discriminant_changed
        PASS [   0.001s]       major        enum_now_doc_hidden
        PASS [   0.001s]       major        enum_repr_int_changed
        PASS [   0.001s]       major        enum_repr_int_removed
        PASS [   0.001s]       major        enum_repr_transparent_removed
        PASS [   0.001s]       major        enum_repr_variant_discriminant_changed
        PASS [   0.001s]       major        enum_struct_variant_field_added
        PASS [   0.001s]       major        enum_struct_variant_field_missing
        PASS [   0.001s]       major        enum_struct_variant_field_now_doc_hidden
        PASS [   0.001s]       major        enum_tuple_variant_changed_kind
        PASS [   0.002s]       major        enum_tuple_variant_field_added
        PASS [   0.001s]       major        enum_tuple_variant_field_missing
        PASS [   0.002s]       major        enum_tuple_variant_field_now_doc_hidden
        PASS [   0.005s]       major        enum_unit_variant_changed_kind
        PASS [   0.002s]       major        enum_variant_added
        PASS [   0.003s]       major        enum_variant_marked_non_exhaustive
        PASS [   0.002s]       major        enum_variant_missing
        PASS [   0.001s]       major        exported_function_changed_abi
        FAIL [   0.001s]       major        feature_missing
        PASS [   0.015s]       major        function_abi_no_longer_unwind
        PASS [   0.001s]       major        function_changed_abi
        PASS [   0.001s]       major        function_const_removed
        PASS [   0.001s]       major        function_export_name_changed
        PASS [   0.000s]       major        function_like_proc_macro_missing
        PASS [   0.001s]       major        function_missing
        PASS [   0.002s]       minor        function_must_use_added
        PASS [   0.001s]       major        function_now_doc_hidden
        PASS [   0.001s]       major        function_parameter_count_changed
        PASS [   0.001s]       major        function_unsafe_added
        PASS [   0.001s]       major        inherent_associated_const_now_doc_hidden
        PASS [   0.001s]       major        inherent_associated_pub_const_missing
        PASS [   0.001s]       major        inherent_method_const_removed
        PASS [   0.002s]       major        inherent_method_missing
        PASS [   0.001s]       minor        inherent_method_must_use_added
        PASS [   0.002s]       major        inherent_method_now_doc_hidden
        PASS [   0.002s]       major        inherent_method_unsafe_added
        PASS [   0.001s]       major        macro_no_longer_exported
        PASS [   0.001s]       major        macro_now_doc_hidden
        PASS [   0.004s]       major        method_parameter_count_changed
        PASS [   0.001s]       major        module_missing
        PASS [   0.001s]       major        non_exhaustive_struct_changed_type
        PASS [   0.001s]       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.001s]       major        pub_static_mut_now_immutable
        PASS [   0.001s]       major        pub_static_now_doc_hidden
        PASS [   0.001s]       major        pub_static_now_mutable
        PASS [   0.001s]       major        repr_c_removed
        PASS [   0.001s]       major        repr_packed_added
        PASS [   0.001s]       major        repr_packed_removed
        PASS [   0.003s]       major        sized_impl_removed
        PASS [   0.001s]       major        struct_marked_non_exhaustive
        PASS [   0.002s]       major        struct_missing
        PASS [   0.001s]       minor        struct_must_use_added
        PASS [   0.001s]       major        struct_now_doc_hidden
        PASS [   0.001s]       major        struct_pub_field_missing
        PASS [   0.001s]       major        struct_pub_field_now_doc_hidden
        PASS [   0.001s]       major        struct_repr_transparent_removed
        PASS [   0.001s]       major        struct_with_no_pub_fields_changed_type
        PASS [   0.001s]       major        struct_with_pub_fields_changed_type
        PASS [   0.001s]       major        trait_added_supertrait
        PASS [   0.001s]       major        trait_associated_const_added
        PASS [   0.001s]       major        trait_associated_const_default_removed
        PASS [   0.001s]       major        trait_associated_const_now_doc_hidden
        PASS [   0.001s]       major        trait_associated_type_added
        PASS [   0.001s]       major        trait_associated_type_default_removed
        PASS [   0.001s]       major        trait_associated_type_now_doc_hidden
        PASS [   0.001s]       major        trait_method_added
        PASS [   0.001s]       major        trait_method_default_impl_removed
        PASS [   0.001s]       major        trait_method_missing
        PASS [   0.001s]       major        trait_method_now_doc_hidden
        PASS [   0.001s]       major        trait_method_unsafe_added
        PASS [   0.001s]       major        trait_method_unsafe_removed
        PASS [   0.001s]       major        trait_mismatched_generic_lifetimes
        PASS [   0.001s]       major        trait_missing
        PASS [   0.001s]       minor        trait_must_use_added
        PASS [   0.001s]       major        trait_newly_sealed
        PASS [   0.001s]       major        trait_no_longer_object_safe
        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.003s]       major        trait_unsafe_removed
        PASS [   0.001s]       major        tuple_struct_to_plain_struct
        PASS [   0.001s]       minor        type_marked_deprecated
        PASS [   0.004s]       major        type_mismatched_generic_lifetimes
        PASS [   0.001s]       major        union_field_missing
        PASS [   0.000s]       major        union_missing
        PASS [   0.001s]       minor        union_must_use_added
        PASS [   0.001s]       major        union_now_doc_hidden
        PASS [   0.000s]       major        union_pub_field_now_doc_hidden
        PASS [   0.001s]       major        unit_struct_changed_kind
     Checked [   0.022s] 107 checks: 106 pass, 1 fail, 0 warn, 0 skip

--- failure feature_missing: package feature removed or renamed ---

Description:
A feature has been removed from this package's Cargo.toml. This will break downstream crates which enable that feature.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#cargo-feature-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/feature_missing.ron

Failed in:
  feature mock_instant in the package's Cargo.toml

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  21.271s] boringtun

Generated System Information

System information:

Software version

cargo-semver-checks 0.38.0

Operating system

Linux 6.12.8

Command-line

/nix/store/hgr8sg2c0dj0jlmm0r7n3faclwjwxx3n-cargo-semver-checks-0.38.0/bin/cargo-semver-checks semver-checks --bugreport

cargo version

> cargo -V
cargo 1.83.0 (5ffbef321 2024-10-29)

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

Cargo build configuration:

Please file an issue on GitHub reporting your bug.
Consider adding the diagnostic information above, either manually or automatically through the link below:

https://github.com/obi1kenobi/cargo-semver-checks/issues/new?labels=C-bug&template=3-bug-report.yml&sys-info=%23%23%23%23%20Software%20version%0A%0Acargo-semver-checks%200.38.0%0A%0A%23%23%23%23%20Operating%20system%0A%0ALinux%206.12.8%0A%0A%23%23%23%23%20Command-line%0A%0A%60%60%60bash%0A%2Fnix%2Fstore%2Fhgr8sg2c0dj0jlmm0r7n3faclwjwxx3n-cargo-semver-checks-0.38.0%2Fbin%2Fcargo-semver-checks%20semver-checks%20--bugreport%20%0A%60%60%60%0A%0A%23%23%23%23%20cargo%20version%0A%0A%60%60%60%0A%3E%20cargo%20-V%20%0Acargo%201.83.0%20%285ffbef321%202024-10-29%29%0A%60%60%60%0A%0A%23%23%23%23%20Compile%20time%20information%0A%0A-%20Profile%3A%20release%0A-%20Target%20triple%3A%20x86_64-unknown-linux-gnu%0A-%20Family%3A%20unix%0A-%20OS%3A%20linux%0A-%20Architecture%3A%20x86_64%0A-%20Pointer%20width%3A%2064%0A-%20Endian%3A%20little%0A-%20CPU%20features%3A%20fxsr%2Csse%2Csse2%0A-%20Host%3A%20x86_64-unknown-linux-gnu%0A%0A&build-config=

Build Configuration

No response

Additional Context

No response

[obi1kenobi]

This is actually a true positive: the optional crate and implicit feature are named mock_instant, whereas the explicit feature is mock-instant, noting the dash instead of the underscore.

Proof:

$ cargo add --git https://github.com/firezone/boringtun.git boringtun
    Updating git repository `https://github.com/firezone/boringtun.git`
      Adding boringtun (git) to dependencies
             Features:
             - device
             - ffi-bindings
             - jni
             - jni-bindings
             - mock-instant
             - mock_instant
             - socket2
             - thiserror
             - tracing-subscriber
[ ... ]

If you remove the optional crate, the implicit feature mock_instant gets removed whereas the explicit mock-instant feature remains.

If anything, I wish cargo-semver-checks more explicitly and forcefully explained why this is really breaking despite looking like a false-positive :) On the other hand, from a cursory glance at the PR it seems like you decided to re-add the implicit dependency to avoid the cargo-semver-checks error, and thereby avoided shipping the breakage? If so, it seems like everything worked as desired 🙌

[thomaseizinger]

Oh wow! Thanks for the investigation and sorry for the bug report! Could cargo semver-checks perhaps hint at the similarity somehow? rustc sometimes does that when you misspell something :)

[obi1kenobi]

No worries at all. It took me a while to spot the problem too, it's definitely a sneaky one.

I also thought cargo-semver-checks should maybe hint at the similarity. The best mechanism we have available in lint queries for picking out similar features is regex, but I couldn't come up with a regex for it. Any ideas?

"Features that differ only in a single confusable character" seems like it would be a good lint candidate for clippy or cargo too, as it's a good warning completely independent of versioning. I've opened an issue in clippy suggesting this lint, and so we should see what clippy folks think before deciding on our own next step: rust-lang/rust-clippy#13969

[thomaseizinger]

Awesome! Yeah I agree that it would be good to prevent such duplicated features in the first place! That is likely the best direction to go in here.

If not added in clippy, it could be added in cargo semver-checks as a "potential semver hazard" in the future? Not sure if you have a plan to add such kinds of lints.

[obi1kenobi]

Yes, definitely a good candidate as a potential semver hazard and we have a number of those kinds of lints in the wish list at #5. I'll add this one as well.