Fix: Use multipart_suggestion for derivable_impls (#13717)
This should address #13099 for the `derivable_impls` test. As I've not contributed to clippy before, I'd like to make sure i'm on the right track before doing more :) changelog: [`derivable_impls`]: Use multipart_suggestion to aggregate feedback
This commit is contained in:
Philipp Krones
GitHub
commit
9b0597d78a
4 changed files with 343 additions and 62 deletions
|
|
@ -132,17 +132,15 @@ fn check_struct<'tcx>(
|
|||
|
||||
if should_emit {
|
||||
let struct_span = cx.tcx.def_span(adt_def.did());
|
||||
let suggestions = vec![
|
||||
(item.span, String::new()), // Remove the manual implementation
|
||||
(struct_span.shrink_to_lo(), "#[derive(Default)]\n".to_string()), // Add the derive attribute
|
||||
];
|
||||
|
||||
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |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(),
|
||||
diag.multipart_suggestion(
|
||||
"replace the manual implementation with a derive attribute",
|
||||
suggestions,
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
});
|
||||
|
|
@ -161,23 +159,23 @@ fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Ex
|
|||
let indent_enum = indent_of(cx, enum_span).unwrap_or(0);
|
||||
let variant_span = cx.tcx.def_span(variant_def.def_id);
|
||||
let indent_variant = indent_of(cx, variant_span).unwrap_or(0);
|
||||
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
|
||||
diag.span_suggestion_hidden(
|
||||
item.span,
|
||||
"remove the manual implementation...",
|
||||
String::new(),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
diag.span_suggestion(
|
||||
|
||||
let suggestions = vec![
|
||||
(item.span, String::new()), // Remove the manual implementation
|
||||
(
|
||||
enum_span.shrink_to_lo(),
|
||||
"...and instead derive it...",
|
||||
format!("#[derive(Default)]\n{indent}", indent = " ".repeat(indent_enum),),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
diag.span_suggestion(
|
||||
format!("#[derive(Default)]\n{}", " ".repeat(indent_enum)),
|
||||
), // Add the derive attribute
|
||||
(
|
||||
variant_span.shrink_to_lo(),
|
||||
"...and mark the default variant",
|
||||
format!("#[default]\n{indent}", indent = " ".repeat(indent_variant),),
|
||||
format!("#[default]\n{}", " ".repeat(indent_variant)),
|
||||
), // Mark the default variant
|
||||
];
|
||||
|
||||
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
|
||||
diag.multipart_suggestion(
|
||||
"replace the manual implementation with a derive attribute and mark the default variant",
|
||||
suggestions,
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
});
|
||||
|
|
|
|||
295
tests/ui/derivable_impls.fixed
Normal file
295
tests/ui/derivable_impls.fixed
Normal file
|
|
@ -0,0 +1,295 @@
|
|||
#![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)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub enum SimpleEnum {
|
||||
Foo,
|
||||
#[default]
|
||||
Bar,
|
||||
}
|
||||
|
||||
|
||||
pub enum NonExhaustiveEnum {
|
||||
Foo,
|
||||
#[non_exhaustive]
|
||||
Bar,
|
||||
}
|
||||
|
||||
impl Default for NonExhaustiveEnum {
|
||||
fn default() -> Self {
|
||||
NonExhaustiveEnum::Bar
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/rust-lang/rust-clippy/issues/10396
|
||||
|
||||
#[derive(Default)]
|
||||
struct DefaultType;
|
||||
|
||||
struct GenericType<T = DefaultType> {
|
||||
t: T,
|
||||
}
|
||||
|
||||
impl Default for GenericType {
|
||||
fn default() -> Self {
|
||||
Self { t: Default::default() }
|
||||
}
|
||||
}
|
||||
|
||||
struct InnerGenericType<T> {
|
||||
t: T,
|
||||
}
|
||||
|
||||
impl Default for InnerGenericType<DefaultType> {
|
||||
fn default() -> Self {
|
||||
Self { t: Default::default() }
|
||||
}
|
||||
}
|
||||
|
||||
struct OtherGenericType<T = DefaultType> {
|
||||
inner: InnerGenericType<T>,
|
||||
}
|
||||
|
||||
impl Default for OtherGenericType {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
inner: Default::default(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod issue10158 {
|
||||
pub trait T {}
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct S {}
|
||||
impl T for S {}
|
||||
|
||||
pub struct Outer {
|
||||
pub inner: Box<dyn T>,
|
||||
}
|
||||
|
||||
impl Default for Outer {
|
||||
fn default() -> Self {
|
||||
Outer {
|
||||
// Box::<S>::default() adjusts to Box<dyn T>
|
||||
inner: Box::<S>::default(),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod issue11368 {
|
||||
pub struct A {
|
||||
a: u32,
|
||||
}
|
||||
|
||||
impl Default for A {
|
||||
#[track_caller]
|
||||
fn default() -> Self {
|
||||
Self { a: 0 }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
@ -1,7 +1,5 @@
|
|||
#![allow(dead_code)]
|
||||
|
||||
//@no-rustfix: need to change the suggestion to a multipart suggestion
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
struct FooDefault<'a> {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:22:1
|
||||
--> tests/ui/derivable_impls.rs:20:1
|
||||
|
|
||||
LL | / impl std::default::Default for FooDefault<'_> {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -12,15 +12,14 @@ LL | | }
|
|||
|
|
||||
= note: `-D clippy::derivable-impls` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::derivable_impls)]`
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | struct FooDefault<'a> {
|
||||
LL ~ struct FooDefault<'a> {
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:43:1
|
||||
--> tests/ui/derivable_impls.rs:41:1
|
||||
|
|
||||
LL | / impl std::default::Default for TupleDefault {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -29,15 +28,14 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | struct TupleDefault(bool, i32, u64);
|
||||
LL ~ struct TupleDefault(bool, i32, u64);
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:95:1
|
||||
--> tests/ui/derivable_impls.rs:93:1
|
||||
|
|
||||
LL | / impl Default for StrDefault<'_> {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -46,15 +44,14 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | struct StrDefault<'a>(&'a str);
|
||||
LL ~ struct StrDefault<'a>(&'a str);
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:121:1
|
||||
--> tests/ui/derivable_impls.rs:119:1
|
||||
|
|
||||
LL | / impl Default for Y {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -63,15 +60,14 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | struct Y(u32);
|
||||
LL ~ struct Y(u32);
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:160:1
|
||||
--> tests/ui/derivable_impls.rs:158:1
|
||||
|
|
||||
LL | / impl Default for WithoutSelfCurly {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -80,15 +76,14 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | struct WithoutSelfCurly {
|
||||
LL ~ struct WithoutSelfCurly {
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:168:1
|
||||
--> tests/ui/derivable_impls.rs:166:1
|
||||
|
|
||||
LL | / impl Default for WithoutSelfParan {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -97,15 +92,14 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | struct WithoutSelfParan(bool);
|
||||
LL ~ struct WithoutSelfParan(bool);
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:218:1
|
||||
--> tests/ui/derivable_impls.rs:216:1
|
||||
|
|
||||
LL | / impl Default for RepeatDefault1 {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -114,15 +108,14 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | pub struct RepeatDefault1 {
|
||||
LL ~ pub struct RepeatDefault1 {
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:252:1
|
||||
--> tests/ui/derivable_impls.rs:250:1
|
||||
|
|
||||
LL | / impl Default for SimpleEnum {
|
||||
LL | | fn default() -> Self {
|
||||
|
|
@ -131,14 +124,11 @@ LL | | }
|
|||
LL | | }
|
||||
| |_^
|
||||
|
|
||||
= help: remove the manual implementation...
|
||||
help: ...and instead derive it...
|
||||
help: replace the manual implementation with a derive attribute and mark the default variant
|
||||
|
|
||||
LL + #[derive(Default)]
|
||||
LL | pub enum SimpleEnum {
|
||||
|
|
||||
help: ...and mark the default variant
|
||||
|
|
||||
LL ~ pub enum SimpleEnum {
|
||||
LL | Foo,
|
||||
LL ~ #[default]
|
||||
LL ~ Bar,
|
||||
|
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue