Skip to content

False positive with unstable exported_private_dependencies lint #2954

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

Closed
0xangelo opened this issue May 10, 2025 · 2 comments
Closed

False positive with unstable exported_private_dependencies lint #2954

0xangelo opened this issue May 10, 2025 · 2 comments

Comments

@0xangelo
Copy link

Hi!

I've been experimenting with the exported_private_dependencies lint on nightly. I found it really useful in helping teams understand when a SemVer-breaking bump in one of their dependencies means the same for their crate. Therefore, I'm trying to have this lint on by default in CIs.

However, when depending on the futures crate, even if declaring it as public = true, I'll still get warnings like the following:

$ cargo +nightly clippy --no-deps -p my-crate --features client
    Checking my-crate v0.21.0 (/home/doom/git/MyOrg/my-repo/crates/my-crate)
warning: trait `futures::Stream` from private dependency 'futures_core' in public interface
   --> crates/my-crate/src/client.rs:285:1
    |
285 | pub struct StreamWrapper<S, Ok>
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(exported_private_dependencies)]` on by default

warning: trait `futures::Stream` from private dependency 'futures_core' in public interface
   --> crates/my-crate/src/client.rs:300:1
    |

The only solution I found for this was to:

  • directly depend on the futures-core crate to declare it as public; and
  • directly declare "dep:futures-core" for any features that use it

This turns out to be a bit annoying, as I was used to importing just futures so I could get both the core items and their extension traits + other utils. Now I have to import futures-core, futures-util, and so on.

Would updating the manifest file for futures to declare futures-util as public solve this issue? Was that not done already because it would be a breaking change?

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 10, 2025
@taiki-e
Copy link
Member

taiki-e commented May 10, 2025

futures::Stream is also a public part of futures.

This appears to be a bug in that lint, or an area that cannot be addressed by our crate that targets stable Rust because that lint is unstable.

Therefore, I recommend that you submit the issue to the upstream project that is implementing this lint, rather than submitting it to a downstream project such as this project that is affected by this bug or limitation.

@taiki-e taiki-e closed this as not planned Won't fix, can't repro, duplicate, stale May 10, 2025
@taiki-e taiki-e added C-question and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 10, 2025
@taiki-e taiki-e changed the title Update manifests to declare public dependencies False positive with unstable exported_private_dependencies May 10, 2025
@taiki-e taiki-e changed the title False positive with unstable exported_private_dependencies False positive with unstable exported_private_dependencies lint May 10, 2025
@0xangelo
Copy link
Author

For those that may come across this, the issue seems to already have a solution underway in rust-lang/rust#119428

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants