diff options
| -rw-r--r-- | clippy_lints/src/comparison_to_empty.rs | 155 | ||||
| -rw-r--r-- | clippy_lints/src/len_zero.rs | 84 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 8 |
3 files changed, 86 insertions, 161 deletions
diff --git a/clippy_lints/src/comparison_to_empty.rs b/clippy_lints/src/comparison_to_empty.rs deleted file mode 100644 index 3b55336722c..00000000000 --- a/clippy_lints/src/comparison_to_empty.rs +++ /dev/null @@ -1,155 +0,0 @@ -use crate::utils::{snippet_with_applicability, span_lint_and_sugg}; -use rustc_ast::ast::LitKind; -use rustc_errors::Applicability; -use rustc_hir::def_id::DefId; -use rustc_hir::{BinOpKind, Expr, ExprKind, ItemKind, TraitItemRef}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::source_map::{Span, Spanned}; - -declare_clippy_lint! { - /// **What it does:** - /// - /// **Why is this bad?** - /// - /// **Known problems:** None. - /// - /// **Example:** - /// - /// ```rust - /// // example code where clippy issues a warning - /// ``` - /// Use instead: - /// ```rust - /// // example code which does not raise clippy warning - /// ``` - pub COMPARISON_TO_EMPTY, - style, - "default lint description" -} - -declare_lint_pass!(ComparisonToEmpty => [COMPARISON_TO_EMPTY]); - -impl LateLintPass<'_> for ComparisonToEmpty { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if expr.span.from_expansion() { - return; - } - - if let ExprKind::Binary(Spanned { node: cmp, .. }, ref left, ref right) = expr.kind { - match cmp { - BinOpKind::Eq => { - check_cmp(cx, expr.span, left, right, "", 0); // len == 0 - check_cmp(cx, expr.span, right, left, "", 0); // 0 == len - }, - BinOpKind::Ne => { - check_cmp(cx, expr.span, left, right, "!", 0); // len != 0 - check_cmp(cx, expr.span, right, left, "!", 0); // 0 != len - }, - BinOpKind::Gt => { - check_cmp(cx, expr.span, left, right, "!", 0); // len > 0 - check_cmp(cx, expr.span, right, left, "", 1); // 1 > len - }, - BinOpKind::Lt => { - check_cmp(cx, expr.span, left, right, "", 1); // len < 1 - check_cmp(cx, expr.span, right, left, "!", 0); // 0 < len - }, - BinOpKind::Ge => check_cmp(cx, expr.span, left, right, "!", 1), // len >= 1 - BinOpKind::Le => check_cmp(cx, expr.span, right, left, "!", 1), // 1 <= len - _ => (), - } - } - } - -} - - -fn check_cmp(cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str, compare_to: u32) { - check_empty_expr(cx, span, lit1, lit2, op) -} - -fn check_empty_expr( - cx: &LateContext<'_>, - span: Span, - lit1: &Expr<'_>, - lit2: &Expr<'_>, - op: &str -) { - if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1) { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - COMPARISON_TO_EMPTY, - span, - &format!("comparison to empty slice"), - &format!("using `{}is_empty` is clearer and more explicit", op), - format!( - "{}{}.is_empty()", - op, - snippet_with_applicability(cx, lit1.span, "_", &mut applicability) - ), - applicability, - ); - } -} - -fn is_empty_string(expr: &Expr<'_>) -> bool { - if let ExprKind::Lit(ref lit) = expr.kind { - if let LitKind::Str(lit, _) = lit.node { - let lit = lit.as_str(); - return lit == ""; - } - } - false -} - -fn is_empty_array(expr: &Expr<'_>) -> bool { - if let ExprKind::Array(ref arr) = expr.kind { - return arr.is_empty(); - } - false -} - - -/// Checks if this type has an `is_empty` method. -fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - /// Gets an `AssocItem` and return true if it matches `is_empty(self)`. - fn is_is_empty(cx: &LateContext<'_>, item: &ty::AssocItem) -> bool { - if let ty::AssocKind::Fn = item.kind { - if item.ident.name.as_str() == "is_empty" { - let sig = cx.tcx.fn_sig(item.def_id); - let ty = sig.skip_binder(); - ty.inputs().len() == 1 - } else { - false - } - } else { - false - } - } - - /// Checks the inherent impl's items for an `is_empty(self)` method. - fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId) -> bool { - cx.tcx.inherent_impls(id).iter().any(|imp| { - cx.tcx - .associated_items(*imp) - .in_definition_order() - .any(|item| is_is_empty(cx, &item)) - }) - } - - let ty = &cx.typeck_results().expr_ty(expr).peel_refs(); - match ty.kind() { - ty::Dynamic(ref tt, ..) => tt.principal().map_or(false, |principal| { - cx.tcx - .associated_items(principal.def_id()) - .in_definition_order() - .any(|item| is_is_empty(cx, &item)) - }), - ty::Projection(ref proj) => has_is_empty_impl(cx, proj.item_def_id), - ty::Adt(id, _) => has_is_empty_impl(cx, id.did), - ty::Array(..) | ty::Slice(..) | ty::Str => true, - _ => false, - } -} diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index c9c4891bb08..75e9c5c973b 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -68,7 +68,45 @@ declare_clippy_lint! { "traits or impls with a public `len` method but no corresponding `is_empty` method" } -declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY]); +declare_clippy_lint! { + /// **What it does:** Checks for comparing to an empty slice such as "" or [],` + /// and suggests using `.is_empty()` where applicable. + /// + /// **Why is this bad?** Some structures can answer `.is_empty()` much faster + /// than checking for equality. So it is good to get into the habit of using + /// `.is_empty()`, and having it is cheap. + /// Besides, it makes the intent clearer than a manual comparison in some contexts. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```ignore + /// if s == "" { + /// .. + /// } + + /// if arr == [] { + /// .. + /// } + /// ``` + /// Use instead: + /// ```ignore + /// if s.is_empty() { + /// .. + /// } + + /// if arr.is_empty() { + /// .. + /// } + /// ``` + pub COMPARISON_TO_EMPTY, + style, + "default lint description" +} + + +declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]); impl<'tcx> LateLintPass<'tcx> for LenZero { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { @@ -221,6 +259,8 @@ fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_> } check_len(cx, span, method_path.ident.name, args, &lit.node, op, compare_to) + } else { + check_empty_expr(cx, span, method, lit, op) } } @@ -258,6 +298,48 @@ fn check_len( } } +fn check_empty_expr( + cx: &LateContext<'_>, + span: Span, + lit1: &Expr<'_>, + lit2: &Expr<'_>, + op: &str +) { + if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1) { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + COMPARISON_TO_EMPTY, + span, + &format!("comparison to empty slice"), + &format!("using `{}is_empty` is clearer and more explicit", op), + format!( + "{}{}.is_empty()", + op, + snippet_with_applicability(cx, lit1.span, "_", &mut applicability) + ), + applicability, + ); + } +} + +fn is_empty_string(expr: &Expr<'_>) -> bool { + if let ExprKind::Lit(ref lit) = expr.kind { + if let LitKind::Str(lit, _) = lit.node { + let lit = lit.as_str(); + return lit == ""; + } + } + false +} + +fn is_empty_array(expr: &Expr<'_>) -> bool { + if let ExprKind::Array(ref arr) = expr.kind { + return arr.is_empty(); + } + false +} + /// Checks if this type has an `is_empty` method. fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { /// Gets an `AssocItem` and return true if it matches `is_empty(self)`. diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 75629e13a8e..3ad2e1e9d57 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -171,7 +171,6 @@ mod checked_conversions; mod cognitive_complexity; mod collapsible_if; mod comparison_chain; -mod comparison_to_empty; mod copies; mod copy_iterator; mod create_dir; @@ -524,7 +523,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &cognitive_complexity::COGNITIVE_COMPLEXITY, &collapsible_if::COLLAPSIBLE_IF, &comparison_chain::COMPARISON_CHAIN, - &comparison_to_empty::COMPARISON_TO_EMPTY, + &len_zero::COMPARISON_TO_EMPTY, &copies::IFS_SAME_COND, &copies::IF_SAME_THEN_ELSE, &copies::MATCH_SAME_ARMS, @@ -1141,7 +1140,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods)); store.register_early_pass(|| box asm_syntax::InlineAsmX86AttSyntax); store.register_early_pass(|| box asm_syntax::InlineAsmX86IntelSyntax); - store.register_late_pass(|| box comparison_to_empty::ComparisonToEmpty); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ @@ -1302,7 +1300,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&bytecount::NAIVE_BYTECOUNT), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&comparison_chain::COMPARISON_CHAIN), - LintId::of(&comparison_to_empty::COMPARISON_TO_EMPTY), + LintId::of(&len_zero::COMPARISON_TO_EMPTY), LintId::of(&copies::IFS_SAME_COND), LintId::of(&copies::IF_SAME_THEN_ELSE), LintId::of(&derive::DERIVE_HASH_XOR_EQ), @@ -1559,7 +1557,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&comparison_chain::COMPARISON_CHAIN), - LintId::of(&comparison_to_empty::COMPARISON_TO_EMPTY), + LintId::of(&len_zero::COMPARISON_TO_EMPTY), LintId::of(&doc::MISSING_SAFETY_DOC), LintId::of(&doc::NEEDLESS_DOCTEST_MAIN), LintId::of(&enum_variants::ENUM_VARIANT_NAMES), |