From a7690cb1e5853f8206eea66018ff08fe393c533a Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Fri, 25 Sep 2015 12:53:25 +0200 Subject: [PATCH] Make rewrite_string return `Option` --- src/comment.rs | 94 ++++++++++++++++++++++++++------------------------ src/expr.rs | 42 ++++++++++++++-------- src/lists.rs | 31 +++++++++-------- src/string.rs | 39 ++++++++++++++++----- src/visitor.rs | 11 +++--- 5 files changed, 129 insertions(+), 88 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index 765d61cfa11..1e22a578eb1 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -21,7 +21,7 @@ pub fn rewrite_comment(orig: &str, width: usize, offset: Indent, config: &Config) - -> String { + -> Option { let s = orig.trim(); // Edge case: block comments. Let's not trim their lines (for now). @@ -47,49 +47,53 @@ pub fn rewrite_comment(orig: &str, let indent_str = offset.to_string(config); let line_breaks = s.chars().filter(|&c| c == '\n').count(); - let (_, mut s) = s.lines() - .enumerate() - .map(|(i, mut line)| { - line = line.trim(); - // Drop old closer. - if i == line_breaks && line.ends_with("*/") && !line.starts_with("//") { - line = &line[..(line.len() - 2)]; - } + let lines = s.lines() + .enumerate() + .map(|(i, mut line)| { + line = line.trim(); + // Drop old closer. + if i == line_breaks && line.ends_with("*/") && !line.starts_with("//") { + line = &line[..(line.len() - 2)]; + } - line.trim_right() - }) - .map(left_trim_comment_line) - .map(|line| { - if line_breaks == 0 { - line.trim_left() - } else { - line - } - }) - .fold((true, opener.to_owned()), - |(first, mut acc), line| { - if !first { - acc.push('\n'); - acc.push_str(&indent_str); - acc.push_str(line_start); - } + line.trim_right() + }) + .map(left_trim_comment_line) + .map(|line| { + if line_breaks == 0 { + line.trim_left() + } else { + line + } + }); - if line.len() > max_chars { - acc.push_str(&rewrite_string(line, &fmt)); - } else { - if line.len() == 0 { - acc.pop(); // Remove space if this is an empty comment. - } else { - acc.push_str(line); - } - } + let mut result = opener.to_owned(); + let mut first = true; - (false, acc) - }); + for line in lines { + if !first { + result.push('\n'); + result.push_str(&indent_str); + result.push_str(line_start); + } - s.push_str(closer); + if line.len() > max_chars { + let rewrite = try_opt!(rewrite_string(line, &fmt)); + result.push_str(&rewrite); + } else { + if line.len() == 0 { + result.pop(); // Remove space if this is an empty comment. + } else { + result.push_str(line); + } + } - s + first = false; + } + + result.push_str(closer); + + Some(result) } fn left_trim_comment_line(line: &str) -> &str { @@ -294,33 +298,33 @@ impl Iterator for CharClasses where T: Iterator, T::Item: RichChar { #[cfg(test)] mod test { use super::{CharClasses, CodeCharKind, contains_comment, rewrite_comment, FindUncommented}; - use Indent; + #[test] #[rustfmt_skip] fn format_comments() { let config = Default::default(); assert_eq!("/* test */", rewrite_comment(" //test", true, 100, Indent::new(0, 100), - &config)); + &config).unwrap()); assert_eq!("// comment\n// on a", rewrite_comment("// comment on a", false, 10, - Indent::empty(), &config)); + Indent::empty(), &config).unwrap()); assert_eq!("// A multi line comment\n // between args.", rewrite_comment("// A multi line comment\n // between args.", false, 60, Indent::new(0, 12), - &config)); + &config).unwrap()); let input = "// comment"; let expected = "/* com\n \ * men\n \ * t */"; - assert_eq!(expected, rewrite_comment(input, true, 9, Indent::new(0, 69), &config)); + assert_eq!(expected, rewrite_comment(input, true, 9, Indent::new(0, 69), &config).unwrap()); assert_eq!("/* trimmed */", rewrite_comment("/* trimmed */", true, 100, - Indent::new(0, 100), &config)); + Indent::new(0, 100), &config).unwrap()); } // This is probably intended to be a non-test fn, but it is not used. I'm diff --git a/src/expr.rs b/src/expr.rs index a2be6ee8fd1..2e87bfbc19a 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -374,7 +374,11 @@ impl Rewrite for ast::Block { // 9 = "unsafe {".len(), 7 = "unsafe ".len() let budget = try_opt!(width.checked_sub(9)); format!("unsafe {} ", - rewrite_comment(trimmed, true, budget, offset + 7, context.config)) + try_opt!(rewrite_comment(trimmed, + true, + budget, + offset + 7, + context.config))) } else { "unsafe ".to_owned() }; @@ -658,7 +662,7 @@ fn rewrite_match_arm_comment(context: &RewriteContext, width: usize, arm_indent: Indent, arm_indent_str: &str) - -> String { + -> Option { // The leading "," is not part of the arm-comment let missed_str = match missed_str.find_uncommented(",") { Some(n) => &missed_str[n+1..], @@ -684,11 +688,17 @@ fn rewrite_match_arm_comment(context: &RewriteContext, } let missed_str = missed_str[first..].trim(); if !missed_str.is_empty() { + let comment = try_opt!(rewrite_comment(&missed_str, + false, + width, + arm_indent, + context.config)); result.push('\n'); result.push_str(arm_indent_str); - result.push_str(&rewrite_comment(&missed_str, false, width, arm_indent, context.config)); + result.push_str(&comment); } - return result; + + Some(result) } fn rewrite_match(context: &RewriteContext, @@ -722,11 +732,12 @@ fn rewrite_match(context: &RewriteContext, } else { context.snippet(mk_sp(arm_end_pos(&arms[i-1]), arm_start_pos(arm))) }; - result.push_str(&rewrite_match_arm_comment(context, - &missed_str, - width, - arm_indent, - &arm_indent_str)); + let comment = try_opt!(rewrite_match_arm_comment(context, + &missed_str, + width, + arm_indent, + &arm_indent_str)); + result.push_str(&comment); result.push('\n'); result.push_str(&arm_indent_str); @@ -742,11 +753,12 @@ fn rewrite_match(context: &RewriteContext, } } let last_comment = context.snippet(mk_sp(arm_end_pos(&arms[arms.len() - 1]), span.hi)); - result.push_str(&rewrite_match_arm_comment(context, - &last_comment, - width, - arm_indent, - &arm_indent_str)); + let comment = try_opt!(rewrite_match_arm_comment(context, + &last_comment, + width, + arm_indent, + &arm_indent_str)); + result.push_str(&comment); result.push('\n'); result.push_str(&(context.block_indent + context.overflow_indent).to_string(context.config)); result.push('}'); @@ -1004,7 +1016,7 @@ fn rewrite_string_lit(context: &RewriteContext, let string_lit = context.snippet(span); let str_lit = &string_lit[1..string_lit.len() - 1]; // Remove the quote characters. - Some(rewrite_string(str_lit, &fmt)) + rewrite_string(str_lit, &fmt) } pub fn rewrite_call(context: &RewriteContext, diff --git a/src/lists.rs b/src/lists.rs index 60e5c7e5b56..aa82ec47964 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -200,11 +200,12 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op let block_mode = tactic != ListTactic::Vertical; // Width restriction is only relevant in vertical mode. let max_width = formatting.v_width; - result.push_str(&rewrite_comment(comment, - block_mode, - max_width, - formatting.indent, - formatting.config)); + let comment = try_opt!(rewrite_comment(comment, + block_mode, + max_width, + formatting.indent, + formatting.config)); + result.push_str(&comment); if tactic == ListTactic::Vertical { result.push('\n'); @@ -221,11 +222,11 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op // Post-comments if tactic != ListTactic::Vertical && item.post_comment.is_some() { let comment = item.post_comment.as_ref().unwrap(); - let formatted_comment = rewrite_comment(comment, - true, - formatting.v_width, - Indent::empty(), - formatting.config); + let formatted_comment = try_opt!(rewrite_comment(comment, + true, + formatting.v_width, + Indent::empty(), + formatting.config)); result.push(' '); result.push_str(&formatted_comment); @@ -246,11 +247,11 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op comment.trim().contains('\n') || comment.trim().len() > width; - let formatted_comment = rewrite_comment(comment, - block_style, - width, - offset, - formatting.config); + let formatted_comment = try_opt!(rewrite_comment(comment, + block_style, + width, + offset, + formatting.config)); result.push(' '); result.push_str(&formatted_comment); diff --git a/src/string.rs b/src/string.rs index 4e40b8e9ede..ffaf52d3380 100644 --- a/src/string.rs +++ b/src/string.rs @@ -31,23 +31,23 @@ pub struct StringFormat<'a> { } // TODO: simplify this! -pub fn rewrite_string<'a>(s: &str, fmt: &StringFormat<'a>) -> String { +pub fn rewrite_string<'a>(s: &str, fmt: &StringFormat<'a>) -> Option { // TODO if lo.col > IDEAL - 10, start a new line (need cur indent for that) // Strip line breaks. let re = Regex::new(r"(\\[:space:]+)").unwrap(); let stripped_str = re.replace_all(s, ""); let graphemes = UnicodeSegmentation::graphemes(&*stripped_str, false).collect::>(); - let indent = fmt.offset.to_string(fmt.config); - let indent = &indent; let mut cur_start = 0; let mut result = String::with_capacity(round_up_to_power_of_two(s.len())); result.push_str(fmt.opener); let ender_length = fmt.line_end.len(); - let max_chars = fmt.width.checked_sub(fmt.opener.len() + ender_length).unwrap_or(1); + // If we cannot put at least a single character per line, the rewrite won't + // succeed. + let max_chars = try_opt!(fmt.width.checked_sub(fmt.opener.len() + ender_length + 1)) + 1; loop { let mut cur_end = cur_start + max_chars; @@ -57,8 +57,9 @@ pub fn rewrite_string<'a>(s: &str, fmt: &StringFormat<'a>) -> String { result.push_str(line); break; } + // Push cur_end left until we reach whitespace. - while !(graphemes[cur_end - 1].trim().len() == 0) { + while !graphemes[cur_end - 1].trim().is_empty() { cur_end -= 1; if cur_end - cur_start < MIN_STRING { // We can't break at whitespace, fall back to splitting @@ -71,7 +72,7 @@ pub fn rewrite_string<'a>(s: &str, fmt: &StringFormat<'a>) -> String { } } // Make sure there is no whitespace to the right of the break. - while cur_end < s.len() && graphemes[cur_end].trim().len() == 0 { + while cur_end < s.len() && graphemes[cur_end].trim().is_empty() { cur_end += 1; } let raw_line = graphemes[cur_start..cur_end].join(""); @@ -85,12 +86,34 @@ pub fn rewrite_string<'a>(s: &str, fmt: &StringFormat<'a>) -> String { result.push_str(line); result.push_str(fmt.line_end); result.push('\n'); - result.push_str(indent); + result.push_str(&indent); result.push_str(fmt.line_start); cur_start = cur_end; } result.push_str(fmt.closer); - result + Some(result) +} + +#[cfg(test)] +mod test { + use super::{StringFormat, rewrite_string}; + + #[test] + fn issue343() { + let config = Default::default(); + let fmt = StringFormat { + opener: "\"", + closer: "\"", + line_start: " ", + line_end: "\\", + width: 2, + offset: ::Indent::empty(), + trim_end: false, + config: &config, + }; + + rewrite_string("eq_", &fmt); + } } diff --git a/src/visitor.rs b/src/visitor.rs index 8c46d69ebfc..1a070a839f5 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -407,11 +407,12 @@ impl<'a> Rewrite for [ast::Attribute] { let multi_line = a_str.starts_with("//") && comment.matches('\n').count() > 1; let comment = comment.trim(); if !comment.is_empty() { - let comment = rewrite_comment(comment, - false, - context.config.max_width - offset.width(), - offset, - context.config); + let comment = try_opt!(rewrite_comment(comment, + false, + context.config.max_width - + offset.width(), + offset, + context.config)); result.push_str(&indent); result.push_str(&comment); result.push('\n');