From b0eb0f5daa9582387c4935d86e6077b7eb89a34b Mon Sep 17 00:00:00 2001 From: DarkDrek Date: Sun, 10 Jan 2016 05:20:35 +0100 Subject: [PATCH 1/3] Keep comments between if and else blocks. Fixes #447 --- src/expr.rs | 25 ++++++++++++++++++++++++- tests/target/issue-447.rs | 7 +++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 tests/target/issue-447.rs diff --git a/src/expr.rs b/src/expr.rs index 269e45f9e9f..096a934a788 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -664,7 +664,30 @@ fn rewrite_if_else(context: &RewriteContext, _ => else_block.rewrite(context, width, offset), }; - result.push_str(" else "); + let snippet = context.codemap + .span_to_snippet(mk_sp(if_block.span.hi, else_block.span.lo)) + .unwrap(); + // Preserve comments that are between the if and else block + if contains_comment(&snippet) { + let close_pos = try_opt!(snippet.find_uncommented("else")); + let trimmed = &snippet[..close_pos].trim(); + let comment_str = format!("{}{}", + offset.to_string(context.config), + try_opt!(rewrite_comment(trimmed, + false, + width, + offset, + context.config)), + ); + let else_str = format!("{}else ", offset.to_string(context.config)); + + result.push('\n'); + result.push_str(&comment_str); + result.push('\n'); + result.push_str(&else_str); + } else { + result.push_str(" else "); + } result.push_str(&&try_opt!(rewrite)); } diff --git a/tests/target/issue-447.rs b/tests/target/issue-447.rs new file mode 100644 index 00000000000..56e6c333aac --- /dev/null +++ b/tests/target/issue-447.rs @@ -0,0 +1,7 @@ +fn main() { + if cond { + } + // This shouldn't be dropped + else { + } +} From 4da91e7df24e833a5800631319f300a304d2f981 Mon Sep 17 00:00:00 2001 From: DarkDrek Date: Tue, 12 Jan 2016 01:09:08 +0100 Subject: [PATCH 2/3] Handle more possible comment position for if else Extended the test with the new possiblecomment positions --- src/expr.rs | 89 +++++++++++++++++++++++++++------------ src/utils.rs | 8 ++++ tests/source/issue-447.rs | 37 ++++++++++++++++ tests/target/issue-447.rs | 38 +++++++++++++++-- 4 files changed, 143 insertions(+), 29 deletions(-) create mode 100644 tests/source/issue-447.rs diff --git a/src/expr.rs b/src/expr.rs index 096a934a788..54a6fb20fa8 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -13,14 +13,15 @@ use std::borrow::Borrow; use std::mem::swap; use std::ops::Deref; use std::iter::ExactSizeIterator; +use std::fmt::Write; use {Indent, Spanned}; use rewrite::{Rewrite, RewriteContext}; use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic, DefinitiveListTactic, definitive_tactic, ListItem, format_fn_args}; use string::{StringFormat, rewrite_string}; -use utils::{span_after, extra_offset, last_line_width, wrap_str, binary_search, first_line_width, - semicolon_for_stmt}; +use utils::{span_after, span_before, extra_offset, last_line_width, wrap_str, binary_search, + first_line_width, semicolon_for_stmt}; use visitor::FmtVisitor; use config::{Config, StructLitStyle, MultilineStyle}; use comment::{FindUncommented, rewrite_comment, contains_comment, recover_comment_removed}; @@ -101,6 +102,7 @@ impl Rewrite for ast::Expr { cond, if_block, else_block.as_ref().map(|e| &**e), + self.span, None, width, offset, @@ -111,6 +113,7 @@ impl Rewrite for ast::Expr { cond, if_block, else_block.as_ref().map(|e| &**e), + self.span, Some(pat), width, offset, @@ -605,12 +608,33 @@ fn rewrite_label(label: Option) -> String { } } +fn extract_comment(span: Span, + context: &RewriteContext, + offset: Indent, + width: usize) + -> Option { + let comment_str = context.snippet(span); + if contains_comment(&comment_str) { + let comment = try_opt!(rewrite_comment(comment_str.trim(), + false, + width, + offset, + context.config)); + Some(format!("\n{indent}{}\n{indent}", + comment, + indent = offset.to_string(context.config))) + } else { + None + } +} + // Rewrites if-else blocks. If let Some(_) = pat, the expression is // treated as an if-let-else expression. fn rewrite_if_else(context: &RewriteContext, cond: &ast::Expr, if_block: &ast::Block, else_block_opt: Option<&ast::Expr>, + span: Span, pat: Option<&ast::Pat>, width: usize, offset: Indent, @@ -635,7 +659,22 @@ fn rewrite_if_else(context: &RewriteContext, } let if_block_string = try_opt!(if_block.rewrite(context, width, offset)); - let mut result = format!("if {} {}", pat_expr_string, if_block_string); + + let between_if_cond = mk_sp(span_after(span, "if", context.codemap), + pat.map_or(cond.span.lo, + |_| span_before(span, "let", context.codemap))); + let between_if_cond_comment = extract_comment(between_if_cond, &context, offset, width); + + let after_cond_comment = extract_comment(mk_sp(cond.span.hi, if_block.span.lo), + context, + offset, + width); + + let mut result = format!("if{}{}{}{}", + between_if_cond_comment.as_ref().map_or(" ", |str| &**str), + pat_expr_string, + after_cond_comment.as_ref().map_or(" ", |str| &**str), + if_block_string); if let Some(else_block) = else_block_opt { let rewrite = match else_block.node { @@ -646,6 +685,7 @@ fn rewrite_if_else(context: &RewriteContext, cond, if_block, else_block.as_ref().map(|e| &**e), + mk_sp(span_after(span, "else", context.codemap), span.hi), Some(pat), width, offset, @@ -656,6 +696,7 @@ fn rewrite_if_else(context: &RewriteContext, cond, if_block, else_block.as_ref().map(|e| &**e), + mk_sp(span_after(span, "else", context.codemap), span.hi), None, width, offset, @@ -664,30 +705,26 @@ fn rewrite_if_else(context: &RewriteContext, _ => else_block.rewrite(context, width, offset), }; - let snippet = context.codemap - .span_to_snippet(mk_sp(if_block.span.hi, else_block.span.lo)) - .unwrap(); - // Preserve comments that are between the if and else block - if contains_comment(&snippet) { - let close_pos = try_opt!(snippet.find_uncommented("else")); - let trimmed = &snippet[..close_pos].trim(); - let comment_str = format!("{}{}", - offset.to_string(context.config), - try_opt!(rewrite_comment(trimmed, - false, - width, - offset, - context.config)), - ); - let else_str = format!("{}else ", offset.to_string(context.config)); + let between_if_else_block = mk_sp(if_block.span.hi, + span_before(mk_sp(if_block.span.hi, else_block.span.lo), + "else", + context.codemap)); + let between_if_else_block_comment = extract_comment(between_if_else_block, + &context, + offset, + width); - result.push('\n'); - result.push_str(&comment_str); - result.push('\n'); - result.push_str(&else_str); - } else { - result.push_str(" else "); - } + let after_else = mk_sp(span_after(mk_sp(if_block.span.hi, else_block.span.lo), + "else", + context.codemap), + else_block.span.lo); + let after_else_comment = extract_comment(after_else, &context, offset, width); + + try_opt!(write!(&mut result, + "{}else{}", + between_if_else_block_comment.as_ref().map_or(" ", |str| &**str), + after_else_comment.as_ref().map_or(" ", |str| &**str)) + .ok()); result.push_str(&&try_opt!(rewrite)); } diff --git a/src/utils.rs b/src/utils.rs index 7014605ec81..3b03ede2be3 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -38,6 +38,14 @@ pub fn span_after(original: Span, needle: &str, codemap: &CodeMap) -> BytePos { original.lo + BytePos(offset as u32) } +#[inline] +pub fn span_before(original: Span, needle: &str, codemap: &CodeMap) -> BytePos { + let snippet = codemap.span_to_snippet(original).unwrap(); + let offset = snippet.find_uncommented(needle).unwrap(); + + original.lo + BytePos(offset as u32) +} + #[inline] pub fn span_after_last(original: Span, needle: &str, codemap: &CodeMap) -> BytePos { let snippet = codemap.span_to_snippet(original).unwrap(); diff --git a/tests/source/issue-447.rs b/tests/source/issue-447.rs new file mode 100644 index 00000000000..cc713af3f18 --- /dev/null +++ b/tests/source/issue-447.rs @@ -0,0 +1,37 @@ +fn main() { + if /* shouldn't be dropped + shouldn't be dropped */ + + cond /* shouldn't be dropped + shouldn't be dropped */ + + { + } /* shouldn't be dropped + shouldn't be dropped */ + + else /* shouldn't be dropped + shouldn't be dropped */ + + if /* shouldn't be dropped + shouldn't be dropped */ + + cond /* shouldn't be dropped + shouldn't be dropped */ + + { + } /* shouldn't be dropped + shouldn't be dropped */ + + else /* shouldn't be dropped + shouldn't be dropped */ + + { + } + + if /* shouldn't be dropped + shouldn't be dropped */ + let Some(x) = y/* shouldn't be dropped + shouldn't be dropped */ + { + } +} diff --git a/tests/target/issue-447.rs b/tests/target/issue-447.rs index 56e6c333aac..7e69c708eb7 100644 --- a/tests/target/issue-447.rs +++ b/tests/target/issue-447.rs @@ -1,7 +1,39 @@ fn main() { - if cond { + if + // shouldn't be dropped + // shouldn't be dropped + cond + // shouldn't be dropped + // shouldn't be dropped + { } - // This shouldn't be dropped - else { + // shouldn't be dropped + // shouldn't be dropped + else + // shouldn't be dropped + // shouldn't be dropped + if + // shouldn't be dropped + // shouldn't be dropped + cond + // shouldn't be dropped + // shouldn't be dropped + { + } + // shouldn't be dropped + // shouldn't be dropped + else + // shouldn't be dropped + // shouldn't be dropped + { + } + + if + // shouldn't be dropped + // shouldn't be dropped + let Some(x) = y + // shouldn't be dropped + // shouldn't be dropped + { } } From 20ccc7bf8e4c3b10e1c05631dc706d8402b2ebdb Mon Sep 17 00:00:00 2001 From: DarkDrek Date: Tue, 12 Jan 2016 01:32:29 +0100 Subject: [PATCH 3/3] Removed the failing test part since it will work when #754 is accepted --- tests/target/comment-not-disappear.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/target/comment-not-disappear.rs b/tests/target/comment-not-disappear.rs index 23c00c418be..646b37e46e2 100644 --- a/tests/target/comment-not-disappear.rs +++ b/tests/target/comment-not-disappear.rs @@ -28,12 +28,6 @@ fn foo() -> Vec { .collect() } -fn d() { - if true /* and ... */ { - a(); - } -} - fn calc_page_len(prefix_len: usize, sofar: usize) -> usize { 2 // page type and flags + 1 // stored depth