Initial implementation of comparison_to_empty

Author: Urcra

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

Cargo.toml

@@ -27,6 +27,13 @@ path = "src/main.rs"
 name = "clippy-driver"
 path = "src/driver.rs"
 
+[target.'cfg(NOT_A_PLATFORM)'.dependencies]
+rustc_data_structures = { path = "/home/urcra/rust/compiler/rustc_data_structures" }
+rustc_driver = { path = "/home/urcra/rust/compiler/rustc_driver" }
+rustc_errors = { path = "/home/urcra/rust/compiler/rustc_errors" }
+rustc_interface = { path = "/home/urcra/rust/compiler/rustc_interface" }
+rustc_middle = { path = "/home/urcra/rust/compiler/rustc_middle" }
+
 [dependencies]
 # begin automatic update
 clippy_lints = { version = "0.0.212", path = "clippy_lints" }

clippy_lints/Cargo.toml

@@ -16,6 +16,28 @@ license = "MIT OR Apache-2.0"
 keywords = ["clippy", "lint", "plugin"]
 edition = "2018"
 
+[target.'cfg(NOT_A_PLATFORM)'.dependencies]
+rustc_ast = { path = "/home/urcra/rust/compiler/rustc_ast" }
+rustc_ast_pretty = { path = "/home/urcra/rust/compiler/rustc_ast_pretty" }
+rustc_attr = { path = "/home/urcra/rust/compiler/rustc_attr" }
+rustc_data_structures = { path = "/home/urcra/rust/compiler/rustc_data_structures" }
+rustc_errors = { path = "/home/urcra/rust/compiler/rustc_errors" }
+rustc_hir = { path = "/home/urcra/rust/compiler/rustc_hir" }
+rustc_hir_pretty = { path = "/home/urcra/rust/compiler/rustc_hir_pretty" }
+rustc_index = { path = "/home/urcra/rust/compiler/rustc_index" }
+rustc_infer = { path = "/home/urcra/rust/compiler/rustc_infer" }
+rustc_lexer = { path = "/home/urcra/rust/compiler/rustc_lexer" }
+rustc_lint = { path = "/home/urcra/rust/compiler/rustc_lint" }
+rustc_middle = { path = "/home/urcra/rust/compiler/rustc_middle" }
+rustc_mir = { path = "/home/urcra/rust/compiler/rustc_mir" }
+rustc_parse = { path = "/home/urcra/rust/compiler/rustc_parse" }
+rustc_parse_format = { path = "/home/urcra/rust/compiler/rustc_parse_format" }
+rustc_session = { path = "/home/urcra/rust/compiler/rustc_session" }
+rustc_span = { path = "/home/urcra/rust/compiler/rustc_span" }
+rustc_target = { path = "/home/urcra/rust/compiler/rustc_target" }
+rustc_trait_selection = { path = "/home/urcra/rust/compiler/rustc_trait_selection" }
+rustc_typeck = { path = "/home/urcra/rust/compiler/rustc_typeck" }
+
 [dependencies]
 cargo_metadata = "0.12"
 if_chain = "1.0.0"

clippy_lints/src/comparison_to_empty.rs

@@ -0,0 +1,155 @@
+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,
+    }
+}

clippy_lints/src/lib.rs

@@ -171,6 +171,7 @@ mod checked_conversions;
 mod cognitive_complexity;
 mod collapsible_if;
 mod comparison_chain;
+mod comparison_to_empty;
 mod copies;
 mod copy_iterator;
 mod create_dir;
@@ -523,6 +524,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,
         &copies::IFS_SAME_COND,
         &copies::IF_SAME_THEN_ELSE,
         &copies::MATCH_SAME_ARMS,
@@ -1139,6 +1141,7 @@ 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![
@@ -1299,6 +1302,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(&copies::IFS_SAME_COND),
         LintId::of(&copies::IF_SAME_THEN_ELSE),
         LintId::of(&derive::DERIVE_HASH_XOR_EQ),
@@ -1555,6 +1559,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(&doc::MISSING_SAFETY_DOC),
         LintId::of(&doc::NEEDLESS_DOCTEST_MAIN),
         LintId::of(&enum_variants::ENUM_VARIANT_NAMES),

src/lintlist/mod.rs

@@ -291,6 +291,13 @@ vec![
         deprecation: None,
         module: "comparison_chain",
     },
+    Lint {
+        name: "comparison_to_empty",
+        group: "style",
+        desc: "default lint description",
+        deprecation: None,
+        module: "comparison_to_empty",
+    },
     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
+