Skip to content

Commit

Permalink
Auto merge of #10813 - y21:issue10755, r=xFrednet
Browse files Browse the repository at this point in the history
[`default_constructed_unit_structs`]: do not lint on type alias paths

Fixes #10755.

Type aliases cannot be used as a constructor, so this lint should not trigger in those cases.
I also changed `clippy_utils::is_ty_alias` to also consider associated types since [they kinda are type aliases too](https://github.com/rust-lang/rust/blob/48ec50ae39d0ca0baa0e78f56c395dcc6d7ebd65/compiler/rustc_resolve/src/late/diagnostics.rs#L1520).

changelog: [`default_constructed_unit_structs`]: do not lint on type alias paths
  • Loading branch information
bors committed May 26, 2023
2 parents 2422594 + 8c82486 commit f1fd467
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 7 deletions.
15 changes: 13 additions & 2 deletions clippy_lints/src/default_constructed_unit_structs.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, match_def_path, paths};
use clippy_utils::{diagnostics::span_lint_and_sugg, is_ty_alias, match_def_path, paths};
use hir::{def::Res, ExprKind};
use rustc_errors::Applicability;
use rustc_hir as hir;
Expand Down Expand Up @@ -43,12 +43,23 @@ declare_clippy_lint! {
}
declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]);

fn is_alias(ty: hir::Ty<'_>) -> bool {
if let hir::TyKind::Path(ref qpath) = ty.kind {
is_ty_alias(qpath)
} else {
false
}
}

impl LateLintPass<'_> for DefaultConstructedUnitStructs {
fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if_chain!(
// make sure we have a call to `Default::default`
if let hir::ExprKind::Call(fn_expr, &[]) = expr.kind;
if let ExprKind::Path(ref qpath@ hir::QPath::TypeRelative(_,_)) = fn_expr.kind;
if let ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(base, _)) = fn_expr.kind;
// make sure this isn't a type alias:
// `<Foo as Bar>::Assoc` cannot be used as a constructor
if !is_alias(*base);
if let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id);
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
// make sure we have a struct with no fields (unit struct)
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ pub fn is_wild(pat: &Pat<'_>) -> bool {
/// Checks if the given `QPath` belongs to a type alias.
pub fn is_ty_alias(qpath: &QPath<'_>) -> bool {
match *qpath {
QPath::Resolved(_, path) => matches!(path.res, Res::Def(DefKind::TyAlias, ..)),
QPath::Resolved(_, path) => matches!(path.res, Res::Def(DefKind::TyAlias | DefKind::AssocTy, ..)),
QPath::TypeRelative(ty, _) if let TyKind::Path(qpath) = ty.kind => { is_ty_alias(&qpath) },
_ => false,
}
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/default_constructed_unit_structs.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ struct EmptyStruct {}
#[non_exhaustive]
struct NonExhaustiveStruct;

mod issue_10755 {
struct Sqlite {}

trait HasArguments<'q> {
type Arguments;
}

impl<'q> HasArguments<'q> for Sqlite {
type Arguments = std::marker::PhantomData<&'q ()>;
}

type SqliteArguments<'q> = <Sqlite as HasArguments<'q>>::Arguments;

fn foo() {
// should not lint
// type alias cannot be used as a constructor
let _ = <Sqlite as HasArguments>::Arguments::default();

let _ = SqliteArguments::default();
}
}

fn main() {
// should lint
let _ = PhantomData::<usize>;
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/default_constructed_unit_structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ struct EmptyStruct {}
#[non_exhaustive]
struct NonExhaustiveStruct;

mod issue_10755 {
struct Sqlite {}

trait HasArguments<'q> {
type Arguments;
}

impl<'q> HasArguments<'q> for Sqlite {
type Arguments = std::marker::PhantomData<&'q ()>;
}

type SqliteArguments<'q> = <Sqlite as HasArguments<'q>>::Arguments;

fn foo() {
// should not lint
// type alias cannot be used as a constructor
let _ = <Sqlite as HasArguments>::Arguments::default();

let _ = SqliteArguments::default();
}
}

fn main() {
// should lint
let _ = PhantomData::<usize>::default();
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/default_constructed_unit_structs.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,25 @@ LL | inner: PhantomData::default(),
| ^^^^^^^^^^^ help: remove this call to `default`

error: use of `default` to create a unit struct
--> $DIR/default_constructed_unit_structs.rs:106:33
--> $DIR/default_constructed_unit_structs.rs:128:33
|
LL | let _ = PhantomData::<usize>::default();
| ^^^^^^^^^^^ help: remove this call to `default`

error: use of `default` to create a unit struct
--> $DIR/default_constructed_unit_structs.rs:107:42
--> $DIR/default_constructed_unit_structs.rs:129:42
|
LL | let _: PhantomData<i32> = PhantomData::default();
| ^^^^^^^^^^^ help: remove this call to `default`

error: use of `default` to create a unit struct
--> $DIR/default_constructed_unit_structs.rs:108:55
--> $DIR/default_constructed_unit_structs.rs:130:55
|
LL | let _: PhantomData<i32> = std::marker::PhantomData::default();
| ^^^^^^^^^^^ help: remove this call to `default`

error: use of `default` to create a unit struct
--> $DIR/default_constructed_unit_structs.rs:109:23
--> $DIR/default_constructed_unit_structs.rs:131:23
|
LL | let _ = UnitStruct::default();
| ^^^^^^^^^^^ help: remove this call to `default`
Expand Down

0 comments on commit f1fd467

Please sign in to comment.