Description
Problem
The exported_private_dependencies lint only take affect in the innermost dependency in a recursively dependent environment.
This inspired by #44663 (comment).
Steps
In order to prove this problem, I purposely contructed a code repository, here
in the repository, the crates folder has three crates,
- grandparent_dep, the init crate provied the pub struct
- parent_dep, the middle crate, add
grandparent_depas public and reexport pub struct from grandparent_dep - pub_dep, the outmost crate, add
parent_depas public and reexport pub struct from parent_dep
(the crates in crates can be treated as download from respority(like github, crates.io))
in src/lib.rs, add pub_dep as dependency but private.
After downloading the resposity,
1、 run cargo build, no lint warning message
2、 change the public = false in crates/pub_dep/Cargo.toml, then run cargo build, no lint warning message
3、 change the public = false in crates/parent_dep/Cargo.toml, run cargo build and a lint warning message comes up.
cargo build
Compiling parent_dep v0.1.0 (/root/workspace/recursive_pub_reexport/crates/parent_dep)
Compiling pub_dep v0.1.0 (/root/workspace/recursive_pub_reexport/crates/pub_dep)
Compiling simple v0.1.0 (/root/workspace/recursive_pub_reexport)
warning: type `FromPriv` from private dependency 'grandparent_dep' in public interface
--> src/lib.rs:3:1
|
3 | pub fn use_pub(_: pub_dep::FromPriv) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(exported_private_dependencies)]` on by default
Possible Solution(s)
The focus of this issue is to verify whether there is a problem with the current situation, so the solution will not be considered for the time being.
Notes
No response
Version
cargo 1.76.0-nightly (623b78849 2023-12-02)
release: 1.76.0-nightly
commit-hash: 623b788496b3e51dc2f9282373cf0f6971a229b5
commit-date: 2023-12-02
host: x86_64-unknown-linux-gnu
libgit2: 1.7.1 (sys:0.18.1 vendored)
libcurl: 8.4.0-DEV (sys:0.4.68+curl-8.4.0 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u 30 May 2023
os: Ubuntu 22.04 (jammy) [64-bit]
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
Participants
Activity
verify public is respected recursivelyrust-lang/cargo#13183epage commentedon Dec 27, 2023
I made a slight change to the repo, renaming
use_pubtouse_privand added the function to each package along the way so you can more easily see what the whole tree was doingepage commentedon Dec 27, 2023
I was curious what is done with diamonds: nothing unexpected.
epage commentedon Dec 27, 2023
Going back to
mainWith line wrapping
epage commentedon Dec 27, 2023
We only pass
--externfor direct dependencies which meansrustcis only told aboutprivfor direct dependencies and rustc then collect that information when recursing through dependencies. I think this should work if everything builds cleanly with--forbid exported_private_dependencies(or--denybut they respect the rules perfectly).In the transitional case where foundational packages have not been updated, users will have to use
#[allow(exported_private_dependencies)]when putting transitive types in their API.In the transitional case where foundation packages have been updated but intermediate packages have not been, no warning will be given even when it should.
So the next questions are
linyihai commentedon Dec 28, 2023
Thank you for dig into this issue.
epage commentedon Dec 28, 2023
Another case I want to test is when a package has a private dependency in its API (so should warn) but it is declared public in a transitive dependency.
epage commentedon Dec 28, 2023
Looks like once a dependency is public, it is always viewed as public.
epage commentedon Dec 28, 2023
Summary: only direct dependencies are marked as public/private; all transitive uses of it respect the highest visibility present in that subtree of the dependency graph.
Example Side effects
This is because we only pass
--externfor direct dependencies and visibility is tied to--extern.Possible solutions (without knowing the compiler side)
externs to see what the effective visibility is for the current crate being compiled--extern@ehuss I think you were involved in the conversations on the cargo/rustc handshake for this. Any thoughts on how to move this forward?
linyihai commentedon Dec 29, 2023
#119428 (comment)
This scenario is more common and worth writing a test case for. I plan to add this in rust-lang/cargo#13183
ehuss commentedon Dec 29, 2023
I believe that is a
rustcissue. It looks like it is checking the wrong thing when it is checking the public-status. It is looking at the DefId of the item, which is where the item is defined. However, I don't think that is correct in the face of re-exports. It should be checking the public status of the crate of theUseitem from where it was re-exported. I don't know much about theDefIdVisitoror the visibility analysis stuff in general, so I can't really suggest a different approach. If you want to work on fixing it yourself, you'll probably want to ask the compiler team for assistance in coming up with a different implementation strategy.I don't think it is necessary to pass more visibility data to rustc. It should be able to compute the effective visibility from the information it has.
It sounds like there needs to be a more principled definition of what it means to "leak" a private item, since it seems like there are some edge cases that the current approach doesn't handle correctly.
26 remaining items