user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #1925 from topecongiro/enhance-comment

Browse source 
Enhance comment formatting
This commit is contained in:
Nick Cameron 2017-08-28 09:33:08 +12:00 • committed by GitHub
commit 7298ce9d52
12 changed files with 232 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Cargo.toml
View file

@ -7,7 +7,7 @@ description = "Tool to find and fix Rust formatting issues"
repository = "https://github.com/rust-lang-nursery/rustfmt"
readme = "README.md"
license = "Apache-2.0/MIT"
include = ["src/*.rs", "Cargo.toml", "build.rs", "LICENSE-*"]
include = ["src/**", "Cargo.toml", "build.rs", "LICENSE-*"]
build = "build.rs"
categories = ["development-tools"]

56
src/comment.rs
View file

@ -153,11 +153,10 @@ pub fn combine_strs_with_missing_comments(
let mut one_line_width =
last_line_width(prev_str) + first_line_width(next_str) + first_sep.len();
let original_snippet = context.snippet(span);
let trimmed_snippet = original_snippet.trim();
let indent_str = shape.indent.to_string(context.config);
let missing_comment = try_opt!(rewrite_missing_comment(span, shape, context));
if trimmed_snippet.is_empty() {
if missing_comment.is_empty() {
if allow_extend && prev_str.len() + first_sep.len() + next_str.len() <= shape.width {
return Some(format!("{}{}{}", prev_str, first_sep, next_str));
} else {
@ -175,18 +174,13 @@ pub fn combine_strs_with_missing_comments(
// Peek the the original source code and find out whether there is a newline between the first
// expression and the second expression or the missing comment. We will preserve the orginal
// layout whenever possible.
let original_snippet = context.snippet(span);
let prefer_same_line = if let Some(pos) = original_snippet.chars().position(|c| c == '/') {
!original_snippet[..pos].contains('\n')
} else {
!original_snippet.contains('\n')
};
let missing_comment = try_opt!(rewrite_comment(
trimmed_snippet,
false,
shape,
context.config
));
one_line_width -= first_sep.len();
let first_sep = if prev_str.is_empty() || missing_comment.is_empty() {
String::new()
@ -365,6 +359,50 @@ fn rewrite_comment_inner(
Some(result)
}
/// Given the span, rewrite the missing comment inside it if available.
/// Note that the given span must only include comments (or leading/trailing whitespaces).
pub fn rewrite_missing_comment(
span: Span,
shape: Shape,
context: &RewriteContext,
) -> Option<String> {
let missing_snippet = context.snippet(span);
let trimmed_snippet = missing_snippet.trim();
if !trimmed_snippet.is_empty() {
rewrite_comment(trimmed_snippet, false, shape, context.config)
} else {
Some(String::new())
}
}
/// Recover the missing comments in the specified span, if available.
/// The layout of the comments will be preserved as long as it does not break the code
/// and its total width does not exceed the max width.
pub fn recover_missing_comment_in_span(
span: Span,
shape: Shape,
context: &RewriteContext,
used_width: usize,
) -> Option<String> {
let missing_comment = try_opt!(rewrite_missing_comment(span, shape, context));
if missing_comment.is_empty() {
Some(String::new())
} else {
let missing_snippet = context.snippet(span);
let pos = missing_snippet.chars().position(|c| c == '/').unwrap_or(0);
// 1 = ` `
let total_width = missing_comment.len() + used_width + 1;
let force_new_line_before_comment =
missing_snippet[..pos].contains('\n') || total_width > context.config.max_width();
let sep = if force_new_line_before_comment {
format!("\n{}", shape.indent.to_string(context.config))
} else {
String::from(" ")
};
Some(format!("{}{}", sep, missing_comment))
}
}
/// Trims whitespace and aligns to indent, but otherwise does not change comments.
fn light_rewrite_comment(orig: &str, offset: Indent, config: &Config) -> Option<String> {
let lines: Vec<&str> = orig.lines()

26
src/expr.rs
View file

@ -20,7 +20,7 @@ use {Indent, Shape, Spanned};
use chains::rewrite_chain;
use codemap::{LineRangeUtils, SpanUtils};
use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed,
rewrite_comment, FindUncommented};
rewrite_comment, rewrite_missing_comment, FindUncommented};
use config::{Config, ControlBraceStyle, IndentStyle, MultilineStyle, Style};
use items::{span_hi_for_arg, span_lo_for_arg};
use lists::{definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting,
@ -1195,9 +1195,13 @@ impl<'a> ControlFlow<'a> {
mk_sp(self.block.span.lo, self.block.span.lo)
};
// for event in event
// `for event in event`
// Do not include label in the span.
let lo = self.label.map_or(self.span.lo, |label| label.span.hi);
let between_kwd_cond = mk_sp(
context.codemap.span_after(self.span, self.keyword.trim()),
context
.codemap
.span_after(mk_sp(lo, self.span.hi), self.keyword.trim()),
self.pat
.map_or(cond_span.lo, |p| if self.matcher.is_empty() {
p.span.lo
@ -1378,21 +1382,13 @@ fn rewrite_label(label: Option<ast::SpannedIdent>) -> String {
}
fn extract_comment(span: Span, context: &RewriteContext, shape: Shape) -> Option<String> {
let comment_str = context.snippet(span);
if contains_comment(&comment_str) {
let comment = try_opt!(rewrite_comment(
comment_str.trim(),
false,
shape,
context.config,
));
Some(format!(
match rewrite_missing_comment(span, shape, context) {
Some(ref comment) if !comment.is_empty() => Some(format!(
"\n{indent}{}\n{indent}",
comment,
indent = shape.indent.to_string(context.config)
))
} else {
None
)),
_ => None,
}
}

149
src/items.rs
View file

@ -19,7 +19,7 @@ use syntax::codemap::{BytePos, Span};
use {Indent, Shape, Spanned};
use codemap::{LineRangeUtils, SpanUtils};
use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed,
rewrite_comment, FindUncommented};
recover_missing_comment_in_span, rewrite_missing_comment, FindUncommented};
use config::{BraceStyle, Config, Density, IndentStyle, ReturnIndent, Style};
use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs,
rewrite_call_inner, ExprType};
@ -66,7 +66,7 @@ impl Rewrite for ast::Local {
// String that is placed within the assignment pattern and expression.
let infix = {
let mut infix = String::new();
let mut infix = String::with_capacity(32);
if let Some(ref ty) = self.ty {
let separator = type_annotation_separator(context.config);
@ -517,8 +517,10 @@ pub fn format_impl(
where_span_end: Option<BytePos>,
) -> Option<String> {
if let ast::ItemKind::Impl(_, _, _, ref generics, _, ref self_ty, ref items) = item.node {
let mut result = String::new();
let mut result = String::with_capacity(128);
let ref_and_type = try_opt!(format_impl_ref_and_type(context, item, offset));
let indent_str = offset.to_string(context.config);
let sep = format!("\n{}", &indent_str);
result.push_str(&ref_and_type);
let where_budget = if result.contains('\n') {
@ -543,6 +545,24 @@ pub fn format_impl(
option,
));
// If there is no where clause, we may have missing comments between the trait name and
// the opening brace.
if generics.where_clause.predicates.is_empty() {
if let Some(hi) = where_span_end {
match recover_missing_comment_in_span(
mk_sp(self_ty.span.hi, hi),
Shape::indented(offset, context.config),
context,
last_line_width(&result),
) {
Some(ref missing_comment) if !missing_comment.is_empty() => {
result.push_str(missing_comment);
}
_ => (),
}
}
}
if try_opt!(is_impl_single_line(
context,
&items,
@ -551,9 +571,8 @@ pub fn format_impl(
&item,
)) {
result.push_str(&where_clause_str);
if where_clause_str.contains('\n') {
let white_space = offset.to_string(context.config);
result.push_str(&format!("\n{}{{\n{}}}", &white_space, &white_space));
if where_clause_str.contains('\n') || last_line_contains_single_line_comment(&result) {
result.push_str(&format!("{}{{{}}}", &sep, &sep));
} else {
result.push_str(" {}");
}
@ -569,14 +588,11 @@ pub fn format_impl(
result.push_str(&where_clause_str);
match context.config.item_brace_style() {
BraceStyle::AlwaysNextLine => {
result.push('\n');
result.push_str(&offset.to_string(context.config));
}
_ if last_line_contains_single_line_comment(&result) => result.push_str(&sep),
BraceStyle::AlwaysNextLine => result.push_str(&sep),
BraceStyle::PreferSameLine => result.push(' '),
BraceStyle::SameLineWhere => if !where_clause_str.is_empty() {
result.push('\n');
result.push_str(&offset.to_string(context.config));
result.push_str(&sep);
} else {
result.push(' ');
},
@ -610,8 +626,7 @@ pub fn format_impl(
}
if result.chars().last().unwrap() == '{' {
result.push('\n');
result.push_str(&offset.to_string(context.config));
result.push_str(&sep);
}
result.push('}');
@ -632,7 +647,7 @@ fn is_impl_single_line(
let open_pos = try_opt!(snippet.find_uncommented("{")) + 1;
Some(
context.config.impl_empty_single_line() && items.is_empty() &&
context.config.impl_empty_single_line() && items.is_empty() && !result.contains('\n') &&
result.len() + where_clause_str.len() <= context.config.max_width() &&
!contains_comment(&snippet[open_pos..]),
)
@ -653,7 +668,7 @@ fn format_impl_ref_and_type(
_,
) = item.node
{
let mut result = String::new();
let mut result = String::with_capacity(128);
result.push_str(&format_visibility(&item.vis));
result.push_str(&format_defaultness(defaultness));
@ -853,7 +868,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent)
if let ast::ItemKind::Trait(unsafety, ref generics, ref type_param_bounds, ref trait_items) =
item.node
{
let mut result = String::new();
let mut result = String::with_capacity(128);
let header = format!(
"{}{}trait {}",
format_visibility(&item.vis),
@ -910,14 +925,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent)
.checked_sub(last_line_width(&result))
);
let pos_before_where = if type_param_bounds.is_empty() {
if generics.where_clause.predicates.is_empty() {
// We do not use this, so it does not matter
item.span.lo
} else {
let snippet = context.snippet(item.span);
let where_pos = snippet.find_uncommented("where");
item.span.lo + where_pos.map_or(BytePos(0), |p| BytePos(p as u32))
}
generics.where_clause.span.lo
} else {
type_param_bounds[type_param_bounds.len() - 1].span().hi
};
@ -946,7 +954,33 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent)
}
result.push_str(&where_clause_str);
if generics.where_clause.predicates.is_empty() {
let item_snippet = context.snippet(item.span);
if let Some(lo) = item_snippet.chars().position(|c| c == '/') {
// 1 = `{`
let comment_hi = body_lo - BytePos(1);
let comment_lo = item.span.lo + BytePos(lo as u32);
if comment_lo < comment_hi {
match recover_missing_comment_in_span(
mk_sp(comment_lo, comment_hi),
Shape::indented(offset, context.config),
context,
last_line_width(&result),
) {
Some(ref missing_comment) if !missing_comment.is_empty() => {
result.push_str(missing_comment);
}
_ => (),
}
}
}
}
match context.config.item_brace_style() {
_ if last_line_contains_single_line_comment(&result) => {
result.push('\n');
result.push_str(&offset.to_string(context.config));
}
BraceStyle::AlwaysNextLine => {
result.push('\n');
result.push_str(&offset.to_string(context.config));
@ -1231,7 +1265,7 @@ pub fn rewrite_type_alias(
vis: &ast::Visibility,
span: Span,
) -> Option<String> {
let mut result = String::new();
let mut result = String::with_capacity(128);
result.push_str(&format_visibility(vis));
result.push_str("type ");
@ -2006,33 +2040,17 @@ fn rewrite_fn_base(
// args and `{`.
if where_clause_str.is_empty() {
if let ast::FunctionRetTy::Default(ret_span) = fd.output {
let sp = mk_sp(args_span.hi, ret_span.hi);
let missing_snippet = context.snippet(sp);
let trimmed_snippet = missing_snippet.trim();
let missing_comment = if trimmed_snippet.is_empty() {
String::new()
} else {
try_opt!(rewrite_comment(
trimmed_snippet,
false,
Shape::indented(indent, context.config),
context.config,
))
};
if !missing_comment.is_empty() {
let pos = missing_snippet.chars().position(|c| c == '/').unwrap_or(0);
// 1 = ` `
let total_width = missing_comment.len() + last_line_width(&result) + 1;
let force_new_line_before_comment = missing_snippet[..pos].contains('\n') ||
total_width > context.config.max_width();
let sep = if force_new_line_before_comment {
format!("\n{}", indent.to_string(context.config))
} else {
String::from(" ")
};
result.push_str(&sep);
result.push_str(&missing_comment);
force_new_line_for_brace = true;
match recover_missing_comment_in_span(
mk_sp(args_span.hi, ret_span.hi),
shape,
context,
last_line_width(&result),
) {
Some(ref missing_comment) if !missing_comment.is_empty() => {
result.push_str(missing_comment);
force_new_line_for_brace = true;
}
_ => (),
}
}
}
@ -2683,34 +2701,17 @@ fn missing_span_before_after_where(
(missing_span_before, missing_span_after)
}
fn rewrite_missing_comment_in_where(
context: &RewriteContext,
comment: &str,
shape: Shape,
) -> Option<String> {
let comment = comment.trim();
if comment.is_empty() {
Some(String::new())
} else {
rewrite_comment(comment, false, shape, context.config)
}
}
fn rewrite_comments_before_after_where(
context: &RewriteContext,
span_before_where: Span,
span_after_where: Span,
shape: Shape,
) -> Option<(String, String)> {
let before_comment = try_opt!(rewrite_missing_comment_in_where(
context,
&context.snippet(span_before_where),
shape,
));
let after_comment = try_opt!(rewrite_missing_comment_in_where(
context,
&context.snippet(span_after_where),
let before_comment = try_opt!(rewrite_missing_comment(span_before_where, shape, context));
let after_comment = try_opt!(rewrite_missing_comment(
span_after_where,
shape.block_indent(context.config.tab_spaces()),
context,
));
Some((before_comment, after_comment))
}

71
src/visitor.rs
View file

@ -18,7 +18,8 @@ use syntax::parse::ParseSess;
use {Indent, Shape, Spanned};
use codemap::{LineRangeUtils, SpanUtils};
use comment::{contains_comment, CodeCharKind, CommentCodeSlices, FindUncommented};
use comment::{contains_comment, recover_missing_comment_in_span, CodeCharKind, CommentCodeSlices,
FindUncommented};
use comment::rewrite_comment;
use config::{BraceStyle, Config};
use expr::{format_expr, ExprType};
@ -919,15 +920,17 @@ impl Rewrite for ast::Attribute {
impl<'a> Rewrite for [ast::Attribute] {
fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
let mut result = String::new();
if self.is_empty() {
return Some(result);
return Some(String::new());
}
let mut result = String::with_capacity(128);
let indent = shape.indent.to_string(context.config);
let mut derive_args = Vec::new();
let mut iter = self.iter().enumerate().peekable();
let mut insert_new_line = true;
let mut is_prev_sugared_doc = false;
while let Some((i, a)) = iter.next() {
let a_str = try_opt!(a.rewrite(context, shape));
@ -937,31 +940,61 @@ impl<'a> Rewrite for [ast::Attribute] {
// This particular horror show is to preserve line breaks in between doc
// comments. An alternative would be to force such line breaks to start
// with the usual doc comment token.
let multi_line = a_str.starts_with("//") && comment.matches('\n').count() > 1;
let comment = comment.trim();
let (multi_line_before, multi_line_after) = if a.is_sugared_doc ||
is_prev_sugared_doc
{
// Look at before and after comment and see if there are any empty lines.
let comment_begin = comment.chars().position(|c| c == '/');
let len = comment_begin.unwrap_or(comment.len());
let mlb = comment.chars().take(len).filter(|c| *c == '\n').count() > 1;
let mla = if comment_begin.is_none() {
mlb
} else {
let comment_end = comment.chars().rev().position(|c| !c.is_whitespace());
let len = comment_end.unwrap();
comment
.chars()
.rev()
.take(len)
.filter(|c| *c == '\n')
.count() > 1
};
(mlb, mla)
} else {
(false, false)
};
let comment = try_opt!(recover_missing_comment_in_span(
mk_sp(self[i - 1].span.hi, a.span.lo),
shape.with_max_width(context.config),
context,
0,
));
if !comment.is_empty() {
let comment = try_opt!(rewrite_comment(
comment,
false,
Shape::legacy(
context.config.comment_width() - shape.indent.width(),
shape.indent,
),
context.config,
));
result.push_str(&indent);
if multi_line_before {
result.push('\n');
}
result.push_str(&comment);
result.push('\n');
} else if multi_line {
if multi_line_after {
result.push('\n')
}
} else if insert_new_line {
result.push('\n');
if multi_line_after {
result.push('\n')
}
}
if derive_args.is_empty() {
result.push_str(&indent);
}
insert_new_line = true;
}
// Write the attribute itself.
let mut insert_new_line = true;
if context.config.merge_derives() {
// If the attribute is `#[derive(...)]`, take the arguments.
if let Some(mut args) = get_derive_args(context, a) {
@ -982,9 +1015,7 @@ impl<'a> Rewrite for [ast::Attribute] {
result.push_str(&a_str);
}
if insert_new_line && i < self.len() - 1 {
result.push('\n');
}
is_prev_sugared_doc = a.is_sugared_doc;
}
Some(result)
}

13
tests/source/attrib.rs
View file

@ -1,6 +1,19 @@
// rustfmt-wrap_comments: true
// Test attributes and doc comments are preserved.
//! Doc comment
#![attribute]
//! Crate doc comment
// Comment
// Comment on attribute
#![the(attribute)]
// Another comment
#[invalid attribute]
fn foo() {}

4
tests/source/trait.rs
View file

@ -53,3 +53,7 @@ trait FooBar<T> : Tttttttttttttttttttttttttttttttttttttttttttttttttttttttttttttt
trait WhereList<T, J> where T: Foo, J: Bar {}
trait X /* comment */ {}
trait Y // comment
{
}

18
tests/target/attrib.rs
View file

@ -1,6 +1,19 @@
// rustfmt-wrap_comments: true
// Test attributes and doc comments are preserved.
//! Doc comment
#![attribute]
//! Crate doc comment
// Comment
// Comment on attribute
#![the(attribute)]
// Another comment
#[invalid attribute]
fn foo() {}
@ -33,11 +46,12 @@ impl Bar {
fn f3(self) -> Dog {}
/// Blah blah bing.
#[attrib1]
/// Blah blah bing.
#[attrib2]
// Another comment that needs rewrite because it's
// tooooooooooooooooooooooooooooooo loooooooooooong.
// Another comment that needs rewrite because it's tooooooooooooooooooooooooooooooo
// loooooooooooong.
/// Blah blah bing.
fn f4(self) -> Cat {}

3
tests/target/comment.rs
View file

@ -83,6 +83,7 @@ fn some_fn3() // some comment some comment some comment some comment some commen
}
fn some_fn4()
// some comment some comment some comment some comment some comment some comment some comment
// some comment some comment some comment some comment some comment some comment
// some comment
{
}

4
tests/target/comment4.rs
View file

@ -1,5 +1,5 @@
#![allow(dead_code)]
// bar
#![allow(dead_code)] // bar
//! Doc comment
fn test() {
// comment

5
tests/target/impl.rs
View file

@ -18,6 +18,11 @@ where
}
}
impl X<T> /* comment */ {}
impl Y<T> // comment
{
}
impl<T> Foo for T
// comment1
where

5
tests/target/trait.rs
View file

@ -79,3 +79,8 @@ where
J: Bar,
{
}
trait X /* comment */ {}
trait Y // comment
{
}