Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not moving the first line comment of blocks #4745

Open
wants to merge 2 commits into
base: rustfmt-2.0.0-rc.2
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/formatting/syntux/session.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::cell::RefCell;
use std::ops::Range;
use std::path::Path;
use std::rc::Rc;

Expand Down Expand Up @@ -189,6 +190,15 @@ impl ParseSess {
}
}

pub(crate) fn line_bounds(&self, pos: BytePos) -> Option<Range<BytePos>> {
let line = self.parse_sess.source_map().lookup_line(pos).ok();

match line {
Some(line_info) => Some(line_info.sf.line_bounds(line_info.line)),
None => None,
}
}

pub(crate) fn line_of_byte_pos(&self, pos: BytePos) -> usize {
self.parse_sess.source_map().lookup_char_pos(pos).line
}
Expand Down
32 changes: 31 additions & 1 deletion src/formatting/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_span::{symbol, BytePos, Pos, Span, DUMMY_SP};
use crate::config::{BraceStyle, Config};
use crate::formatting::{
attr::*,
comment::{contains_comment, rewrite_comment, CodeCharKind, CommentCodeSlices},
comment::{comment_style, contains_comment, rewrite_comment, CodeCharKind, CommentCodeSlices},
items::{
format_impl, format_trait, format_trait_alias, is_mod_decl, is_use_item,
rewrite_associated_impl_type, rewrite_extern_crate, rewrite_opaque_impl_type,
Expand Down Expand Up @@ -263,6 +263,36 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
self.block_indent = self.block_indent.block_indent(self.config);
self.push_str("{");

if has_braces {
let block_line_range = self.parse_sess.lookup_line_range(b.span);
if block_line_range.lo != block_line_range.hi { // Skipping if a single line block
let first_line_contains_stmt = if let Some(first_stmt) = b.stmts.first() {
self.parse_sess.lookup_line_range(first_stmt.span).lo == block_line_range.lo
} else {
false
};

let first_line_bounds = self.parse_sess.line_bounds(self.last_pos).unwrap();
let first_line_snip = self
.snippet(mk_sp(self.last_pos, first_line_bounds.end))
.trim();

if !first_line_contains_stmt
&& contains_comment(first_line_snip)
&& comment_style(first_line_snip, self.config.normalize_comments())
.is_line_comment()
{
if let Some(comment) =
rewrite_comment(first_line_snip, false, self.shape(), self.config)
{
self.push_str(" ");
self.push_str(&comment);
self.last_pos = self.last_pos + BytePos(first_line_snip.len() as u32);
}
}
}
}

let first_non_ws = inner_attrs
.and_then(|attrs| attrs.first().map(|attr| attr.span.lo()))
.or_else(|| b.stmts.first().map(|s| s.span().lo()));
Expand Down
26 changes: 26 additions & 0 deletions tests/source/issue-3255.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
fn foo(){
if true { // Sample comment
1
}
}


fn foo(){
if true {
// Sample comment
1
}
}

fn foo(){
if true { /* Sample comment */
1
}
}

fn foo(){
if true { /* Sample
comment */
1
}
}
3 changes: 1 addition & 2 deletions tests/target/async_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ async unsafe fn foo() {
}

async unsafe fn rust() {
async move {
// comment
async move { // comment
Ok(())
}
}
Expand Down
3 changes: 1 addition & 2 deletions tests/target/control-brace-style-always-next-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ fn main() {


'while_label: while cond
{
// while comment
{ // while comment
let foo = ();
}

Expand Down
3 changes: 1 addition & 2 deletions tests/target/control-brace-style-always-same-line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ fn main() {
}


'while_label: while cond {
// while comment
'while_label: while cond { // while comment
let foo = ();
}

Expand Down
27 changes: 27 additions & 0 deletions tests/target/issue-3255.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
fn foo() {
if true { // Sample comment
1
}
}

fn foo() {
if true {
// Sample comment
1
}
}

fn foo() {
if true {
/* Sample comment */
1
}
}

fn foo() {
if true {
/* Sample
comment */
1
Comment on lines +22 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally, I think it makes more sense to not move the comment one line down. For example, the following use cases

if true { /* this is a supppppppppppppppppppppppppppper 
   long comment to describe the if condition */
   1
}

}
}
3 changes: 1 addition & 2 deletions tests/target/match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ fn foo() {
// Some comment.
a => foo(),
b if 0 < 42 => foo(),
c => {
// Another comment.
c => { // Another comment.
// Comment.
an_expression;
foo()
Expand Down