Auto merge of #8429 - nsunderland1:master, r=llogiq

Document `pub` requirement for `new_without_default` lint

fixes #8415

Also adds some UI tests that ensure that `pub` is required on both the struct _and_ the field. The only thing I'm not sure about is that the lint actually [checks](https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/new_without_default.rs#L102) if `new` is _reachable_, not _public_. To the best of my understanding, both the struct and the method need to be public for the method to be reachable for external crates (I certainly didn't manage to craft a counterexample).

changelog: Document `pub` requirement for ``[`new_without_default`]`` lint.
This commit is contained in:
bors 2022-02-14 05:55:34 +00:00
commit 083697014c
3 changed files with 25 additions and 9 deletions

View file

@ -13,7 +13,7 @@ use rustc_span::sym;
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
/// Checks for types with a `fn new() -> Self` method and no /// Checks for public types with a `pub fn new() -> Self` method and no
/// implementation of /// implementation of
/// [`Default`](https://doc.rust-lang.org/std/default/trait.Default.html). /// [`Default`](https://doc.rust-lang.org/std/default/trait.Default.html).
/// ///
@ -24,10 +24,10 @@ declare_clippy_lint! {
/// ///
/// ### Example /// ### Example
/// ```ignore /// ```ignore
/// struct Foo(Bar); /// pub struct Foo(Bar);
/// ///
/// impl Foo { /// impl Foo {
/// fn new() -> Self { /// pub fn new() -> Self {
/// Foo(Bar::new()) /// Foo(Bar::new())
/// } /// }
/// } /// }
@ -36,7 +36,7 @@ declare_clippy_lint! {
/// To fix the lint, add a `Default` implementation that delegates to `new`: /// To fix the lint, add a `Default` implementation that delegates to `new`:
/// ///
/// ```ignore /// ```ignore
/// struct Foo(Bar); /// pub struct Foo(Bar);
/// ///
/// impl Default for Foo { /// impl Default for Foo {
/// fn default() -> Self { /// fn default() -> Self {
@ -47,7 +47,7 @@ declare_clippy_lint! {
#[clippy::version = "pre 1.29.0"] #[clippy::version = "pre 1.29.0"]
pub NEW_WITHOUT_DEFAULT, pub NEW_WITHOUT_DEFAULT,
style, style,
"`fn new() -> Self` method without `Default` implementation" "`pub fn new() -> Self` method without `Default` implementation"
} }
#[derive(Clone, Default)] #[derive(Clone, Default)]

View file

@ -90,6 +90,22 @@ impl Private {
} // We don't lint private items } // We don't lint private items
} }
struct PrivateStruct;
impl PrivateStruct {
pub fn new() -> PrivateStruct {
unimplemented!()
} // We don't lint public items on private structs
}
pub struct PrivateItem;
impl PrivateItem {
fn new() -> PrivateItem {
unimplemented!()
} // We don't lint private items on public structs
}
struct Const; struct Const;
impl Const { impl Const {

View file

@ -51,7 +51,7 @@ LL + }
| |
error: you should consider adding a `Default` implementation for `NewNotEqualToDerive` error: you should consider adding a `Default` implementation for `NewNotEqualToDerive`
--> $DIR/new_without_default.rs:156:5 --> $DIR/new_without_default.rs:172:5
| |
LL | / pub fn new() -> Self { LL | / pub fn new() -> Self {
LL | | NewNotEqualToDerive { foo: 1 } LL | | NewNotEqualToDerive { foo: 1 }
@ -68,7 +68,7 @@ LL + }
| |
error: you should consider adding a `Default` implementation for `FooGenerics<T>` error: you should consider adding a `Default` implementation for `FooGenerics<T>`
--> $DIR/new_without_default.rs:164:5 --> $DIR/new_without_default.rs:180:5
| |
LL | / pub fn new() -> Self { LL | / pub fn new() -> Self {
LL | | Self(Default::default()) LL | | Self(Default::default())
@ -85,7 +85,7 @@ LL + }
| |
error: you should consider adding a `Default` implementation for `BarGenerics<T>` error: you should consider adding a `Default` implementation for `BarGenerics<T>`
--> $DIR/new_without_default.rs:171:5 --> $DIR/new_without_default.rs:187:5
| |
LL | / pub fn new() -> Self { LL | / pub fn new() -> Self {
LL | | Self(Default::default()) LL | | Self(Default::default())
@ -102,7 +102,7 @@ LL + }
| |
error: you should consider adding a `Default` implementation for `Foo<T>` error: you should consider adding a `Default` implementation for `Foo<T>`
--> $DIR/new_without_default.rs:182:9 --> $DIR/new_without_default.rs:198:9
| |
LL | / pub fn new() -> Self { LL | / pub fn new() -> Self {
LL | | todo!() LL | | todo!()