diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 515e5af339bc..6f67acb29219 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -2,7 +2,7 @@ use crate::reexport::*; use crate::utils::{ - is_present_in_source, last_line_of_span, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg, + first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then, without_block_comments, }; use if_chain::if_chain; @@ -261,7 +261,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes { _ => {}, } } - let line_span = last_line_of_span(cx, attr.span); + let line_span = first_line_of_span(cx, attr.span); if let Some(mut sugg) = snippet_opt(cx, line_span) { if sugg.contains("#[") { diff --git a/clippy_lints/src/block_in_if_condition.rs b/clippy_lints/src/block_in_if_condition.rs index 5a64e444136e..325e12617142 100644 --- a/clippy_lints/src/block_in_if_condition.rs +++ b/clippy_lints/src/block_in_if_condition.rs @@ -2,6 +2,7 @@ use crate::utils::*; use matches::matches; use rustc::hir::map::Map; use rustc::lint::in_external_macro; +use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::*; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -79,8 +80,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { if in_external_macro(cx.sess(), expr.span) { return; } - if let Some((check, then, _)) = higher::if_block(&expr) { - if let ExprKind::Block(block, _) = &check.kind { + if let Some((cond, _, _)) = higher::if_block(&expr) { + if let ExprKind::Block(block, _) = &cond.kind { if block.rules == BlockCheckMode::DefaultBlock { if block.stmts.is_empty() { if let Some(ex) = &block.expr { @@ -89,16 +90,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) { return; } - span_lint_and_help( + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( cx, BLOCK_IN_IF_CONDITION_EXPR, - check.span, + cond.span, BRACED_EXPR_MESSAGE, - &format!( - "try\nif {} {} ... ", - snippet_block(cx, ex.span, ".."), - snippet_block(cx, then.span, "..") + "try", + format!( + "{}", + snippet_block_with_applicability( + cx, + ex.span, + "..", + Some(expr.span), + &mut applicability + ) ), + applicability, ); } } else { @@ -107,22 +116,30 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { return; } // move block higher - span_lint_and_help( + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( cx, BLOCK_IN_IF_CONDITION_STMT, - check.span, + expr.span.with_hi(cond.span.hi()), COMPLEX_BLOCK_MESSAGE, - &format!( - "try\nlet res = {};\nif res {} ... ", - snippet_block(cx, block.span, ".."), - snippet_block(cx, then.span, "..") + "try", + format!( + "let res = {}; if res", + snippet_block_with_applicability( + cx, + block.span, + "..", + Some(expr.span), + &mut applicability + ), ), + applicability, ); } } } else { let mut visitor = ExVisitor { found_block: None, cx }; - walk_expr(&mut visitor, check); + walk_expr(&mut visitor, cond); if let Some(block) = visitor.found_block { span_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE); } diff --git a/clippy_lints/src/collapsible_if.rs b/clippy_lints/src/collapsible_if.rs index af309b512699..c6ca85b0cdf4 100644 --- a/clippy_lints/src/collapsible_if.rs +++ b/clippy_lints/src/collapsible_if.rs @@ -95,7 +95,7 @@ fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) { fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool { // We trim all opening braces and whitespaces and then check if the next string is a comment. - let trimmed_block_text = snippet_block(cx, expr.span, "..") + let trimmed_block_text = snippet_block(cx, expr.span, "..", None) .trim_start_matches(|c: char| c.is_whitespace() || c == '{') .to_owned(); trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*") @@ -116,7 +116,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) { block.span, "this `else { if .. }` block can be collapsed", "try", - snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(), + snippet_block_with_applicability(cx, else_.span, "..", Some(block.span), &mut applicability).into_owned(), applicability, ); } @@ -146,7 +146,7 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: & format!( "if {} {}", lhs.and(&rhs), - snippet_block(cx, content.span, ".."), + snippet_block(cx, content.span, "..", Some(expr.span)), ), Applicability::MachineApplicable, // snippet ); diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 1d8c5ec9038d..d753b664045d 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -3,9 +3,9 @@ use crate::utils::paths; use crate::utils::sugg::Sugg; use crate::utils::usage::is_unused; use crate::utils::{ - expr_block, get_arg_name, in_macro, is_allowed, is_expn_of, is_refutable, is_wild, match_qpath, match_type, - match_var, multispan_sugg, remove_blocks, snippet, snippet_block, snippet_with_applicability, span_lint_and_help, - span_lint_and_note, span_lint_and_sugg, span_lint_and_then, walk_ptrs_ty, + expr_block, get_arg_name, in_macro, indent_of, is_allowed, is_expn_of, is_refutable, is_wild, match_qpath, + match_type, match_var, multispan_sugg, remove_blocks, snippet, snippet_block, snippet_with_applicability, + span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, walk_ptrs_ty, }; use if_chain::if_chain; use rustc::lint::in_external_macro; @@ -434,7 +434,7 @@ fn report_single_match_single_pattern( ) { let lint = if els.is_some() { SINGLE_MATCH_ELSE } else { SINGLE_MATCH }; let els_str = els.map_or(String::new(), |els| { - format!(" else {}", expr_block(cx, els, None, "..")) + format!(" else {}", expr_block(cx, els, None, "..", Some(expr.span))) }); span_lint_and_sugg( cx, @@ -447,7 +447,7 @@ fn report_single_match_single_pattern( "if let {} = {} {}{}", snippet(cx, arms[0].pat.span, ".."), snippet(cx, ex.span, ".."), - expr_block(cx, &arms[0].body, None, ".."), + expr_block(cx, &arms[0].body, None, "..", Some(expr.span)), els_str, ), Applicability::HasPlaceholders, @@ -523,17 +523,21 @@ fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], e (false, false) => Some(format!( "if {} {} else {}", snippet(cx, ex.span, "b"), - expr_block(cx, true_expr, None, ".."), - expr_block(cx, false_expr, None, "..") + expr_block(cx, true_expr, None, "..", Some(expr.span)), + expr_block(cx, false_expr, None, "..", Some(expr.span)) )), (false, true) => Some(format!( "if {} {}", snippet(cx, ex.span, "b"), - expr_block(cx, true_expr, None, "..") + expr_block(cx, true_expr, None, "..", Some(expr.span)) )), (true, false) => { let test = Sugg::hir(cx, ex, ".."); - Some(format!("if {} {}", !test, expr_block(cx, false_expr, None, ".."))) + Some(format!( + "if {} {}", + !test, + expr_block(cx, false_expr, None, "..", Some(expr.span)) + )) }, (true, true) => None, }; @@ -832,7 +836,7 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A let mut snippet_body = if match_body.span.from_expansion() { Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string() } else { - snippet_block(cx, match_body.span, "..").to_owned().to_string() + snippet_block(cx, match_body.span, "..", Some(expr.span)).to_string() }; // Do we need to add ';' to suggestion ? @@ -861,10 +865,11 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A "this match could be written as a `let` statement", "consider using `let` statement", format!( - "let {} = {};\n{}", + "let {} = {};\n{}{}", snippet_with_applicability(cx, bind_names, "..", &mut applicability), snippet_with_applicability(cx, matched_vars, "..", &mut applicability), - snippet_body + " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), + snippet_body, ), applicability, ); diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index ac14b113cc58..660aae25f557 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -36,10 +36,10 @@ use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::{original_sp, DUMMY_SP}; -use std::borrow::Cow; +use rustc_span::Span; use syntax::ast; -use crate::utils::{snippet, snippet_block, span_lint_and_help, trim_multiline}; +use crate::utils::{indent_of, snippet, snippet_block, span_lint_and_help}; declare_clippy_lint! { /// **What it does:** The lint checks for `if`-statements appearing in loops @@ -119,9 +119,9 @@ declare_clippy_lint! { declare_lint_pass!(NeedlessContinue => [NEEDLESS_CONTINUE]); impl EarlyLintPass for NeedlessContinue { - fn check_expr(&mut self, ctx: &EarlyContext<'_>, expr: &ast::Expr) { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) { if !expr.span.from_expansion() { - check_and_warn(ctx, expr); + check_and_warn(cx, expr); } } } @@ -273,93 +273,96 @@ struct LintData<'a> { block_stmts: &'a [ast::Stmt], } -const MSG_REDUNDANT_ELSE_BLOCK: &str = "This `else` block is redundant.\n"; +const MSG_REDUNDANT_ELSE_BLOCK: &str = "this `else` block is redundant"; -const MSG_ELSE_BLOCK_NOT_NEEDED: &str = "There is no need for an explicit `else` block for this `if` \ - expression\n"; +const MSG_ELSE_BLOCK_NOT_NEEDED: &str = "there is no need for an explicit `else` block for this `if` \ + expression"; -const DROP_ELSE_BLOCK_AND_MERGE_MSG: &str = "Consider dropping the `else` clause and merging the code that \ - follows (in the loop) with the `if` block, like so:\n"; +const DROP_ELSE_BLOCK_AND_MERGE_MSG: &str = "consider dropping the `else` clause and merging the code that \ + follows (in the loop) with the `if` block"; -const DROP_ELSE_BLOCK_MSG: &str = "Consider dropping the `else` clause, and moving out the code in the `else` \ - block, like so:\n"; +const DROP_ELSE_BLOCK_MSG: &str = "consider dropping the `else` clause"; -fn emit_warning<'a>(ctx: &EarlyContext<'_>, data: &'a LintData<'_>, header: &str, typ: LintType) { +fn emit_warning<'a>(cx: &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. let (snip, message, expr) = match typ { LintType::ContinueInsideElseBlock => ( - suggestion_snippet_for_continue_inside_else(ctx, data, header), + suggestion_snippet_for_continue_inside_else(cx, data), MSG_REDUNDANT_ELSE_BLOCK, data.else_expr, ), LintType::ContinueInsideThenBlock => ( - suggestion_snippet_for_continue_inside_if(ctx, data, header), + suggestion_snippet_for_continue_inside_if(cx, data), MSG_ELSE_BLOCK_NOT_NEEDED, data.if_expr, ), }; - span_lint_and_help(ctx, NEEDLESS_CONTINUE, expr.span, message, &snip); + span_lint_and_help( + cx, + NEEDLESS_CONTINUE, + expr.span, + message, + &format!("{}\n{}", header, snip), + ); } -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); - /* ^^^^--- Four spaces of indentation. */ - // region B - let else_code = snippet(ctx, data.else_expr.span, "..").into_owned(); - let else_code = erode_block(&else_code); - let else_code = trim_multiline(Cow::from(else_code), false); - - let mut ret = String::from(header); - ret.push_str(&if_code); - ret.push_str(&else_code); - ret.push_str("\n..."); - ret +fn suggestion_snippet_for_continue_inside_if<'a>(cx: &EarlyContext<'_>, data: &'a LintData<'_>) -> String { + let cond_code = snippet(cx, data.if_cond.span, ".."); + + let continue_code = snippet_block(cx, data.if_block.span, "..", Some(data.if_expr.span)); + + let else_code = snippet_block(cx, data.else_expr.span, "..", Some(data.if_expr.span)); + + let indent_if = indent_of(cx, data.if_expr.span).unwrap_or(0); + format!( + "{indent}if {} {}\n{indent}{}", + cond_code, + continue_code, + else_code, + indent = " ".repeat(indent_if), + ) } -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); +fn suggestion_snippet_for_continue_inside_else<'a>(cx: &EarlyContext<'_>, data: &'a LintData<'_>) -> String { + let cond_code = snippet(cx, data.if_cond.span, ".."); // Region B - let block_code = &snippet(ctx, data.if_block.span, "..").into_owned(); - let block_code = erode_block(block_code); - let block_code = trim_multiline(Cow::from(block_code), false); - - if_code.push_str(&block_code); + let block_code = erode_from_back(&snippet_block(cx, data.if_block.span, "..", Some(data.if_expr.span))); // Region C // These is the code in the loop block that follows the if/else construction // we are complaining about. We want to pull all of this code into the // `then` block of the `if` statement. + let indent = span_of_first_expr_in_block(data.if_block) + .and_then(|span| indent_of(cx, span)) + .unwrap_or(0); let to_annex = data.block_stmts[data.stmt_idx + 1..] .iter() .map(|stmt| original_sp(stmt.span, DUMMY_SP)) - .map(|span| snippet_block(ctx, span, "..").into_owned()) + .map(|span| { + let snip = snippet_block(cx, span, "..", None).into_owned(); + snip.lines() + .map(|line| format!("{}{}", " ".repeat(indent), line)) + .collect::>() + .join("\n") + }) .collect::>() .join("\n"); - let mut ret = String::from(header); - - ret.push_str(&if_code); - ret.push_str("\n// Merged code follows..."); - ret.push_str(&to_annex); - ret.push_str("\n}\n"); - ret + let indent_if = indent_of(cx, data.if_expr.span).unwrap_or(0); + format!( + "{indent_if}if {} {}\n{indent}// merged code follows:\n{}\n{indent_if}}}", + cond_code, + block_code, + to_annex, + indent = " ".repeat(indent), + indent_if = " ".repeat(indent_if), + ) } -fn check_and_warn<'a>(ctx: &EarlyContext<'_>, expr: &'a ast::Expr) { +fn check_and_warn<'a>(cx: &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| { @@ -373,22 +376,22 @@ fn check_and_warn<'a>(ctx: &EarlyContext<'_>, expr: &'a ast::Expr) { }; if needless_continue_in_else(else_expr, label) { emit_warning( - ctx, + cx, 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); + emit_warning(cx, data, DROP_ELSE_BLOCK_MSG, LintType::ContinueInsideThenBlock); } }); } }); } -/// Eats at `s` from the end till a closing brace `}` is encountered, and then -/// continues eating till a non-whitespace character is found. -/// e.g., the string +/// Eats at `s` from the end till a closing brace `}` is encountered, and then continues eating +/// till a non-whitespace character is found. e.g., the string. If no closing `}` is present, the +/// string will be preserved. /// /// ```rust /// { @@ -402,12 +405,9 @@ fn check_and_warn<'a>(ctx: &EarlyContext<'_>, expr: &'a ast::Expr) { /// { /// let x = 5; /// ``` -/// -/// NOTE: when there is no closing brace in `s`, `s` is _not_ preserved, i.e., -/// an empty string will be returned in that case. #[must_use] -pub fn erode_from_back(s: &str) -> String { - let mut ret = String::from(s); +fn erode_from_back(s: &str) -> String { + let mut ret = s.to_string(); while ret.pop().map_or(false, |c| c != '}') {} while let Some(c) = ret.pop() { if !c.is_whitespace() { @@ -415,41 +415,48 @@ pub fn erode_from_back(s: &str) -> String { break; } } - ret + if ret.is_empty() { + s.to_string() + } else { + ret + } } -/// Eats at `s` from the front by first skipping all leading whitespace. Then, -/// 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(); -/// } -/// ``` -#[must_use] -pub fn erode_from_front(s: &str) -> String { - s.chars() - .skip_while(|c| c.is_whitespace()) - .skip_while(|c| *c == '{') - .skip_while(|c| *c == '\n') - .collect::() +fn span_of_first_expr_in_block(block: &ast::Block) -> Option { + block.stmts.iter().next().map(|stmt| stmt.span) } -/// If `s` contains the code for a block, delimited by braces, this function -/// tries to get the contents of the block. If there is no closing brace -/// present, -/// an empty string is returned. -#[must_use] -pub fn erode_block(s: &str) -> String { - erode_from_back(&erode_from_front(s)) +#[cfg(test)] +mod test { + use super::erode_from_back; + + #[test] + #[rustfmt::skip] + fn test_erode_from_back() { + let input = "\ +{ + let x = 5; + let y = format!(\"{}\", 42); +}"; + + let expected = "\ +{ + let x = 5; + let y = format!(\"{}\", 42);"; + + let got = erode_from_back(input); + assert_eq!(expected, got); + } + + #[test] + #[rustfmt::skip] + fn test_erode_from_back_no_brace() { + let input = "\ +let x = 5; +let y = something(); +"; + let expected = input; + let got = erode_from_back(input); + assert_eq!(expected, got); + } } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 4b7e21bc3bad..ed6d9f81cc9f 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -44,6 +44,7 @@ use rustc_hir::Node; use rustc_hir::*; use rustc_lint::{LateContext, Level, Lint, LintContext}; use rustc_span::hygiene::{ExpnKind, MacroKind}; +use rustc_span::source_map::original_sp; use rustc_span::symbol::{self, kw, Symbol}; use rustc_span::{BytePos, Pos, Span, DUMMY_SP}; use smallvec::SmallVec; @@ -533,19 +534,49 @@ pub fn snippet_opt(cx: &T, span: Span) -> Option { cx.sess().source_map().span_to_snippet(span).ok() } -/// Converts a span (from a block) to a code snippet if available, otherwise use -/// default. -/// This trims the code of indentation, except for the first line. Use it for -/// blocks or block-like +/// Converts a span (from a block) to a code snippet if available, otherwise use default. +/// +/// This trims the code of indentation, except for the first line. Use it for blocks or block-like /// things which need to be printed as such. /// +/// The `indent_relative_to` arg can be used, to provide a span, where the indentation of the +/// resulting snippet of the given span. +/// /// # Example +/// +/// ```rust,ignore +/// snippet_block(cx, block.span, "..", None) +/// // where, `block` is the block of the if expr +/// if x { +/// y; +/// } +/// // will return the snippet +/// { +/// y; +/// } +/// ``` +/// /// ```rust,ignore -/// snippet_block(cx, expr.span, "..") +/// snippet_block(cx, block.span, "..", Some(if_expr.span)) +/// // where, `block` is the block of the if expr +/// if x { +/// y; +/// } +/// // will return the snippet +/// { +/// y; +/// } // aligned with `if` /// ``` -pub fn snippet_block<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> { +/// Note that the first line of the snippet always has 0 indentation. +pub fn snippet_block<'a, T: LintContext>( + cx: &T, + span: Span, + default: &'a str, + indent_relative_to: Option, +) -> Cow<'a, str> { let snip = snippet(cx, span, default); - trim_multiline(snip, true) + let indent = indent_relative_to.and_then(|s| indent_of(cx, s)); + trim_multiline(snip, true, indent) } /// Same as `snippet_block`, but adapts the applicability level by the rules of @@ -554,27 +585,73 @@ pub fn snippet_block_with_applicability<'a, T: LintContext>( cx: &T, span: Span, default: &'a str, + indent_relative_to: Option, applicability: &mut Applicability, ) -> Cow<'a, str> { let snip = snippet_with_applicability(cx, span, default, applicability); - trim_multiline(snip, true) + let indent = indent_relative_to.and_then(|s| indent_of(cx, s)); + trim_multiline(snip, true, indent) +} + +/// Returns a new Span that extends the original Span to the first non-whitespace char of the first +/// line. +/// +/// ```rust,ignore +/// let x = (); +/// // ^^ +/// // will be converted to +/// let x = (); +/// // ^^^^^^^^^^ +/// ``` +pub fn first_line_of_span(cx: &T, span: Span) -> Span { + if let Some(first_char_pos) = first_char_in_first_line(cx, span) { + span.with_lo(first_char_pos) + } else { + span + } } -/// Returns a new Span that covers the full last line of the given Span -pub fn last_line_of_span(cx: &T, span: Span) -> Span { +fn first_char_in_first_line(cx: &T, span: Span) -> Option { + let line_span = line_span(cx, span); + if let Some(snip) = snippet_opt(cx, line_span) { + snip.find(|c: char| !c.is_whitespace()) + .map(|pos| line_span.lo() + BytePos::from_usize(pos)) + } else { + None + } +} + +/// Returns the indentation of the line of a span +/// +/// ```rust,ignore +/// let x = (); +/// // ^^ -- will return 0 +/// let x = (); +/// // ^^ -- will return 4 +/// ``` +pub fn indent_of(cx: &T, span: Span) -> Option { + if let Some(snip) = snippet_opt(cx, line_span(cx, span)) { + snip.find(|c: char| !c.is_whitespace()) + } else { + None + } +} + +/// Extends the span to the beginning of the spans line, incl. whitespaces. +/// +/// ```rust,ignore +/// let x = (); +/// // ^^ +/// // will be converted to +/// let x = (); +/// // ^^^^^^^^^^^^^^ +/// ``` +fn line_span(cx: &T, span: Span) -> Span { + let span = original_sp(span, DUMMY_SP); let source_map_and_line = cx.sess().source_map().lookup_line(span.lo()).unwrap(); let line_no = source_map_and_line.line; - let line_start = &source_map_and_line.sf.lines[line_no]; - let span = Span::new(*line_start, span.hi(), span.ctxt()); - if_chain! { - if let Some(snip) = snippet_opt(cx, span); - if let Some(first_ch_pos) = snip.find(|c: char| !c.is_whitespace()); - then { - span.with_lo(span.lo() + BytePos::from_usize(first_ch_pos)) - } else { - span - } - } + let line_start = source_map_and_line.sf.lines[line_no]; + Span::new(line_start, span.hi(), span.ctxt()) } /// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`. @@ -584,8 +661,9 @@ pub fn expr_block<'a, T: LintContext>( expr: &Expr<'_>, option: Option, default: &'a str, + indent_relative_to: Option, ) -> Cow<'a, str> { - let code = snippet_block(cx, expr.span, default); + let code = snippet_block(cx, expr.span, default, indent_relative_to); let string = option.unwrap_or_default(); if expr.span.from_expansion() { Cow::Owned(format!("{{ {} }}", snippet_with_macro_callsite(cx, expr.span, default))) @@ -600,14 +678,14 @@ pub fn expr_block<'a, T: LintContext>( /// Trim indentation from a multiline string with possibility of ignoring the /// first line. -pub fn trim_multiline(s: Cow<'_, str>, ignore_first: bool) -> Cow<'_, str> { - let s_space = trim_multiline_inner(s, ignore_first, ' '); - let s_tab = trim_multiline_inner(s_space, ignore_first, '\t'); - trim_multiline_inner(s_tab, ignore_first, ' ') +fn trim_multiline(s: Cow<'_, str>, ignore_first: bool, indent: Option) -> Cow<'_, str> { + let s_space = trim_multiline_inner(s, ignore_first, indent, ' '); + let s_tab = trim_multiline_inner(s_space, ignore_first, indent, '\t'); + trim_multiline_inner(s_tab, ignore_first, indent, ' ') } -fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, ch: char) -> Cow<'_, str> { - let x = s +fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, indent: Option, ch: char) -> Cow<'_, str> { + let mut x = s .lines() .skip(ignore_first as usize) .filter_map(|l| { @@ -620,6 +698,9 @@ fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, ch: char) -> Cow<'_ }) .min() .unwrap_or(0); + if let Some(indent) = indent { + x = x.saturating_sub(indent); + } if x > 0 { Cow::Owned( s.lines() @@ -1141,87 +1222,6 @@ pub fn is_normalizable<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, param_env: ty::Para }) } -#[cfg(test)] -mod test { - use super::{trim_multiline, without_block_comments}; - - #[test] - fn test_trim_multiline_single_line() { - assert_eq!("", trim_multiline("".into(), false)); - assert_eq!("...", trim_multiline("...".into(), false)); - assert_eq!("...", trim_multiline(" ...".into(), false)); - assert_eq!("...", trim_multiline("\t...".into(), false)); - assert_eq!("...", trim_multiline("\t\t...".into(), false)); - } - - #[test] - #[rustfmt::skip] - fn test_trim_multiline_block() { - assert_eq!("\ - if x { - y - } else { - z - }", trim_multiline(" if x { - y - } else { - z - }".into(), false)); - assert_eq!("\ - if x { - \ty - } else { - \tz - }", trim_multiline(" if x { - \ty - } else { - \tz - }".into(), false)); - } - - #[test] - #[rustfmt::skip] - fn test_trim_multiline_empty_line() { - assert_eq!("\ - if x { - y - - } else { - z - }", trim_multiline(" if x { - y - - } else { - z - }".into(), false)); - } - - #[test] - fn test_without_block_comments_lines_without_block_comments() { - let result = without_block_comments(vec!["/*", "", "*/"]); - println!("result: {:?}", result); - assert!(result.is_empty()); - - let result = without_block_comments(vec!["", "/*", "", "*/", "#[crate_type = \"lib\"]", "/*", "", "*/", ""]); - assert_eq!(result, vec!["", "#[crate_type = \"lib\"]", ""]); - - let result = without_block_comments(vec!["/* rust", "", "*/"]); - assert!(result.is_empty()); - - let result = without_block_comments(vec!["/* one-line comment */"]); - assert!(result.is_empty()); - - let result = without_block_comments(vec!["/* nested", "/* multi-line", "comment", "*/", "test", "*/"]); - assert!(result.is_empty()); - - let result = without_block_comments(vec!["/* nested /* inline /* comment */ test */ */"]); - assert!(result.is_empty()); - - let result = without_block_comments(vec!["foo", "bar", "baz"]); - assert_eq!(result, vec!["foo", "bar", "baz"]); - } -} - pub fn match_def_path<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, did: DefId, syms: &[&str]) -> bool { let path = cx.get_def_path(did); path.len() == syms.len() && path.into_iter().zip(syms.iter()).all(|(a, &b)| a.as_str() == b) @@ -1369,3 +1369,84 @@ pub fn is_trait_impl_item(cx: &LateContext<'_, '_>, hir_id: HirId) -> bool { false } } + +#[cfg(test)] +mod test { + use super::{trim_multiline, without_block_comments}; + + #[test] + fn test_trim_multiline_single_line() { + assert_eq!("", trim_multiline("".into(), false, None)); + assert_eq!("...", trim_multiline("...".into(), false, None)); + assert_eq!("...", trim_multiline(" ...".into(), false, None)); + assert_eq!("...", trim_multiline("\t...".into(), false, None)); + assert_eq!("...", trim_multiline("\t\t...".into(), false, None)); + } + + #[test] + #[rustfmt::skip] + fn test_trim_multiline_block() { + assert_eq!("\ + if x { + y + } else { + z + }", trim_multiline(" if x { + y + } else { + z + }".into(), false, None)); + assert_eq!("\ + if x { + \ty + } else { + \tz + }", trim_multiline(" if x { + \ty + } else { + \tz + }".into(), false, None)); + } + + #[test] + #[rustfmt::skip] + fn test_trim_multiline_empty_line() { + assert_eq!("\ + if x { + y + + } else { + z + }", trim_multiline(" if x { + y + + } else { + z + }".into(), false, None)); + } + + #[test] + fn test_without_block_comments_lines_without_block_comments() { + let result = without_block_comments(vec!["/*", "", "*/"]); + println!("result: {:?}", result); + assert!(result.is_empty()); + + let result = without_block_comments(vec!["", "/*", "", "*/", "#[crate_type = \"lib\"]", "/*", "", "*/", ""]); + assert_eq!(result, vec!["", "#[crate_type = \"lib\"]", ""]); + + let result = without_block_comments(vec!["/* rust", "", "*/"]); + assert!(result.is_empty()); + + let result = without_block_comments(vec!["/* one-line comment */"]); + assert!(result.is_empty()); + + let result = without_block_comments(vec!["/* nested", "/* multi-line", "comment", "*/", "test", "*/"]); + assert!(result.is_empty()); + + let result = without_block_comments(vec!["/* nested /* inline /* comment */ test */ */"]); + assert!(result.is_empty()); + + let result = without_block_comments(vec!["foo", "bar", "baz"]); + assert_eq!(result, vec!["foo", "bar", "baz"]); + } +} diff --git a/tests/needless_continue_helpers.rs b/tests/needless_continue_helpers.rs deleted file mode 100644 index 255653b4737d..000000000000 --- a/tests/needless_continue_helpers.rs +++ /dev/null @@ -1,87 +0,0 @@ -// Tests for the various helper functions used by the needless_continue -// lint that don't belong in utils. - -use clippy_lints::needless_continue::{erode_block, erode_from_back, erode_from_front}; - -#[test] -#[rustfmt::skip] -fn test_erode_from_back() { - let input = "\ -{ - let x = 5; - let y = format!(\"{}\", 42); -}"; - - let expected = "\ -{ - let x = 5; - let y = format!(\"{}\", 42);"; - - let got = erode_from_back(input); - assert_eq!(expected, got); -} - -#[test] -#[rustfmt::skip] -fn test_erode_from_back_no_brace() { - let input = "\ -let x = 5; -let y = something(); -"; - let expected = ""; - let got = erode_from_back(input); - assert_eq!(expected, got); -} - -#[test] -#[rustfmt::skip] -fn test_erode_from_front() { - let input = " - { - something(); - inside_a_block(); - } - "; - let expected = -" something(); - inside_a_block(); - } - "; - let got = erode_from_front(input); - println!("input: {}\nexpected:\n{}\ngot:\n{}", input, expected, got); - assert_eq!(expected, got); -} - -#[test] -#[rustfmt::skip] -fn test_erode_from_front_no_brace() { - let input = " - something(); - inside_a_block(); - "; - let expected = -"something(); - inside_a_block(); - "; - let got = erode_from_front(input); - println!("input: {}\nexpected:\n{}\ngot:\n{}", input, expected, got); - assert_eq!(expected, got); -} - -#[test] -#[rustfmt::skip] -fn test_erode_block() { - - let input = " - { - something(); - inside_a_block(); - } - "; - let expected = -" something(); - inside_a_block();"; - let got = erode_block(input); - println!("input: {}\nexpected:\n{}\ngot:\n{}", input, expected, got); - assert_eq!(expected, got); -} diff --git a/tests/ui/block_in_if_condition.fixed b/tests/ui/block_in_if_condition.fixed new file mode 100644 index 000000000000..955801e40f9b --- /dev/null +++ b/tests/ui/block_in_if_condition.fixed @@ -0,0 +1,75 @@ +// run-rustfix +#![warn(clippy::block_in_if_condition_expr)] +#![warn(clippy::block_in_if_condition_stmt)] +#![allow(unused, clippy::let_and_return)] +#![warn(clippy::nonminimal_bool)] + +macro_rules! blocky { + () => {{ + true + }}; +} + +macro_rules! blocky_too { + () => {{ + let r = true; + r + }}; +} + +fn macro_if() { + if blocky!() {} + + if blocky_too!() {} +} + +fn condition_has_block() -> i32 { + let res = { + let x = 3; + x == 3 + }; if res { + 6 + } else { + 10 + } +} + +fn condition_has_block_with_single_expression() -> i32 { + if true { + 6 + } else { + 10 + } +} + +fn condition_is_normal() -> i32 { + let x = 3; + if x == 3 { + 6 + } else { + 10 + } +} + +fn condition_is_unsafe_block() { + let a: i32 = 1; + + // this should not warn because the condition is an unsafe block + if unsafe { 1u32 == std::mem::transmute(a) } { + println!("1u32 == a"); + } +} + +fn block_in_assert() { + let opt = Some(42); + assert!(opt + .as_ref() + .and_then(|val| { + let mut v = val * 2; + v -= 1; + Some(v * 3) + }) + .is_some()); +} + +fn main() {} diff --git a/tests/ui/block_in_if_condition.rs b/tests/ui/block_in_if_condition.rs index 50f238814a31..a6ea01d5fc5f 100644 --- a/tests/ui/block_in_if_condition.rs +++ b/tests/ui/block_in_if_condition.rs @@ -1,3 +1,4 @@ +// run-rustfix #![warn(clippy::block_in_if_condition_expr)] #![warn(clippy::block_in_if_condition_stmt)] #![allow(unused, clippy::let_and_return)] @@ -41,37 +42,6 @@ fn condition_has_block_with_single_expression() -> i32 { } } -fn predicate bool, T>(pfn: F, val: T) -> bool { - pfn(val) -} - -fn pred_test() { - let v = 3; - let sky = "blue"; - // This is a sneaky case, where the block isn't directly in the condition, - // but is actually nside a closure that the condition is using. - // The same principle applies -- add some extra expressions to make sure - // linter isn't confused by them. - if v == 3 - && sky == "blue" - && predicate( - |x| { - let target = 3; - x == target - }, - v, - ) - {} - - if predicate( - |x| { - let target = 3; - x == target - }, - v, - ) {} -} - fn condition_is_normal() -> i32 { let x = 3; if true && x == 3 { @@ -81,10 +51,6 @@ fn condition_is_normal() -> i32 { } } -fn closure_without_block() { - if predicate(|x| x == 3, 6) {} -} - fn condition_is_unsafe_block() { let a: i32 = 1; @@ -94,16 +60,6 @@ fn condition_is_unsafe_block() { } } -fn main() {} - -fn macro_in_closure() { - let option = Some(true); - - if option.unwrap_or_else(|| unimplemented!()) { - unimplemented!() - } -} - fn block_in_assert() { let opt = Some(42); assert!(opt @@ -115,3 +71,5 @@ fn block_in_assert() { }) .is_some()); } + +fn main() {} diff --git a/tests/ui/block_in_if_condition.stderr b/tests/ui/block_in_if_condition.stderr index d75f3c02f197..b0a0a276c890 100644 --- a/tests/ui/block_in_if_condition.stderr +++ b/tests/ui/block_in_if_condition.stderr @@ -1,62 +1,36 @@ error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` - --> $DIR/block_in_if_condition.rs:26:8 + --> $DIR/block_in_if_condition.rs:27:5 | -LL | if { - | ________^ +LL | / if { LL | | let x = 3; LL | | x == 3 LL | | } { | |_____^ | = note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings` - = help: try - let res = { - let x = 3; - x == 3 - }; - if res { - 6 - } ... +help: try + | +LL | let res = { +LL | let x = 3; +LL | x == 3 +LL | }; if res { + | error: omit braces around single expression condition - --> $DIR/block_in_if_condition.rs:37:8 + --> $DIR/block_in_if_condition.rs:38:8 | LL | if { true } { - | ^^^^^^^^ + | ^^^^^^^^ help: try: `true` | = note: `-D clippy::block-in-if-condition-expr` implied by `-D warnings` - = help: try - if true { - 6 - } ... - -error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` - --> $DIR/block_in_if_condition.rs:58:17 - | -LL | |x| { - | _________________^ -LL | | let target = 3; -LL | | x == target -LL | | }, - | |_____________^ - -error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` - --> $DIR/block_in_if_condition.rs:67:13 - | -LL | |x| { - | _____________^ -LL | | let target = 3; -LL | | x == target -LL | | }, - | |_________^ error: this boolean expression can be simplified - --> $DIR/block_in_if_condition.rs:77:8 + --> $DIR/block_in_if_condition.rs:47:8 | LL | if true && x == 3 { | ^^^^^^^^^^^^^^ help: try: `x == 3` | = note: `-D clippy::nonminimal-bool` implied by `-D warnings` -error: aborting due to 5 previous errors +error: aborting due to 3 previous errors diff --git a/tests/ui/block_in_if_condition_closure.rs b/tests/ui/block_in_if_condition_closure.rs new file mode 100644 index 000000000000..bac3eda5e7f3 --- /dev/null +++ b/tests/ui/block_in_if_condition_closure.rs @@ -0,0 +1,48 @@ +#![warn(clippy::block_in_if_condition_expr)] +#![warn(clippy::block_in_if_condition_stmt)] +#![allow(unused, clippy::let_and_return)] + +fn predicate bool, T>(pfn: F, val: T) -> bool { + pfn(val) +} + +fn pred_test() { + let v = 3; + let sky = "blue"; + // This is a sneaky case, where the block isn't directly in the condition, + // but is actually nside a closure that the condition is using. + // The same principle applies -- add some extra expressions to make sure + // linter isn't confused by them. + if v == 3 + && sky == "blue" + && predicate( + |x| { + let target = 3; + x == target + }, + v, + ) + {} + + if predicate( + |x| { + let target = 3; + x == target + }, + v, + ) {} +} + +fn closure_without_block() { + if predicate(|x| x == 3, 6) {} +} + +fn macro_in_closure() { + let option = Some(true); + + if option.unwrap_or_else(|| unimplemented!()) { + unimplemented!() + } +} + +fn main() {} diff --git a/tests/ui/block_in_if_condition_closure.stderr b/tests/ui/block_in_if_condition_closure.stderr new file mode 100644 index 000000000000..86cd24fe7632 --- /dev/null +++ b/tests/ui/block_in_if_condition_closure.stderr @@ -0,0 +1,24 @@ +error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` + --> $DIR/block_in_if_condition_closure.rs:19:17 + | +LL | |x| { + | _________________^ +LL | | let target = 3; +LL | | x == target +LL | | }, + | |_____________^ + | + = note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings` + +error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` + --> $DIR/block_in_if_condition_closure.rs:28:13 + | +LL | |x| { + | _____________^ +LL | | let target = 3; +LL | | x == target +LL | | }, + | |_________^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/collapsible_else_if.fixed b/tests/ui/collapsible_else_if.fixed index a31f748670ce..c4149ad19c1e 100644 --- a/tests/ui/collapsible_else_if.fixed +++ b/tests/ui/collapsible_else_if.fixed @@ -10,57 +10,57 @@ fn main() { if x == "hello" { print!("Hello "); } else if y == "world" { - println!("world!") -} + println!("world!") + } if x == "hello" { print!("Hello "); } else if let Some(42) = Some(42) { - println!("world!") -} + println!("world!") + } if x == "hello" { print!("Hello "); } else if y == "world" { - println!("world") -} -else { - println!("!") -} + println!("world") + } + else { + println!("!") + } if x == "hello" { print!("Hello "); } else if let Some(42) = Some(42) { - println!("world") -} -else { - println!("!") -} + println!("world") + } + else { + println!("!") + } if let Some(42) = Some(42) { print!("Hello "); } else if let Some(42) = Some(42) { - println!("world") -} -else { - println!("!") -} + println!("world") + } + else { + println!("!") + } if let Some(42) = Some(42) { print!("Hello "); } else if x == "hello" { - println!("world") -} -else { - println!("!") -} + println!("world") + } + else { + println!("!") + } if let Some(42) = Some(42) { print!("Hello "); } else if let Some(42) = Some(42) { - println!("world") -} -else { - println!("!") -} + println!("world") + } + else { + println!("!") + } } diff --git a/tests/ui/collapsible_else_if.stderr b/tests/ui/collapsible_else_if.stderr index 26e3635c2d1c..28048999e8ec 100644 --- a/tests/ui/collapsible_else_if.stderr +++ b/tests/ui/collapsible_else_if.stderr @@ -13,8 +13,8 @@ LL | | } help: try | LL | } else if y == "world" { -LL | println!("world!") -LL | } +LL | println!("world!") +LL | } | error: this `else { if .. }` block can be collapsed @@ -31,8 +31,8 @@ LL | | } help: try | LL | } else if let Some(42) = Some(42) { -LL | println!("world!") -LL | } +LL | println!("world!") +LL | } | error: this `else { if .. }` block can be collapsed @@ -51,11 +51,11 @@ LL | | } help: try | LL | } else if y == "world" { -LL | println!("world") -LL | } -LL | else { -LL | println!("!") -LL | } +LL | println!("world") +LL | } +LL | else { +LL | println!("!") +LL | } | error: this `else { if .. }` block can be collapsed @@ -74,11 +74,11 @@ LL | | } help: try | LL | } else if let Some(42) = Some(42) { -LL | println!("world") -LL | } -LL | else { -LL | println!("!") -LL | } +LL | println!("world") +LL | } +LL | else { +LL | println!("!") +LL | } | error: this `else { if .. }` block can be collapsed @@ -97,11 +97,11 @@ LL | | } help: try | LL | } else if let Some(42) = Some(42) { -LL | println!("world") -LL | } -LL | else { -LL | println!("!") -LL | } +LL | println!("world") +LL | } +LL | else { +LL | println!("!") +LL | } | error: this `else { if .. }` block can be collapsed @@ -120,11 +120,11 @@ LL | | } help: try | LL | } else if x == "hello" { -LL | println!("world") -LL | } -LL | else { -LL | println!("!") -LL | } +LL | println!("world") +LL | } +LL | else { +LL | println!("!") +LL | } | error: this `else { if .. }` block can be collapsed @@ -143,11 +143,11 @@ LL | | } help: try | LL | } else if let Some(42) = Some(42) { -LL | println!("world") -LL | } -LL | else { -LL | println!("!") -LL | } +LL | println!("world") +LL | } +LL | else { +LL | println!("!") +LL | } | error: aborting due to 7 previous errors diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index e8f90b3164a9..076771f5c57e 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -7,28 +7,28 @@ fn main() { let x = "hello"; let y = "world"; if x == "hello" && y == "world" { - println!("Hello world!"); -} + println!("Hello world!"); + } if (x == "hello" || x == "world") && (y == "world" || y == "hello") { - println!("Hello world!"); -} + println!("Hello world!"); + } if x == "hello" && x == "world" && (y == "world" || y == "hello") { - println!("Hello world!"); -} + println!("Hello world!"); + } if (x == "hello" || x == "world") && y == "world" && y == "hello" { - println!("Hello world!"); -} + println!("Hello world!"); + } if x == "hello" && x == "world" && y == "world" && y == "hello" { - println!("Hello world!"); -} + println!("Hello world!"); + } if 42 == 1337 && 'a' != 'A' { - println!("world!") -} + println!("world!") + } // Works because any if with an else statement cannot be collapsed. if x == "hello" { @@ -81,8 +81,8 @@ fn main() { } if x == "hello" && y == "world" { // Collapsible - println!("Hello world!"); -} + println!("Hello world!"); + } if x == "hello" { print!("Hello "); diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr index b123bc1c7bd7..6440ff41be81 100644 --- a/tests/ui/collapsible_if.stderr +++ b/tests/ui/collapsible_if.stderr @@ -12,8 +12,8 @@ LL | | } help: try | LL | if x == "hello" && y == "world" { -LL | println!("Hello world!"); -LL | } +LL | println!("Hello world!"); +LL | } | error: this `if` statement can be collapsed @@ -29,8 +29,8 @@ LL | | } help: try | LL | if (x == "hello" || x == "world") && (y == "world" || y == "hello") { -LL | println!("Hello world!"); -LL | } +LL | println!("Hello world!"); +LL | } | error: this `if` statement can be collapsed @@ -46,8 +46,8 @@ LL | | } help: try | LL | if x == "hello" && x == "world" && (y == "world" || y == "hello") { -LL | println!("Hello world!"); -LL | } +LL | println!("Hello world!"); +LL | } | error: this `if` statement can be collapsed @@ -63,8 +63,8 @@ LL | | } help: try | LL | if (x == "hello" || x == "world") && y == "world" && y == "hello" { -LL | println!("Hello world!"); -LL | } +LL | println!("Hello world!"); +LL | } | error: this `if` statement can be collapsed @@ -80,8 +80,8 @@ LL | | } help: try | LL | if x == "hello" && x == "world" && y == "world" && y == "hello" { -LL | println!("Hello world!"); -LL | } +LL | println!("Hello world!"); +LL | } | error: this `if` statement can be collapsed @@ -97,8 +97,8 @@ LL | | } help: try | LL | if 42 == 1337 && 'a' != 'A' { -LL | println!("world!") -LL | } +LL | println!("world!") +LL | } | error: this `if` statement can be collapsed @@ -114,8 +114,8 @@ LL | | } help: try | LL | if x == "hello" && y == "world" { // Collapsible -LL | println!("Hello world!"); -LL | } +LL | println!("Hello world!"); +LL | } | error: aborting due to 7 previous errors diff --git a/tests/ui/match_bool.stderr b/tests/ui/match_bool.stderr index 42f20862939b..d0c20eb2696b 100644 --- a/tests/ui/match_bool.stderr +++ b/tests/ui/match_bool.stderr @@ -40,8 +40,8 @@ LL | | }; help: consider using an `if`/`else` expression | LL | if !test { -LL | println!("Noooo!"); -LL | }; +LL | println!("Noooo!"); +LL | }; | error: you seem to be trying to match on a boolean expression @@ -58,8 +58,8 @@ LL | | }; help: consider using an `if`/`else` expression | LL | if !test { -LL | println!("Noooo!"); -LL | }; +LL | println!("Noooo!"); +LL | }; | error: you seem to be trying to match on a boolean expression @@ -76,8 +76,8 @@ LL | | }; help: consider using an `if`/`else` expression | LL | if !(test && test) { -LL | println!("Noooo!"); -LL | }; +LL | println!("Noooo!"); +LL | }; | error: equal expressions as operands to `&&` @@ -103,10 +103,10 @@ LL | | }; help: consider using an `if`/`else` expression | LL | if test { -LL | println!("Yes!"); -LL | } else { -LL | println!("Noooo!"); -LL | }; +LL | println!("Yes!"); +LL | } else { +LL | println!("Noooo!"); +LL | }; | error: aborting due to 8 previous errors diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed index 8fb8dc323e43..932bd6783a15 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -14,12 +14,12 @@ fn main() { let c = 3; // Lint let (x, y, z) = (a, b, c); -{ - println!("{} {} {}", x, y, z); -} + { + println!("{} {} {}", x, y, z); + } // Lint let (x, y, z) = (a, b, c); -println!("{} {} {}", x, y, z); + println!("{} {} {}", x, y, z); // Ok match a { 2 => println!("2"), @@ -35,29 +35,29 @@ println!("{} {} {}", x, y, z); println!("whatever"); // Lint { - let x = 29; - println!("x has a value of {}", x); -} + let x = 29; + println!("x has a value of {}", x); + } // Lint { - let e = 5 * a; - if e >= 5 { - println!("e is superior to 5"); + let e = 5 * a; + if e >= 5 { + println!("e is superior to 5"); + } } -} // Lint let p = Point { x: 0, y: 7 }; let Point { x, y } = p; -println!("Coords: ({}, {})", x, y); + println!("Coords: ({}, {})", x, y); // Lint let Point { x: x1, y: y1 } = p; -println!("Coords: ({}, {})", x1, y1); + println!("Coords: ({}, {})", x1, y1); // Lint let x = 5; let ref r = x; -println!("Got a reference to {}", r); + println!("Got a reference to {}", r); // Lint let mut x = 5; let ref mut mr = x; -println!("Got a mutable reference to {}", mr); + println!("Got a mutable reference to {}", mr); } diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index d76e229adff0..471aec780d23 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -12,9 +12,9 @@ LL | | } help: consider using `let` statement | LL | let (x, y, z) = (a, b, c); -LL | { -LL | println!("{} {} {}", x, y, z); -LL | } +LL | { +LL | println!("{} {} {}", x, y, z); +LL | } | error: this match could be written as a `let` statement @@ -28,7 +28,7 @@ LL | | } help: consider using `let` statement | LL | let (x, y, z) = (a, b, c); -LL | println!("{} {} {}", x, y, z); +LL | println!("{} {} {}", x, y, z); | error: this match could be replaced by its body itself @@ -53,9 +53,9 @@ LL | | } help: consider using the match body instead | LL | { -LL | let x = 29; -LL | println!("x has a value of {}", x); -LL | } +LL | let x = 29; +LL | println!("x has a value of {}", x); +LL | } | error: this match could be replaced by its body itself @@ -73,11 +73,11 @@ LL | | } help: consider using the match body instead | LL | { -LL | let e = 5 * a; -LL | if e >= 5 { -LL | println!("e is superior to 5"); +LL | let e = 5 * a; +LL | if e >= 5 { +LL | println!("e is superior to 5"); +LL | } LL | } -LL | } | error: this match could be written as a `let` statement @@ -91,7 +91,7 @@ LL | | } help: consider using `let` statement | LL | let Point { x, y } = p; -LL | println!("Coords: ({}, {})", x, y); +LL | println!("Coords: ({}, {})", x, y); | error: this match could be written as a `let` statement @@ -105,7 +105,7 @@ LL | | } help: consider using `let` statement | LL | let Point { x: x1, y: y1 } = p; -LL | println!("Coords: ({}, {})", x1, y1); +LL | println!("Coords: ({}, {})", x1, y1); | error: this match could be written as a `let` statement @@ -119,7 +119,7 @@ LL | | } help: consider using `let` statement | LL | let ref r = x; -LL | println!("Got a reference to {}", r); +LL | println!("Got a reference to {}", r); | error: this match could be written as a `let` statement @@ -133,7 +133,7 @@ LL | | } help: consider using `let` statement | LL | let ref mut mr = x; -LL | println!("Got a mutable reference to {}", mr); +LL | println!("Got a mutable reference to {}", mr); | error: aborting due to 9 previous errors diff --git a/tests/ui/needless_continue.stderr b/tests/ui/needless_continue.stderr index b9215885877d..8d6a37df9601 100644 --- a/tests/ui/needless_continue.stderr +++ b/tests/ui/needless_continue.stderr @@ -1,5 +1,4 @@ -error: This `else` block is redundant. - +error: this `else` block is redundant --> $DIR/needless_continue.rs:28:16 | LL | } else { @@ -9,34 +8,33 @@ LL | | } | |_________^ | = note: `-D clippy::needless-continue` implied by `-D warnings` - = help: Consider dropping the `else` clause and merging the code that follows (in the loop) with the `if` block, like so: - if i % 2 == 0 && i % 3 == 0 { - println!("{}", i); - println!("{}", i + 1); - if i % 5 == 0 { - println!("{}", i + 2); - } - let i = 0; - println!("bar {} ", i); - // Merged code follows...println!("bleh"); - { - println!("blah"); - } - if !(!(i == 2) || !(i == 5)) { - println!("lama"); - } - if (zero!(i % 2) || nonzero!(i % 5)) && i % 3 != 0 { - continue; - } else { - println!("Blabber"); - println!("Jabber"); - } - println!("bleh"); - } - - -error: There is no need for an explicit `else` block for this `if` expression + = help: consider dropping the `else` clause and merging the code that follows (in the loop) with the `if` block + if i % 2 == 0 && i % 3 == 0 { + println!("{}", i); + println!("{}", i + 1); + if i % 5 == 0 { + println!("{}", i + 2); + } + let i = 0; + println!("bar {} ", i); + // merged code follows: + println!("bleh"); + { + println!("blah"); + } + if !(!(i == 2) || !(i == 5)) { + println!("lama"); + } + if (zero!(i % 2) || nonzero!(i % 5)) && i % 3 != 0 { + continue; + } else { + println!("Blabber"); + println!("Jabber"); + } + println!("bleh"); + } +error: there is no need for an explicit `else` block for this `if` expression --> $DIR/needless_continue.rs:43:9 | LL | / if (zero!(i % 2) || nonzero!(i % 5)) && i % 3 != 0 { @@ -47,16 +45,16 @@ LL | | println!("Jabber"); LL | | } | |_________^ | - = help: Consider dropping the `else` clause, and moving out the code in the `else` block, like so: - if (zero!(i % 2) || nonzero!(i % 5)) && i % 3 != 0 { - continue; - } - println!("Blabber"); - println!("Jabber"); - ... - -error: This `else` block is redundant. + = help: consider dropping the `else` clause + if (zero!(i % 2) || nonzero!(i % 5)) && i % 3 != 0 { + continue; + } + { + println!("Blabber"); + println!("Jabber"); + } +error: this `else` block is redundant --> $DIR/needless_continue.rs:100:24 | LL | } else { @@ -65,22 +63,21 @@ LL | | continue 'inner; // should lint here LL | | } | |_________________^ | - = help: Consider dropping the `else` clause and merging the code that follows (in the loop) with the `if` block, like so: - if condition() { - println!("bar-3"); - // Merged code follows...println!("bar-4"); - update_condition(); - if condition() { - continue; // should lint here - } else { - println!("bar-5"); - } - println!("bar-6"); - } - - -error: There is no need for an explicit `else` block for this `if` expression + = help: consider dropping the `else` clause and merging the code that follows (in the loop) with the `if` block + if condition() { + println!("bar-3"); + // merged code follows: + println!("bar-4"); + update_condition(); + if condition() { + continue; // should lint here + } else { + println!("bar-5"); + } + println!("bar-6"); + } +error: there is no need for an explicit `else` block for this `if` expression --> $DIR/needless_continue.rs:106:17 | LL | / if condition() { @@ -90,12 +87,13 @@ LL | | println!("bar-5"); LL | | } | |_________________^ | - = help: Consider dropping the `else` clause, and moving out the code in the `else` block, like so: - if condition() { - continue; - } - println!("bar-5"); - ... + = help: consider dropping the `else` clause + if condition() { + continue; // should lint here + } + { + println!("bar-5"); + } error: aborting due to 4 previous errors diff --git a/tests/ui/single_match.stderr b/tests/ui/single_match.stderr index 80ddeabdb2fd..f69554d75f9b 100644 --- a/tests/ui/single_match.stderr +++ b/tests/ui/single_match.stderr @@ -13,8 +13,8 @@ LL | | }; help: try this | LL | if let Some(y) = x { -LL | println!("{:?}", y); -LL | }; +LL | println!("{:?}", y); +LL | }; | error: you seem to be trying to use match for destructuring a single pattern. Consider using `if let` diff --git a/tests/ui/single_match_else.stderr b/tests/ui/single_match_else.stderr index 3f29f5aaf6a3..59861d46eb34 100644 --- a/tests/ui/single_match_else.stderr +++ b/tests/ui/single_match_else.stderr @@ -14,9 +14,9 @@ LL | | } help: try this | LL | if let ExprNode::ExprAddrOf = ExprNode::Butterflies { Some(&NODE) } else { -LL | let x = 5; -LL | None -LL | } +LL | let x = 5; +LL | None +LL | } | error: aborting due to previous error