Skip to content

fix(fmt): single line comments orphaned words overflow into next line #11132

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

Open
wants to merge 3 commits into
base: master
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
3 changes: 3 additions & 0 deletions crates/fmt/foundry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[fmt]
line_length = 80
wrap_comments = true
182 changes: 160 additions & 22 deletions crates/fmt/src/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,40 @@ impl<'a, W: Write> Formatter<'a, W> {
Ok(())
}

/// Returns false if line starts new semantic block (NatSpec, markdown, etc.)
fn should_merge_comment_line(line: &str) -> bool {
let trimmed = line.trim();

// Don't merge NatSpec tags
if trimmed.starts_with('@') {
return false;
}

// Don't merge markdown structure
if trimmed.starts_with("- ")
|| trimmed.starts_with("* ")
|| trimmed.starts_with("> ")
|| trimmed.starts_with("+ ")
{
return false;
}

// Don't merge numbered lists
if let Some(first_word) = trimmed.split_whitespace().next()
&& first_word.ends_with('.')
&& first_word[..first_word.len() - 1].chars().all(|c| c.is_ascii_digit())
{
return false;
}

// Don't merge empty lines
if trimmed.is_empty() {
return false;
}

true
}

/// Write a comment line that might potentially overflow the maximum line length
/// and, if configured, will be wrapped to the next line.
fn write_comment_line(&mut self, comment: &CommentWithMetadata, line: &str) -> Result<bool> {
Expand Down Expand Up @@ -621,12 +655,11 @@ impl<'a, W: Write> Formatter<'a, W> {

if let Some(next) = words.peek() {
if !word.is_empty() && !self.will_it_fit(next) {
// the next word doesn't fit on this line,
// write remaining words on the next
let remaining_text = words.join(" ");
// Wrap remaining text on next line
self.write_whitespace_separator(true)?;
// write newline wrap token
write!(self.buf(), "{}", comment.wrap_token())?;
self.write_comment_line(comment, &words.join(" "))?;
self.write_comment_line(comment, &remaining_text)?;
return Ok(true);
}

Expand All @@ -652,28 +685,133 @@ impl<'a, W: Write> Formatter<'a, W> {
&mut self,
comments: impl IntoIterator<Item = &'b CommentWithMetadata>,
) -> Result<()> {
let mut comments = comments.into_iter().peekable();
let mut last_byte_written = match comments.peek() {
Some(comment) => comment.loc.start(),
None => return Ok(()),
};
let mut is_first = true;
for comment in comments {
let unwritten_whitespace_loc =
Loc::File(comment.loc.file_no(), last_byte_written, comment.loc.start());
if self.inline_config.is_disabled(unwritten_whitespace_loc) {
self.write_raw(&self.source[unwritten_whitespace_loc.range()])?;
self.write_raw_comment(comment)?;
last_byte_written = if comment.is_line() {
self.find_next_line(comment.loc.end()).unwrap_or_else(|| comment.loc.end())
let comments: Vec<_> = comments.into_iter().collect();

let mut i = 0;
while i < comments.len() {
let comment = comments[i];

if comment.ty == CommentType::DocLine && self.config.wrap_comments {
let mut continuation_comments = vec![comment];
let mut j = i + 1;

while j < comments.len()
&& comments[j].ty == CommentType::DocLine
&& Self::should_merge_comment_line(comments[j].contents())
{
continuation_comments.push(comments[j]);
j += 1;
}

self.write_comment_with_overflow_handling(&continuation_comments)?;
i = j;
} else {
self.write_comment(comment, i == 0)?;
i += 1;
}
}
Ok(())
}

/// Write comment with overflow to continuation lines
fn write_comment_with_overflow_handling(
&mut self,
comments: &[&CommentWithMetadata],
) -> Result<()> {
if comments.is_empty() {
return Ok(());
}

let first_comment = comments[0];

// Collect text from continuation comments
let mut all_text = first_comment.contents().to_string();
for &comment in &comments[1..] {
all_text.push(' ');
all_text.push_str(comment.contents());
}

// Create merged comment for wrapping
let mut merged_comment = first_comment.clone();
merged_comment.comment = format!("/// {all_text}");

self.write_comment_with_strategic_wrapping(&merged_comment)?;
Ok(())
}

/// Write comment with wrapping to prevent orphaned words
fn write_comment_with_strategic_wrapping(
&mut self,
comment: &CommentWithMetadata,
) -> Result<()> {
if comment.is_line() && comment.has_newline_before && !self.is_beginning_of_line() {
self.write_whitespace_separator(true)?;
}

let content = comment.contents();
let prefix = comment.start_token();

// Break text into words for optimal line distribution
let words: Vec<&str> = content.split_whitespace().collect();
if words.is_empty() {
write!(self.buf(), "{}", comment.comment)?;
return Ok(());
}

let mut current_line = String::new();
let mut word_index = 0;
let mut is_first_line = true;

while word_index < words.len() {
let line_prefix = if is_first_line {
format!("{prefix} ")
} else {
format!("{} ", comment.wrap_token())
};

// Calculate available width for this line
let available_width = self.config.line_length.saturating_sub(if is_first_line {
self.buf.current_indent_len() + line_prefix.len()
} else {
self.buf.current_indent_len() + comment.wrap_token().len()
});

current_line.clear();
let mut line_len = 0;

while word_index < words.len() {
let word = words[word_index];
let space_needed = if current_line.is_empty() { 0 } else { 1 };
let word_len = word.len();

if line_len + space_needed + word_len <= available_width {
// Word fits on current line
if !current_line.is_empty() {
current_line.push(' ');
line_len += 1;
}
current_line.push_str(word);
line_len += word_len;
word_index += 1;
} else {
comment.loc.end()
};
// Word doesn't fit, break to next line
break;
}
}

if is_first_line {
write!(self.buf(), "{line_prefix}{current_line}")?;
is_first_line = false;
} else {
self.write_comment(comment, is_first)?;
self.write_whitespace_separator(true)?;
write!(self.buf(), "{}{current_line}", comment.wrap_token())?;
}
is_first = false;
}

if comment.is_line() {
self.write_whitespace_separator(true)?;
}

Ok(())
}

Expand Down
50 changes: 50 additions & 0 deletions crates/fmt/testdata/WrapCommentOverflow/fmt.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// config: line_length = 80
// config: wrap_comments = true
pragma solidity ^0.8.13;

contract WrapCommentOverflowTest {
/// @notice This is a very long single-line comment that should demonstrate
/// strategic overflow wrapping behavior without creating orphaned words
function singleLineOverflow() public {}

/// @notice Calculates the amount that the sender would be refunded if the
/// stream were canceled, denominated in units of the token's decimals.
function originalGitHubIssue() public {}

/// @notice Short comment that fits on one line
function singleLineNoWrap() public {}

/// @notice This is a notice section that is quite long and should wrap
/// nicely
/// @param value This parameter description should remain separate from the
/// notice above
/// @return result The return value description should also stay separate
function natspecBoundaries(uint256 value) public returns (uint256 result) {}

/// @notice Another example with multiple sections
/// @dev Implementation details that are separate from notice
/// @param amount Should not merge with dev section above
function multipleSections(uint256 amount) public {}

/// @notice Function with markdown list that should preserve structure:
/// - First item in the list should stay as a list item
/// - Second item should also remain properly formatted
/// - Third item completes the list structure
function markdownList() public {}

/// @notice Another markdown example:
/// 1. Numbered list item one
/// 2. Numbered list item two that is longer
/// 3. Final numbered item
function numberedList() public {}

/// @notice Block quote example:
/// > This is a block quote that should remain intact
/// > Second line of the block quote
function blockQuote() public {}

/// @notice First paragraph of documentation
///
/// Second paragraph should remain separate due to empty line above
function emptyLineSeparation() public {}
}
44 changes: 44 additions & 0 deletions crates/fmt/testdata/WrapCommentOverflow/original.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
pragma solidity ^0.8.13;

contract WrapCommentOverflowTest {
/// @notice This is a very long single-line comment that should demonstrate strategic overflow wrapping behavior without creating orphaned words
function singleLineOverflow() public {}

/// @notice Calculates the amount that the sender would be refunded if the stream were canceled, denominated in units of the token's decimals.
function originalGitHubIssue() public {}

/// @notice Short comment that fits on one line
function singleLineNoWrap() public {}

/// @notice This is a notice section that is quite long and should wrap nicely
/// @param value This parameter description should remain separate from the notice above
/// @return result The return value description should also stay separate
function natspecBoundaries(uint256 value) public returns (uint256 result) {}

/// @notice Another example with multiple sections
/// @dev Implementation details that are separate from notice
/// @param amount Should not merge with dev section above
function multipleSections(uint256 amount) public {}

/// @notice Function with markdown list that should preserve structure:
/// - First item in the list should stay as a list item
/// - Second item should also remain properly formatted
/// - Third item completes the list structure
function markdownList() public {}

/// @notice Another markdown example:
/// 1. Numbered list item one
/// 2. Numbered list item two that is longer
/// 3. Final numbered item
function numberedList() public {}

/// @notice Block quote example:
/// > This is a block quote that should remain intact
/// > Second line of the block quote
function blockQuote() public {}

/// @notice First paragraph of documentation
///
/// Second paragraph should remain separate due to empty line above
function emptyLineSeparation() public {}
}
1 change: 1 addition & 0 deletions crates/fmt/tests/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ test_directories! {
BlockComments,
BlockCommentsFunction,
EnumVariants,
WrapCommentOverflow,
}

test_dir!(SortedImports, TestConfig::skip_compare_ast_eq());
Expand Down
Loading