From 8ea6d6a8d285d8a09885ebb535051346bf22cd79 Mon Sep 17 00:00:00 2001 From: Yati Sagade Date: Sun, 9 Apr 2017 14:07:11 +0200 Subject: [PATCH] needless_continue: Refactor to use stuff from utils, and move some stuff to utils. I had my own implementation of what `trim_multiline()` seems to be doing, so I just started using `trim_multiline()`. Some other functions, like those block alignment, are general enough to be used elsewhere, so moved them to utils. --- clippy_lints/src/needless_continue.rs | 176 ++++++------------------ clippy_lints/src/utils/mod.rs | 188 ++++++++++++++++++++++++++ 2 files changed, 229 insertions(+), 135 deletions(-) diff --git a/clippy_lints/src/needless_continue.rs b/clippy_lints/src/needless_continue.rs index c6ae0113f91f..5784032b750e 100644 --- a/clippy_lints/src/needless_continue.rs +++ b/clippy_lints/src/needless_continue.rs @@ -27,12 +27,13 @@ //! ``` //! //! This lint is **warn** by default. -use std; use rustc::lint::*; use syntax::ast; use syntax::codemap::{original_sp,DUMMY_SP}; +use std::borrow::Cow; -use utils::{in_macro, span_help_and_lint, snippet_block, snippet}; +use utils::{in_macro, span_help_and_lint, snippet_block, snippet, trim_multiline, + left_pad_lines_with_spaces, align_snippets}; /// **What it does:** The lint checks for `if`-statements appearing in loops /// that contain a `continue` statement in either their main blocks or their @@ -110,7 +111,7 @@ impl LintPass for NeedlessContinue { impl EarlyLintPass for NeedlessContinue { fn check_expr(&mut self, ctx: &EarlyContext, expr: &ast::Expr) { - if !in_macro(ctx, expr.span) { + if !in_macro(expr.span) { check_and_warn(ctx, expr); } } @@ -290,7 +291,7 @@ fn suggestion_snippet_for_continue_inside_if<'a>(ctx: &EarlyContext, // region B let else_code = snippet(ctx, data.else_expr.span, "..").into_owned(); let else_code = erode_block(&else_code); - let else_code = trim_indent(&else_code, false); + let else_code = trim_multiline(Cow::from(else_code), false); let mut ret = String::from(header); ret.push_str(&if_code); @@ -309,7 +310,7 @@ fn suggestion_snippet_for_continue_inside_else<'a>(ctx: &EarlyContext, // Region B let block_code = &snippet(ctx, data.if_block.span, "..").into_owned(); let block_code = erode_block(block_code); - let block_code = trim_indent(&block_code, false); + let block_code = trim_multiline(Cow::from(block_code), false); let block_code = left_pad_lines_with_spaces(&block_code, 4_usize); if_code.push_str(&block_code); @@ -321,7 +322,7 @@ fn suggestion_snippet_for_continue_inside_else<'a>(ctx: &EarlyContext, let to_annex = data.block_stmts[data.stmt_idx+1..] .iter() .map(|stmt| { - original_sp(ctx.sess().codemap(), stmt.span, DUMMY_SP) + original_sp(stmt.span, DUMMY_SP) }) .map(|span| snippet_block(ctx, span, "..").into_owned()) .collect::>().join("\n"); @@ -360,19 +361,21 @@ fn check_and_warn<'a>(ctx: &EarlyContext, expr: &'a ast::Expr) { /// continues eating till a non-whitespace character is found. /// e.g., the string /// -/// " -/// { -/// let x = 5; -/// } -/// " +/// " +/// { +/// let x = 5; +/// } +/// " /// /// is transformed to /// -/// " -/// { -/// let x = 5;" +/// " +/// { +/// let x = 5;" /// -fn erode_from_back(s: &str) -> String { +/// NOTE: when there is no closing brace in `s`, `s` is _not_ preserved, i.e., +/// an empty string will be returned in that case. +pub fn erode_from_back(s: &str) -> String { let mut ret = String::from(s); while ret.pop().map_or(false, |c| c != '}') { } while let Some(c) = ret.pop() { @@ -384,7 +387,25 @@ fn erode_from_back(s: &str) -> String { ret } -fn erode_from_front(s: &str) -> String { +/// 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 +/// +/// " +/// { +/// something(); +/// inside_a_block(); +/// } +/// " +/// +/// is transformed to +/// +/// " something(); +/// inside_a_block(); +/// } +/// " +/// +pub fn erode_from_front(s: &str) -> String { s.chars() .skip_while(|c| c.is_whitespace()) .skip_while(|c| *c == '{') @@ -392,124 +413,9 @@ fn erode_from_front(s: &str) -> String { .collect::() } -fn erode_block(s: &str) -> String { +/// 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. +pub fn erode_block(s: &str) -> String { erode_from_back(&erode_from_front(s)) } - -fn is_all_whitespace(s: &str) -> bool { s.chars().all(|c| c.is_whitespace()) } - -/// Returns true if a string is empty or just spaces. -fn is_null(s: &str) -> bool { s.is_empty() || is_all_whitespace(s) } - -/// Returns the indentation level of a string. It just returns the count of -/// whitespace characters in the string before a non-whitespace character -/// is encountered. -fn indent_level(s: &str) -> usize { - s.chars() - .enumerate() - .find(|&(_, c)| !c.is_whitespace()) - .map_or(0_usize, |(i, _)| i) -} - -/// Trims indentation from a snippet such that the line with the minimum -/// indentation has no indentation after the trasformation. -fn trim_indent(s: &str, skip_first_line: bool) -> String { - let min_indent_level = s.lines() - .filter(|line| !is_null(line)) - .skip(skip_first_line as usize) - .map(indent_level) - .min() - .unwrap_or(0_usize); - let ret = s.lines().map(|line| { - if is_null(line) { - String::from(line) - } else { - line.chars() - .enumerate() - .skip_while(|&(i, c)| c.is_whitespace() && i < min_indent_level) - .map(|pair| pair.1) - .collect::() - } - }).collect::>(); - ret.join("\n") -} - -/// Add `n` spaces to the left of `s`. -fn left_pad_with_spaces(s: &str, n: usize) -> String { - let mut new_s = std::iter::repeat(' '/* <-space */).take(n).collect::(); - new_s.push_str(s); - new_s -} - -/// Add `n` spaces to the left of each line in `s` and return the result -/// in a new String. -fn left_pad_lines_with_spaces(s: &str, n: usize) -> String { - s.lines() - .map(|line| left_pad_with_spaces(line, n)) - .collect::>() - .join("\n") -} - -/// Remove upto `n` whitespace characters from the beginning of `s`. -fn remove_whitespace_from_left(s: &str, n: usize) -> String { - s.chars() - .enumerate() - .skip_while(|&(i, c)| i < n && c.is_whitespace()) - .map(|(_, c)| c) - .collect::() -} - -/// Aligns two snippets such that the indentation level of the last non-empty, -/// non-space line of the first snippet matches the first non-empty, non-space -/// line of the second. -fn align_two_snippets(s: &str, t: &str) -> String { - // indent level of the last nonempty, non-whitespace line of s. - let target_ilevel = s.lines() - .rev() - .skip_while(|line| line.is_empty() || is_all_whitespace(line)) - .next() - .map_or(0_usize, indent_level); - - // We want to align the first nonempty, non-all-whitespace line of t to - // have the same indent level as target_ilevel - let level = t.lines() - .skip_while(|line| line.is_empty() || is_all_whitespace(line)) - .next() - .map_or(0_usize, indent_level); - - let add_or_not_remove = target_ilevel > level; /* when true, we add spaces, - otherwise eat. */ - - let delta = if add_or_not_remove { - target_ilevel - level - } else { - level - target_ilevel - }; - - let new_t = t.lines() - .filter_map(|line| { - if is_null(line) { - None - } else if add_or_not_remove { - Some(left_pad_with_spaces(line, delta)) - } else { - Some(remove_whitespace_from_left(line, delta)) - } - }) - .collect::>().join("\n"); - - format!("{}\n{}", s, new_t) -} - -fn align_snippets(xs: &[&str]) -> String { - if xs.is_empty() { - String::from("") - } else { - let mut ret = xs[0].to_string(); - for x in xs.iter().skip(1_usize) { - ret = align_two_snippets(&ret, x); - } - ret - } -} - diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index db595b079699..2e35140cdf55 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -22,6 +22,7 @@ use syntax::codemap::{ExpnFormat, ExpnInfo, MultiSpan, Span, DUMMY_SP}; use syntax::errors::DiagnosticBuilder; use syntax::ptr::P; use syntax::symbol::keywords; +use std::iter; pub mod comparisons; pub mod conf; @@ -978,3 +979,190 @@ pub fn type_size<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>) -> Opti .infer_ctxt((), Reveal::All) .enter(|infcx| ty.layout(&infcx).ok().map(|lay| lay.size(&TargetDataLayout::parse(cx.sess())).bytes())) } + +/// Add `n` spaces to the left of `s`. +pub fn left_pad_with_spaces(s: &str, n: usize) -> String { + let mut new_s = iter::repeat(' ').take(n).collect::(); + new_s.push_str(s); + new_s +} + +/// Add `n` spaces to the left of each line in `s` and return the result +/// in a new String. +/// e.g., when n = 2, the string +/// +/// " fn foo() { +/// bar() +/// }" +/// +/// becomes +/// +/// " fn foo() { +/// bar() +/// }" +/// +/// # Example +/// +/// ``` +/// use clippy_lints::utils::left_pad_with_spaces; +/// +/// let input = "\ +/// fn main() { +/// println!("hello world!"); +/// }"; +/// +/// let expected = +/// " fn main() { +/// println!("hello world!"); +/// }"; +/// +/// assert_eq!(expected, input); +/// ``` +pub fn left_pad_lines_with_spaces(s: &str, n: usize) -> String { + s.lines() + .map(|line| left_pad_with_spaces(line, n)) + .collect::>() + .join("\n") +} + +/// Remove upto `n` whitespace characters from the beginning of `s`. +/// +/// # Examples +/// +/// ``` +/// let s = " foobar "; +/// assert_eq!("foobar ", remove_whitespace_from_left(s, 100)); +/// assert_eq!(" foobar ", remove_whitespace_from_left(s, 2)); +/// assert_eq("", remove_whitespace_from_left(" ", 50)); +/// ``` +pub fn remove_whitespace_from_left(s: &str, n: usize) -> String { + s.chars() + .enumerate() + .skip_while(|&(i, c)| i < n && c.is_whitespace()) + .map(|(_, c)| c) + .collect::() +} + +/// Aligns two snippets such that the indentation level of the last non-empty, +/// non-space line of the first snippet matches the first non-empty, non-space +/// line of the second. +pub fn align_two_snippets(s: &str, t: &str) -> String { + // indent level of the last nonempty, non-whitespace line of s. + let target_ilevel = s.lines() + .rev() + .skip_while(|line| line.is_empty() || is_all_whitespace(line)) + .next() + .map_or(0_usize, indent_level); + + // We want to align the first nonempty, non-all-whitespace line of t to + // have the same indent level as target_ilevel + let level = t.lines() + .skip_while(|line| line.is_empty() || is_all_whitespace(line)) + .next() + .map_or(0_usize, indent_level); + + // When add_spaces=true, we add spaces, otherwise eat. + let add_spaces = target_ilevel > level; + + let delta = if add_spaces { + target_ilevel - level + } else { + level - target_ilevel + }; + + let new_t = t.lines() + .map(|line| { + if is_null(line) { + // leave empty lines alone + String::from(line) + } else if add_spaces { + left_pad_with_spaces(line, delta) + } else { + remove_whitespace_from_left(line, delta) + } + }) + .collect::>().join("\n"); + + format!("{}\n{}", s, new_t) +} + +/// Aligns strings in `xs` pairwise from the start, such that for any pair of +/// strings, the first string's last line is aligned with the first line of +/// the second string. See `align_two_snippets`. Use this to merge code regions +/// into a reasonably aligned chunk of code. +/// +/// For example, consider +/// +/// let s1 = "\ +/// if (condition()) { +/// do_something()"; +/// +/// let s2 = "\ +/// code_from_somewhere_else();" +/// +/// let s3 = "\ +/// another_piece_of_code(); +/// indented_here();"; +/// +/// +/// +/// +/// Now calling `align_snippets(&[s1, s2, s3])` will yield the following: +/// +/// "\ +/// if (condition()) { +/// do_something(); +/// code_from_somewhere_else(); +/// another_piece_of_code(); +/// indented_here();" +pub fn align_snippets(xs: &[&str]) -> String { + if xs.is_empty() { + String::from("") + } else { + let mut ret = xs[0].to_string(); + for x in xs.iter().skip(1_usize) { + ret = align_two_snippets(&ret, x); + } + ret + } +} + + +/// # Examples +/// ``` +/// use clippy_lints::utils::is_all_whitespace; +/// +/// assert_eq!(true, " \n\t "); +/// assert_eq!(false, ""); +/// assert_eq!(false, "hello world!\n"); +/// ``` +pub fn is_all_whitespace(s: &str) -> bool { + s.chars().all(|c| c.is_whitespace()) +} + +/// Returns true if a string is empty or just spaces. +pub fn is_null(s: &str) -> bool { + s.is_empty() || is_all_whitespace(s) +} + +/// Returns the indentation level of a string. It just returns the count of +/// whitespace characters in the string before a non-whitespace character +/// is encountered. +/// +/// # Examples +/// +/// ``` +/// use clippy_lints::utils::indent_level; +/// +/// let s = " fn foo() { "; +/// assert_eq!(4, indent_level(s)); +/// +/// let s = "fn foo() { "; +/// assert_eq!(0, indent_level(s)); +/// ``` +pub fn indent_level(s: &str) -> usize { + s.chars() + .enumerate() + .find(|&(_, c)| !c.is_whitespace()) + .map_or(0_usize, |(i, _)| i) +}