-use crate::utils::{get_item_name, higher, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty};
+use crate::utils::{get_item_name, snippet_with_applicability, span_lint, span_lint_and_sugg};
use rustc_ast::ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
-use rustc_hir::{AssocItemKind, BinOpKind, Expr, ExprKind, ImplItemRef, Item, ItemKind, TraitItemRef};
+use rustc_hir::{AssocItemKind, BinOpKind, Expr, ExprKind, Impl, ImplItemRef, Item, ItemKind, TraitItemRef};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_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,
+ "checking `x == \"\"` or `x == []` (or similar) when `.is_empty()` could be used instead"
+}
+
+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<'_>) {
match item.kind {
ItemKind::Trait(_, _, _, _, ref trait_items) => check_trait_items(cx, item, trait_items),
- ItemKind::Impl {
+ ItemKind::Impl(Impl {
of_trait: None,
items: ref impl_items,
..
- } => check_impl_items(cx, item, impl_items),
+ }) => check_impl_items(cx, item, impl_items),
_ => (),
}
}
fn is_named_self(cx: &LateContext<'_>, item: &TraitItemRef, name: &str) -> bool {
item.ident.name.as_str() == name
&& if let AssocItemKind::Fn { has_self } = item.kind {
- has_self && {
- let did = cx.tcx.hir().local_def_id(item.id.hir_id);
- cx.tcx.fn_sig(did).inputs().skip_binder().len() == 1
- }
+ has_self && { cx.tcx.fn_sig(item.id.def_id).inputs().skip_binder().len() == 1 }
} else {
false
}
}
}
- if cx.access_levels.is_exported(visited_trait.hir_id) && trait_items.iter().any(|i| is_named_self(cx, i, "len")) {
+ if cx.access_levels.is_exported(visited_trait.hir_id()) && trait_items.iter().any(|i| is_named_self(cx, i, "len")) {
let mut current_and_super_traits = FxHashSet::default();
- let visited_trait_def_id = cx.tcx.hir().local_def_id(visited_trait.hir_id);
- fill_trait_set(visited_trait_def_id.to_def_id(), &mut current_and_super_traits, cx);
+ fill_trait_set(visited_trait.def_id.to_def_id(), &mut current_and_super_traits, cx);
let is_empty_method_found = current_and_super_traits
.iter()
fn is_named_self(cx: &LateContext<'_>, item: &ImplItemRef<'_>, name: &str) -> bool {
item.ident.name.as_str() == name
&& if let AssocItemKind::Fn { has_self } = item.kind {
- has_self && {
- let did = cx.tcx.hir().local_def_id(item.id.hir_id);
- cx.tcx.fn_sig(did).inputs().skip_binder().len() == 1
- }
+ has_self && cx.tcx.fn_sig(item.id.def_id).inputs().skip_binder().len() == 1
} else {
false
}
}
let is_empty = if let Some(is_empty) = impl_items.iter().find(|i| is_named_self(cx, i, "is_empty")) {
- if cx.access_levels.is_exported(is_empty.id.hir_id) {
+ if cx.access_levels.is_exported(is_empty.id.hir_id()) {
return;
- } else {
- "a private"
}
+ "a private"
} else {
"no corresponding"
};
if let Some(i) = impl_items.iter().find(|i| is_named_self(cx, i, "len")) {
- if cx.access_levels.is_exported(i.id.hir_id) {
- let def_id = cx.tcx.hir().local_def_id(item.hir_id);
- let ty = cx.tcx.type_of(def_id);
+ if cx.access_levels.is_exported(i.id.hir_id()) {
+ let ty = cx.tcx.type_of(item.def_id);
span_lint(
cx,
}
check_len(cx, span, method_path.ident.name, args, &lit.node, op, compare_to)
+ } else {
+ check_empty_expr(cx, span, method, lit, op)
}
}
}
}
-/// Checks if this type has an `is_empty` method.
-fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
- /// Special case ranges until `range_is_empty` is stabilized. See issue 3807.
- fn should_skip_range(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
- higher::range(cx, expr).map_or(false, |_| {
- !cx.tcx
- .features()
- .declared_lib_features
- .iter()
- .any(|(name, _)| name.as_str() == "range_is_empty")
- })
+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,
+ "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 should_skip_range(cx, expr) {
- return false;
- }
-
- let ty = &walk_ptrs_ty(cx.tables().expr_ty(expr));
- match ty.kind {
- ty::Dynamic(ref tt, ..) => {
- if let Some(principal) = tt.principal() {
- cx.tcx
- .associated_items(principal.def_id())
- .in_definition_order()
- .any(|item| is_is_empty(cx, &item))
- } else {
- false
- }
- },
+ 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,
projects / rust.git / blobdiff