New lint: suspicious_unary_op_formatting

Lints when, on the RHS of a BinOp, there is a UnOp without a space
before the operator but with a space after (e.g. foo >- 1).

Signed-off-by: Nikos Filippakis <[email protected]>

Author: nikofil

CHANGELOG.md

@@ -1190,6 +1190,7 @@ Released 2018-09-13
 [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
 [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
 [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
+[`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 318 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 319 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/formatting.rs

@@ -22,6 +22,28 @@ declare_clippy_lint! {
     "suspicious formatting of `*=`, `-=` or `!=`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks the formatting of a unary operator on the right hand side
+    /// of a binary operator. It lints if there is no space between the binary and unary operators,
+    /// but there is a space between the unary and its operand.
+    ///
+    /// **Why is this bad?** This is either a typo in the binary operator or confusing.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust,ignore
+    /// if foo <- 30 { // this should be `foo < -30` but looks like a different operator
+    /// }
+    ///
+    /// if foo &&! bar { // this should be `foo && !bar` but looks like a different operator
+    /// }
+    /// ```
+    pub SUSPICIOUS_UNARY_OP_FORMATTING,
+    style,
+    "suspicious formatting of unary `-` or `!` on the RHS of a BinOp"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for formatting of `else`. It lints if the `else`
     /// is followed immediately by a newline or the `else` seems to be missing.
@@ -80,6 +102,7 @@ declare_clippy_lint! {
 
 declare_lint_pass!(Formatting => [
     SUSPICIOUS_ASSIGNMENT_FORMATTING,
+    SUSPICIOUS_UNARY_OP_FORMATTING,
     SUSPICIOUS_ELSE_FORMATTING,
     POSSIBLE_MISSING_COMMA
 ]);
@@ -99,6 +122,7 @@ impl EarlyLintPass for Formatting {
 
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
         check_assign(cx, expr);
+        check_unop(cx, expr);
         check_else(cx, expr);
         check_array(cx, expr);
     }
@@ -133,6 +157,49 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) {
     }
 }
 
+/// Implementation of the `SUSPICIOUS_UNARY_OP_FORMATTING` lint.
+fn check_unop(cx: &EarlyContext<'_>, expr: &Expr) {
+    if let ExprKind::Binary(ref binop, ref lhs, ref rhs) = expr.kind {
+        if !differing_macro_contexts(lhs.span, rhs.span) && !lhs.span.from_expansion() {
+            // span between BinOp LHS and RHS
+            let binop_span = lhs.span.between(rhs.span);
+            // if RHS is a UnOp
+            if let ExprKind::Unary(op, ref un_rhs) = rhs.kind {
+                // from UnOp operator to UnOp operand
+                let unop_operand_span = rhs.span.until(un_rhs.span);
+                if let (Some(binop_snippet), Some(unop_operand_snippet)) =
+                    (snippet_opt(cx, binop_span), snippet_opt(cx, unop_operand_span))
+                {
+                    let binop_str = BinOpKind::to_string(&binop.node);
+                    // no space after BinOp operator and space after UnOp operator
+                    if binop_snippet.ends_with(binop_str) && unop_operand_snippet.ends_with(' ') {
+                        let unop_str = UnOp::to_string(op);
+                        let eqop_span = lhs.span.between(un_rhs.span);
+                        span_note_and_lint(
+                            cx,
+                            SUSPICIOUS_UNARY_OP_FORMATTING,
+                            eqop_span,
+                            &format!(
+                                "by not having a space between `{binop}` and `{unop}` it looks like \
+                                 `{binop}{unop}` is a single operator",
+                                binop = binop_str,
+                                unop = unop_str
+                            ),
+                            eqop_span,
+                            &format!(
+                                "to remove this lint, put a space between `{binop}` and `{unop}` \
+                                 or remove the space after `{binop}`",
+                                binop = binop_str,
+                                unop = unop_str
+                            ),
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
+
 /// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else`.
 fn check_else(cx: &EarlyContext<'_>, expr: &Expr) {
     if_chain! {

clippy_lints/src/lib.rs

@@ -739,6 +739,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         formatting::POSSIBLE_MISSING_COMMA,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
+        formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
         functions::NOT_UNSAFE_PTR_ARG_DEREF,
         functions::TOO_MANY_ARGUMENTS,
         get_last_with_len::GET_LAST_WITH_LEN,
@@ -946,6 +947,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         excessive_precision::EXCESSIVE_PRECISION,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
+        formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
         infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
         inherent_to_string::INHERENT_TO_STRING,
         len_zero::LEN_WITHOUT_IS_EMPTY,

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 318] = [
+pub const ALL_LINTS: [Lint; 319] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1792,6 +1792,13 @@ pub const ALL_LINTS: [Lint; 318] = [
         deprecation: None,
         module: "suspicious_trait_impl",
     },
+    Lint {
+        name: "suspicious_unary_op_formatting",
+        group: "style",
+        desc: "suspicious formatting of unary `-` or `!` on the RHS of a BinOp",
+        deprecation: None,
+        module: "formatting",
+    },
     Lint {
         name: "temporary_assignment",
         group: "complexity",

tests/ui/suspicious_unary_op_formatting.rs

@@ -0,0 +1,24 @@
+#![warn(clippy::all)]
+#![allow(unused_variables)]
+
+#[rustfmt::skip]
+fn main() {
+    // weird binary operator formatting:
+    let a = 42;
+
+    if a >- 30 {
+    } else if a >=- 30 {
+    }
+
+    let b = true;
+    let c = false;
+
+    if b &&! c {
+    }
+
+    // those are ok:
+    if a >-30 {
+    } else if a < -30 {
+    } else if b && !c {
+    }
+}

tests/ui/suspicious_unary_op_formatting.stderr

@@ -0,0 +1,27 @@
+error: by not having a space between `>` and `-` it looks like `>-` is a single operator
+  --> $DIR/suspicious_unary_op_formatting.rs:9:9
+   |
+LL |     if a >- 30 {
+   |         ^^^^
+   |
+   = note: `-D clippy::suspicious-unary-op-formatting` implied by `-D warnings`
+   = note: to remove this lint, put a space between `>` and `-` or remove the space after `>`
+
+error: by not having a space between `>=` and `-` it looks like `>=-` is a single operator
+  --> $DIR/suspicious_unary_op_formatting.rs:10:16
+   |
+LL |     } else if a >=- 30 {
+   |                ^^^^^
+   |
+   = note: to remove this lint, put a space between `>=` and `-` or remove the space after `>=`
+
+error: by not having a space between `&&` and `!` it looks like `&&!` is a single operator
+  --> $DIR/suspicious_unary_op_formatting.rs:16:9
+   |
+LL |     if b &&! c {
+   |         ^^^^^
+   |
+   = note: to remove this lint, put a space between `&&` and `!` or remove the space after `&&`
+
+error: aborting due to 3 previous errors
+