]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/needless_continue.rs
Merge pull request #2203 from clippered/float_cmp_const
[rust.git] / clippy_lints / src / needless_continue.rs
1 //! Checks for continue statements in loops that are redundant.
2 //!
3 //! For example, the lint would catch
4 //!
5 //! ```
6 //! while condition() {
7 //!     update_condition();
8 //!     if x {
9 //!         // ...
10 //!     } else {
11 //!         continue;
12 //!     }
13 //!     println!("Hello, world");
14 //! }
15 //! ```
16 //!
17 //! And suggest something like this:
18 //!
19 //! ```
20 //! while condition() {
21 //!     update_condition();
22 //!     if x {
23 //!         // ...
24 //!         println!("Hello, world");
25 //!     }
26 //! }
27 //! ```
28 //!
29 //! This lint is **warn** by default.
30 use rustc::lint::*;
31 use syntax::ast;
32 use syntax::codemap::{original_sp, DUMMY_SP};
33 use std::borrow::Cow;
34
35 use utils::{in_macro, snippet, snippet_block, span_help_and_lint, trim_multiline};
36
37 /// **What it does:** The lint checks for `if`-statements appearing in loops
38 /// that contain a `continue` statement in either their main blocks or their
39 /// `else`-blocks, when omitting the `else`-block possibly with some
40 /// rearrangement of code can make the code easier to understand.
41 ///
42 /// **Why is this bad?** Having explicit `else` blocks for `if` statements
43 /// containing `continue` in their THEN branch adds unnecessary branching and
44 /// nesting to the code. Having an else block containing just `continue` can
45 /// also be better written by grouping the statements following the whole `if`
46 /// statement within the THEN block and omitting the else block completely.
47 ///
48 /// **Known problems:** None
49 ///
50 /// **Example:**
51 /// ```rust
52 /// while condition() {
53 ///     update_condition();
54 ///     if x {
55 ///         // ...
56 ///     } else {
57 ///         continue;
58 ///     }
59 ///     println!("Hello, world");
60 /// }
61 /// ```
62 ///
63 /// Could be rewritten as
64 ///
65 /// ```rust
66 /// while condition() {
67 ///     update_condition();
68 ///     if x {
69 ///         // ...
70 ///         println!("Hello, world");
71 ///     }
72 /// }
73 /// ```
74 ///
75 /// As another example, the following code
76 ///
77 /// ```rust
78 /// loop {
79 ///     if waiting() {
80 ///         continue;
81 ///     } else {
82 ///         // Do something useful
83 ///     }
84 /// }
85 /// ```
86 /// Could be rewritten as
87 ///
88 /// ```rust
89 /// loop {
90 ///     if waiting() {
91 ///         continue;
92 ///     }
93 ///     // Do something useful
94 /// }
95 /// ```
96 declare_lint! {
97     pub NEEDLESS_CONTINUE,
98     Warn,
99     "`continue` statements that can be replaced by a rearrangement of code"
100 }
101
102 #[derive(Copy, Clone)]
103 pub struct NeedlessContinue;
104
105 impl LintPass for NeedlessContinue {
106     fn get_lints(&self) -> LintArray {
107         lint_array!(NEEDLESS_CONTINUE)
108     }
109 }
110
111 impl EarlyLintPass for NeedlessContinue {
112     fn check_expr(&mut self, ctx: &EarlyContext, expr: &ast::Expr) {
113         if !in_macro(expr.span) {
114             check_and_warn(ctx, expr);
115         }
116     }
117 }
118
119 /* This lint has to mainly deal with two cases of needless continue
120  * statements. */
121 // Case 1 [Continue inside else block]:
122 //
123 //     loop {
124 //         // region A
125 //         if cond {
126 //             // region B
127 //         } else {
128 //             continue;
129 //         }
130 //         // region C
131 //     }
132 //
133 // This code can better be written as follows:
134 //
135 //     loop {
136 //         // region A
137 //         if cond {
138 //             // region B
139 //             // region C
140 //         }
141 //     }
142 //
143 // Case 2 [Continue inside then block]:
144 //
145 //     loop {
146 //       // region A
147 //       if cond {
148 //           continue;
149 //           // potentially more code here.
150 //       } else {
151 //           // region B
152 //       }
153 //       // region C
154 //     }
155 //
156 //
157 // This snippet can be refactored to:
158 //
159 //     loop {
160 //       // region A
161 //       if !cond {
162 //           // region B
163 //           // region C
164 //       }
165 //     }
166 //
167
168 /// Given an expression, returns true if either of the following is true
169 ///
170 /// - The expression is a `continue` node.
171 /// - The expression node is a block with the first statement being a
172 /// `continue`.
173 ///
174 fn needless_continue_in_else(else_expr: &ast::Expr) -> bool {
175     match else_expr.node {
176         ast::ExprKind::Block(ref else_block) => is_first_block_stmt_continue(else_block),
177         ast::ExprKind::Continue(_) => true,
178         _ => false,
179     }
180 }
181
182 fn is_first_block_stmt_continue(block: &ast::Block) -> bool {
183     block.stmts.get(0).map_or(false, |stmt| match stmt.node {
184         ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => if let ast::ExprKind::Continue(_) = e.node {
185             true
186         } else {
187             false
188         },
189         _ => false,
190     })
191 }
192
193 /// If `expr` is a loop expression (while/while let/for/loop), calls `func` with
194 /// the AST object representing the loop block of `expr`.
195 fn with_loop_block<F>(expr: &ast::Expr, mut func: F)
196 where
197     F: FnMut(&ast::Block),
198 {
199     match expr.node {
200         ast::ExprKind::While(_, ref loop_block, _) |
201         ast::ExprKind::WhileLet(_, _, ref loop_block, _) |
202         ast::ExprKind::ForLoop(_, _, ref loop_block, _) |
203         ast::ExprKind::Loop(ref loop_block, _) => func(loop_block),
204         _ => {},
205     }
206 }
207
208 /// If `stmt` is an if expression node with an `else` branch, calls func with
209 /// the
210 /// following:
211 ///
212 /// - The `if` expression itself,
213 /// - The `if` condition expression,
214 /// - The `then` block, and
215 /// - The `else` expression.
216 ///
217 fn with_if_expr<F>(stmt: &ast::Stmt, mut func: F)
218 where
219     F: FnMut(&ast::Expr, &ast::Expr, &ast::Block, &ast::Expr),
220 {
221     match stmt.node {
222         ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => {
223             if let ast::ExprKind::If(ref cond, ref if_block, Some(ref else_expr)) = e.node {
224                 func(e, cond, if_block, else_expr);
225             }
226         },
227         _ => {},
228     }
229 }
230
231 /// A type to distinguish between the two distinct cases this lint handles.
232 #[derive(Copy, Clone, Debug)]
233 enum LintType {
234     ContinueInsideElseBlock,
235     ContinueInsideThenBlock,
236 }
237
238 /// Data we pass around for construction of help messages.
239 struct LintData<'a> {
240     /// The `if` expression encountered in the above loop.
241     if_expr: &'a ast::Expr,
242     /// The condition expression for the above `if`.
243     if_cond: &'a ast::Expr,
244     /// The `then` block of the `if` statement.
245     if_block: &'a ast::Block,
246     /// The `else` block of the `if` statement.
247     /// Note that we only work with `if` exprs that have an `else` branch.
248     else_expr: &'a ast::Expr,
249     /// The 0-based index of the `if` statement in the containing loop block.
250     stmt_idx: usize,
251     /// The statements of the loop block.
252     block_stmts: &'a [ast::Stmt],
253 }
254
255 const MSG_REDUNDANT_ELSE_BLOCK: &str = "This else block is redundant.\n";
256
257 const MSG_ELSE_BLOCK_NOT_NEEDED: &str = "There is no need for an explicit `else` block for this `if` \
258                                          expression\n";
259
260 const DROP_ELSE_BLOCK_AND_MERGE_MSG: &str = "Consider dropping the else clause and merging the code that \
261                                              follows (in the loop) with the if block, like so:\n";
262
263 const DROP_ELSE_BLOCK_MSG: &str = "Consider dropping the else clause, and moving out the code in the else \
264                                    block, like so:\n";
265
266
267 fn emit_warning<'a>(ctx: &EarlyContext, data: &'a LintData, header: &str, typ: LintType) {
268     // snip    is the whole *help* message that appears after the warning.
269     // message is the warning message.
270     // expr    is the expression which the lint warning message refers to.
271     let (snip, message, expr) = match typ {
272         LintType::ContinueInsideElseBlock => (
273             suggestion_snippet_for_continue_inside_else(ctx, data, header),
274             MSG_REDUNDANT_ELSE_BLOCK,
275             data.else_expr,
276         ),
277         LintType::ContinueInsideThenBlock => (
278             suggestion_snippet_for_continue_inside_if(ctx, data, header),
279             MSG_ELSE_BLOCK_NOT_NEEDED,
280             data.if_expr,
281         ),
282     };
283     span_help_and_lint(ctx, NEEDLESS_CONTINUE, expr.span, message, &snip);
284 }
285
286 fn suggestion_snippet_for_continue_inside_if<'a>(ctx: &EarlyContext, data: &'a LintData, header: &str) -> String {
287     let cond_code = snippet(ctx, data.if_cond.span, "..");
288
289     let if_code = format!("if {} {{\n    continue;\n}}\n", cond_code);
290     /* ^^^^--- Four spaces of indentation. */
291     // region B
292     let else_code = snippet(ctx, data.else_expr.span, "..").into_owned();
293     let else_code = erode_block(&else_code);
294     let else_code = trim_multiline(Cow::from(else_code), false);
295
296     let mut ret = String::from(header);
297     ret.push_str(&if_code);
298     ret.push_str(&else_code);
299     ret.push_str("\n...");
300     ret
301 }
302
303 fn suggestion_snippet_for_continue_inside_else<'a>(ctx: &EarlyContext, data: &'a LintData, header: &str) -> String {
304     let cond_code = snippet(ctx, data.if_cond.span, "..");
305     let mut if_code = format!("if {} {{\n", cond_code);
306
307     // Region B
308     let block_code = &snippet(ctx, data.if_block.span, "..").into_owned();
309     let block_code = erode_block(block_code);
310     let block_code = trim_multiline(Cow::from(block_code), false);
311
312     if_code.push_str(&block_code);
313
314     // Region C
315     // These is the code in the loop block that follows the if/else construction
316     // we are complaining about. We want to pull all of this code into the
317     // `then` block of the `if` statement.
318     let to_annex = data.block_stmts[data.stmt_idx + 1..]
319         .iter()
320         .map(|stmt| original_sp(stmt.span, DUMMY_SP))
321         .map(|span| snippet_block(ctx, span, "..").into_owned())
322         .collect::<Vec<_>>()
323         .join("\n");
324
325     let mut ret = String::from(header);
326
327     ret.push_str(&if_code);
328     ret.push_str("\n// Merged code follows...");
329     ret.push_str(&to_annex);
330     ret.push_str("\n}\n");
331     ret
332 }
333
334 fn check_and_warn<'a>(ctx: &EarlyContext, expr: &'a ast::Expr) {
335     with_loop_block(expr, |loop_block| {
336         for (i, stmt) in loop_block.stmts.iter().enumerate() {
337             with_if_expr(stmt, |if_expr, cond, then_block, else_expr| {
338                 let data = &LintData {
339                     stmt_idx: i,
340                     if_expr: if_expr,
341                     if_cond: cond,
342                     if_block: then_block,
343                     else_expr: else_expr,
344                     block_stmts: &loop_block.stmts,
345                 };
346                 if needless_continue_in_else(else_expr) {
347                     emit_warning(ctx, data, DROP_ELSE_BLOCK_AND_MERGE_MSG, LintType::ContinueInsideElseBlock);
348                 } else if is_first_block_stmt_continue(then_block) {
349                     emit_warning(ctx, data, DROP_ELSE_BLOCK_MSG, LintType::ContinueInsideThenBlock);
350                 }
351             });
352         }
353     });
354 }
355
356 /// Eats at `s` from the end till a closing brace `}` is encountered, and then
357 /// continues eating till a non-whitespace character is found.
358 /// e.g., the string
359 ///
360 /// ```
361 ///     {
362 ///         let x = 5;
363 ///     }
364 /// ```
365 ///
366 /// is transformed to
367 ///
368 /// ```
369 ///     {
370 ///         let x = 5;
371 /// ```
372 ///
373 /// NOTE: when there is no closing brace in `s`, `s` is _not_ preserved, i.e.,
374 /// an empty string will be returned in that case.
375 pub fn erode_from_back(s: &str) -> String {
376     let mut ret = String::from(s);
377     while ret.pop().map_or(false, |c| c != '}') {}
378     while let Some(c) = ret.pop() {
379         if !c.is_whitespace() {
380             ret.push(c);
381             break;
382         }
383     }
384     ret
385 }
386
387 /// Eats at `s` from the front by first skipping all leading whitespace. Then,
388 /// any number of opening braces are eaten, followed by any number of newlines.
389 /// e.g.,  the string
390 ///
391 /// ```
392 ///         {
393 ///             something();
394 ///             inside_a_block();
395 ///         }
396 /// ```
397 ///
398 /// is transformed to
399 ///
400 /// ```
401 ///             something();
402 ///             inside_a_block();
403 ///         }
404 /// ```
405 ///
406 pub fn erode_from_front(s: &str) -> String {
407     s.chars()
408         .skip_while(|c| c.is_whitespace())
409         .skip_while(|c| *c == '{')
410         .skip_while(|c| *c == '\n')
411         .collect::<String>()
412 }
413
414 /// If `s` contains the code for a block, delimited by braces, this function
415 /// tries to get the contents of the block. If there is no closing brace
416 /// present,
417 /// an empty string is returned.
418 pub fn erode_block(s: &str) -> String {
419     erode_from_back(&erode_from_front(s))
420 }