Skip to content

Commit

Permalink
add derivable impls lint
Browse files Browse the repository at this point in the history
  • Loading branch information
HKalbasi committed Aug 19, 2021
1 parent 6d36d1a commit 6498b8f
Show file tree
Hide file tree
Showing 8 changed files with 327 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2483,6 +2483,7 @@ Released 2018-09-13
[`deprecated_cfg_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_cfg_attr
[`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
[`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
[`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls
[`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
[`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
Expand Down
114 changes: 114 additions & 0 deletions clippy_lints/src/derivable_impls.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::{in_macro, is_automatically_derived, is_default, match_def_path, paths};
use rustc_hir::{Block, Expr, ExprField, ExprKind, Impl, ImplItemKind, Item, ItemKind, Node, QPath};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks for double comparisons that could be simplified to a single expression.
///
///
/// ### Why is this bad?
/// Readability.
///
/// ### Example
/// ```rust
/// # let x = 1;
/// # let y = 2;
/// if x == y || x < y {}
/// ```
///
/// Could be written as:
///
/// ```rust
/// # let x = 1;
/// # let y = 2;
/// if x <= y {}
/// ```
pub DERIVABLE_IMPLS,
complexity,
"manual implementation of a trait which is equal to a derive"
}

declare_lint_pass!(DerivableImpls => [DERIVABLE_IMPLS]);

fn struct_fields<'hir>(e: &'hir Expr<'hir>) -> Option<&'hir [ExprField<'hir>]> {
match e.kind {
ExprKind::Struct(_, fields, _) => Some(fields),
ExprKind::Block(Block { expr, .. }, _) => expr.and_then(struct_fields),
_ => None,
}
}

fn is_path_self(e: &Expr<'_>) -> bool {
if let ExprKind::Path(QPath::Resolved(_, p)) = e.kind {
if p.segments.len() != 1 {
return false;
}
if p.segments[0].ident.name == rustc_span::symbol::kw::SelfUpper {
return true;
}
}
false
}

fn tuple_fields<'hir>(e: &'hir Expr<'hir>) -> Option<&'hir [Expr<'hir>]> {
match e.kind {
ExprKind::Tup(fields) => Some(fields),
ExprKind::Call(callee, args) => {
if is_path_self(callee) {
Some(args)
} else {
None
}
},
ExprKind::Block(Block { expr, .. }, _) => expr.and_then(tuple_fields),
_ => None,
}
}

impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if let ItemKind::Impl(Impl {
of_trait: Some(ref trait_ref),
items,
..
}) = item.kind
{
let attrs = cx.tcx.hir().attrs(item.hir_id());
if is_automatically_derived(attrs) {
return;
}
if in_macro(item.span) {
return;
}
if let Some(def_id) = trait_ref.trait_def_id() {
if !match_def_path(cx, def_id, &paths::DEFAULT_TRAIT) {
return;
}
}
let impl_item_hir = items[0].id.hir_id();
let func_expr = match cx.tcx.hir().find(impl_item_hir) {
Some(Node::ImplItem(x)) => match &x.kind {
ImplItemKind::Fn(_, b) => match cx.tcx.hir().find(b.hir_id) {
Some(Node::Expr(e)) => e,
_ => return,
},
_ => return,
},
_ => return,
};
let should_emit = if let Some(s) = struct_fields(func_expr) {
s.iter().all(|ef| is_default(cx, ef.expr))
} else if let Some(s) = tuple_fields(func_expr) {
s.iter().all(|e| is_default(cx, e))
} else {
return;
};
if should_emit {
span_lint(cx, DERIVABLE_IMPLS, item.span, "derivable impl of trait Default:");
}
}
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ mod dbg_macro;
mod default;
mod default_numeric_fallback;
mod dereference;
mod derivable_impls;
mod derive;
mod disallowed_method;
mod disallowed_script_idents;
Expand Down Expand Up @@ -587,6 +588,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
default::FIELD_REASSIGN_WITH_DEFAULT,
default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK,
dereference::EXPLICIT_DEREF_METHODS,
derivable_impls::DERIVABLE_IMPLS,
derive::DERIVE_HASH_XOR_EQ,
derive::DERIVE_ORD_XOR_PARTIAL_ORD,
derive::EXPL_IMPL_CLONE_ON_COPY,
Expand Down Expand Up @@ -1200,6 +1202,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(copies::IFS_SAME_COND),
LintId::of(copies::IF_SAME_THEN_ELSE),
LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
LintId::of(derivable_impls::DERIVABLE_IMPLS),
LintId::of(derive::DERIVE_HASH_XOR_EQ),
LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD),
LintId::of(doc::MISSING_SAFETY_DOC),
Expand Down Expand Up @@ -1583,6 +1586,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(casts::CHAR_LIT_AS_U8),
LintId::of(casts::UNNECESSARY_CAST),
LintId::of(copies::BRANCHES_SHARING_CODE),
LintId::of(derivable_impls::DERIVABLE_IMPLS),
LintId::of(double_comparison::DOUBLE_COMPARISONS),
LintId::of(double_parens::DOUBLE_PARENS),
LintId::of(duration_subsec::DURATION_SUBSEC),
Expand Down Expand Up @@ -1922,6 +1926,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented);
store.register_late_pass(|| box strings::StringLitAsBytes);
store.register_late_pass(|| box derive::Derive);
store.register_late_pass(|| box derivable_impls::DerivableImpls);
store.register_late_pass(|| box get_last_with_len::GetLastWithLen);
store.register_late_pass(|| box drop_forget_ref::DropForgetRef);
store.register_late_pass(|| box empty_enum::EmptyEnum);
Expand Down
77 changes: 19 additions & 58 deletions clippy_lints/src/mem_replace.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::{in_macro, is_diag_trait_item, is_lang_ctor, match_def_path, meets_msrv, msrvs, paths};
use clippy_utils::{in_macro, is_default, is_lang_ctor, match_def_path, meets_msrv, msrvs, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
use rustc_hir::LangItem::OptionNone;
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
Expand Down Expand Up @@ -194,64 +193,26 @@ fn check_replace_with_uninit(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'
}
}

