Skip to content
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

default_trait_access: Fix wrong suggestion #5993

Merged
merged 1 commit into from
Sep 1, 2020
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
4 changes: 2 additions & 2 deletions clippy_lints/src/default_trait_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ impl<'tcx> LateLintPass<'tcx> for DefaultTraitAccess {
// TODO: Work out a way to put "whatever the imported way of referencing
// this type in this file" rather than a fully-qualified type.
let expr_ty = cx.typeck_results().expr_ty(expr);
if let ty::Adt(..) = expr_ty.kind {
let replacement = format!("{}::default()", expr_ty);
if let ty::Adt(def, ..) = expr_ty.kind {
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did));
span_lint_and_sugg(
cx,
DEFAULT_TRAIT_ACCESS,
Expand Down
106 changes: 106 additions & 0 deletions tests/ui/default_trait_access.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// run-rustfix

#![allow(unused_imports)]
#![deny(clippy::default_trait_access)]

use std::default;
use std::default::Default as D2;
use std::string;

fn main() {
let s1: String = std::string::String::default();

let s2 = String::default();

let s3: String = std::string::String::default();

let s4: String = std::string::String::default();

let s5 = string::String::default();

let s6: String = std::string::String::default();

let s7 = std::string::String::default();

let s8: String = DefaultFactory::make_t_badly();

let s9: String = DefaultFactory::make_t_nicely();

let s10 = DerivedDefault::default();

let s11: GenericDerivedDefault<String> = GenericDerivedDefault::default();

let s12 = GenericDerivedDefault::<String>::default();

let s13 = TupleDerivedDefault::default();

let s14: TupleDerivedDefault = TupleDerivedDefault::default();

let s15: ArrayDerivedDefault = ArrayDerivedDefault::default();

let s16 = ArrayDerivedDefault::default();

let s17: TupleStructDerivedDefault = TupleStructDerivedDefault::default();

let s18 = TupleStructDerivedDefault::default();

let s19 = <DerivedDefault as Default>::default();

println!(
"[{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}] [{:?}], [{:?}]",
s1,
s2,
s3,
s4,
s5,
s6,
s7,
s8,
s9,
s10,
s11,
s12,
s13,
s14,
s15,
s16,
s17,
s18,
s19,
);
}

struct DefaultFactory;

impl DefaultFactory {
pub fn make_t_badly<T: Default>() -> T {
Default::default()
}

pub fn make_t_nicely<T: Default>() -> T {
T::default()
}
}

#[derive(Debug, Default)]
struct DerivedDefault {
pub s: String,
}

#[derive(Debug, Default)]
struct GenericDerivedDefault<T: Default + std::fmt::Debug> {
pub s: T,
}

#[derive(Debug, Default)]
struct TupleDerivedDefault {
pub s: (String, String),
}

#[derive(Debug, Default)]
struct ArrayDerivedDefault {
pub s: [String; 10],
}

#[derive(Debug, Default)]
struct TupleStructDerivedDefault(String);
5 changes: 4 additions & 1 deletion tests/ui/default_trait_access.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#![warn(clippy::default_trait_access)]
// run-rustfix

#![allow(unused_imports)]
#![deny(clippy::default_trait_access)]

use std::default;
use std::default::Default as D2;
Expand Down
26 changes: 15 additions & 11 deletions tests/ui/default_trait_access.stderr
Original file line number Diff line number Diff line change
@@ -1,49 +1,53 @@
error: calling `std::string::String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:8:22
--> $DIR/default_trait_access.rs:11:22
|
LL | let s1: String = Default::default();
| ^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()`
|
= note: `-D clippy::default-trait-access` implied by `-D warnings`
note: the lint level is defined here
--> $DIR/default_trait_access.rs:4:9
|
LL | #![deny(clippy::default_trait_access)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: calling `std::string::String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:12:22
--> $DIR/default_trait_access.rs:15:22
|
LL | let s3: String = D2::default();
| ^^^^^^^^^^^^^ help: try: `std::string::String::default()`

error: calling `std::string::String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:14:22
--> $DIR/default_trait_access.rs:17:22
|
LL | let s4: String = std::default::Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()`

error: calling `std::string::String::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:18:22
--> $DIR/default_trait_access.rs:21:22
|
LL | let s6: String = default::Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::string::String::default()`

error: calling `GenericDerivedDefault<std::string::String>::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:28:46
error: calling `GenericDerivedDefault::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:31:46
|
LL | let s11: GenericDerivedDefault<String> = Default::default();
| ^^^^^^^^^^^^^^^^^^ help: try: `GenericDerivedDefault<std::string::String>::default()`
| ^^^^^^^^^^^^^^^^^^ help: try: `GenericDerivedDefault::default()`
Comment on lines -27 to +35
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already had a test for this, but it seems that the broken suggestion was missed.


error: calling `TupleDerivedDefault::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:34:36
--> $DIR/default_trait_access.rs:37:36
|
LL | let s14: TupleDerivedDefault = Default::default();
| ^^^^^^^^^^^^^^^^^^ help: try: `TupleDerivedDefault::default()`

error: calling `ArrayDerivedDefault::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:36:36
--> $DIR/default_trait_access.rs:39:36
|
LL | let s15: ArrayDerivedDefault = Default::default();
| ^^^^^^^^^^^^^^^^^^ help: try: `ArrayDerivedDefault::default()`

error: calling `TupleStructDerivedDefault::default()` is more clear than this expression
--> $DIR/default_trait_access.rs:40:42
--> $DIR/default_trait_access.rs:43:42
|
LL | let s17: TupleStructDerivedDefault = Default::default();
| ^^^^^^^^^^^^^^^^^^ help: try: `TupleStructDerivedDefault::default()`
Expand Down