//!
//! For example, the lint would catch
//!
-//! ```
-//! while condition() {
-//! update_condition();
+//! ```rust
+//! let mut a = 1;
+//! let x = true;
+//!
+//! while a < 5 {
+//! a = 6;
//! if x {
//! // ...
//! } else {
//!
//! And suggest something like this:
//!
-//! ```
-//! while condition() {
-//! update_condition();
+//! ```rust
+//! let mut a = 1;
+//! let x = true;
+//!
+//! while a < 5 {
+//! a = 6;
//! if x {
//! // ...
//! println!("Hello, world");
//! ```
//!
//! This lint is **warn** by default.
-use rustc::lint::*;
-use syntax::ast;
-use syntax::codemap::{original_sp, DUMMY_SP};
+use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
+use rustc::{declare_tool_lint, lint_array};
use std::borrow::Cow;
+use syntax::ast;
+use syntax::source_map::{original_sp, DUMMY_SP};
use crate::utils::{in_macro, snippet, snippet_block, span_help_and_lint, trim_multiline};
-/// **What it does:** The lint checks for `if`-statements appearing in loops
-/// that contain a `continue` statement in either their main blocks or their
-/// `else`-blocks, when omitting the `else`-block possibly with some
-/// rearrangement of code can make the code easier to understand.
-///
-/// **Why is this bad?** Having explicit `else` blocks for `if` statements
-/// containing `continue` in their THEN branch adds unnecessary branching and
-/// nesting to the code. Having an else block containing just `continue` can
-/// also be better written by grouping the statements following the whole `if`
-/// statement within the THEN block and omitting the else block completely.
-///
-/// **Known problems:** None
-///
-/// **Example:**
-/// ```rust
-/// while condition() {
-/// update_condition();
-/// if x {
-/// // ...
-/// } else {
-/// continue;
-/// }
-/// println!("Hello, world");
-/// }
-/// ```
-///
-/// Could be rewritten as
-///
-/// ```rust
-/// while condition() {
-/// update_condition();
-/// if x {
-/// // ...
-/// println!("Hello, world");
-/// }
-/// }
-/// ```
-///
-/// As another example, the following code
-///
-/// ```rust
-/// loop {
-/// if waiting() {
-/// continue;
-/// } else {
-/// // Do something useful
-/// }
-/// }
-/// ```
-/// Could be rewritten as
-///
-/// ```rust
-/// loop {
-/// if waiting() {
-/// continue;
-/// }
-/// // Do something useful
-/// }
-/// ```
declare_clippy_lint! {
+ /// **What it does:** The lint checks for `if`-statements appearing in loops
+ /// that contain a `continue` statement in either their main blocks or their
+ /// `else`-blocks, when omitting the `else`-block possibly with some
+ /// rearrangement of code can make the code easier to understand.
+ ///
+ /// **Why is this bad?** Having explicit `else` blocks for `if` statements
+ /// containing `continue` in their THEN branch adds unnecessary branching and
+ /// nesting to the code. Having an else block containing just `continue` can
+ /// also be better written by grouping the statements following the whole `if`
+ /// statement within the THEN block and omitting the else block completely.
+ ///
+ /// **Known problems:** None
+ ///
+ /// **Example:**
+ /// ```rust
+ /// while condition() {
+ /// update_condition();
+ /// if x {
+ /// // ...
+ /// } else {
+ /// continue;
+ /// }
+ /// println!("Hello, world");
+ /// }
+ /// ```
+ ///
+ /// Could be rewritten as
+ ///
+ /// ```rust
+ /// while condition() {
+ /// update_condition();
+ /// if x {
+ /// // ...
+ /// println!("Hello, world");
+ /// }
+ /// }
+ /// ```
+ ///
+ /// As another example, the following code
+ ///
+ /// ```rust
+ /// loop {
+ /// if waiting() {
+ /// continue;
+ /// } else {
+ /// // Do something useful
+ /// }
+ /// }
+ /// ```
+ /// Could be rewritten as
+ ///
+ /// ```rust
+ /// loop {
+ /// if waiting() {
+ /// continue;
+ /// }
+ /// // Do something useful
+ /// }
+ /// ```
pub NEEDLESS_CONTINUE,
pedantic,
"`continue` statements that can be replaced by a rearrangement of code"
fn get_lints(&self) -> LintArray {
lint_array!(NEEDLESS_CONTINUE)
}
+
+ fn name(&self) -> &'static str {
+ "NeedlessContinue"
+ }
}
impl EarlyLintPass for NeedlessContinue {
- fn check_expr(&mut self, ctx: &EarlyContext, expr: &ast::Expr) {
+ fn check_expr(&mut self, ctx: &EarlyContext<'_>, expr: &ast::Expr) {
if !in_macro(expr.span) {
check_and_warn(ctx, expr);
}
/// - The expression is a `continue` node.
/// - The expression node is a block with the first statement being a
/// `continue`.
-///
-fn needless_continue_in_else(else_expr: &ast::Expr) -> bool {
+fn needless_continue_in_else(else_expr: &ast::Expr, label: Option<&ast::Label>) -> bool {
match else_expr.node {
- ast::ExprKind::Block(ref else_block, _) => is_first_block_stmt_continue(else_block),
- ast::ExprKind::Continue(_) => true,
+ ast::ExprKind::Block(ref else_block, _) => is_first_block_stmt_continue(else_block, label),
+ ast::ExprKind::Continue(l) => compare_labels(label, l.as_ref()),
_ => false,
}
}
-fn is_first_block_stmt_continue(block: &ast::Block) -> bool {
+fn is_first_block_stmt_continue(block: &ast::Block, label: Option<&ast::Label>) -> bool {
block.stmts.get(0).map_or(false, |stmt| match stmt.node {
- ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => if let ast::ExprKind::Continue(_) = e.node {
- true
- } else {
- false
+ ast::StmtKind::Semi(ref e) | ast::StmtKind::Expr(ref e) => {
+ if let ast::ExprKind::Continue(ref l) = e.node {
+ compare_labels(label, l.as_ref())
+ } else {
+ false
+ }
},
_ => false,
})
}
+/// If the `continue` has a label, check it matches the label of the loop.
+fn compare_labels(loop_label: Option<&ast::Label>, continue_label: Option<&ast::Label>) -> bool {
+ match (loop_label, continue_label) {
+ // `loop { continue; }` or `'a loop { continue; }`
+ (_, None) => true,
+ // `loop { continue 'a; }`
+ (None, _) => false,
+ // `'a loop { continue 'a; }` or `'a loop { continue 'b; }`
+ (Some(x), Some(y)) => x.ident == y.ident,
+ }
+}
+
/// If `expr` is a loop expression (while/while let/for/loop), calls `func` with
/// the AST object representing the loop block of `expr`.
fn with_loop_block<F>(expr: &ast::Expr, mut func: F)
where
- F: FnMut(&ast::Block),
+ F: FnMut(&ast::Block, Option<&ast::Label>),
{
match expr.node {
- ast::ExprKind::While(_, ref loop_block, _) |
- ast::ExprKind::WhileLet(_, _, ref loop_block, _) |
- ast::ExprKind::ForLoop(_, _, ref loop_block, _) |
- ast::ExprKind::Loop(ref loop_block, _) => func(loop_block),
+ ast::ExprKind::While(_, ref loop_block, ref label)
+ | ast::ExprKind::WhileLet(_, _, ref loop_block, ref label)
+ | ast::ExprKind::ForLoop(_, _, ref loop_block, ref label)
+ | ast::ExprKind::Loop(ref loop_block, ref label) => func(loop_block, label.as_ref()),
_ => {},
}
}
/// - The `if` condition expression,
/// - The `then` block, and
/// - The `else` expression.
-///
fn with_if_expr<F>(stmt: &ast::Stmt, mut func: F)
where
F: FnMut(&ast::Expr, &ast::Expr, &ast::Block, &ast::Expr),
const DROP_ELSE_BLOCK_MSG: &str = "Consider dropping the else clause, and moving out the code in the else \
block, like so:\n";
-
-fn emit_warning<'a>(ctx: &EarlyContext, data: &'a LintData, header: &str, typ: LintType) {
+fn emit_warning<'a>(ctx: &EarlyContext<'_>, data: &'a LintData<'_>, header: &str, typ: LintType) {
// snip is the whole *help* message that appears after the warning.
// message is the warning message.
// expr is the expression which the lint warning message refers to.
span_help_and_lint(ctx, NEEDLESS_CONTINUE, expr.span, message, &snip);
}
-fn suggestion_snippet_for_continue_inside_if<'a>(ctx: &EarlyContext, data: &'a LintData, header: &str) -> String {
+fn suggestion_snippet_for_continue_inside_if<'a>(
+ ctx: &EarlyContext<'_>,
+ data: &'a LintData<'_>,
+ header: &str,
+) -> String {
let cond_code = snippet(ctx, data.if_cond.span, "..");
let if_code = format!("if {} {{\n continue;\n}}\n", cond_code);
ret
}
-fn suggestion_snippet_for_continue_inside_else<'a>(ctx: &EarlyContext, data: &'a LintData, header: &str) -> String {
+fn suggestion_snippet_for_continue_inside_else<'a>(
+ ctx: &EarlyContext<'_>,
+ data: &'a LintData<'_>,
+ header: &str,
+) -> String {
let cond_code = snippet(ctx, data.if_cond.span, "..");
let mut if_code = format!("if {} {{\n", cond_code);
ret
}
-fn check_and_warn<'a>(ctx: &EarlyContext, expr: &'a ast::Expr) {
- with_loop_block(expr, |loop_block| {
+fn check_and_warn<'a>(ctx: &EarlyContext<'_>, expr: &'a ast::Expr) {
+ with_loop_block(expr, |loop_block, label| {
for (i, stmt) in loop_block.stmts.iter().enumerate() {
with_if_expr(stmt, |if_expr, cond, then_block, else_expr| {
let data = &LintData {
else_expr,
block_stmts: &loop_block.stmts,
};
- if needless_continue_in_else(else_expr) {
- emit_warning(ctx, data, DROP_ELSE_BLOCK_AND_MERGE_MSG, LintType::ContinueInsideElseBlock);
- } else if is_first_block_stmt_continue(then_block) {
+ if needless_continue_in_else(else_expr, label) {
+ emit_warning(
+ ctx,
+ data,
+ DROP_ELSE_BLOCK_AND_MERGE_MSG,
+ LintType::ContinueInsideElseBlock,
+ );
+ } else if is_first_block_stmt_continue(then_block, label) {
emit_warning(ctx, data, DROP_ELSE_BLOCK_MSG, LintType::ContinueInsideThenBlock);
}
});
/// continues eating till a non-whitespace character is found.
/// e.g., the string
///
-/// ```
-/// {
-/// let x = 5;
-/// }
+/// ```rust
+/// {
+/// let x = 5;
+/// }
/// ```
///
/// is transformed to
///
-/// ```
+/// ```ignore
/// {
/// let x = 5;
/// ```
/// any number of opening braces are eaten, followed by any number of newlines.
/// e.g., the string
///
-/// ```
+/// ```ignore
/// {
/// something();
/// inside_a_block();
///
/// is transformed to
///
-/// ```
+/// ```ignore
/// something();
/// inside_a_block();
/// }
/// ```
-///
pub fn erode_from_front(s: &str) -> String {
s.chars()
.skip_while(|c| c.is_whitespace())