Skip to content

Fix: Use multipart_suggestion for derivable_impls #13717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 23 additions & 25 deletions clippy_lints/src/derivable_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
});
Expand All @@ -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,
);
});
Expand Down
295 changes: 295 additions & 0 deletions tests/ui/derivable_impls.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
2 changes: 0 additions & 2 deletions tests/ui/derivable_impls.rs
Original file line number Diff line number Diff line change
@@ -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> {
Expand Down
Loading