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

fix(forge): format new expressions #6408

Merged
merged 7 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
54 changes: 29 additions & 25 deletions crates/fmt/src/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,12 @@ impl<'a, W: Write> Formatter<'a, W> {
/// Does the next written character require whitespace before
fn next_char_needs_space(&self, next_char: char) -> bool {
if self.is_beginning_of_line() {
return false
return false;
}
let last_char =
if let Some(last_char) = self.last_char() { last_char } else { return false };
if last_char.is_whitespace() || next_char.is_whitespace() {
return false
return false;
}
match last_char {
'{' => match next_char {
Expand All @@ -231,10 +231,10 @@ impl<'a, W: Write> Formatter<'a, W> {
fn will_it_fit(&self, text: impl AsRef<str>) -> bool {
let text = text.as_ref();
if text.is_empty() {
return true
return true;
}
if text.contains('\n') {
return false
return false;
}
let space: usize = self.next_char_needs_space(text.chars().next().unwrap()).into();
self.config.line_length >=
Expand Down Expand Up @@ -446,7 +446,7 @@ impl<'a, W: Write> Formatter<'a, W> {
/// or if the comment are wrapped
fn write_comment(&mut self, comment: &CommentWithMetadata, is_first: bool) -> Result<()> {
if self.inline_config.is_disabled(comment.loc) {
return self.write_raw_comment(comment)
return self.write_raw_comment(comment);
}

match comment.position {
Expand Down Expand Up @@ -474,7 +474,7 @@ impl<'a, W: Write> Formatter<'a, W> {
lines.try_for_each(|l| self.write_doc_block_line(comment, l))?;
write!(self.buf(), " {}", comment.end_token().unwrap())?;
self.write_preserved_line()?;
return Ok(())
return Ok(());
}

write!(self.buf(), "{}", comment.start_token())?;
Expand Down Expand Up @@ -549,7 +549,7 @@ impl<'a, W: Write> Formatter<'a, W> {
write!(self.buf(), " *{}", if needs_space { " " } else { "" })?;
self.write_comment_line(comment, line)?;
self.write_whitespace_separator(true)?;
return Ok(())
return Ok(());
}

let indent_whitespace_count = line
Expand All @@ -571,7 +571,7 @@ impl<'a, W: Write> Formatter<'a, W> {
line.chars().next().map(|ch| ch.is_whitespace()).unwrap_or_default();
if !self.is_beginning_of_line() || !start_with_ws {
write!(self.buf(), "{line}")?;
return Ok(false)
return Ok(false);
}

// if this is the beginning of the line,
Expand All @@ -583,7 +583,7 @@ impl<'a, W: Write> Formatter<'a, W> {
.map(|(_, ch)| ch);
let padded = format!("{}{}", " ".repeat(indent), chars.join(""));
self.write_raw(padded)?;
return Ok(false)
return Ok(false);
}

let mut words = line.split(' ').peekable();
Expand All @@ -602,7 +602,7 @@ impl<'a, W: Write> Formatter<'a, W> {
// write newline wrap token
write!(self.buf(), "{}", comment.wrap_token())?;
self.write_comment_line(comment, &words.join(" "))?;
return Ok(true)
return Ok(true);
}

self.write_whitespace_separator(false)?;
Expand Down Expand Up @@ -1074,14 +1074,14 @@ impl<'a, W: Write> Formatter<'a, W> {
/// expression decide how to split itself on multiple lines
fn visit_assignment(&mut self, expr: &mut Expression) -> Result<()> {
if self.try_on_single_line(|fmt| expr.visit(fmt))? {
return Ok(())
return Ok(());
}

self.write_postfix_comments_before(expr.loc().start())?;
self.write_prefix_comments_before(expr.loc().start())?;

if self.try_on_single_line(|fmt| fmt.indented(1, |fmt| expr.visit(fmt)))? {
return Ok(())
return Ok(());
}

let mut fit_on_next_line = false;
Expand Down Expand Up @@ -1173,7 +1173,7 @@ impl<'a, W: Write> Formatter<'a, W> {
})?;

if fits_on_single {
return Ok(true)
return Ok(true);
}
}

Expand Down Expand Up @@ -1337,7 +1337,7 @@ impl<'a, W: Write> Formatter<'a, W> {

let add_underscores = |string: &str, reversed: bool| -> String {
if !matches!(config, NumberUnderscore::Thousands) || string.len() < 5 {
return string.to_string()
return string.to_string();
}
if reversed {
Box::new(string.as_bytes().chunks(3)) as Box<dyn Iterator<Item = &[u8]>>
Expand Down Expand Up @@ -1741,7 +1741,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
fmt.write_postfix_comments_before(first.loc().start())?;
fmt.write_whitespace_separator(true)?;
} else {
return Ok(())
return Ok(());
}

if fmt.config.contract_new_lines {
Expand Down Expand Up @@ -1877,7 +1877,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
fmt.write_semicolon()?;
Ok(())
})?;
return Ok(())
return Ok(());
}

let imports_start = imports.first().unwrap().0.loc.start();
Expand Down Expand Up @@ -2286,6 +2286,10 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
expr.visit(self)?;
stmt.visit(self)?;
}
Expression::New(_, expr) => {
write_chunk!(self, "new ")?;
self.visit_expr(expr.loc(), expr)?;
}
_ => self.visit_source(loc)?,
};

Expand All @@ -2302,7 +2306,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
#[instrument(name = "ident_path", skip_all)]
fn visit_ident_path(&mut self, idents: &mut IdentifierPath) -> Result<(), Self::Error> {
if idents.identifiers.is_empty() {
return Ok(())
return Ok(());
}
return_source_if_disabled!(self, idents.loc);

Expand Down Expand Up @@ -2472,7 +2476,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
name.content.push_str("()");
}
self.write_chunk(&name)?;
return Ok(())
return Ok(());
}

let args = base.args.as_mut().unwrap();
Expand Down Expand Up @@ -2526,7 +2530,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
write_chunk!(fmt, struct_name.loc.start(), "struct")?;
struct_name.visit(fmt)?;
if structure.fields.is_empty() {
return fmt.write_empty_brackets()
return fmt.write_empty_brackets();
}

write!(fmt.buf(), " {{")?;
Expand Down Expand Up @@ -2978,7 +2982,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {

if !is_first_stmt {
self.write_if_stmt(loc, cond, if_branch, else_branch)?;
return Ok(())
return Ok(());
}

self.context.if_stmt_single_line = Some(true);
Expand Down Expand Up @@ -3088,7 +3092,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {

if args.is_empty() {
write!(self.buf(), "({{}});")?;
return Ok(())
return Ok(());
}

write!(self.buf(), "(")?;
Expand All @@ -3108,7 +3112,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {

if expr.is_none() {
write_chunk!(self, loc.end(), "return;")?;
return Ok(())
return Ok(());
}

let expr = expr.as_mut().unwrap();
Expand All @@ -3125,7 +3129,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
expr.visit(fmt)
})?;
if fits_on_single {
return Ok(())
return Ok(());
}

let mut fit_on_next_line = false;
Expand All @@ -3142,7 +3146,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
})?;
if fit_on_next_line {
tx.commit()?;
return Ok(())
return Ok(());
}

write_return(fmt)?;
Expand Down Expand Up @@ -3230,7 +3234,7 @@ impl<'a, W: Write> Visitor for Formatter<'a, W> {
let multiline = self.are_chunks_separated_multiline("{}", &chunks, "")?;
if !multiline {
self.write_chunks_separated(&chunks, "", false)?;
return Ok(())
return Ok(());
}

let mut chunks = chunks.iter_mut().peekable();
Expand Down
2 changes: 1 addition & 1 deletion crates/fmt/testdata/ArrayExpressions/fmt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ contract ArrayExpressions {
uint256[10] memory sample;

uint256 length = 10;
uint256[] memory sample2 = new uint[](length);
uint256[] memory sample2 = new uint256[](length);

uint256[] /* comment1 */ memory /* comment2 */ sample3; // comment3

Expand Down
3 changes: 2 additions & 1 deletion crates/fmt/testdata/ArrayExpressions/original.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ contract ArrayExpressions {
uint[10] memory sample;

uint256 length = 10;
uint[] memory sample2 = new uint[](length);
uint[] memory sample2 = new uint[](
length);

uint /* comment1 */ [] memory /* comment2 */ sample3 // comment3
;
Expand Down
5 changes: 5 additions & 0 deletions crates/fmt/testdata/FunctionCall/bracket-spacing.fmt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,13 @@ contract FunctionCall {

function a(uint256 foo) {
foo;
MyContract c = new MyContract(address(0), hex"beef");
}

function b() {
a({ foo: 5 });
}

contract MyContract {
constructor(address arg, bytes memory data) { }
}
5 changes: 5 additions & 0 deletions crates/fmt/testdata/FunctionCall/fmt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ contract FunctionCall {

function a(uint256 foo) {
foo;
MyContract c = new MyContract(address(0), hex"beef");
}

function b() {
a({foo: 5});
}

contract MyContract {
constructor(address arg, bytes memory data) {}
}
5 changes: 5 additions & 0 deletions crates/fmt/testdata/FunctionCall/original.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@ contract FunctionCall {

function a(uint256 foo) {
foo;
MyContract c = new MyContract(address( 0),hex"beef");
}

function b() {
a( {foo: 5} );
}

contract MyContract {
constructor(address arg, bytes memory data) {}
}
4 changes: 2 additions & 2 deletions crates/fmt/testdata/FunctionType/fmt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ library ArrayUtils {
pure
returns (uint256[] memory r)
{
r = new uint[](self.length);
r = new uint256[](self.length);
for (uint256 i = 0; i < self.length; i++) {
r[i] = f(self[i]);
}
Expand All @@ -23,7 +23,7 @@ library ArrayUtils {
}

function range(uint256 length) internal pure returns (uint256[] memory r) {
r = new uint[](length);
r = new uint256[](length);
for (uint256 i = 0; i < r.length; i++) {
r[i] = i;
}
Expand Down
4 changes: 2 additions & 2 deletions crates/fmt/testdata/FunctionType/original.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ library ArrayUtils {
uint[] memory r
)
{
r = new uint[](self.length);
r = new uint[]( self.length);
for (uint i = 0; i < self.length; i++) {
r[i] = f(self[i]);
}
Expand All @@ -23,7 +23,7 @@ library ArrayUtils {
}

function range(uint256 length) internal pure returns (uint[] memory r) {
r = new uint[](length);
r = new uint256[](length );
for (uint i = 0; i < r.length; i++) {
r[i] = i;
}
Expand Down
15 changes: 3 additions & 12 deletions testdata/cheats/Prank.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,7 @@ contract PrankTest is DSTest {
function testPrankConstructorSender(address sender) public {
vm.prank(sender);
ConstructorVictim victim = new ConstructorVictim(
sender,
"msg.sender was not set during prank",
tx.origin,
"tx.origin invariant failed"
sender, "msg.sender was not set during prank", tx.origin, "tx.origin invariant failed"
);

// Ensure we cleaned up correctly
Expand All @@ -290,10 +287,7 @@ contract PrankTest is DSTest {
// Perform the prank
vm.prank(sender, origin);
ConstructorVictim victim = new ConstructorVictim(
sender,
"msg.sender was not set during prank",
origin,
"tx.origin was not set during prank"
sender, "msg.sender was not set during prank", origin, "tx.origin was not set during prank"
);

// Ensure we cleaned up correctly
Expand Down Expand Up @@ -329,10 +323,7 @@ contract PrankTest is DSTest {
// Perform the prank
vm.startPrank(sender, origin);
ConstructorVictim victim = new ConstructorVictim(
sender,
"msg.sender was not set during prank",
origin,
"tx.origin was not set during prank"
sender, "msg.sender was not set during prank", origin, "tx.origin was not set during prank"
);
new ConstructorVictim(
sender,
Expand Down
6 changes: 3 additions & 3 deletions testdata/cheats/RecordAccountAccesses.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ contract RecordAccountAccessesTest is DSTest {
(succ,) = address(123469).call("hello world");
(succ,) = address(5678).call("");
// contract calls to self in constructor
SelfCaller caller = new SelfCaller{value: 2 ether}('hello2 world2');
SelfCaller caller = new SelfCaller{value: 2 ether}("hello2 world2");

Vm.AccountAccess[] memory called = cheats.stopAndReturnStateDiff();
assertEq(called.length, 6);
Expand Down Expand Up @@ -1011,8 +1011,8 @@ contract RecordAccountAccessesTest is DSTest {
function testSelfDestruct() public {
uint256 startingBalance = address(this).balance;
this.startRecordingFromLowerDepth();
address a = address(new SelfDestructor{value:1 ether}(address(this)));
address b = address(new SelfDestructor{value:1 ether}(address(bytes20("doesn't exist yet"))));
address a = address(new SelfDestructor{value: 1 ether}(address(this)));
address b = address(new SelfDestructor{value: 1 ether}(address(bytes20("doesn't exist yet"))));
Vm.AccountAccess[] memory called = cheats.stopAndReturnStateDiff();
assertEq(called.length, 5, "incorrect length");
assertEq(
Expand Down