Skip to content

Commit 0836970

Browse files
committed
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.
2 parents c37d6ee + 78c2e0b commit 0836970

File tree

3 files changed

+25
-9
lines changed

3 files changed

+25
-9
lines changed

clippy_lints/src/new_without_default.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_span::sym;
1313

1414
declare_clippy_lint! {
1515
/// ### What it does
16-
/// Checks for types with a `fn new() -> Self` method and no
16+
/// Checks for public types with a `pub fn new() -> Self` method and no
1717
/// implementation of
1818
/// [`Default`](https://doc.rust-lang.org/std/default/trait.Default.html).
1919
///
@@ -24,10 +24,10 @@ declare_clippy_lint! {
2424
///
2525
/// ### Example
2626
/// ```ignore
27-
/// struct Foo(Bar);
27+
/// pub struct Foo(Bar);
2828
///
2929
/// impl Foo {
30-
/// fn new() -> Self {
30+
/// pub fn new() -> Self {
3131
/// Foo(Bar::new())
3232
/// }
3333
/// }
@@ -36,7 +36,7 @@ declare_clippy_lint! {
3636
/// To fix the lint, add a `Default` implementation that delegates to `new`:
3737
///
3838
/// ```ignore
39-
/// struct Foo(Bar);
39+
/// pub struct Foo(Bar);
4040
///
4141
/// impl Default for Foo {
4242
/// fn default() -> Self {
@@ -47,7 +47,7 @@ declare_clippy_lint! {
4747
#[clippy::version = "pre 1.29.0"]
4848
pub NEW_WITHOUT_DEFAULT,
4949
style,
50-
"`fn new() -> Self` method without `Default` implementation"
50+
"`pub fn new() -> Self` method without `Default` implementation"
5151
}
5252

5353
#[derive(Clone, Default)]

tests/ui/new_without_default.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,22 @@ impl Private {
9090
} // We don't lint private items
9191
}
9292

93+
struct PrivateStruct;
94+
95+
impl PrivateStruct {
96+
pub fn new() -> PrivateStruct {
97+
unimplemented!()
98+
} // We don't lint public items on private structs
99+
}
100+
101+
pub struct PrivateItem;
102+
103+
impl PrivateItem {
104+
fn new() -> PrivateItem {
105+
unimplemented!()
106+
} // We don't lint private items on public structs
107+
}
108+
93109
struct Const;
94110

95111
impl Const {

tests/ui/new_without_default.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ LL + }
5151
|
5252

5353
error: you should consider adding a `Default` implementation for `NewNotEqualToDerive`
54-
--> $DIR/new_without_default.rs:156:5
54+
--> $DIR/new_without_default.rs:172:5
5555
|
5656
LL | / pub fn new() -> Self {
5757
LL | | NewNotEqualToDerive { foo: 1 }
@@ -68,7 +68,7 @@ LL + }
6868
|
6969

7070
error: you should consider adding a `Default` implementation for `FooGenerics<T>`
71-
--> $DIR/new_without_default.rs:164:5
71+
--> $DIR/new_without_default.rs:180:5
7272
|
7373
LL | / pub fn new() -> Self {
7474
LL | | Self(Default::default())
@@ -85,7 +85,7 @@ LL + }
8585
|
8686

8787
error: you should consider adding a `Default` implementation for `BarGenerics<T>`
88-
--> $DIR/new_without_default.rs:171:5
88+
--> $DIR/new_without_default.rs:187:5
8989
|
9090
LL | / pub fn new() -> Self {
9191
LL | | Self(Default::default())
@@ -102,7 +102,7 @@ LL + }
102102
|
103103

104104
error: you should consider adding a `Default` implementation for `Foo<T>`
105-
--> $DIR/new_without_default.rs:182:9
105+
--> $DIR/new_without_default.rs:198:9
106106
|
107107
LL | / pub fn new() -> Self {
108108
LL | | todo!()

0 commit comments

Comments
 (0)