diff --git a/CHANGELOG.md b/CHANGELOG.md index 7415c5fa226..86088231109 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index aff8a36eca7..3ccf5fcc250 100644 --- a/README.md +++ b/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: diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs index 52ebaa4af7d..821e79e8c77 100644 --- a/clippy_lints/src/formatting.rs +++ b/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! { diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 452e4e9787d..ec12e06b1aa 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8793d98f29f..28f212fb2b2 100644 --- a/src/lintlist/mod.rs +++ b/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", diff --git a/tests/ui/suspicious_unary_op_formatting.rs b/tests/ui/suspicious_unary_op_formatting.rs new file mode 100644 index 00000000000..9564e373c24 --- /dev/null +++ b/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 {} +} diff --git a/tests/ui/suspicious_unary_op_formatting.stderr b/tests/ui/suspicious_unary_op_formatting.stderr new file mode 100644 index 00000000000..581527dcff8 --- /dev/null +++ b/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 +