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

Lint exported_private_dependencies misses public dependency via trait impl #71043

Open
Tracked by #44663
CAD97 opened this issue Apr 11, 2020 · 4 comments
Open
Tracked by #44663
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-visibility Area: Visibility / privacy C-bug Category: This is a bug. F-public_private_dependencies feature: public_private_dependencies T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@CAD97
Copy link
Contributor

CAD97 commented Apr 11, 2020

Tracking issue: #44663, RFC: rust-lang/rfcs#1977

Cargo.toml:

cargo-features = ["public-dependency"]

[package]
name = "playground"
version = "0.0.0"
edition = "2018"

[dependencies]
num-traits = "0.2"

lib.rs:

pub struct S;

impl std::ops::Add for S {
    type Output = S;

    fn add(self, _: Self) -> Self::Output {
        unimplemented!()
    }
}

impl num_traits::Zero for S {
    fn zero() -> Self {
        unimplemented!()
    }
    fn is_zero(&self) -> bool {
        unimplemented!()
    }
}

Also, a plain pub use seems to be missed as well.

@CAD97 CAD97 added the C-bug Category: This is a bug. label Apr 11, 2020
@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@epage
Copy link
Contributor

epage commented Nov 30, 2023

This is still a problem and is much wider than this one case

What is linted today

  • Private dep types as argument types
  • Private dep types as return types
  • Private dep traits as supertraits
  • Private dep traits in trait bounds

What is missing

  • Re-exports of a private dep
  • Implement a private dep trait on a local type
  • Implement a local trait on a private dep type
  • Implement a standard trait with a type as a generic argument on a local type

Some cases that might be worth trying:

  • Private dep types wrapped in a standard type (e.g. Saturating) as argument or return types

My reproduction case:

foo/Cargo.toml

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

foo/src/lib.rs

pub struct FromFoo;

pub trait FooTrait {}

impl FooTrait for FromFoo {}

pub fn add(left: usize, right: usize) -> usize {
    left + right
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn it_works() {
        let result = add(2, 2);
        assert_eq!(result, 4);
    }
}

Cargo.toml

cargo-features = ["public-dependency"]

[package]
name = "cargo-pub-priv"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
foo.path = "foo"

src/lib.rs

pub use foo;
pub use foo::add;
pub use foo::FromFoo;

pub fn use_foo_struct(_: foo::FromFoo) {}

pub fn returns_foo() -> foo::FromFoo {
    unreachable!()
}

pub trait UseFoo: foo::FooTrait {}

pub struct Local;

impl foo::FooTrait for Local {}

impl UseFoo for foo::FromFoo {}

impl From<foo::FromFoo> for Local {
    fn from(_: foo::FromFoo) -> Self {
        Local
    }
}

pub fn use_foo_trait<F: foo::FooTrait>(_: Local) {}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn it_works() {
        let result = add(2, 2);
        assert_eq!(result, 4);
    }
}
$ RUSTFLAGS=-Wexported-private-dependencies cargo +nightly check
    Checking foo v0.1.0 (/home/epage/src/personal/dump/cargo-pub-priv/foo)
    Checking cargo-pub-priv v0.1.0 (/home/epage/src/personal/dump/cargo-pub-priv)
warning: type `FromFoo` from private dependency 'foo' in public interface
 --> src/lib.rs:5:1
  |
5 | pub fn use_foo_struct(_: foo::FromFoo) {}
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: requested on the command line with `-W exported-private-dependencies`

warning: type `FromFoo` from private dependency 'foo' in public interface
 --> src/lib.rs:7:1
  |
7 | pub fn returns_foo() -> foo::FromFoo {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: trait `FooTrait` from private dependency 'foo' in public interface
  --> src/lib.rs:11:1
   |
11 | pub trait UseFoo: foo::FooTrait {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: type `FromFoo` from private dependency 'foo' in public interface
  --> src/lib.rs:20:5
   |
20 |     fn from(_: foo::FromFoo) -> Self {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: trait `FooTrait` from private dependency 'foo' in public interface
  --> src/lib.rs:25:1
   |
25 | pub fn use_foo_trait<F: foo::FooTrait>(_: Local) {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: `cargo-pub-priv` (lib) generated 5 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s

@epage
Copy link
Contributor

epage commented Dec 4, 2023

Some other cases to validate as suggested by @obi1kenobi

  • making a value of a private dep's type a pub static or const
  • making a private dep's type an associated type on a pub trait impl
  • making a private dep's trait a bound on associated type of a trait
  • as a pub field of a pub tuple / plain struct
  • as a field of a tuple / struct enum variant
  • re-export of a macro_rules macro from the private dep
  • re-export of a proc / derive macro from the private dep
  • making a pub type alias of a private dep's type (unclear if this should lint or not, but a good test case to pin down either way)
  • re-exporting an enum variant where the enum is from a private dep, like so:
mod other_crate {
    pub enum Example {
        First,
        Second,
    }
}

pub use other_crate::Example::First;

(arguably this last one should lint regardless of the private deps because it's fishy in general, but that's just my opinion)

@obi1kenobi
Copy link
Member

To clarify, the opinions expressed above are mine (though possibly shared by @epage, I'm not sure) 😁

Sorry @epage, I should have posted this comment directly in this issue instead of just in a DM, and saved you from needing to copy-paste.

@fmease fmease added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-visibility Area: Visibility / privacy T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Jan 25, 2024
@epage
Copy link
Contributor

epage commented Jan 30, 2024

FYI @matthewjasper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-visibility Area: Visibility / privacy C-bug Category: This is a bug. F-public_private_dependencies feature: public_private_dependencies T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants