Skip to content

Commit

Permalink
Auto merge of rust-lang#9429 - kraktus:deriv_ma, r=xFrednet
Browse files Browse the repository at this point in the history
Make `derivable_impls` machine applicable

changelog: [`derivable_impls`]: Now machine applicable
  • Loading branch information
bors committed Sep 13, 2022
2 parents 2e55b42 + 6f13203 commit 5564158
Show file tree
Hide file tree
Showing 4 changed files with 278 additions and 19 deletions.
24 changes: 19 additions & 5 deletions clippy_lints/src/derivable_impls.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{is_default_equivalent, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{
def::{DefKind, Res},
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
Expand Down Expand Up @@ -100,15 +101,28 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
_ => false,
};

if should_emit {
let path_string = cx.tcx.def_path_str(adt_def.did());
span_lint_and_help(
let struct_span = cx.tcx.def_span(adt_def.did());
span_lint_and_then(
cx,
DERIVABLE_IMPLS,
item.span,
"this `impl` can be derived",
None,
&format!("try annotating `{}` with `#[derive(Default)]`", path_string),
|diag| {
diag.span_suggestion_hidden(
item.span,
"remove the manual implementation...",
String::new(),
Applicability::MachineApplicable
);
diag.span_suggestion(
struct_span.shrink_to_lo(),
"...and instead derive it",
"#[derive(Default)]\n".to_string(),
Applicability::MachineApplicable
);
}
);
}
}
Expand Down
213 changes: 213 additions & 0 deletions tests/ui/derivable_impls.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
// run-rustfix

#![allow(dead_code)]

use std::collections::HashMap;

#[derive(Default)]
struct FooDefault<'a> {
a: bool,
b: i32,
c: u64,
d: Vec<i32>,
e: FooND1,
f: FooND2,
g: HashMap<i32, i32>,
h: (i32, Vec<i32>),
i: [Vec<i32>; 3],
j: [i32; 5],
k: Option<i32>,
l: &'a [i32],
}



#[derive(Default)]
struct TupleDefault(bool, i32, u64);



struct FooND1 {
a: bool,
}

impl std::default::Default for FooND1 {
fn default() -> Self {
Self { a: true }
}
}

struct FooND2 {
a: i32,
}

impl std::default::Default for FooND2 {
fn default() -> Self {
Self { a: 5 }
}
}

struct FooNDNew {
a: bool,
}

impl FooNDNew {
fn new() -> Self {
Self { a: true }
}
}

impl Default for FooNDNew {
fn default() -> Self {
Self::new()
}
}

struct FooNDVec(Vec<i32>);

impl Default for FooNDVec {
fn default() -> Self {
Self(vec![5, 12])
}
}

#[derive(Default)]
struct StrDefault<'a>(&'a str);



#[derive(Default)]
struct AlreadyDerived(i32, bool);

macro_rules! mac {
() => {
0
};
($e:expr) => {
struct X(u32);
impl Default for X {
fn default() -> Self {
Self($e)
}
}
};
}

mac!(0);

#[derive(Default)]
struct Y(u32);


struct RustIssue26925<T> {
a: Option<T>,
}

// We should watch out for cases where a manual impl is needed because a
// derive adds different type bounds (https://github.com/rust-lang/rust/issues/26925).
// For example, a struct with Option<T> does not require T: Default, but a derive adds
// that type bound anyways. So until #26925 get fixed we should disable lint
// for the following case
impl<T> Default for RustIssue26925<T> {
fn default() -> Self {
Self { a: None }
}
}

struct SpecializedImpl<A, B> {
a: A,
b: B,
}

impl<T: Default> Default for SpecializedImpl<T, T> {
fn default() -> Self {
Self {
a: T::default(),
b: T::default(),
}
}
}

#[derive(Default)]
struct WithoutSelfCurly {
a: bool,
}



#[derive(Default)]
struct WithoutSelfParan(bool);



// https://github.com/rust-lang/rust-clippy/issues/7655

pub struct SpecializedImpl2<T> {
v: Vec<T>,
}

impl Default for SpecializedImpl2<String> {
fn default() -> Self {
Self { v: Vec::new() }
}
}

// https://github.com/rust-lang/rust-clippy/issues/7654

pub struct Color {
pub r: u8,
pub g: u8,
pub b: u8,
}

/// `#000000`
impl Default for Color {
fn default() -> Self {
Color { r: 0, g: 0, b: 0 }
}
}

pub struct Color2 {
pub r: u8,
pub g: u8,
pub b: u8,
}

impl Default for Color2 {
/// `#000000`
fn default() -> Self {
Self { r: 0, g: 0, b: 0 }
}
}

#[derive(Default)]
pub struct RepeatDefault1 {
a: [i8; 32],
}



pub struct RepeatDefault2 {
a: [i8; 33],
}

impl Default for RepeatDefault2 {
fn default() -> Self {
RepeatDefault2 { a: [0; 33] }
}
}

// https://github.com/rust-lang/rust-clippy/issues/7753

pub enum IntOrString {
Int(i32),
String(String),
}

impl Default for IntOrString {
fn default() -> Self {
IntOrString::Int(0)
}
}

fn main() {}
4 changes: 4 additions & 0 deletions tests/ui/derivable_impls.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// run-rustfix

#![allow(dead_code)]

use std::collections::HashMap;

struct FooDefault<'a> {
Expand Down
56 changes: 42 additions & 14 deletions tests/ui/derivable_impls.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this `impl` can be derived
--> $DIR/derivable_impls.rs:18:1
--> $DIR/derivable_impls.rs:22:1
|
LL | / impl std::default::Default for FooDefault<'_> {
LL | | fn default() -> Self {
Expand All @@ -11,10 +11,14 @@ LL | | }
| |_^
|
= note: `-D clippy::derivable-impls` implied by `-D warnings`
= help: try annotating `FooDefault` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: this `impl` can be derived
--> $DIR/derivable_impls.rs:39:1
--> $DIR/derivable_impls.rs:43:1
|
LL | / impl std::default::Default for TupleDefault {
LL | | fn default() -> Self {
Expand All @@ -23,10 +27,14 @@ LL | | }
LL | | }
| |_^
|
= help: try annotating `TupleDefault` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: this `impl` can be derived
--> $DIR/derivable_impls.rs:91:1
--> $DIR/derivable_impls.rs:95:1
|
LL | / impl Default for StrDefault<'_> {
LL | | fn default() -> Self {
Expand All @@ -35,10 +43,14 @@ LL | | }
LL | | }
| |_^
|
= help: try annotating `StrDefault` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: this `impl` can be derived
--> $DIR/derivable_impls.rs:117:1
--> $DIR/derivable_impls.rs:121:1
|
LL | / impl Default for Y {
LL | | fn default() -> Self {
Expand All @@ -47,10 +59,14 @@ LL | | }
LL | | }
| |_^
|
= help: try annotating `Y` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: this `impl` can be derived
--> $DIR/derivable_impls.rs:156:1
--> $DIR/derivable_impls.rs:160:1
|
LL | / impl Default for WithoutSelfCurly {
LL | | fn default() -> Self {
Expand All @@ -59,10 +75,14 @@ LL | | }
LL | | }
| |_^
|
= help: try annotating `WithoutSelfCurly` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: this `impl` can be derived
--> $DIR/derivable_impls.rs:164:1
--> $DIR/derivable_impls.rs:168:1
|
LL | / impl Default for WithoutSelfParan {
LL | | fn default() -> Self {
Expand All @@ -71,10 +91,14 @@ LL | | }
LL | | }
| |_^
|
= help: try annotating `WithoutSelfParan` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: this `impl` can be derived
--> $DIR/derivable_impls.rs:214:1
--> $DIR/derivable_impls.rs:218:1
|
LL | / impl Default for RepeatDefault1 {
LL | | fn default() -> Self {
Expand All @@ -83,7 +107,11 @@ LL | | }
LL | | }
| |_^
|
= help: try annotating `RepeatDefault1` with `#[derive(Default)]`
= help: remove the manual implementation...
help: ...and instead derive it
|
LL | #[derive(Default)]
|

error: aborting due to 7 previous errors

0 comments on commit 5564158

Please sign in to comment.