Improve UX when a deprecated #[doc(hidden)] item is removed, since it isn't obvious why the item was public API in the first place

[joshlf]

Which lint or lints are the issue

trait_method_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

Download google/zerocopy@0bee231 and run cargo +1.81.0 semver-checks --baseline-version 0.8.5.

Actual Behaviour

The removed methods were previously #[doc(hidden)], and so I would expect it not to be considered semver-breaking to remove them. However, I get this output:

$ cargo +1.81.0 semver-checks --baseline-version 0.8.5
     Parsing zerocopy v0.8.6 (current)
      Parsed [   5.061s] (current)
    Blocking waiting for cargo global package lock
     Parsing zerocopy v0.8.5 (baseline, cached)
      Parsed [   0.277s] (baseline)
    Checking zerocopy v0.8.5 -> v0.8.6 (minor change)
     Checked [   0.034s] 82 checks: 81 pass, 1 fail, 0 warn, 7 skip

--- failure trait_method_missing: pub trait method removed or renamed ---

Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/trait_method_missing.ron

Failed in:
  method slice_from of trait FromBytes, previously in file /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:4551
  method mut_slice_from of trait FromBytes, previously in file /Users/josh/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:4584

     Summary semver requires new major version: 1 major and 0 minor checks failed

(This was originally discovered in CI.)

Expected Behaviour

I would expect removing #[doc(hidden)] trait methods with default implementations to not be a semver-breaking change.

Verbose Lint Output

$ cargo +1.81.0 semver-checks --baseline-version 0.8.5 --verbose
     Parsing zerocopy v0.8.6 (current)
             Features: alloc,derive,simd,simd-nightly,std
    Updating crates.io index
     Locking 7 packages to latest compatible versions
 Documenting zerocopy v0.8.6 (...)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.65s
      Parsed [   1.588s] (current)
     Parsing zerocopy v0.8.5 (baseline, cached)
      Parsed [   0.333s] (baseline)
    Checking zerocopy v0.8.5 -> v0.8.6 (minor change)
    Starting 82 checks, 7 unnecessary on 16 threads
        PASS [   0.015s]       major        auto_trait_impl_removed
        PASS [   0.002s]       major        constructible_struct_adds_field
        PASS [   0.003s]       major        constructible_struct_adds_private_field
        PASS [   0.003s]       major        constructible_struct_changed_type
        PASS [   0.029s]       major        derive_trait_impl_removed
        PASS [   0.002s]       major        enum_marked_non_exhaustive
        PASS [   0.001s]       major        enum_missing
        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.003s]       major        enum_repr_transparent_removed
        PASS [   0.002s]       major        enum_struct_variant_field_added
        PASS [   0.002s]       major        enum_struct_variant_field_missing
        PASS [   0.002s]       major        enum_struct_variant_field_now_doc_hidden
        PASS [   0.002s]       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.001s]       major        enum_tuple_variant_field_now_doc_hidden
        PASS [   0.001s]       major        enum_unit_variant_changed_kind
        PASS [   0.001s]       major        enum_variant_added
        PASS [   0.007s]       major        enum_variant_marked_non_exhaustive
        PASS [   0.001s]       major        enum_variant_missing
        PASS [   0.001s]       major        exported_function_changed_abi
        PASS [   0.002s]       major        function_abi_no_longer_unwind
        PASS [   0.003s]       major        function_changed_abi
        PASS [   0.002s]       major        function_const_removed
        PASS [   0.001s]       major        function_export_name_changed
        PASS [   0.002s]       major        function_missing
        PASS [   0.003s]       major        function_now_doc_hidden
        PASS [   0.002s]       major        function_parameter_count_changed
        PASS [   0.002s]       major        function_unsafe_added
        PASS [   0.003s]       major        inherent_associated_const_now_doc_hidden
        PASS [   0.002s]       major        inherent_associated_pub_const_missing
        PASS [   0.006s]       major        inherent_method_const_removed
        PASS [   0.006s]       major        inherent_method_missing
        PASS [   0.008s]       major        inherent_method_now_doc_hidden
        PASS [   0.008s]       major        inherent_method_unsafe_added
        PASS [   0.013s]       major        method_parameter_count_changed
        PASS [   0.001s]       major        module_missing
        PASS [   0.001s]       major        pub_module_level_const_missing
        PASS [   0.001s]       major        pub_module_level_const_now_doc_hidden
        PASS [   0.002s]       major        pub_static_missing
        PASS [   0.001s]       major        pub_static_mut_now_immutable
        PASS [   0.001s]       major        pub_static_now_doc_hidden
        PASS [   0.002s]       major        repr_c_removed
        PASS [   0.002s]       major        repr_packed_added
        PASS [   0.001s]       major        repr_packed_removed
        PASS [   0.005s]       major        sized_impl_removed
        PASS [   0.001s]       major        struct_marked_non_exhaustive
        PASS [   0.002s]       major        struct_missing
        PASS [   0.002s]       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.002s]       major        struct_repr_transparent_removed
        PASS [   0.001s]       major        struct_with_pub_fields_changed_type
        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.002s]       major        trait_method_added
        PASS [   0.004s]       major        trait_method_default_impl_removed
        FAIL [   0.002s]       major        trait_method_missing
        PASS [   0.002s]       major        trait_method_now_doc_hidden
        PASS [   0.002s]       major        trait_method_unsafe_added
        PASS [   0.001s]       major        trait_method_unsafe_removed
        PASS [   0.001s]       major        trait_missing
        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.002s]       major        trait_removed_supertrait
        PASS [   0.001s]       major        trait_unsafe_added
        PASS [   0.001s]       major        trait_unsafe_removed
        PASS [   0.002s]       major        tuple_struct_to_plain_struct
        PASS [   0.002s]       major        union_field_missing
        PASS [   0.001s]       major        union_missing
        PASS [   0.001s]       major        union_now_doc_hidden
        PASS [   0.001s]       major        union_pub_field_now_doc_hidden
        PASS [   0.001s]       major        unit_struct_changed_kind
     Checked [   0.031s] 82 checks: 81 pass, 1 fail, 0 warn, 7 skip

