clippy_lints/src/formatting.rs

   1 use crate::utils::{differing_macro_contexts, in_macro, snippet_opt, span_note_and_lint};
   2 use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
   3 use rustc::{declare_tool_lint, lint_array};
   4 use syntax::ast;
   5 use syntax::ptr::P;
   6
   7 /// **What it does:** Checks for use of the non-existent `=*`, `=!` and `=-`
   8 /// operators.
   9 ///
  10 /// **Why is this bad?** This is either a typo of `*=`, `!=` or `-=` or
  11 /// confusing.
  12 ///
  13 /// **Known problems:** None.
  14 ///
  15 /// **Example:**
  16 /// ```rust,ignore
  17 /// a =- 42; // confusing, should it be `a -= 42` or `a = -42`?
  18 /// ```
  19 declare_clippy_lint! {
  20     pub SUSPICIOUS_ASSIGNMENT_FORMATTING,
  21     style,
  22     "suspicious formatting of `*=`, `-=` or `!=`"
  23 }
  24
  25 /// **What it does:** Checks for formatting of `else`. It lints if the `else`
  26 /// is followed immediately by a newline or the `else` seems to be missing.
  27 ///
  28 /// **Why is this bad?** This is probably some refactoring remnant, even if the
  29 /// code is correct, it might look confusing.
  30 ///
  31 /// **Known problems:** None.
  32 ///
  33 /// **Example:**
  34 /// ```rust,ignore
  35 /// if foo {
  36 /// } { // looks like an `else` is missing here
  37 /// }
  38 ///
  39 /// if foo {
  40 /// } if bar { // looks like an `else` is missing here
  41 /// }
  42 ///
  43 /// if foo {
  44 /// } else
  45 ///
  46 /// { // this is the `else` block of the previous `if`, but should it be?
  47 /// }
  48 ///
  49 /// if foo {
  50 /// } else
  51 ///
  52 /// if bar { // this is the `else` block of the previous `if`, but should it be?
  53 /// }
  54 /// ```
  55 declare_clippy_lint! {
  56     pub SUSPICIOUS_ELSE_FORMATTING,
  57     style,
  58     "suspicious formatting of `else`"
  59 }
  60
  61 /// **What it does:** Checks for possible missing comma in an array. It lints if
  62 /// an array element is a binary operator expression and it lies on two lines.
  63 ///
  64 /// **Why is this bad?** This could lead to unexpected results.
  65 ///
  66 /// **Known problems:** None.
  67 ///
  68 /// **Example:**
  69 /// ```rust,ignore
  70 /// let a = &[
  71 ///     -1, -2, -3 // <= no comma here
  72 ///     -4, -5, -6
  73 /// ];
  74 /// ```
  75 declare_clippy_lint! {
  76     pub POSSIBLE_MISSING_COMMA,
  77     correctness,
  78     "possible missing comma in array"
  79 }
  80
  81 #[derive(Copy, Clone)]
  82 pub struct Formatting;
  83
  84 impl LintPass for Formatting {
  85     fn get_lints(&self) -> LintArray {
  86         lint_array!(
  87             SUSPICIOUS_ASSIGNMENT_FORMATTING,
  88             SUSPICIOUS_ELSE_FORMATTING,
  89             POSSIBLE_MISSING_COMMA
  90         )
  91     }
  92
  93     fn name(&self) -> &'static str {
  94         "Formatting"
  95     }
  96 }
  97
  98 impl EarlyLintPass for Formatting {
  99     fn check_block(&mut self, cx: &EarlyContext<'_>, block: &ast::Block) {
 100         for w in block.stmts.windows(2) {
 101             match (&w[0].node, &w[1].node) {
 102                 (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Expr(ref second))
 103                 | (&ast::StmtKind::Expr(ref first), &ast::StmtKind::Semi(ref second)) => {
 104                     check_missing_else(cx, first, second);
 105                 },
 106                 _ => (),
 107             }
 108         }
 109     }
 110
 111     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
 112         check_assign(cx, expr);
 113         check_else(cx, expr);
 114         check_array(cx, expr);
 115     }
 116 }
 117
 118 /// Implementation of the `SUSPICIOUS_ASSIGNMENT_FORMATTING` lint.
 119 fn check_assign(cx: &EarlyContext<'_>, expr: &ast::Expr) {
 120     if let ast::ExprKind::Assign(ref lhs, ref rhs) = expr.node {
 121         if !differing_macro_contexts(lhs.span, rhs.span) && !in_macro(lhs.span) {
 122             let eq_span = lhs.span.between(rhs.span);
 123             if let ast::ExprKind::Unary(op, ref sub_rhs) = rhs.node {
 124                 if let Some(eq_snippet) = snippet_opt(cx, eq_span) {
 125                     let op = ast::UnOp::to_string(op);
 126                     let eqop_span = lhs.span.between(sub_rhs.span);
 127                     if eq_snippet.ends_with('=') {
 128                         span_note_and_lint(
 129                             cx,
 130                             SUSPICIOUS_ASSIGNMENT_FORMATTING,
 131                             eqop_span,
 132                             &format!(
 133                                 "this looks like you are trying to use `.. {op}= ..`, but you \
 134                                  really are doing `.. = ({op} ..)`",
 135                                 op = op
 136                             ),
 137                             eqop_span,
 138                             &format!("to remove this lint, use either `{op}=` or `= {op}`", op = op),
 139                         );
 140                     }
 141                 }
 142             }
 143         }
 144     }
 145 }
 146
 147 /// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else`.
 148 fn check_else(cx: &EarlyContext<'_>, expr: &ast::Expr) {
 149     if let Some((then, &Some(ref else_))) = unsugar_if(expr) {
 150         if (is_block(else_) || unsugar_if(else_).is_some())
 151             && !differing_macro_contexts(then.span, else_.span)
 152             && !in_macro(then.span)
 153         {
 154             // workaround for rust-lang/rust#43081
 155             if expr.span.lo().0 == 0 && expr.span.hi().0 == 0 {
 156                 return;
 157             }
 158
 159             // this will be a span from the closing ‘}’ of the “then” block (excluding) to
 160             // the
 161             // “if” of the “else if” block (excluding)
 162             let else_span = then.span.between(else_.span);
 163
 164             // the snippet should look like " else \n    " with maybe comments anywhere
 165             // it’s bad when there is a ‘\n’ after the “else”
 166             if let Some(else_snippet) = snippet_opt(cx, else_span) {
 167                 let else_pos = else_snippet.find("else").expect("there must be a `else` here");
 168
 169                 if else_snippet[else_pos..].contains('\n') {
 170                     let else_desc = if unsugar_if(else_).is_some() { "if" } else { "{..}" };
 171
 172                     span_note_and_lint(
 173                         cx,
 174                         SUSPICIOUS_ELSE_FORMATTING,
 175                         else_span,
 176                         &format!("this is an `else {}` but the formatting might hide it", else_desc),
 177                         else_span,
 178                         &format!(
 179                             "to remove this lint, remove the `else` or remove the new line between \
 180                              `else` and `{}`",
 181                             else_desc,
 182                         ),
 183                     );
 184                 }
 185             }
 186         }
 187     }
 188 }
 189
 190 fn has_unary_equivalent(bin_op: ast::BinOpKind) -> bool {
 191     // &, *, -
 192     bin_op == ast::BinOpKind::And || bin_op == ast::BinOpKind::Mul || bin_op == ast::BinOpKind::Sub
 193 }
 194
 195 /// Implementation of the `POSSIBLE_MISSING_COMMA` lint for array
 196 fn check_array(cx: &EarlyContext<'_>, expr: &ast::Expr) {
 197     if let ast::ExprKind::Array(ref array) = expr.node {
 198         for element in array {
 199             if let ast::ExprKind::Binary(ref op, ref lhs, _) = element.node {
 200                 if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span) {
 201                     let space_span = lhs.span.between(op.span);
 202                     if let Some(space_snippet) = snippet_opt(cx, space_span) {
 203                         let lint_span = lhs.span.with_lo(lhs.span.hi());
 204                         if space_snippet.contains('\n') {
 205                             span_note_and_lint(
 206                                 cx,
 207                                 POSSIBLE_MISSING_COMMA,
 208                                 lint_span,
 209                                 "possibly missing a comma here",
 210                                 lint_span,
 211                                 "to remove this lint, add a comma or write the expr in a single line",
 212                             );
 213                         }
 214                     }
 215                 }
 216             }
 217         }
 218     }
 219 }
 220
 221 fn check_missing_else(cx: &EarlyContext<'_>, first: &ast::Expr, second: &ast::Expr) {
 222     if !differing_macro_contexts(first.span, second.span)
 223         && !in_macro(first.span)
 224         && unsugar_if(first).is_some()
 225         && (is_block(second) || unsugar_if(second).is_some())
 226     {
 227         // where the else would be
 228         let else_span = first.span.between(second.span);
 229
 230         if let Some(else_snippet) = snippet_opt(cx, else_span) {
 231             if !else_snippet.contains('\n') {
 232                 let (looks_like, next_thing) = if unsugar_if(second).is_some() {
 233                     ("an `else if`", "the second `if`")
 234                 } else {
 235                     ("an `else {..}`", "the next block")
 236                 };
 237
 238                 span_note_and_lint(
 239                     cx,
 240                     SUSPICIOUS_ELSE_FORMATTING,
 241                     else_span,
 242                     &format!("this looks like {} but the `else` is missing", looks_like),
 243                     else_span,
 244                     &format!(
 245                         "to remove this lint, add the missing `else` or add a new line before {}",
 246                         next_thing,
 247                     ),
 248                 );
 249             }
 250         }
 251     }
 252 }
 253
 254 fn is_block(expr: &ast::Expr) -> bool {
 255     if let ast::ExprKind::Block(..) = expr.node {
 256         true
 257     } else {
 258         false
 259     }
 260 }
 261
 262 /// Match `if` or `if let` expressions and return the `then` and `else` block.
 263 fn unsugar_if(expr: &ast::Expr) -> Option<(&P<ast::Block>, &Option<P<ast::Expr>>)> {
 264     match expr.node {
 265         ast::ExprKind::If(_, ref then, ref else_) | ast::ExprKind::IfLet(_, _, ref then, ref else_) => {
 266             Some((then, else_))
 267         },
 268         _ => None,
 269     }
 270 }