Improving printing performance

[Janther]

Removing groups that will always break:

• UncheckedStatement
• EnumDefinition
• ModifierDefinition

Removing group that just affect a closing bracket ) or ] that will be outside the tabWidth

• TupleExpression

Member Access Chain is already properly grouped

• MemberAccess

Just reordering the group indent calls:

• FunctionDefinition
• FunctionTypeName

Prefer using shouldBreak at the group level instead of choosing between hardline and line

• IfStatement

The rest is wrapping strings together whenever possible.
Finally I rebased some functions that would push and concat into a variable and returned an built array instead.

[Janther]

if this functions always returns an array, then we can always ask for comments.length instead of comments?.length

[fvictorio]

This results in some code changes in the OpenZeppelin repository:

diff --git a/contracts/access/extensions/AccessControlDefaultAdminRules.sol b/contracts/access/extensions/AccessControlDefaultAdminRules.sol
index ef71a648..73a8b66b 100644
--- a/contracts/access/extensions/AccessControlDefaultAdminRules.sol
+++ b/contracts/access/extensions/AccessControlDefaultAdminRules.sol
@@ -183,7 +183,7 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      */
     function defaultAdminDelay() public view virtual returns (uint48) {
         uint48 schedule = _pendingDelaySchedule;
-        return (_isScheduleSet(schedule) && _hasSchedulePassed(schedule)) ? _pendingDelay : _currentDelay;
+        return _isScheduleSet(schedule) && _hasSchedulePassed(schedule) ? _pendingDelay : _currentDelay;
     }
 
     /**
@@ -191,7 +191,7 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      */
     function pendingDefaultAdminDelay() public view virtual returns (uint48 newDelay, uint48 schedule) {
         schedule = _pendingDelaySchedule;
-        return (_isScheduleSet(schedule) && !_hasSchedulePassed(schedule)) ? (_pendingDelay, schedule) : (0, 0);
+        return _isScheduleSet(schedule) && !_hasSchedulePassed(schedule) ? (_pendingDelay, schedule) : (0, 0);
     }
 
     /**
diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/contracts/governance/extensions/GovernorTimelockCompound.sol
index 77cf369f..2837ca80 100644
--- a/contracts/governance/extensions/GovernorTimelockCompound.sol
+++ b/contracts/governance/extensions/GovernorTimelockCompound.sol
@@ -40,8 +40,8 @@ abstract contract GovernorTimelockCompound is Governor {
         ProposalState currentState = super.state(proposalId);
 
         return
-            (currentState == ProposalState.Queued &&
-                block.timestamp >= proposalEta(proposalId) + _timelock.GRACE_PERIOD())
+            currentState == ProposalState.Queued &&
+                block.timestamp >= proposalEta(proposalId) + _timelock.GRACE_PERIOD()
                 ? ProposalState.Expired
                 : currentState;
     }
diff --git a/lib/forge-std b/lib/forge-std
index c2236853..eb980e1d 160000
--- a/lib/forge-std
+++ b/lib/forge-std
@@ -1 +1 @@
-Subproject commit c2236853aadb8e2d9909bbecdc490099519b70a4
+Subproject commit eb980e1d4f0e8173ec27da77297ae411840c8ccb

Maybe we are missing tests for ternaries whose condition is inside parentheses?

Also, I used hyperfine to benchmark this change, and these are the results (again, in OZ) before and after the change:

// Before
  Time (mean ± σ):     11.554 s ±  0.315 s    [User: 16.764 s, System: 3.959 s]
  Range (min … max):   10.990 s … 11.857 s    10 runs


// After
  Time (mean ± σ):     11.458 s ±  0.246 s    [User: 16.539 s, System: 3.937 s]
  Range (min … max):   11.024 s … 11.824 s    10 runs

So I don't think it has an impact, at least in that repo. That doesn't mean I'm against this change though, because it simplifies things anyway.

[Janther]

Thank you for this,
after this comment addressing the same issue I decided to keep the original behaviour although in the future if prettier decides to adopt experimentalTernaries as the default formatting choice and we decide to follow that choice we will need to sometimes add parentheses the thing will be when to remove them.

But that's for the future.

[Janther]

All of the changes of this PR will be included in the TypeScript #968 step by step migration so I decided to close this one in favor to that one