fix(ci): apply return arg fmt rule to tests (#12546)

Applies the semgrep rule for return-arg-fmt to Solidity tests.

packages/contracts-bedrock/test/L1/ResourceMetering.t.sol

@@ -374,8 +374,8 @@ contract ArtifactResourceMetering_Test is Test {
                                 // Call the metering code and catch the various
                                 // types of errors.
                                 uint256 gasConsumed = 0;
-                                try meter.use{ gas: 30_000_000 }(requestedGas) returns (uint256 _gasConsumed) {
-                                    gasConsumed = _gasConsumed;
+                                try meter.use{ gas: 30_000_000 }(requestedGas) returns (uint256 gasConsumed_) {
+                                    gasConsumed = gasConsumed_;
                                 } catch (bytes memory err) {
                                     bytes32 hash = keccak256(err);
                                     if (hash == cannotBuyMoreGas) {

packages/contracts-bedrock/test/L2/L2Genesis.t.sol

@@ -170,7 +170,7 @@ contract L2GenesisTest is Test {
     }
 
     /// @notice Creates mock L1Dependencies for testing purposes.
-    function _dummyL1Deps() internal pure returns (L1Dependencies memory _deps) {
+    function _dummyL1Deps() internal pure returns (L1Dependencies memory deps_) {
         return L1Dependencies({
             l1CrossDomainMessengerProxy: payable(address(0x100000)),
             l1StandardBridgeProxy: payable(address(0x100001)),

packages/contracts-bedrock/test/cannon/MIPS.t.sol

@@ -1801,22 +1801,22 @@ contract MIPS_Test is CommonTest {
         uint32 val
     )
         internal
-        returns (IMIPS.State memory state, bytes memory proof)
+        returns (IMIPS.State memory state_, bytes memory proof_)
     {
-        (state.memRoot, proof) = ffi.getCannonMemoryProof(pc, insn, addr, val);
-        state.pc = pc;
-        state.nextPC = pc + 4;
+        (state_.memRoot, proof_) = ffi.getCannonMemoryProof(pc, insn, addr, val);
+        state_.pc = pc;
+        state_.nextPC = pc + 4;
     }
 
-    function encodeitype(uint8 opcode, uint8 rs, uint8 rt, uint16 imm) internal pure returns (uint32 insn) {
-        insn = uint32(opcode) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | imm;
+    function encodeitype(uint8 opcode, uint8 rs, uint8 rt, uint16 imm) internal pure returns (uint32 insn_) {
+        insn_ = uint32(opcode) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | imm;
     }
 
-    function encodespec(uint8 rs, uint8 rt, uint8 rd, uint16 funct) internal pure returns (uint32 insn) {
-        insn = uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
+    function encodespec(uint8 rs, uint8 rt, uint8 rd, uint16 funct) internal pure returns (uint32 insn_) {
+        insn_ = uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
     }
 
-    function encodespec2(uint8 rs, uint8 rt, uint8 rd, uint8 funct) internal pure returns (uint32 insn) {
-        insn = uint32(28) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
+    function encodespec2(uint8 rs, uint8 rt, uint8 rd, uint8 funct) internal pure returns (uint32 insn_) {
+        insn_ = uint32(28) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
     }
 }

packages/contracts-bedrock/test/cannon/MIPS2.t.sol

@@ -2832,16 +2832,16 @@ contract MIPS2_Test is CommonTest {
         }
     }
 
-    function encodeitype(uint8 opcode, uint8 rs, uint8 rt, uint16 imm) internal pure returns (uint32 insn) {
-        insn = uint32(opcode) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | imm;
+    function encodeitype(uint8 opcode, uint8 rs, uint8 rt, uint16 imm) internal pure returns (uint32 insn_) {
+        insn_ = uint32(opcode) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | imm;
     }
 
-    function encodespec(uint8 rs, uint8 rt, uint8 rd, uint16 funct) internal pure returns (uint32 insn) {
-        insn = uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
+    function encodespec(uint8 rs, uint8 rt, uint8 rd, uint16 funct) internal pure returns (uint32 insn_) {
+        insn_ = uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
     }
 
-    function encodespec2(uint8 rs, uint8 rt, uint8 rd, uint8 funct) internal pure returns (uint32 insn) {
-        insn = uint32(28) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
+    function encodespec2(uint8 rs, uint8 rt, uint8 rd, uint8 funct) internal pure returns (uint32 insn_) {
+        insn_ = uint32(28) << 26 | uint32(rs) << 21 | uint32(rt) << 16 | uint32(rd) << 11 | uint32(funct);
     }
 }
 

packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/handlers/Protocol.t.sol

@@ -151,7 +151,7 @@ contract ProtocolHandler is TestBase, StdUtils, Actors {
         uint256 chainId
     )
         internal
-        returns (OptimismSuperchainERC20 supertoken)
+        returns (OptimismSuperchainERC20 supertoken_)
     {
         // this salt would be used in production. Tokens sharing it will be bridgable with each other
         bytes32 realSalt = keccak256(abi.encode(remoteToken, name, symbol, decimals));
@@ -162,7 +162,7 @@ contract ProtocolHandler is TestBase, StdUtils, Actors {
         // what we use in the tests to walk around two contracts needing two different addresses
         // tbf we could be using CREATE1, but this feels more verbose
         bytes32 hackySalt = keccak256(abi.encode(remoteToken, name, symbol, decimals, chainId));
-        supertoken = OptimismSuperchainERC20(
+        supertoken_ = OptimismSuperchainERC20(
             address(
                 // TODO: Use the OptimismSuperchainERC20 Beacon Proxy
                 new ERC1967Proxy{ salt: hackySalt }(
@@ -171,7 +171,7 @@ contract ProtocolHandler is TestBase, StdUtils, Actors {
                 )
             )
         );
-        MESSENGER.registerSupertoken(realSalt, chainId, address(supertoken));
-        allSuperTokens.push(address(supertoken));
+        MESSENGER.registerSupertoken(realSalt, chainId, address(supertoken_));
+        allSuperTokens.push(address(supertoken_));
     }
 }

packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/helpers/HandlerGetters.t.sol

@@ -7,16 +7,16 @@ import { EnumerableMap } from "@openzeppelin/contracts/utils/structs/EnumerableM
 contract HandlerGetters is ProtocolHandler {
     using EnumerableMap for EnumerableMap.Bytes32ToUintMap;
 
-    function deploySaltsLength() external view returns (uint256 length) {
+    function deploySaltsLength() external view returns (uint256 length_) {
         return ghost_totalSupplyAcrossChains.length();
     }
 
-    function totalSupplyAcrossChainsAtIndex(uint256 index) external view returns (bytes32 salt, uint256 supply) {
-        return ghost_totalSupplyAcrossChains.at(index);
+    function totalSupplyAcrossChainsAtIndex(uint256 _index) external view returns (bytes32 salt_, uint256 supply_) {
+        return ghost_totalSupplyAcrossChains.at(_index);
     }
 
-    function tokensInTransitForDeploySalt(bytes32 salt) external view returns (uint256 amount) {
-        (, amount) = ghost_tokensInTransit.tryGet(salt);
-        return amount;
+    function tokensInTransitForDeploySalt(bytes32 _salt) external view returns (uint256 amount_) {
+        (, amount_) = ghost_tokensInTransit.tryGet(_salt);
+        return amount_;
     }
 }

packages/contracts-bedrock/test/invariants/OptimismSuperchainERC20/helpers/MockL2ToL2CrossDomainMessenger.t.sol

@@ -112,7 +112,7 @@ contract MockL2ToL2CrossDomainMessenger {
     //  Internal helpers  //
     ////////////////////////
 
-    function _decodePayload(bytes calldata payload) internal pure returns (address recipient, uint256 amount) {
-        (, recipient, amount) = abi.decode(payload[4:], (address, address, uint256));
+    function _decodePayload(bytes calldata payload) internal pure returns (address recipient_, uint256 amount_) {
+        (, recipient_, amount_) = abi.decode(payload[4:], (address, address, uint256));
     }
 }

packages/contracts-bedrock/test/mocks/TestERC1271Wallet.sol

@@ -13,7 +13,15 @@ contract TestERC1271Wallet is Ownable, IERC1271 {
         transferOwnership(originalOwner);
     }
 
-    function isValidSignature(bytes32 hash, bytes memory signature) public view override returns (bytes4 magicValue) {
-        return ECDSA.recover(hash, signature) == owner() ? this.isValidSignature.selector : bytes4(0);
+    function isValidSignature(
+        bytes32 _hash,
+        bytes memory _signature
+    )
+        public
+        view
+        override
+        returns (bytes4 magicValue_)
+    {
+        return ECDSA.recover(_hash, _signature) == owner() ? this.isValidSignature.selector : bytes4(0);
     }
 }

semgrep/sol-rules.yaml

@@ -65,7 +65,7 @@ rules:
     paths:
       exclude:
         - op-chain-ops/script/testdata/scripts/ScriptExample.s.sol
-        - packages/contracts-bedrock/test
+        - packages/contracts-bedrock/test/safe-tools
         - packages/contracts-bedrock/scripts/libraries/Solarray.sol
         - packages/contracts-bedrock/scripts/interfaces/IGnosisSafe.sol
         - packages/contracts-bedrock/src/dispute/interfaces/IPermissionedDisputeGame.sol