From d46ddb85529a29be7ca05af86b971c469861a893 Mon Sep 17 00:00:00 2001 From: 0xheartcode <0xheartcode@gmail.com> Date: Mon, 28 Jul 2025 18:59:33 +0200 Subject: [PATCH 1/3] fix(fmt): single line comments orphaned words overflow into next line --- crates/fmt/src/formatter.rs | 237 ++++++++++++++++++++++++++++++++++-- 1 file changed, 230 insertions(+), 7 deletions(-) diff --git a/crates/fmt/src/formatter.rs b/crates/fmt/src/formatter.rs index c0fcbdfc65fb8..2c5f5c2f06f45 100644 --- a/crates/fmt/src/formatter.rs +++ b/crates/fmt/src/formatter.rs @@ -587,6 +587,86 @@ 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() { + if 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 + } + + /// Merge consecutive DocLine comments into single logical block + fn merge_consecutive_doc_comments( + comments: Vec<&CommentWithMetadata> + ) -> Vec { + let mut result = Vec::new(); + let mut i = 0; + + while i < comments.len() { + let current_comment = comments[i]; + + if current_comment.ty == CommentType::DocLine { + let mut j = i + 1; + let mut should_merge_group = false; + + // Look ahead for consecutive DocLine comments + while j < comments.len() && + comments[j].ty == CommentType::DocLine && + Self::should_merge_comment_line(comments[j].contents()) { + should_merge_group = true; + j += 1; + } + + if should_merge_group { + let mut merged_content = current_comment.contents().to_string(); + for k in (i + 1)..j { + merged_content.push(' '); + merged_content.push_str(comments[k].contents()); + } + + let mut merged_comment = current_comment.clone(); + merged_comment.comment = format!("/// {}", merged_content); + result.push(merged_comment); + + i = j; + } else { + result.push(current_comment.clone()); + i += 1; + } + } else { + result.push(current_comment.clone()); + i += 1; + } + } + + result + } + /// 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 { @@ -621,13 +701,21 @@ 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 - self.write_whitespace_separator(true)?; - // write newline wrap token - write!(self.buf(), "{}", comment.wrap_token())?; - self.write_comment_line(comment, &words.join(" "))?; - return Ok(true); + let remaining_text = words.join(" "); + // Check if remaining text should be merged for wrapping + if Self::should_merge_comment_line(&remaining_text) { + // Wrap within same semantic block + self.write_whitespace_separator(true)?; + write!(self.buf(), "{}", comment.wrap_token())?; + self.write_comment_line(comment, &remaining_text)?; + return Ok(true); + } else { + // Don't merge - treat as separate semantic block + self.write_whitespace_separator(true)?; + write!(self.buf(), "{}", comment.wrap_token())?; + self.write_comment_line(comment, &remaining_text)?; + return Ok(true); + } } self.write_whitespace_separator(false)?; @@ -651,6 +739,141 @@ impl<'a, W: Write> Formatter<'a, W> { fn write_comments<'b>( &mut self, comments: impl IntoIterator, + ) -> Result<()> { + let mut 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() { + if 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 { + // 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_whitespace_separator(true)?; + write!(self.buf(), "{}{}", comment.wrap_token(), current_line)?; + } + } + + if comment.is_line() { + self.write_whitespace_separator(true)?; + } + + Ok(()) + } + + /// Write multiple comments individually + fn write_comments_individual<'b>( + &mut self, + comments: impl IntoIterator, ) -> Result<()> { let mut comments = comments.into_iter().peekable(); let mut last_byte_written = match comments.peek() { From 971362f0894594e0c6c302d346d964976024c21c Mon Sep 17 00:00:00 2001 From: 0xheartcode <0xheartcode@gmail.com> Date: Mon, 28 Jul 2025 19:39:14 +0200 Subject: [PATCH 2/3] fix(fmt): single line comments tests --- crates/fmt/foundry.toml | 3 ++ .../fmt/testdata/WrapCommentOverflow/fmt.sol | 50 +++++++++++++++++++ .../testdata/WrapCommentOverflow/original.sol | 44 ++++++++++++++++ crates/fmt/tests/formatter.rs | 1 + 4 files changed, 98 insertions(+) create mode 100644 crates/fmt/foundry.toml create mode 100644 crates/fmt/testdata/WrapCommentOverflow/fmt.sol create mode 100644 crates/fmt/testdata/WrapCommentOverflow/original.sol diff --git a/crates/fmt/foundry.toml b/crates/fmt/foundry.toml new file mode 100644 index 0000000000000..81be16870a0b6 --- /dev/null +++ b/crates/fmt/foundry.toml @@ -0,0 +1,3 @@ +[fmt] +line_length = 80 +wrap_comments = true diff --git a/crates/fmt/testdata/WrapCommentOverflow/fmt.sol b/crates/fmt/testdata/WrapCommentOverflow/fmt.sol new file mode 100644 index 0000000000000..f213e1800de7d --- /dev/null +++ b/crates/fmt/testdata/WrapCommentOverflow/fmt.sol @@ -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 {} +} diff --git a/crates/fmt/testdata/WrapCommentOverflow/original.sol b/crates/fmt/testdata/WrapCommentOverflow/original.sol new file mode 100644 index 0000000000000..465d21a268fa8 --- /dev/null +++ b/crates/fmt/testdata/WrapCommentOverflow/original.sol @@ -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 {} +} diff --git a/crates/fmt/tests/formatter.rs b/crates/fmt/tests/formatter.rs index 132b73902d1d9..40dfee9fac70a 100644 --- a/crates/fmt/tests/formatter.rs +++ b/crates/fmt/tests/formatter.rs @@ -238,6 +238,7 @@ test_directories! { BlockComments, BlockCommentsFunction, EnumVariants, + WrapCommentOverflow, } test_dir!(SortedImports, TestConfig::skip_compare_ast_eq()); From 86944cc47a565071a1ac97024fbf7cba4fc96d29 Mon Sep 17 00:00:00 2001 From: 0xheartcode <0xheartcode@gmail.com> Date: Mon, 28 Jul 2025 20:53:14 +0200 Subject: [PATCH 3/3] fix(fmt): cargo fmt & cargo clippy fixed --- crates/fmt/src/formatter.rs | 207 +++++++++++------------------------- 1 file changed, 61 insertions(+), 146 deletions(-) diff --git a/crates/fmt/src/formatter.rs b/crates/fmt/src/formatter.rs index 2c5f5c2f06f45..eea4b1e95cb5e 100644 --- a/crates/fmt/src/formatter.rs +++ b/crates/fmt/src/formatter.rs @@ -590,81 +590,35 @@ impl<'a, W: Write> Formatter<'a, W> { /// 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("+ ") { + 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() { - if first_word.ends_with('.') && - first_word[..first_word.len()-1].chars().all(|c| c.is_ascii_digit()) { - return false; - } + 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 - } - /// Merge consecutive DocLine comments into single logical block - fn merge_consecutive_doc_comments( - comments: Vec<&CommentWithMetadata> - ) -> Vec { - let mut result = Vec::new(); - let mut i = 0; - - while i < comments.len() { - let current_comment = comments[i]; - - if current_comment.ty == CommentType::DocLine { - let mut j = i + 1; - let mut should_merge_group = false; - - // Look ahead for consecutive DocLine comments - while j < comments.len() && - comments[j].ty == CommentType::DocLine && - Self::should_merge_comment_line(comments[j].contents()) { - should_merge_group = true; - j += 1; - } - - if should_merge_group { - let mut merged_content = current_comment.contents().to_string(); - for k in (i + 1)..j { - merged_content.push(' '); - merged_content.push_str(comments[k].contents()); - } - - let mut merged_comment = current_comment.clone(); - merged_comment.comment = format!("/// {}", merged_content); - result.push(merged_comment); - - i = j; - } else { - result.push(current_comment.clone()); - i += 1; - } - } else { - result.push(current_comment.clone()); - i += 1; - } - } - - result + true } /// Write a comment line that might potentially overflow the maximum line length @@ -702,20 +656,11 @@ impl<'a, W: Write> Formatter<'a, W> { if let Some(next) = words.peek() { if !word.is_empty() && !self.will_it_fit(next) { let remaining_text = words.join(" "); - // Check if remaining text should be merged for wrapping - if Self::should_merge_comment_line(&remaining_text) { - // Wrap within same semantic block - self.write_whitespace_separator(true)?; - write!(self.buf(), "{}", comment.wrap_token())?; - self.write_comment_line(comment, &remaining_text)?; - return Ok(true); - } else { - // Don't merge - treat as separate semantic block - self.write_whitespace_separator(true)?; - write!(self.buf(), "{}", comment.wrap_token())?; - self.write_comment_line(comment, &remaining_text)?; - return Ok(true); - } + // Wrap remaining text on next line + self.write_whitespace_separator(true)?; + write!(self.buf(), "{}", comment.wrap_token())?; + self.write_comment_line(comment, &remaining_text)?; + return Ok(true); } self.write_whitespace_separator(false)?; @@ -740,23 +685,24 @@ impl<'a, W: Write> Formatter<'a, W> { &mut self, comments: impl IntoIterator, ) -> Result<()> { - let mut comments: Vec<_> = comments.into_iter().collect(); - + 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()) { + + 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 { @@ -769,76 +715,75 @@ impl<'a, W: Write> Formatter<'a, W> { /// Write comment with overflow to continuation lines fn write_comment_with_overflow_handling( - &mut self, - comments: &[&CommentWithMetadata] + &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); - + 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() { - if comment.has_newline_before && !self.is_beginning_of_line() { - self.write_whitespace_separator(true)?; - } + 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 { + 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() - } - ); - + 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() { @@ -853,50 +798,20 @@ impl<'a, W: Write> Formatter<'a, W> { break; } } - + if is_first_line { - write!(self.buf(), "{}{}", line_prefix, current_line)?; + write!(self.buf(), "{line_prefix}{current_line}")?; is_first_line = false; } else { self.write_whitespace_separator(true)?; - write!(self.buf(), "{}{}", comment.wrap_token(), current_line)?; + write!(self.buf(), "{}{current_line}", comment.wrap_token())?; } } - + if comment.is_line() { self.write_whitespace_separator(true)?; } - - Ok(()) - } - /// Write multiple comments individually - fn write_comments_individual<'b>( - &mut self, - comments: impl IntoIterator, - ) -> 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()) - } else { - comment.loc.end() - }; - } else { - self.write_comment(comment, is_first)?; - } - is_first = false; - } Ok(()) }