--- failure trait_method_missing: pub trait method removed or renamed ---

Description:
A trait method is no longer callable, and may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#major-any-change-to-trait-item-signatures
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/trait_method_missing.ron

Failed in:
  method slice_from of trait FromBytes, previously in file ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:4551
  method mut_slice_from of trait FromBytes, previously in file ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/zerocopy-0.8.5/src/lib.rs:4584

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

Generated System Information

$ cargo +1.81.0 semver-checks --bugreport
System information:
-------------------
#### Software version

cargo-semver-checks 0.35.0

#### Operating system

macOS 14.6.1 (Darwin 23.6.0)

#### Command-line

```bash
~/.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: 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

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.35.0%0A%0A%23%23%23%23%20Operating%20system%0A%0AmacOS%2014.6.1%20%28Darwin%2023.6.0%29%0A%0A%23%23%23%23%20Command-line%0A%0A%60%60%60bash%0A%2FUsers%2Fjosh%2F.cargo%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.81.0%20%282dbb1af80%202024-08-20%29%0A%60%60%60%0A%0A%23%23%23%23%20Compile%20time%20information%0A%0A-%20Profile%3A%20release%0A-%20Target%20triple%3A%20aarch64-apple-darwin%0A-%20Family%3A%20unix%0A-%20OS%3A%20macos%0A-%20Architecture%3A%20aarch64%0A-%20Pointer%20width%3A%2064%0A-%20Endian%3A%20little%0A-%20CPU%20features%3A%20aes%2Ccrc%2Cdit%2Cdotprod%2Cdpb%2Cdpb2%2Cfcma%2Cfhm%2Cflagm%2Cfp16%2Cfrintts%2Cjsconv%2Clor%2Clse%2Cneon%2Cpaca%2Cpacg%2Cpan%2Cpmuv3%2Cras%2Crcpc%2Crcpc2%2Crdm%2Csb%2Csha2%2Csha3%2Cssbs%2Cvh%0A-%20Host%3A%20aarch64-apple-darwin%0A%0A&build-config=

### Build Configuration

_No response_

### Additional Context

_No response_

[obi1kenobi]

Reproduced, and I agree with your analysis. Triaging now. Thanks for the report, and sorry for the inconvenience.

I'm planning to cut a new release tomorrow, and I'll try to include a fix for this if feasible.

[joshlf]

Amazing, thank you! We're not blocked since we've now published 0.8.6 (which includes this change), so there's no rush on our side.

[obi1kenobi]

Ah, this is actually "works as intended" because of an unfortunate interaction between deprecations and #[doc(hidden)].

#[doc(hidden)] is used for two arguably orthogonal purposes: (1) marking something as "not public API", and (2) hiding items from docs.rs to discourage people from using them (even if they are public API).

A couple of observations here:

• Adding #[doc(hidden)] by itself is a breaking change, since it's a removal from the public API (use case 1 above).
• Adding #[deprecated] and #[doc(hidden)] simultaneously should not be flagged a breaking change, since it's a common pattern in Rust where deprecated items are hidden from the docs index (use case 2 above).

Here's a key property we want: if an item goes from non-hidden public API to completely deleted, then we want to report a breaking change at some point regardless of the sequence of steps taken.

If we aren't careful, here's a possible sequence that could break that property:

• mark item #[deprecated] (still public API)
• mark item #[doc(hidden)] (use case 2 above)
• delete the item completely (the case in this issue)

Assuming you agree that property is valuable, then this case ("deprecated hidden item is deleted") has to be reported as breaking. AFAICT there's simply no other place to report a breaking change in the sequence above.

In an ideal world, we'd have separate attributes for "not public API" and "hidden from docs.rs," where one implies the other but not vice versa. Alas, that is not Rust today.

Let me know if that makes sense, and if you have any ideas on how to make the UX here better. This is hardly obvious, and I don't think the current lint explained it particularly well — even I was confused about why the lint triggered.

[joshlf]

Ahh interesting. Philosophically I disagree that case (2) should be supported, but I'm sure you have more experience with how these patterns are used in the wild than I do. Luckily we should rarely or even never need to deal with this again, and if we ever do, we can just manually bypass cargo-semver-checks in that one instance. So this isn't blocking us. Thanks for the analysis!

[obi1kenobi]

I also dislike case (2) personally, but it's unfortunately common enough in practice that we can't realistically ignore it =/

That said, we have some options for local improvements. We can write some more refined lints that separate the "deprecated + hidden" case from the regular case. We can adjust the wording of the message that the lint displays. Etc.

I'll tweak the issue title and labels, and keep it open so we can circle back to those UX improvements when we get a chance.