Auto merge of #4615 - nikofil:suspicious_unary_op_formatting, r=flip1995

New lint: suspicious_unary_op_formatting

fixes #4228

changelog: New lint: [`suspicious_unary_op_formatting`]

CHANGELOG.md

@@ -1191,6 +1191,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 320 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 321 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

@@ -1,4 +1,4 @@
-use crate::utils::{differing_macro_contexts, snippet_opt, span_note_and_lint};
+use crate::utils::{differing_macro_contexts, snippet_opt, span_help_and_lint, span_note_and_lint};
 use if_chain::if_chain;
 use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
@@ -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,45 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) {
     }
 }
 
+/// Implementation of the `SUSPICIOUS_UNARY_OP_FORMATTING` lint.
+fn check_unop(cx: &EarlyContext<'_>, expr: &Expr) {
+    if_chain! {
+        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) = snippet_opt(cx, binop_span);
+        if let Some(unop_operand_snippet) = 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(' ');
+        then {
+            let unop_str = UnOp::to_string(op);
+            let eqop_span = lhs.span.between(un_rhs.span);
+            span_help_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
+                ),
+                &format!(
+                    "put a space between `{binop}` and `{unop}` and remove the space after `{unop}`",
+                    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

@@ -743,6 +743,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,
@@ -953,6 +954,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; 320] = [
+pub const ALL_LINTS: [Lint; 321] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -1799,6 +1799,13 @@ pub const ALL_LINTS: [Lint; 320] = [
         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,23 @@
+#![warn(clippy::suspicious_unary_op_formatting)]
+
+#[rustfmt::skip]
+fn main() {
+    // weird binary operator formatting:
+    let a = 42;
+
+    if a >- 30 {}
+    if a >=- 30 {}
+
+    let b = true;
+    let c = false;
+
+    if b &&! c {}
+
+    if a >-   30 {}
+
+    // those are ok:
+    if a >-30 {}
+    if a < -30 {}
+    if b && !c {}
+    if a > -   30 {}
+}

tests/ui/suspicious_unary_op_formatting.stderr

@@ -0,0 +1,35 @@
+error: by not having a space between `>` and `-` it looks like `>-` is a single operator
+  --> $DIR/suspicious_unary_op_formatting.rs:8:9
+   |
+LL |     if a >- 30 {}
+   |         ^^^^
+   |
+   = note: `-D clippy::suspicious-unary-op-formatting` implied by `-D warnings`
+   = help: put a space between `>` and `-` and 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:9:9
+   |
+LL |     if a >=- 30 {}
+   |         ^^^^^
+   |
+   = help: put a space between `>=` and `-` and 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:14:9
+   |
+LL |     if b &&! c {}
+   |         ^^^^^
+   |
+   = help: put a space between `&&` and `!` and 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 a >-   30 {}
+   |         ^^^^^^
+   |
+   = help: put a space between `>` and `-` and remove the space after `-`
+
+error: aborting due to 4 previous errors
+