Lint for possible missing comma in an array #1177

CHANGELOG.md

@@ -386,6 +386,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_in_array_formatting`]: https://github.com/Manishearth/rust-clippy/wiki#possible_missing_comma_in_array_formatting
 [`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

@@ -180,10 +180,10 @@ transparently:
 
 ## Lints
 
-There are 184 lints included in this crate:
+There are 185 lints included in this crate:
 
 name                                                                                                                                     | default | triggers on
------------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
+-----------------------------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
 [absurd_extreme_comparisons](https://github.com/Manishearth/rust-clippy/wiki#absurd_extreme_comparisons)                                 | warn    | a comparison with a maximum or minimum value that is always true or false
 [almost_swapped](https://github.com/Manishearth/rust-clippy/wiki#almost_swapped)                                                         | warn    | `foo = bar; bar = foo` sequence
 [approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant)                                                       | warn    | the approximate of a known float constant (in `std::fXX::consts`)
@@ -304,6 +304,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_in_array_formatting](https://github.com/Manishearth/rust-clippy/wiki#possible_missing_comma_in_array_formatting) | 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 op 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_IN_ARRAY_FORMATTING,
+    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_IN_ARRAY_FORMATTING]
     }
 }
 
@@ -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,28 @@ fn check_else_if(cx: &EarlyContext, expr: &ast::Expr) {
     }
 }
 
+/// Implementation of the `POSSIBLE_MISSING_COMMA_IN_ARRAY_FORMATTING` 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 {
+                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_IN_ARRAY_FORMATTING,
+                                           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

@@ -380,6 +380,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_IN_ARRAY_FORMATTING,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
         functions::NOT_UNSAFE_PTR_ARG_DEREF,

tests/compile-fail/formatting.rs

@@ -96,4 +96,12 @@ fn main() {
     a = -35;
     a = *&191;
     b = !false;
+
+    // possible missing comma in an array
+    let mut c = &[
+        -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
+    ];
 }