Add a leading underscore to internal and private functions. (#1257)

* Add a leading underscore to internal and private functions.

Fixes #1176

* Remove super

* update the ERC721 changes

* add missing underscore after merge

* Fix mock

Author: Leo Arias

CODE_STYLE.md

@@ -43,8 +43,22 @@ Any exception or additions specific to our project are documented below.
 
     ```
     function test() {
-       uint256 functionVar;
-       ...
+      uint256 functionVar;
+      ...
+    }
+    ```
+
+* Internal and private functions should have an underscore prefix.
+
+    ```
+    function _testInternal() internal {
+      ...
+    }
+    ```
+
+    ```
+    function _testPrivate() private {
+      ...
     }
     ```
 

contracts/access/SignatureBouncer.sol

@@ -17,7 +17,7 @@ import "../cryptography/ECDSA.sol";
  * valid addresses on-chain, simply sign a grant of the form
  * keccak256(abi.encodePacked(`:contractAddress` + `:granteeAddress`)) using a valid bouncer address.
  * Then restrict access to your crowdsale/whitelist/airdrop using the
- * `onlyValidSignature` modifier (or implement your own using isValidSignature).
+ * `onlyValidSignature` modifier (or implement your own using _isValidSignature).
  * In addition to `onlyValidSignature`, `onlyValidSignatureAndMethod` and
  * `onlyValidSignatureAndData` can be used to restrict access to only a given method
  * or a given method with given parameters respectively.
@@ -42,7 +42,7 @@ contract SignatureBouncer is Ownable, RBAC {
    */
   modifier onlyValidSignature(bytes _signature)
   {
-    require(isValidSignature(msg.sender, _signature));
+    require(_isValidSignature(msg.sender, _signature));
     _;
   }
 
@@ -51,7 +51,7 @@ contract SignatureBouncer is Ownable, RBAC {
    */
   modifier onlyValidSignatureAndMethod(bytes _signature)
   {
-    require(isValidSignatureAndMethod(msg.sender, _signature));
+    require(_isValidSignatureAndMethod(msg.sender, _signature));
     _;
   }
 
@@ -60,7 +60,7 @@ contract SignatureBouncer is Ownable, RBAC {
    */
   modifier onlyValidSignatureAndData(bytes _signature)
   {
-    require(isValidSignatureAndData(msg.sender, _signature));
+    require(_isValidSignatureAndData(msg.sender, _signature));
     _;
   }
 
@@ -72,7 +72,7 @@ contract SignatureBouncer is Ownable, RBAC {
     onlyOwner
   {
     require(_bouncer != address(0));
-    addRole(_bouncer, ROLE_BOUNCER);
+    _addRole(_bouncer, ROLE_BOUNCER);
   }
 
   /**
@@ -82,19 +82,19 @@ contract SignatureBouncer is Ownable, RBAC {
     public
     onlyOwner
   {
-    removeRole(_bouncer, ROLE_BOUNCER);
+    _removeRole(_bouncer, ROLE_BOUNCER);
   }
 
   /**
    * @dev is the signature of `this + sender` from a bouncer?
    * @return bool
    */
-  function isValidSignature(address _address, bytes _signature)
+  function _isValidSignature(address _address, bytes _signature)
     internal
     view
     returns (bool)
   {
-    return isValidDataHash(
+    return _isValidDataHash(
       keccak256(abi.encodePacked(address(this), _address)),
       _signature
     );
@@ -104,7 +104,7 @@ contract SignatureBouncer is Ownable, RBAC {
    * @dev is the signature of `this + sender + methodId` from a bouncer?
    * @return bool
    */
-  function isValidSignatureAndMethod(address _address, bytes _signature)
+  function _isValidSignatureAndMethod(address _address, bytes _signature)
     internal
     view
     returns (bool)
@@ -113,7 +113,7 @@ contract SignatureBouncer is Ownable, RBAC {
     for (uint i = 0; i < data.length; i++) {
       data[i] = msg.data[i];
     }
-    return isValidDataHash(
+    return _isValidDataHash(
       keccak256(abi.encodePacked(address(this), _address, data)),
       _signature
     );
@@ -124,7 +124,7 @@ contract SignatureBouncer is Ownable, RBAC {
     * @notice the _signature parameter of the method being validated must be the "last" parameter
     * @return bool
     */
-  function isValidSignatureAndData(address _address, bytes _signature)
+  function _isValidSignatureAndData(address _address, bytes _signature)
     internal
     view
     returns (bool)
@@ -134,7 +134,7 @@ contract SignatureBouncer is Ownable, RBAC {
     for (uint i = 0; i < data.length; i++) {
       data[i] = msg.data[i];
     }
-    return isValidDataHash(
+    return _isValidDataHash(
       keccak256(abi.encodePacked(address(this), _address, data)),
       _signature
     );
@@ -145,7 +145,7 @@ contract SignatureBouncer is Ownable, RBAC {
    * and then recover the signature and check it against the bouncer role
    * @return bool
    */
-  function isValidDataHash(bytes32 _hash, bytes _signature)
+  function _isValidDataHash(bytes32 _hash, bytes _signature)
     internal
     view
     returns (bool)

contracts/access/Whitelist.sol

@@ -31,7 +31,7 @@ contract Whitelist is Ownable, RBAC {
     public
     onlyOwner
   {
-    addRole(_operator, ROLE_WHITELISTED);
+    _addRole(_operator, ROLE_WHITELISTED);
   }
 
   /**
@@ -70,7 +70,7 @@ contract Whitelist is Ownable, RBAC {
     public
     onlyOwner
   {
-    removeRole(_operator, ROLE_WHITELISTED);
+    _removeRole(_operator, ROLE_WHITELISTED);
   }
 
   /**

contracts/access/rbac/RBAC.sol

@@ -52,7 +52,7 @@ contract RBAC {
    * @param _operator address
    * @param _role the name of the role
    */
-  function addRole(address _operator, string _role)
+  function _addRole(address _operator, string _role)
     internal
   {
     roles[_role].add(_operator);
@@ -64,7 +64,7 @@ contract RBAC {
    * @param _operator address
    * @param _role the name of the role
    */
-  function removeRole(address _operator, string _role)
+  function _removeRole(address _operator, string _role)
     internal
   {
     roles[_role].remove(_operator);

contracts/bounties/BreakInvariantBounty.sol

@@ -27,7 +27,7 @@ contract BreakInvariantBounty is PullPayment, Ownable {
    * @return A target contract
    */
   function createTarget() public returns(Target) {
-    Target target = Target(deployContract());
+    Target target = Target(_deployContract());
     researchers[target] = msg.sender;
     emit TargetCreated(target);
     return target;
@@ -42,7 +42,7 @@ contract BreakInvariantBounty is PullPayment, Ownable {
     require(researcher != address(0));
     // Check Target contract invariants
     require(!_target.checkInvariant());
-    asyncTransfer(researcher, address(this).balance);
+    _asyncTransfer(researcher, address(this).balance);
     claimed = true;
   }
 
@@ -57,7 +57,7 @@ contract BreakInvariantBounty is PullPayment, Ownable {
    * @dev Internal function to deploy the target contract.
    * @return A target contract address
    */
-  function deployContract() internal returns(address);
+  function _deployContract() internal returns(address);
 
 }
 

contracts/crowdsale/distribution/FinalizableCrowdsale.sol

@@ -25,18 +25,18 @@ contract FinalizableCrowdsale is Ownable, TimedCrowdsale {
     require(!isFinalized);
     require(hasClosed());
 
-    finalization();
+    _finalization();
     emit CrowdsaleFinalized();
 
     isFinalized = true;
   }
 
   /**
    * @dev Can be overridden to add finalization logic. The overriding function
-   * should call super.finalization() to ensure the chain of finalization is
+   * should call super._finalization() to ensure the chain of finalization is
    * executed entirely.
    */
-  function finalization() internal {
+  function _finalization() internal {
   }
 
 }

contracts/crowdsale/distribution/RefundableCrowdsale.sol

@@ -51,15 +51,15 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
   /**
    * @dev escrow finalization task, called when owner calls finalize()
    */
-  function finalization() internal {
+  function _finalization() internal {
     if (goalReached()) {
       escrow_.close();
       escrow_.beneficiaryWithdraw();
     } else {
       escrow_.enableRefunds();
     }
 
-    super.finalization();
+    super._finalization();
   }
 
   /**

contracts/examples/RBACWithAdmin.sol

@@ -37,7 +37,7 @@ contract RBACWithAdmin is RBAC {
   constructor()
     public
   {
-    addRole(msg.sender, ROLE_ADMIN);
+    _addRole(msg.sender, ROLE_ADMIN);
   }
 
   /**
@@ -49,7 +49,7 @@ contract RBACWithAdmin is RBAC {
     public
     onlyAdmin
   {
-    addRole(_account, _roleName);
+    _addRole(_account, _roleName);
   }
 
   /**
@@ -61,6 +61,6 @@ contract RBACWithAdmin is RBAC {
     public
     onlyAdmin
   {
-    removeRole(_account, _roleName);
+    _removeRole(_account, _roleName);
   }
 }

contracts/mocks/BouncerMock.sol

@@ -9,7 +9,7 @@ contract SignatureBouncerMock is SignatureBouncer {
     view
     returns (bool)
   {
-    return isValidSignature(_address, _signature);
+    return _isValidSignature(_address, _signature);
   }
 
   function onlyWithValidSignature(bytes _signature)
@@ -25,7 +25,7 @@ contract SignatureBouncerMock is SignatureBouncer {
     view
     returns (bool)
   {
-    return isValidSignatureAndMethod(_address, _signature);
+    return _isValidSignatureAndMethod(_address, _signature);
   }
 
   function onlyWithValidSignatureAndMethod(bytes _signature)
@@ -46,7 +46,7 @@ contract SignatureBouncerMock is SignatureBouncer {
     view
     returns (bool)
   {
-    return isValidSignatureAndData(_address, _signature);
+    return _isValidSignatureAndData(_address, _signature);
   }
 
   function onlyWithValidSignatureAndData(uint, bytes _signature)

contracts/mocks/ERC721Mock.sol

@@ -14,22 +14,22 @@ contract ERC721Mock is ERC721 {
   { }
 
   function mint(address _to, uint256 _tokenId) public {
-    super._mint(_to, _tokenId);
+    _mint(_to, _tokenId);
   }
 
   function burn(uint256 _tokenId) public {
-    super._burn(ownerOf(_tokenId), _tokenId);
+    _burn(ownerOf(_tokenId), _tokenId);
   }
 
   function exists(uint256 _tokenId) public view returns (bool) {
-    return super._exists(_tokenId);
+    return _exists(_tokenId);
   }
 
   function setTokenURI(uint256 _tokenId, string _uri) public {
-    super._setTokenURI(_tokenId, _uri);
+    _setTokenURI(_tokenId, _uri);
   }
 
-  function _removeTokenFrom(address _from, uint256 _tokenId) public {
-    super.removeTokenFrom(_from, _tokenId);
+  function removeTokenFrom(address _from, uint256 _tokenId) public {
+    _removeTokenFrom(_from, _tokenId);
   }
 }