Merge pull request #445 from marcusklaas/comments

Format more comments
This commit is contained in:
Marcus Klaas de Vries 2015-10-19 23:09:04 +02:00
commit 526ccff690
14 changed files with 371 additions and 76 deletions

4
Cargo.lock generated
View file

@ -7,7 +7,7 @@ dependencies = [
"rustc-serialize 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)",
"strings 0.0.1 (git+https://github.com/nrc/strings.rs.git)",
"term 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)",
"toml 0.1.22 (registry+https://github.com/rust-lang/crates.io-index)",
"toml 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)",
"unicode-segmentation 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
]
@ -93,7 +93,7 @@ dependencies = [
[[package]]
name = "toml"
version = "0.1.22"
version = "0.1.23"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"rustc-serialize 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)",

View file

@ -81,11 +81,11 @@ pub fn rewrite_comment(orig: &str,
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);
if line.len() == 0 || line.starts_with('!') {
// Remove space if this is an empty comment or a doc comment.
result.pop();
}
result.push_str(line);
}
first = false;
@ -195,17 +195,17 @@ enum CharClassesStatus {
LitCharEscape,
// The u32 is the nesting deepness of the comment
BlockComment(u32),
// Status when the '/' has been consumed, but not yet the '*', deepness is the new deepness
// (after the comment opening).
// Status when the '/' has been consumed, but not yet the '*', deepness is
// the new deepness (after the comment opening).
BlockCommentOpening(u32),
// Status when the '*' has been consumed, but not yet the '/', deepness is the new deepness
// (after the comment closing).
// Status when the '*' has been consumed, but not yet the '/', deepness is
// the new deepness (after the comment closing).
BlockCommentClosing(u32),
LineComment,
}
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
enum CodeCharKind {
pub enum CodeCharKind {
Normal,
Comment,
}
@ -298,11 +298,137 @@ impl<T> Iterator for CharClasses<T> where T: Iterator, T::Item: RichChar {
}
}
/// Iterator over an alternating sequence of functional and commented parts of
/// a string. The first item is always a, possibly zero length, subslice of
/// functional text. Line style comments contain their ending newlines.
pub struct CommentCodeSlices<'a> {
slice: &'a str,
last_slice_kind: CodeCharKind,
last_slice_end: usize,
}
impl<'a> CommentCodeSlices<'a> {
pub fn new(slice: &'a str) -> CommentCodeSlices<'a> {
CommentCodeSlices {
slice: slice,
last_slice_kind: CodeCharKind::Comment,
last_slice_end: 0,
}
}
}
impl<'a> Iterator for CommentCodeSlices<'a> {
type Item = (CodeCharKind, usize, &'a str);
fn next(&mut self) -> Option<Self::Item> {
if self.last_slice_end == self.slice.len() {
return None;
}
let mut sub_slice_end = self.last_slice_end;
let mut first_whitespace = None;
let subslice = &self.slice[self.last_slice_end..];
let mut iter = CharClasses::new(subslice.char_indices());
for (kind, (i, c)) in &mut iter {
let is_comment_connector = self.last_slice_kind == CodeCharKind::Normal &&
&subslice[..2] == "//" &&
[' ', '\t'].contains(&c);
if is_comment_connector && first_whitespace.is_none() {
first_whitespace = Some(i);
}
if kind == self.last_slice_kind && !is_comment_connector {
let last_index = match first_whitespace {
Some(j) => j,
None => i,
};
sub_slice_end = self.last_slice_end + last_index;
break;
}
if !is_comment_connector {
first_whitespace = None;
}
}
if let (None, true) = (iter.next(), sub_slice_end == self.last_slice_end) {
// This was the last subslice.
sub_slice_end = match first_whitespace {
Some(i) => self.last_slice_end + i,
None => self.slice.len(),
};
}
let kind = match self.last_slice_kind {
CodeCharKind::Comment => CodeCharKind::Normal,
CodeCharKind::Normal => CodeCharKind::Comment,
};
let res = (kind,
self.last_slice_end,
&self.slice[self.last_slice_end..sub_slice_end]);
self.last_slice_end = sub_slice_end;
self.last_slice_kind = kind;
Some(res)
}
}
#[cfg(test)]
mod test {
use super::{CharClasses, CodeCharKind, contains_comment, rewrite_comment, FindUncommented};
use super::{CharClasses, CodeCharKind, contains_comment, rewrite_comment, FindUncommented,
CommentCodeSlices};
use Indent;
#[test]
fn char_classes() {
let mut iter = CharClasses::new("//\n\n".chars());
assert_eq!((CodeCharKind::Comment, '/'), iter.next().unwrap());
assert_eq!((CodeCharKind::Comment, '/'), iter.next().unwrap());
assert_eq!((CodeCharKind::Comment, '\n'), iter.next().unwrap());
assert_eq!((CodeCharKind::Normal, '\n'), iter.next().unwrap());
assert_eq!(None, iter.next());
}
#[test]
fn comment_code_slices() {
let input = "code(); /* test */ 1 + 1";
let mut iter = CommentCodeSlices::new(input);
assert_eq!((CodeCharKind::Normal, 0, "code(); "), iter.next().unwrap());
assert_eq!((CodeCharKind::Comment, 8, "/* test */"),
iter.next().unwrap());
assert_eq!((CodeCharKind::Normal, 18, " 1 + 1"), iter.next().unwrap());
assert_eq!(None, iter.next());
}
#[test]
fn comment_code_slices_two() {
let input = "// comment\n test();";
let mut iter = CommentCodeSlices::new(input);
assert_eq!((CodeCharKind::Normal, 0, ""), iter.next().unwrap());
assert_eq!((CodeCharKind::Comment, 0, "// comment\n"),
iter.next().unwrap());
assert_eq!((CodeCharKind::Normal, 11, " test();"),
iter.next().unwrap());
assert_eq!(None, iter.next());
}
#[test]
fn comment_code_slices_three() {
let input = "1 // comment\n // comment2\n\n";
let mut iter = CommentCodeSlices::new(input);
assert_eq!((CodeCharKind::Normal, 0, "1 "), iter.next().unwrap());
assert_eq!((CodeCharKind::Comment, 2, "// comment\n // comment2\n"),
iter.next().unwrap());
assert_eq!((CodeCharKind::Normal, 29, "\n"), iter.next().unwrap());
assert_eq!(None, iter.next());
}
#[test]
#[rustfmt_skip]
fn format_comments() {
@ -362,7 +488,6 @@ mod test {
#[test]
fn test_find_uncommented() {
fn check(haystack: &str, needle: &str, expected: Option<usize>) {
println!("haystack {:?}, needle: {:?}", haystack, needle);
assert_eq!(expected, haystack.find_uncommented(needle));
}

View file

@ -423,7 +423,7 @@ impl Rewrite for ast::Block {
}
let mut visitor = FmtVisitor::from_codemap(context.codemap, context.config);
visitor.block_indent = context.block_indent + context.overflow_indent;
visitor.block_indent = context.block_indent;
let prefix = match self.rules {
ast::BlockCheckMode::UnsafeBlock(..) => {
@ -471,10 +471,6 @@ impl Rewrite for ast::Block {
visitor.visit_block(self);
// Push text between last block item and end of block
let snippet = visitor.snippet(mk_sp(visitor.last_pos, self.span.hi));
visitor.buffer.push_str(&snippet);
Some(format!("{}{}", prefix, visitor.buffer))
}
}
@ -751,7 +747,7 @@ fn rewrite_match(context: &RewriteContext,
let mut result = format!("match {} {{", cond_str);
let nested_context = context.nested_context();
let arm_indent = nested_context.block_indent + context.overflow_indent;
let arm_indent = nested_context.block_indent;
let arm_indent_str = arm_indent.to_string(context.config);
let open_brace_pos = span_after(mk_sp(cond.span.hi, arm_start_pos(&arms[0])),
@ -795,7 +791,7 @@ fn rewrite_match(context: &RewriteContext,
&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_str(&context.block_indent.to_string(context.config));
result.push('}');
Some(result)
}
@ -1534,12 +1530,11 @@ pub fn rewrite_assign_rhs<S: Into<String>>(context: &RewriteContext,
let new_offset = offset.block_indent(context.config);
result.push_str(&format!("\n{}", new_offset.to_string(context.config)));
// FIXME: we probably should related max_width to width instead of config.max_width
// where is the 1 coming from anyway?
// FIXME: we probably should related max_width to width instead of
// config.max_width where is the 1 coming from anyway?
let max_width = try_opt!(context.config.max_width.checked_sub(new_offset.width() + 1));
let rhs_indent = Indent::new(context.config.tab_spaces, 0);
let overflow_context = context.overflow_context(rhs_indent);
let rhs = ex.rewrite(&overflow_context, max_width, new_offset);
let inner_context = context.nested_context();
let rhs = ex.rewrite(&inner_context, max_width, new_offset);
result.push_str(&&try_opt!(rhs));
}

View file

@ -287,8 +287,8 @@ impl<'a> FmtVisitor<'a> {
has_body: bool)
-> Option<(String, bool)> {
let mut force_new_line_for_brace = false;
// FIXME we'll lose any comments in between parts of the function decl, but anyone
// who comments there probably deserves what they get.
// FIXME we'll lose any comments in between parts of the function decl, but
// anyone who comments there probably deserves what they get.
let where_clause = &generics.where_clause;
@ -1008,7 +1008,8 @@ impl<'a> FmtVisitor<'a> {
span_start,
span_end);
let item_vec = items.collect::<Vec<_>>();
// FIXME: we don't need to collect here if the where_layout isnt horizontalVertical
// FIXME: we don't need to collect here if the where_layout isnt
// HorizontalVertical.
let tactic = definitive_tactic(&item_vec, self.config.where_layout, budget);
let fmt = ListFormatting {

View file

@ -10,7 +10,8 @@
use visitor::FmtVisitor;
use syntax::codemap::{self, BytePos};
use syntax::codemap::{self, BytePos, Span, Pos};
use comment::{CodeCharKind, CommentCodeSlices, rewrite_comment};
impl<'a> FmtVisitor<'a> {
// TODO these format_missing methods are ugly. Refactor and add unit tests
@ -37,16 +38,12 @@ impl<'a> FmtVisitor<'a> {
end: BytePos,
process_last_snippet: F) {
let start = self.last_pos;
debug!("format_missing_inner: {:?} to {:?}",
self.codemap.lookup_char_pos(start),
self.codemap.lookup_char_pos(end));
if start == end {
// Do nothing if this is the beginning of the file.
if start == self.codemap.lookup_char_pos(start).file.start_pos {
return;
if start != self.codemap.lookup_char_pos(start).file.start_pos {
process_last_snippet(self, "", "");
}
process_last_snippet(self, "", "");
return;
}
@ -57,24 +54,117 @@ impl<'a> FmtVisitor<'a> {
self.last_pos = end;
let span = codemap::mk_sp(start, end);
self.write_snippet(span, &process_last_snippet);
}
fn write_snippet<F>(&mut self, span: Span, process_last_snippet: F)
where F: Fn(&mut FmtVisitor, &str, &str)
{
// Get a snippet from the file start to the span's hi without allocating.
// We need it to determine what precedes the current comment. If the comment
// follows code on the same line, we won't touch it.
let big_span_lo = self.codemap.lookup_char_pos(span.lo).file.start_pos;
let local_begin = self.codemap.lookup_byte_offset(big_span_lo);
let local_end = self.codemap.lookup_byte_offset(span.hi);
let start_index = local_begin.pos.to_usize();
let end_index = local_end.pos.to_usize();
let big_snippet = &local_begin.fm.src.as_ref().unwrap()[start_index..end_index];
let big_diff = (span.lo - big_span_lo).to_usize();
let snippet = self.snippet(span);
self.write_snippet(&snippet, &process_last_snippet);
self.write_snippet_inner(big_snippet, big_diff, &snippet, process_last_snippet);
}
fn write_snippet<F: Fn(&mut FmtVisitor, &str, &str)>(&mut self,
snippet: &str,
process_last_snippet: F) {
let mut lines: Vec<&str> = snippet.lines().collect();
let last_snippet = if snippet.ends_with("\n") {
""
} else {
lines.pop().unwrap()
};
for line in lines.iter() {
self.buffer.push_str(line.trim_right());
self.buffer.push_str("\n");
fn write_snippet_inner<F>(&mut self,
big_snippet: &str,
big_diff: usize,
snippet: &str,
process_last_snippet: F)
where F: Fn(&mut FmtVisitor, &str, &str)
{
// Trim whitespace from the right hand side of each line.
// Annoyingly, the library functions for splitting by lines etc. are not
// quite right, so we must do it ourselves.
let mut line_start = 0;
let mut last_wspace = None;
let mut rewrite_next_comment = true;
for (kind, offset, subslice) in CommentCodeSlices::new(snippet) {
if let CodeCharKind::Comment = kind {
let last_char = big_snippet[..(offset + big_diff)]
.chars()
.rev()
.skip_while(|rev_c| [' ', '\t'].contains(&rev_c))
.next();
let fix_indent = last_char.map(|rev_c| ['{', '\n'].contains(&rev_c))
.unwrap_or(true);
if rewrite_next_comment && fix_indent {
if let Some('{') = last_char {
self.buffer.push_str("\n");
}
let comment_width = ::std::cmp::min(self.config.ideal_width,
self.config.max_width -
self.block_indent.width());
self.buffer.push_str(&self.block_indent.to_string(self.config));
self.buffer.push_str(&rewrite_comment(subslice,
false,
comment_width,
self.block_indent,
self.config)
.unwrap());
last_wspace = None;
line_start = offset + subslice.len();
if let Some('/') = subslice.chars().skip(1).next() {
self.buffer.push_str("\n");
} else if line_start < snippet.len() {
let x = (&snippet[line_start..]).chars().next().unwrap() != '\n';
if x {
self.buffer.push_str("\n");
}
}
continue;
} else {
rewrite_next_comment = false;
}
}
for (mut i, c) in subslice.char_indices() {
i += offset;
if c == '\n' {
if let Some(lw) = last_wspace {
self.buffer.push_str(&snippet[line_start..lw]);
self.buffer.push_str("\n");
} else {
self.buffer.push_str(&snippet[line_start..i + 1]);
}
line_start = i + 1;
last_wspace = None;
rewrite_next_comment = rewrite_next_comment || kind == CodeCharKind::Normal;
} else {
if c.is_whitespace() {
if last_wspace.is_none() {
last_wspace = Some(i);
}
} else {
rewrite_next_comment = rewrite_next_comment || kind == CodeCharKind::Normal;
last_wspace = None;
}
}
}
}
process_last_snippet(self, &last_snippet, snippet);
process_last_snippet(self, &snippet[line_start..], &snippet);
}
}

View file

@ -29,14 +29,8 @@ pub trait Rewrite {
pub struct RewriteContext<'a> {
pub codemap: &'a CodeMap,
pub config: &'a Config,
// Indentation due to nesting of blocks.
pub block_indent: Indent,
// *Extra* indentation due to overflowing to the next line, e.g.,
// let foo =
// bar();
// The extra 4 spaces when formatting `bar()` is overflow_indent.
pub overflow_indent: Indent,
}
impl<'a> RewriteContext<'a> {
@ -45,16 +39,6 @@ impl<'a> RewriteContext<'a> {
codemap: self.codemap,
config: self.config,
block_indent: self.block_indent.block_indent(self.config),
overflow_indent: self.overflow_indent,
}
}
pub fn overflow_context(&self, overflow: Indent) -> RewriteContext<'a> {
RewriteContext {
codemap: self.codemap,
config: self.config,
block_indent: self.block_indent,
overflow_indent: overflow,
}
}

View file

@ -101,11 +101,22 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
}
}
self.block_indent = self.block_indent.block_unindent(self.config);
// TODO: we should compress any newlines here to just one
self.format_missing_with_indent(b.span.hi - brace_compensation);
// FIXME: this is a terrible hack to indent the comments between the last
// item in the block and the closing brace to the block's level.
// The closing brace itself, however, should be indented at a shallower
// level.
let total_len = self.buffer.len;
let chars_too_many = if self.config.hard_tabs {
1
} else {
self.config.tab_spaces
};
self.buffer.truncate(total_len - chars_too_many);
self.buffer.push_str("}");
self.last_pos = b.span.hi;
self.block_indent = self.block_indent.block_unindent(self.config);
}
// Note that this only gets called for function definitions. Required methods
@ -177,6 +188,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
// FIXME(#78): format traits and impl definitions.
ast::Item_::ItemImpl(..) |
ast::Item_::ItemTrait(..) => {
self.format_missing_with_indent(item.span.lo);
self.block_indent = self.block_indent.block_indent(self.config);
visit::walk_item(self, item);
self.block_indent = self.block_indent.block_unindent(self.config);
@ -215,6 +227,9 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
}
ast::Item_::ItemMac(..) => {
self.format_missing_with_indent(item.span.lo);
let snippet = self.snippet(item.span);
self.buffer.push_str(&snippet);
self.last_pos = item.span.hi;
// FIXME: we cannot format these yet, because of a bad span.
// See rust lang issue #28424.
// visit::walk_item(self, item);
@ -387,7 +402,6 @@ impl<'a> FmtVisitor<'a> {
codemap: self.codemap,
config: self.config,
block_indent: self.block_indent,
overflow_indent: Indent::empty(),
};
// 1 = ";"
match vp.rewrite(&context, self.config.max_width - offset.width() - 1, offset) {
@ -419,7 +433,6 @@ impl<'a> FmtVisitor<'a> {
codemap: self.codemap,
config: self.config,
block_indent: self.block_indent,
overflow_indent: Indent::empty(),
}
}
}

39
tests/source/comment.rs Normal file
View file

@ -0,0 +1,39 @@
//! Doc comment
fn test() {
// comment
// comment2
code(); /* leave this comment alone!
* ok? */
/* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec a
* diam lectus. Sed sit amet ipsum mauris. Maecenas congue ligula ac quam
* viverra nec consectetur ante hendrerit. Donec et mollis dolor.
* Praesent et diam eget libero egestas mattis sit amet vitae augue. Nam
* tincidunt congue enim, ut porta lorem lacinia consectetur. Donec ut
* libero sed arcu vehicula ultricies a non tortor. Lorem ipsum dolor sit
* amet, consectetur adipiscing elit. Aenean ut gravida lorem. Ut turpis
* felis, pulvinar a semper sed, adipiscing id dolor. */
// Very looooooooooooooooooooooooooooooooooooooooooooooooooooooooong comment that should be split
// println!("{:?}", rewrite_comment(subslice,
// false,
// comment_width,
// self.block_indent,
// self.config)
// .unwrap());
funk(); //dontchangeme
// or me
}
/// test123
fn doc_comment() {
}
fn chains() {
foo.bar(|| {
let x = 10;
/* comment */ x })
}

View file

@ -19,7 +19,7 @@ fn main() {
};
let loooooooooooooong_name = |field| {
// TODO(#27): format comments.
// TODO(#27): format comments.
if field.node.attrs.len() > 0 {
field.node.attrs[0].span.lo
} else {
@ -39,7 +39,8 @@ fn main() {
let empty = |arg| {};
let simple = |arg| { /* TODO(#27): comment formatting */
let simple = |arg| {
// TODO(#27): comment formatting
foo(arg)
};

42
tests/target/comment.rs Normal file
View file

@ -0,0 +1,42 @@
//! Doc comment
fn test() {
// comment
// comment2
code(); /* leave this comment alone!
* ok? */
// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec a
// diam lectus. Sed sit amet ipsum mauris. Maecenas congue ligula ac quam
// viverra nec consectetur ante hendrerit. Donec et mollis dolor.
// Praesent et diam eget libero egestas mattis sit amet vitae augue. Nam
// tincidunt congue enim, ut porta lorem lacinia consectetur. Donec ut
// libero sed arcu vehicula ultricies a non tortor. Lorem ipsum dolor sit
// amet, consectetur adipiscing elit. Aenean ut gravida lorem. Ut turpis
// felis, pulvinar a semper sed, adipiscing id dolor.
// Very looooooooooooooooooooooooooooooooooooooooooooooooooooooooong comment
// that should be split
// println!("{:?}", rewrite_comment(subslice,
// false,
// comment_width,
// self.block_indent,
// self.config)
// .unwrap());
funk(); //dontchangeme
// or me
}
/// test123
fn doc_comment() {
}
fn chains() {
foo.bar(|| {
let x = 10;
// comment
x
})
}

View file

@ -1,3 +1,3 @@
// sadfsdfa
//sdffsdfasdf
// sdffsdfasdf

View file

@ -142,7 +142,8 @@ fn baz() {
fn qux() {
{}
// FIXME this one could be done better.
{ /* a block with a comment */
{
// a block with a comment
}
{

View file

@ -6,7 +6,8 @@ fn foo() {
// Some comment.
a => foo(),
b if 0 < 42 => foo(),
c => { // Another comment.
c => {
// Another comment.
// Comment.
an_expression;
foo()
@ -112,7 +113,8 @@ fn issue339() {
// collapsing here exceeds line length
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffg => {
}
h => { // comment above block
h => {
// comment above block
}
i => {} // comment below block
j => {
@ -133,7 +135,8 @@ fn issue339() {
m => {}
n => {}
o => {}
p => { // Dont collapse me
p => {
// Dont collapse me
}
q => {}
r => {}

View file

@ -22,7 +22,8 @@ mod doc;
mod other;
// sfdgfffffffffffffffffffffffffffffffffffffffffffffffffffffff ffffffffffffffffffffffffffffffffffffffffff
// sfdgfffffffffffffffffffffffffffffffffffffffffffffffffffffff
// ffffffffffffffffffffffffffffffffffffffffff
fn foo(a: isize, b: u32 /* blah blah */, c: f64) {