Merge pull request #1506 from bood/master

Lint for possible missing comma in an array #1177

CHANGELOG.md

@@ -392,6 +392,7 @@ All notable changes to this project will be documented in this file.
 [`overflow_check_conditional`]: https://github.com/Manishearth/rust-clippy/wiki#overflow_check_conditional
 [`panic_params`]: https://github.com/Manishearth/rust-clippy/wiki#panic_params
 [`partialeq_ne_impl`]: https://github.com/Manishearth/rust-clippy/wiki#partialeq_ne_impl
+[`possible_missing_comma`]: https://github.com/Manishearth/rust-clippy/wiki#possible_missing_comma
 [`precedence`]: https://github.com/Manishearth/rust-clippy/wiki#precedence
 [`print_stdout`]: https://github.com/Manishearth/rust-clippy/wiki#print_stdout
 [`print_with_newline`]: https://github.com/Manishearth/rust-clippy/wiki#print_with_newline

README.md

@@ -305,6 +305,7 @@ name
 [overflow_check_conditional](https://github.com/Manishearth/rust-clippy/wiki#overflow_check_conditional)               | warn    | overflow checks inspired by C which are likely to panic
 [panic_params](https://github.com/Manishearth/rust-clippy/wiki#panic_params)                                           | warn    | missing parameters in `panic!` calls
 [partialeq_ne_impl](https://github.com/Manishearth/rust-clippy/wiki#partialeq_ne_impl)                                 | warn    | re-implementing `PartialEq::ne`
+[possible_missing_comma](https://github.com/Manishearth/rust-clippy/wiki#possible_missing_comma)                       | warn    | possible missing comma in array
 [precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence)                                               | warn    | operations where precedence may be unclear
 [print_stdout](https://github.com/Manishearth/rust-clippy/wiki#print_stdout)                                           | allow   | printing on stdout
 [print_with_newline](https://github.com/Manishearth/rust-clippy/wiki#print_with_newline)                               | warn    | using `print!()` with a format string that ends in a newline

clippy_lints/src/formatting.rs

@@ -46,12 +46,33 @@ declare_lint! {
     "suspicious formatting of `else if`"
 }
 
+/// **What it does:** Checks for possible missing comma in an array. It lints if
+/// an array element is a binary operator expression and it lies on two lines.
+///
+/// **Why is this bad?** This could lead to unexpected results.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust,ignore
+/// let a = &[
+///     -1, -2, -3 // <= no coma here
+///     -4, -5, -6
+/// ];
+/// ```
+declare_lint! {
+    pub POSSIBLE_MISSING_COMMA,
+    Warn,
+    "possible missing comma in array"
+}
+
+
 #[derive(Copy,Clone)]
 pub struct Formatting;
 
 impl LintPass for Formatting {
     fn get_lints(&self) -> LintArray {
-        lint_array![SUSPICIOUS_ASSIGNMENT_FORMATTING, SUSPICIOUS_ELSE_FORMATTING]
+        lint_array![SUSPICIOUS_ASSIGNMENT_FORMATTING, SUSPICIOUS_ELSE_FORMATTING, POSSIBLE_MISSING_COMMA]
     }
 }
 
@@ -71,6 +92,7 @@ impl EarlyLintPass for Formatting {
     fn check_expr(&mut self, cx: &EarlyContext, expr: &ast::Expr) {
         check_assign(cx, expr);
         check_else_if(cx, expr);
+        check_array(cx, expr);
     }
 }
 
@@ -127,6 +149,30 @@ fn check_else_if(cx: &EarlyContext, expr: &ast::Expr) {
     }
 }
 
+/// 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 = mk_sp(lhs.span.hi, op.span.lo);
+                    if let Some(space_snippet) = snippet_opt(cx, space_span) {
+                        let lint_span = mk_sp(lhs.span.hi, 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");
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
 /// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for consecutive ifs.
 fn check_consecutive_ifs(cx: &EarlyContext, first: &ast::Expr, second: &ast::Expr) {
     if !differing_macro_contexts(first.span, second.span) && !in_macro(cx, first.span) &&

clippy_lints/src/lib.rs

@@ -382,6 +382,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         eval_order_dependence::DIVERGING_SUB_EXPRESSION,
         eval_order_dependence::EVAL_ORDER_DEPENDENCE,
         format::USELESS_FORMAT,
+        formatting::POSSIBLE_MISSING_COMMA,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
         functions::NOT_UNSAFE_PTR_ARG_DEREF,

tests/compile-fail/formatting.rs

@@ -96,4 +96,32 @@ fn main() {
     a = -35;
     a = *&191;
     b = !false;
+
+    // possible missing comma in an array
+    let _ = &[
+        -1, -2, -3 // <= no coma here
+        //~^ ERROR possibly missing a comma here
+        //~| NOTE to remove this lint, add a comma or write the expr in a single line
+        -4, -5, -6
+    ];
+    let _ = &[
+        -1, -2, -3 // <= no coma here
+        //~^ ERROR possibly missing a comma here
+        //~| NOTE to remove this lint, add a comma or write the expr in a single line
+        *4, -5, -6
+    ];
+
+    // those are ok:
+    let _ = &[
+        -1, -2, -3,
+        -4, -5, -6
+    ];
+    let _ = &[
+        -1, -2, -3,
+        -4, -5, -6,
+    ];
+    let _ = &[
+        1 + 2, 3 +
+        4, 5 + 6,
+    ];
 }