rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.57k stars 12.62k forks source link

Lint exported_private_dependencies misses public dependency via trait impl #71043

Open CAD97 opened 4 years ago

CAD97 commented 4 years ago

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.

epage commented 10 months ago

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

What is linted today

What is missing

Some cases that might be worth trying:

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 commented 10 months ago

Some other cases to validate as suggested by @obi1kenobi

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 commented 10 months ago

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.

epage commented 8 months ago

FYI @matthewjasper