Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions .changelog/fmt-forloop-comment-corruption.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
forge: patch
---

Fixed `forge fmt` silently corrupting `for` loops that have a trailing `//` comment on or after the header's closing brace, which could destroy source code with no error or warning.
18 changes: 15 additions & 3 deletions crates/fmt/src/state/sol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2006,12 +2006,22 @@ impl<'ast> State<'_, 'ast> {
/// Prints the given statement in the source code, handling formatting, inline documentation,
/// trailing comments and layout logic for various statement kinds.
fn print_stmt(&mut self, stmt: &'ast ast::Stmt<'ast>) {
self.print_stmt_bound(stmt, None);
}

/// Like [`Self::print_stmt`], but bounds the statement's own trailing-comment scan by
/// `next_pos` instead of scanning unbounded. Every caller other than a `for`-loop's init
/// clause prints one statement per line, where an unbounded scan is harmless because
/// nothing else follows on that line. A `for`-loop's init clause is different: it shares
/// its source line with the condition/increment clauses that get printed afterwards, so an
/// unbounded scan can swallow those clauses into the comment (see `print_for_stmt`).
Comment thread
figtracer marked this conversation as resolved.
Outdated
fn print_stmt_bound(&mut self, stmt: &'ast ast::Stmt<'ast>, next_pos: Option<BytePos>) {
let ast::Stmt { ref docs, span, ref kind } = *stmt;
self.print_docs(docs);

// Handle disabled statements.
if self.handle_span(span, false) {
self.print_trailing_comment_no_break(stmt.span.hi(), None);
self.print_trailing_comment_no_break(stmt.span.hi(), next_pos);
return;
}

Expand Down Expand Up @@ -2080,7 +2090,7 @@ impl<'ast> State<'_, 'ast> {
stmt.span.hi(),
CommentConfig::default().trailing_no_break().mixed_no_break().mixed_prev_space(),
);
self.print_trailing_comment_no_break(stmt.span.hi(), None);
self.print_trailing_comment_no_break(stmt.span.hi(), next_pos);
}

/// Prints an `assembly` statement, including optional dialect and flags,
Expand Down Expand Up @@ -2180,7 +2190,9 @@ impl<'ast> State<'_, 'ast> {
// Print init.
self.s.cbox(0);
match init {
Some(init_stmt) => self.print_stmt(init_stmt),
Some(init_stmt) => {
self.print_stmt_bound(init_stmt, Some(init_stmt.span.hi()));
}
None => self.print_word(";"),
}

Expand Down
49 changes: 49 additions & 0 deletions crates/fmt/testdata/ForStatementComments/fmt.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
pragma solidity ^0.8.8;

contract ForStatementComments {
function bracedTrailingComment() external {
uint256 x;
for (uint256 i = 0; i < 10; ++i) {
// step
x++;
}
}

function bracelessTrailingComment() external {
uint256 x;
for (uint256 i = 0; i < 10; ++i) {
x++; // step
}
}

function emptyBodyTrailingComment() external {
for (uint256 i = 0; i < 10; ++i) { // step
}
}

function leadingCommentUnaffected() external {
// leading comment, must not move
for (uint256 i = 0; i < 10; ++i) {
i;
}
}

function commentAfterHeaderNoCondNoNext() external {
for (uint256 i = 0;;) // after header
{}
}

function missingConditionTrailingComment() external {
for (uint256 i = 0;; ++i) {
// c
i;
}
}

function missingIncrementTrailingComment() external {
for (uint256 i = 0; i < 10;) {
// c
i;
}
}
}
44 changes: 44 additions & 0 deletions crates/fmt/testdata/ForStatementComments/original.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
pragma solidity ^0.8.8;

contract ForStatementComments {
function bracedTrailingComment() external {
uint256 x;
for (uint256 i = 0; i < 10; ++i) { // step
x++;
}
}

function bracelessTrailingComment() external {
uint256 x;
for (uint256 i = 0; i < 10; ++i) x++; // step
}

function emptyBodyTrailingComment() external {
for (uint256 i = 0; i < 10; ++i) { // step
}
}

function leadingCommentUnaffected() external {
// leading comment, must not move
for (uint256 i = 0; i < 10; ++i) {
i;
}
}

function commentAfterHeaderNoCondNoNext() external {
for (uint256 i = 0;;) // after header
{}
}

function missingConditionTrailingComment() external {
for (uint256 i = 0; ; ++i) { // c
i;
}
}

function missingIncrementTrailingComment() external {
for (uint256 i = 0; i < 10;) { // c
i;
}
}
}
1 change: 1 addition & 0 deletions crates/fmt/tests/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ fmt_tests! {
ErrorDefinition,
EventDefinition,
ForStatement,
ForStatementComments,
FunctionCall,
FunctionCallArgsStatement,
FunctionDefinition,
Expand Down