Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: suspicious_unary_op_formatting #4615

Merged
merged 1 commit into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
65 changes: 64 additions & 1 deletion clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
/// 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.
Expand Down Expand Up @@ -80,6 +102,7 @@ declare_clippy_lint! {

declare_lint_pass!(Formatting => [
SUSPICIOUS_ASSIGNMENT_FORMATTING,
SUSPICIOUS_UNARY_OP_FORMATTING,
SUSPICIOUS_ELSE_FORMATTING,
POSSIBLE_MISSING_COMMA
]);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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! {
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/suspicious_unary_op_formatting.rs
Original file line number Diff line number Diff line change
@@ -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 {}
}
35 changes: 35 additions & 0 deletions tests/ui/suspicious_unary_op_formatting.stderr
Original file line number Diff line number Diff line change
@@ -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