Use leading underscore solhint rule for private constants (#4542)

Co-authored-by: Francisco Giordano <fg@frang.io>
OpenZeppelin · Sep 8, 2023 · 28037d5 · 28037d5
1 parent d6426d1
commit 28037d5
Show file tree

Hide file tree

Showing 12 changed files with 37 additions and 39 deletions.
diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
@@ -43,7 +43,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint48 eta;
     }
 
-    bytes32 private constant _ALL_PROPOSAL_STATES_BITMAP = bytes32((2 ** (uint8(type(ProposalState).max) + 1)) - 1);
+    bytes32 private constant ALL_PROPOSAL_STATES_BITMAP = bytes32((2 ** (uint8(type(ProposalState).max) + 1)) - 1);
     string private _name;
 
     mapping(uint256 proposalId => ProposalCore) private _proposals;
@@ -479,7 +479,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
 
         _validateStateBitmap(
             proposalId,
-            _ALL_PROPOSAL_STATES_BITMAP ^
+            ALL_PROPOSAL_STATES_BITMAP ^
                 _encodeStateBitmap(ProposalState.Canceled) ^
                 _encodeStateBitmap(ProposalState.Expired) ^
                 _encodeStateBitmap(ProposalState.Executed)

diff --git a/contracts/governance/utils/Votes.sol b/contracts/governance/utils/Votes.sol
@@ -31,7 +31,7 @@ import {ECDSA} from "../../utils/cryptography/ECDSA.sol";
 abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
     using Checkpoints for Checkpoints.Trace224;
 
-    bytes32 private constant _DELEGATION_TYPEHASH =
+    bytes32 private constant DELEGATION_TYPEHASH =
         keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
 
     mapping(address account => address) private _delegatee;
@@ -150,7 +150,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 {
             revert VotesExpiredSignature(expiry);
         }
         address signer = ECDSA.recover(
-            _hashTypedDataV4(keccak256(abi.encode(_DELEGATION_TYPEHASH, delegatee, nonce, expiry))),
+            _hashTypedDataV4(keccak256(abi.encode(DELEGATION_TYPEHASH, delegatee, nonce, expiry))),
             v,
             r,
             s

diff --git a/contracts/proxy/utils/Initializable.sol b/contracts/proxy/utils/Initializable.sol
@@ -74,8 +74,7 @@ abstract contract Initializable {
     }
 
     // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.Initializable")) - 1))
-    bytes32 private constant _INITIALIZABLE_STORAGE =
-        0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a0e;
+    bytes32 private constant INITIALIZABLE_STORAGE = 0xf0c57e16840df040f15088dc2f81fe391c3923bec73e23a9662efc9c229c6a0e;
 
     /**
      * @dev The contract is already initialized.
@@ -212,7 +211,7 @@ abstract contract Initializable {
     // solhint-disable-next-line var-name-mixedcase
     function _getInitializableStorage() private pure returns (InitializableStorage storage $) {
         assembly {
-            $.slot := _INITIALIZABLE_STORAGE
+            $.slot := INITIALIZABLE_STORAGE
         }
     }
 }
diff --git a/contracts/security/ReentrancyGuard.sol b/contracts/security/ReentrancyGuard.sol
@@ -31,8 +31,8 @@ abstract contract ReentrancyGuard {
     // amount. Since refunds are capped to a percentage of the total
     // transaction's gas, it is best to keep them low in cases like this one, to
     // increase the likelihood of the full refund coming into effect.
-    uint256 private constant _NOT_ENTERED = 1;
-    uint256 private constant _ENTERED = 2;
+    uint256 private constant NOT_ENTERED = 1;
+    uint256 private constant ENTERED = 2;
 
     uint256 private _status;
 
@@ -42,7 +42,7 @@ abstract contract ReentrancyGuard {
     error ReentrancyGuardReentrantCall();
 
     constructor() {
-        _status = _NOT_ENTERED;
+        _status = NOT_ENTERED;
     }
 
     /**
@@ -59,26 +59,26 @@ abstract contract ReentrancyGuard {
     }
 
     function _nonReentrantBefore() private {
-        // On the first call to nonReentrant, _status will be _NOT_ENTERED
-        if (_status == _ENTERED) {
+        // On the first call to nonReentrant, _status will be NOT_ENTERED
+        if (_status == ENTERED) {
             revert ReentrancyGuardReentrantCall();
         }
 
         // Any calls to nonReentrant after this point will fail
-        _status = _ENTERED;
+        _status = ENTERED;
     }
 
     function _nonReentrantAfter() private {
         // By storing the original value once again, a refund is triggered (see
         // https://eips.ethereum.org/EIPS/eip-2200)
-        _status = _NOT_ENTERED;
+        _status = NOT_ENTERED;
     }
 
     /**
      * @dev Returns true if the reentrancy guard is currently set to "entered", which indicates there is a
      * `nonReentrant` function in the call stack.
      */
     function _reentrancyGuardEntered() internal view returns (bool) {
-        return _status == _ENTERED;
+        return _status == ENTERED;
     }
 }
diff --git a/contracts/token/ERC20/extensions/ERC20FlashMint.sol b/contracts/token/ERC20/extensions/ERC20FlashMint.sol
@@ -19,7 +19,7 @@ import {ERC20} from "../ERC20.sol";
  * overriding {maxFlashLoan} so that it correctly reflects the supply cap.
  */
 abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender {
-    bytes32 private constant _RETURN_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan");
+    bytes32 private constant RETURN_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan");
 
     /**
      * @dev The loan token is not valid.
@@ -118,7 +118,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender {
         }
         uint256 fee = flashFee(token, value);
         _mint(address(receiver), value);
-        if (receiver.onFlashLoan(_msgSender(), token, value, fee, data) != _RETURN_VALUE) {
+        if (receiver.onFlashLoan(_msgSender(), token, value, fee, data) != RETURN_VALUE) {
             revert ERC3156InvalidReceiver(address(receiver));
         }
         address flashFeeReceiver = _flashFeeReceiver();

diff --git a/contracts/token/ERC20/extensions/ERC20Permit.sol b/contracts/token/ERC20/extensions/ERC20Permit.sol
@@ -18,8 +18,7 @@ import {Nonces} from "../../../utils/Nonces.sol";
  * need to send a transaction, and thus is not required to hold Ether at all.
  */
 abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
-    // solhint-disable-next-line var-name-mixedcase
-    bytes32 private constant _PERMIT_TYPEHASH =
+    bytes32 private constant PERMIT_TYPEHASH =
         keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
 
     /**
@@ -55,7 +54,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
             revert ERC2612ExpiredSignature(deadline);
         }
 
-        bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline));
+        bytes32 structHash = keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline));
 
         bytes32 hash = _hashTypedDataV4(structHash);
 

diff --git a/contracts/utils/ShortStrings.sol b/contracts/utils/ShortStrings.sol
@@ -39,7 +39,7 @@ type ShortString is bytes32;
  */
 library ShortStrings {
     // Used as an identifier for strings longer than 31 bytes.
-    bytes32 private constant _FALLBACK_SENTINEL = 0x00000000000000000000000000000000000000000000000000000000000000FF;
+    bytes32 private constant FALLBACK_SENTINEL = 0x00000000000000000000000000000000000000000000000000000000000000FF;
 
     error StringTooLong(string str);
     error InvalidShortString();
@@ -91,15 +91,15 @@ library ShortStrings {
             return toShortString(value);
         } else {
             StorageSlot.getStringSlot(store).value = value;
-            return ShortString.wrap(_FALLBACK_SENTINEL);
+            return ShortString.wrap(FALLBACK_SENTINEL);
         }
     }
 
     /**
      * @dev Decode a string that was encoded to `ShortString` or written to storage using {setWithFallback}.
      */
     function toStringWithFallback(ShortString value, string storage store) internal pure returns (string memory) {
-        if (ShortString.unwrap(value) != _FALLBACK_SENTINEL) {
+        if (ShortString.unwrap(value) != FALLBACK_SENTINEL) {
             return toString(value);
         } else {
             return store;
@@ -113,7 +113,7 @@ library ShortStrings {
      * actual characters as the UTF-8 encoding of a single character can span over multiple bytes.
      */
     function byteLengthWithFallback(ShortString value, string storage store) internal view returns (uint256) {
-        if (ShortString.unwrap(value) != _FALLBACK_SENTINEL) {
+        if (ShortString.unwrap(value) != FALLBACK_SENTINEL) {
             return byteLength(value);
         } else {
             return bytes(store).length;

diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol
@@ -10,8 +10,8 @@ import {SignedMath} from "./math/SignedMath.sol";
  * @dev String operations.
  */
 library Strings {
-    bytes16 private constant _HEX_DIGITS = "0123456789abcdef";
-    uint8 private constant _ADDRESS_LENGTH = 20;
+    bytes16 private constant HEX_DIGITS = "0123456789abcdef";
+    uint8 private constant ADDRESS_LENGTH = 20;
 
     /**
      * @dev The `value` string doesn't fit in the specified `length`.
@@ -34,7 +34,7 @@ library Strings {
                 ptr--;
                 /// @solidity memory-safe-assembly
                 assembly {
-                    mstore8(ptr, byte(mod(value, 10), _HEX_DIGITS))
+                    mstore8(ptr, byte(mod(value, 10), HEX_DIGITS))
                 }
                 value /= 10;
                 if (value == 0) break;
@@ -68,7 +68,7 @@ library Strings {
         buffer[0] = "0";
         buffer[1] = "x";
         for (uint256 i = 2 * length + 1; i > 1; --i) {
-            buffer[i] = _HEX_DIGITS[localValue & 0xf];
+            buffer[i] = HEX_DIGITS[localValue & 0xf];
             localValue >>= 4;
         }
         if (localValue != 0) {
@@ -81,7 +81,7 @@ library Strings {
      * @dev Converts an `address` with fixed length of 20 bytes to its not checksummed ASCII `string` hexadecimal representation.
      */
     function toHexString(address addr) internal pure returns (string memory) {
-        return toHexString(uint256(uint160(addr)), _ADDRESS_LENGTH);
+        return toHexString(uint256(uint160(addr)), ADDRESS_LENGTH);
     }
 
     /**

diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol
@@ -34,7 +34,7 @@ import {IERC5267} from "../../interfaces/IERC5267.sol";
 abstract contract EIP712 is IERC5267 {
     using ShortStrings for *;
 
-    bytes32 private constant _TYPE_HASH =
+    bytes32 private constant TYPE_HASH =
         keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
 
     // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to
@@ -86,7 +86,7 @@ abstract contract EIP712 is IERC5267 {
     }
 
     function _buildDomainSeparator() private view returns (bytes32) {
-        return keccak256(abi.encode(_TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this)));
+        return keccak256(abi.encode(TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this)));
     }
 
     /**

diff --git a/contracts/utils/introspection/ERC165Checker.sol b/contracts/utils/introspection/ERC165Checker.sol
@@ -14,7 +14,7 @@ import {IERC165} from "./IERC165.sol";
  */
 library ERC165Checker {
     // As per the EIP-165 spec, no interface should ever match 0xffffffff
-    bytes4 private constant _INTERFACE_ID_INVALID = 0xffffffff;
+    bytes4 private constant INTERFACE_ID_INVALID = 0xffffffff;
 
     /**
      * @dev Returns true if `account` supports the {IERC165} interface.
@@ -24,7 +24,7 @@ library ERC165Checker {
         // InterfaceId_ERC165 and explicitly indicate non-support of InterfaceId_Invalid
         return
             supportsERC165InterfaceUnchecked(account, type(IERC165).interfaceId) &&
-            !supportsERC165InterfaceUnchecked(account, _INTERFACE_ID_INVALID);
+            !supportsERC165InterfaceUnchecked(account, INTERFACE_ID_INVALID);
     }
 
     /**

diff --git a/scripts/solhint-custom/index.js b/scripts/solhint-custom/index.js
@@ -48,9 +48,9 @@ module.exports = [
 
     VariableDeclaration(node) {
       if (node.isDeclaredConst) {
-        if (/^_/.test(node.name)) {
-          // TODO: re-enable and fix
-          // this.error(node, 'Constant variables should not have leading underscore');
+        // TODO: expand visibility and fix
+        if (node.visibility === 'private' && /^_/.test(node.name)) {
+          this.error(node, 'Constant variables should not have leading underscore');
         }
       } else if (node.visibility === 'private' && !/^_/.test(node.name)) {
         this.error(node, 'Non-constant private variables must have leading underscore');

diff --git a/scripts/upgradeable/upgradeable.patch b/scripts/upgradeable/upgradeable.patch
@@ -151,7 +151,7 @@ index 3800804a..90c1db78 100644
  abstract contract EIP712 is IERC5267 {
 -    using ShortStrings for *;
 -
-     bytes32 private constant _TYPE_HASH =
+     bytes32 private constant TYPE_HASH =
          keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
 
 -    // Cache the domain separator as an immutable value, but also store the chain id that it corresponds to, in order to
@@ -207,8 +207,8 @@ index 3800804a..90c1db78 100644
      }
 
      function _buildDomainSeparator() private view returns (bytes32) {
--        return keccak256(abi.encode(_TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this)));
-+        return keccak256(abi.encode(_TYPE_HASH, _EIP712NameHash(), _EIP712VersionHash(), block.chainid, address(this)));
+-        return keccak256(abi.encode(TYPE_HASH, _hashedName, _hashedVersion, block.chainid, address(this)));
++        return keccak256(abi.encode(TYPE_HASH, _EIP712NameHash(), _EIP712VersionHash(), block.chainid, address(this)));
      }
 
      /**