Add lint for comparing to empty slices instead of using .is_empty()

CHANGELOG.md

@@ -1664,6 +1664,7 @@ Released 2018-09-13
 [`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
 [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
 [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
+[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
 [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir
 [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute

clippy_lints/src/len_zero.rs

@@ -68,7 +68,44 @@ 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,
+ "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<'_>) {
@@ -221,6 +258,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 +297,42 @@ 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,
+ "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)`.

clippy_lints/src/lib.rs

@@ -608,6 +608,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  &large_const_arrays::LARGE_CONST_ARRAYS,
  &large_enum_variant::LARGE_ENUM_VARIANT,
  &large_stack_arrays::LARGE_STACK_ARRAYS,
+ &len_zero::COMPARISON_TO_EMPTY,
  &len_zero::LEN_WITHOUT_IS_EMPTY,
  &len_zero::LEN_ZERO,
  &let_if_seq::USELESS_LET_IF_SEQ,
@@ -1348,6 +1349,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(&int_plus_one::INT_PLUS_ONE),
  LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
  LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
+ LintId::of(&len_zero::COMPARISON_TO_EMPTY),
  LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
  LintId::of(&len_zero::LEN_ZERO),
  LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
@@ -1570,6 +1572,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(&functions::RESULT_UNIT_ERR),
  LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
  LintId::of(&inherent_to_string::INHERENT_TO_STRING),
+ LintId::of(&len_zero::COMPARISON_TO_EMPTY),
  LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
  LintId::of(&len_zero::LEN_ZERO),
  LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),

src/lintlist/mod.rs

@@ -291,6 +291,13 @@ vec![
  deprecation: None,
  module: "comparison_chain",
  },
+ Lint {
+ name: "comparison_to_empty",
+ group: "style",
+ desc: "checking `x == \"\"` or `x == []` (or similar) when `.is_empty()` could be used instead",
+ deprecation: None,
+ module: "len_zero",
+ },
  Lint {
  name: "copy_iterator",
  group: "pedantic",

tests/ui/comparison_to_empty.fixed

@@ -0,0 +1,23 @@
+// run-rustfix
+
+#![warn(clippy::comparison_to_empty)]
+
+fn main() {
+ // Disallow comparisons to empty
+ let s = String::new();
+ let _ = s.is_empty();
+ let _ = !s.is_empty();
+
+ let v = vec![0];
+ let _ = v.is_empty();
+ let _ = !v.is_empty();
+
+ // Allow comparisons to non-empty
+ let s = String::new();
+ let _ = s == " ";
+ let _ = s != " ";
+
+ let v = vec![0];
+ let _ = v == [0];
+ let _ = v != [0];
+}

tests/ui/comparison_to_empty.rs

@@ -0,0 +1,23 @@
+// run-rustfix
+
+#![warn(clippy::comparison_to_empty)]
+
+fn main() {
+ // Disallow comparisons to empty
+ let s = String::new();
+ let _ = s == "";
+ let _ = s != "";
+
+ let v = vec![0];
+ let _ = v == [];
+ let _ = v != [];
+
+ // Allow comparisons to non-empty
+ let s = String::new();
+ let _ = s == " ";
+ let _ = s != " ";
+
+ let v = vec![0];
+ let _ = v == [0];
+ let _ = v != [0];
+}

tests/ui/comparison_to_empty.stderr

@@ -0,0 +1,28 @@
+error: comparison to empty slice
+ --> $DIR/comparison_to_empty.rs:8:13
+ |
+LL | let _ = s == "";
+ | ^^^^^^^ help: using `is_empty` is clearer and more explicit: `s.is_empty()`
+ |
+ = note: `-D clippy::comparison-to-empty` implied by `-D warnings`
+
+error: comparison to empty slice
+ --> $DIR/comparison_to_empty.rs:9:13
+ |
+LL | let _ = s != "";
+ | ^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!s.is_empty()`
+
+error: comparison to empty slice
+ --> $DIR/comparison_to_empty.rs:12:13
+ |
+LL | let _ = v == [];
+ | ^^^^^^^ help: using `is_empty` is clearer and more explicit: `v.is_empty()`
+
+error: comparison to empty slice
+ --> $DIR/comparison_to_empty.rs:13:13
+ |
+LL | let _ = v != [];
+ | ^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!v.is_empty()`
+
+error: aborting due to 4 previous errors
+