/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent"
/// constructor from the std library
fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool {
let std_types_symbols = &[
sym::string_type,
sym::vec_type,
sym::vecdeque_type,
sym::LinkedList,
sym::hashmap_type,
sym::BTreeMap,
sym::hashset_type,
sym::BTreeSet,
sym::BinaryHeap,
];

if let QPath::TypeRelative(_, method) = path {
if method.ident.name == sym::new {
if let Some(impl_did) = cx.tcx.impl_of_method(def_id) {
if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() {
return std_types_symbols
.iter()
.any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did));
}
}
}
}
false
}

fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
if_chain! {
if let ExprKind::Call(repl_func, _) = src.kind;
if !in_external_macro(cx.tcx.sess, expr_span);
if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
if is_diag_trait_item(cx, repl_def_id, sym::Default)
|| is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);

then {
span_lint_and_then(
cx,
MEM_REPLACE_WITH_DEFAULT,
expr_span,
"replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`",
|diag| {
if !in_macro(expr_span) {
let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, ""));

diag.span_suggestion(
expr_span,
"consider using",
suggestion,
Applicability::MachineApplicable
);
}
if is_default(cx, src) && !in_external_macro(cx.tcx.sess, expr_span) {
span_lint_and_then(
cx,
MEM_REPLACE_WITH_DEFAULT,
expr_span,
"replacing a value of type `T` with `T::default()` is better expressed using `std::mem::take`",
|diag| {
if !in_macro(expr_span) {
let suggestion = format!("std::mem::take({})", snippet(cx, dest.span, ""));

diag.span_suggestion(
expr_span,
"consider using",
suggestion,
Applicability::MachineApplicable,
);
}
);
}
},
);
}
}

Expand Down
55 changes: 55 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,61 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
false
}

/// Returns true if the `def_id` associated with the `path` is recognized as a "default-equivalent"
/// constructor from the std library
fn is_default_equivalent_ctor(cx: &LateContext<'_>, def_id: DefId, path: &QPath<'_>) -> bool {
let std_types_symbols = &[
sym::string_type,
sym::vec_type,
sym::vecdeque_type,
sym::LinkedList,
sym::hashmap_type,
sym::BTreeMap,
sym::hashset_type,
sym::BTreeSet,
sym::BinaryHeap,
];

if let QPath::TypeRelative(_, method) = path {
if method.ident.name == sym::new {
if let Some(impl_did) = cx.tcx.impl_of_method(def_id) {
if let Some(adt) = cx.tcx.type_of(impl_did).ty_adt_def() {
return std_types_symbols
.iter()
.any(|&symbol| cx.tcx.is_diagnostic_item(symbol, adt.did));
}
}
}
}
false
}

pub fn is_default(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
match &e.kind {
ExprKind::Lit(lit) => match lit.node {
LitKind::Bool(x) => !x,
LitKind::Int(x, _) => x == 0,
LitKind::Str(s, _) => s.is_empty(),
_ => false,
},
ExprKind::Tup(items) | ExprKind::Array(items) => items.iter().all(|x| is_default(cx, x)),
ExprKind::Repeat(x, _) => is_default(cx, x),
ExprKind::Call(repl_func, _) => if_chain! {
if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
if let Some(repl_def_id) = cx.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
if is_diag_trait_item(cx, repl_def_id, sym::Default)
|| is_default_equivalent_ctor(cx, repl_def_id, repl_func_qpath);
then {
true
}
else {
false
}
},
_ => false,
}
}

/// Checks if the top level expression can be moved into a closure as is.
pub fn can_move_expr_to_closure_no_visit(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, jump_targets: &[HirId]) -> bool {
match expr.kind {
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"];
pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"];
pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
pub const CSTRING_AS_C_STR: [&str; 5] = ["std", "ffi", "c_str", "CString", "as_c_str"];
pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"];
/// Preferably use the diagnostic item `sym::deref_method` where possible
Expand Down
Loading

0 comments on commit 6498b8f

Please sign in to comment.