Skip to content

Commit

Permalink
Auto merge of rust-lang#8429 - nsunderland1:master, r=llogiq
Browse files Browse the repository at this point in the history
Document `pub` requirement for `new_without_default` lint

fixes rust-lang#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.
  • Loading branch information
bors committed Feb 14, 2022
2 parents c37d6ee + 78c2e0b commit 0836970
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
10 changes: 5 additions & 5 deletions clippy_lints/src/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_span::sym;

declare_clippy_lint! {
/// ### 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
/// [`Default`](https://doc.rust-lang.org/std/default/trait.Default.html).
///
Expand All @@ -24,10 +24,10 @@ declare_clippy_lint! {
///
/// ### Example
/// ```ignore
/// struct Foo(Bar);
/// pub struct Foo(Bar);
///
/// impl Foo {
/// fn new() -> Self {
/// pub fn new() -> Self {
/// Foo(Bar::new())
/// }
/// }
Expand All @@ -36,7 +36,7 @@ declare_clippy_lint! {
/// To fix the lint, add a `Default` implementation that delegates to `new`:
///
/// ```ignore
/// struct Foo(Bar);
/// pub struct Foo(Bar);
///
/// impl Default for Foo {
/// fn default() -> Self {
Expand All @@ -47,7 +47,7 @@ declare_clippy_lint! {
#[clippy::version = "pre 1.29.0"]
pub NEW_WITHOUT_DEFAULT,
style,
"`fn new() -> Self` method without `Default` implementation"
"`pub fn new() -> Self` method without `Default` implementation"
}

#[derive(Clone, Default)]
Expand Down
16 changes: 16 additions & 0 deletions tests/ui/new_without_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,22 @@ impl Private {
} // 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;

impl Const {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/new_without_default.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ LL + }
|

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 | | NewNotEqualToDerive { foo: 1 }
Expand All @@ -68,7 +68,7 @@ LL + }
|

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 | | Self(Default::default())
Expand All @@ -85,7 +85,7 @@ LL + }
|

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 | | Self(Default::default())
Expand All @@ -102,7 +102,7 @@ LL + }
|

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 | | todo!()
Expand Down

0 comments on commit 0836970

Please sign in to comment.