about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Wright <mikerite@lavabit.com>2018-11-04 10:02:49 +0200
committerMichael Wright <mikerite@lavabit.com>2018-11-04 10:02:49 +0200
commit0c1ffc1d1fdfd73bf326d1e7a886f59367374dc3 (patch)
tree719526c69e0e02c9b2b19adf7098efd71cd6bd07
parenta07c2715594b17fca900c6c4a497c8fb6abde609 (diff)
downloadrust-0c1ffc1d1fdfd73bf326d1e7a886f59367374dc3.tar.gz
rust-0c1ffc1d1fdfd73bf326d1e7a886f59367374dc3.zip
Fix `possible_missing_comma` false positives
`possible_missing_comma` should only trigger when the binary operator has
unary equivalent. Otherwise, it's not possible to insert a comma without
breaking compilation. The operators identified were `+`, `&`, `*` and `-`.

This fixes the specific examples given in issues #3244 and #3396
but doesn't address the conflict this lint has with the style of starting
a line with a binary operator.
-rw-r--r--clippy_lints/src/formatting.rs36
-rw-r--r--tests/ui/formatting.rs12
2 files changed, 35 insertions, 13 deletions
diff --git a/clippy_lints/src/formatting.rs b/clippy_lints/src/formatting.rs
index 7f5715ef691..8649834e267 100644
--- a/clippy_lints/src/formatting.rs
+++ b/clippy_lints/src/formatting.rs
@@ -173,24 +173,34 @@ fn check_else_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
     }
 }
 
+fn has_unary_equivalent(bin_op: ast::BinOpKind) -> bool {
+    //+, &, *, -
+    bin_op == ast::BinOpKind::Add
+    || bin_op == ast::BinOpKind::And
+    || bin_op == ast::BinOpKind::Mul
+    || bin_op == ast::BinOpKind::Sub
+}
+
 /// Implementation of the `POSSIBLE_MISSING_COMMA` lint for array
 fn check_array(cx: &EarlyContext<'_>, expr: &ast::Expr) {
     if let ast::ExprKind::Array(ref array) = expr.node {
         for element in array {
             if let ast::ExprKind::Binary(ref op, ref lhs, _) = element.node {
-                if !differing_macro_contexts(lhs.span, op.span) {
-                    let space_span = lhs.span.between(op.span);
-                    if let Some(space_snippet) = snippet_opt(cx, space_span) {
-                        let lint_span = lhs.span.with_lo(lhs.span.hi());
-                        if space_snippet.contains('\n') {
-                            span_note_and_lint(
-                                cx,
-                                POSSIBLE_MISSING_COMMA,
-                                lint_span,
-                                "possibly missing a comma here",
-                                lint_span,
-                                "to remove this lint, add a comma or write the expr in a single line",
-                            );
+                if has_unary_equivalent(op.node) {
+                    if !differing_macro_contexts(lhs.span, op.span) {
+                        let space_span = lhs.span.between(op.span);
+                        if let Some(space_snippet) = snippet_opt(cx, space_span) {
+                            let lint_span = lhs.span.with_lo(lhs.span.hi());
+                            if space_snippet.contains('\n') {
+                                span_note_and_lint(
+                                    cx,
+                                    POSSIBLE_MISSING_COMMA,
+                                    lint_span,
+                                    "possibly missing a comma here",
+                                    lint_span,
+                                    "to remove this lint, add a comma or write the expr in a single line",
+                                );
+                            }
                         }
                     }
                 }
diff --git a/tests/ui/formatting.rs b/tests/ui/formatting.rs
index 0dca8c8585b..88f6e497d12 100644
--- a/tests/ui/formatting.rs
+++ b/tests/ui/formatting.rs
@@ -112,4 +112,16 @@ fn main() {
         1 + 2, 3 +
         4, 5 + 6,
     ];
+
+    // don't lint for bin op without unary equiv
+    // issue 3244
+    vec![
+        1
+        / 2,
+    ];
+    // issue 3396
+    vec![
+        true
+        | false,
+    ];
 }