rustfmt
rustfmt copied to clipboard
avoid inserting newline between opening curly brace and comment
This ports https://github.com/rust-lang/rustfmt/pull/4745 to the current master branch. That led to a bunch of test failures so I hacked around until they were all gone... but the codebase is entirely foreign to me so the result may not make any sense at all.^^
In particular, I have no idea what the contract of rewrite_comment is (and it doesn't have a doc comment that would clarify): is the result guaranteed to contain everything "relevant" from the input, so I can just advance last_pos to the end of the line, or do I have to worry about this only formatting a prefix of the input and last_pos needs to be advancd more carefully?
Fixes https://github.com/rust-lang/rustfmt/issues/3255
I spent some time looking at this one tonight. One issue I can see is that the shape used to format the first line comment is technically incorrect. It doesn't take into account the length of the conditional logic that already occupies the line, e.g if true {, so rewrite_comment thinks it has more room to work with than it actually does.
As a result, this interacts poorly with wrap_comments=true since it'll wraps at an incorrect location and reports a max_width error (assuming you've got error_on_unformatted=true` set). Also, after we wrap the comment, part of the comment will be aligned with the opening brace and the rest of it will be aligned with the statements (not necessarily a bad thing). here's an example input:
fn foo() {
if true { // some long comment explaining what this condition is all about. when `wrap_comments=true` is used this will wrap.
1
}
}
This is the output I get when running this with wrap_comments=true:
fn foo() {
if true { // some long comment explaining what this condition is all about. when `wrap_comments=true`
// is used this will wrap.
1
}
}
and I'm also seeing this error:
error[internal]: line formatted, but exceeded maximum width (maximum: 100 (see `max_width` option), found: 105)
--> <stdin>:2:2:101
|
2 | if true { // some long comment explaining what this condition is all about. when `wrap_comments=true`
| ^^^^^
|
= note: set `error_on_unformatted = false` to suppress the warning against comments or string literals
warning: rustfmt has failed to format. See previous 1 errors.
Here's one idea I had to try and handle block comments formatting. There are likely other issues with this code, but I also ran into the same issue that I described above. Since we don't know how much room if true { occupies on the first line we can't align things correctly, and the block comments end up getting aligned with the statements + 1 since it's trying to align the *. This might not be what we go with, but I thought I'd share what I'd looked into.
diff --git a/src/visitor.rs b/src/visitor.rs
index b57a49e5..62533d82 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -1,6 +1,7 @@
use std::cell::{Cell, RefCell};
use std::rc::Rc;
+use rustc_ast::HasAttrs;
use rustc_ast::{ast, token::Delimiter, visit};
use rustc_data_structures::sync::Lrc;
use rustc_span::{symbol, BytePos, Pos, Span};
@@ -244,14 +245,42 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
.trim();
if contains_comment(first_line_snip) {
- if let Ok(comment) =
- rewrite_comment(first_line_snip, false, self.shape(), self.config)
- {
+ // Now that we know the first line contains a comment, lets determine the
+ // bounds of the comment(s).
+ let comment_byte_pos_hi = match inner_attrs {
+ Some(attrs) if !attrs.is_empty() => attrs.first().unwrap().span.hi(),
+ _ => b
+ .stmts
+ .first()
+ .map(|s| {
+ match s
+ .attrs()
+ .iter()
+ .filter(|a| a.style == ast::AttrStyle::Outer)
+ .next()
+ {
+ Some(attr) => attr.span.hi() - BytePos(1),
+ None => s.span.hi() - BytePos(1),
+ }
+ })
+ .unwrap_or_else(|| self.snippet_provider.span_before(b.span, "}")),
+ };
+ let comment_snippet = self.snippet(mk_sp(self.last_pos, comment_byte_pos_hi)).trim();
+
+ // FIXME(ytmimi) it would be nice to update the Shape's width
+ // when formatting these first line comments, especially block comments
+ // since those comment lines are logically grouped, but the current
+ // FmtVisitor doesn't have any context about how much room is already
+ // occupied on the line we're adding the comment to
+ if let Ok(comment) = rewrite_comment(
+ comment_snippet,
+ false,
+ self.shape(),
+ self.config,
+ ) {
self.push_str(" ");
self.push_str(&comment);
- // This line has no statements, so we can jump to the end of it
- // (but *before* the newline).
- self.last_pos = first_line_bounds.end - BytePos::from_usize(1);
+ self.last_pos = comment_byte_pos_hi;
}
}
}
@RalfJung and @ytmimi any updates on this?