Replaced Solium in favor of Solhint (#1575)

* Adding solhint, working on style fixes.

* Upgraded to solhint 1.5.0.

* Removed all references to Solium

* Updated mocks to make the pass the new linter rules.

* Reformatted the .solhint.json file a bit.

* Removed Solium configuration files.

* Remove Solium dependency.

* Add comment explaing disabled time rule in TokenVesting.

* Revert to the old (ugly?) style.

* Revert SignatureBouncerMock style.

* Fix ERC165InterfacesSupported interface.

Author: nventuro

.solhint.json

@@ -0,0 +1,11 @@
+{
+  "extends": "default",
+  "rules": {
+    "indent": ["error", 4],
+
+    "bracket-align": false,
+    "compiler-fixed": false,
+    "no-simple-event-func-name": false,
+    "two-lines-top-level-separator": false
+  }
+}

.soliumignore

@@ -63,10 +63,6 @@ contract Crowdsale is ReentrancyGuard {
         _token = token;
     }
 
-    // -----------------------------------------
-    // Crowdsale external interface
-    // -----------------------------------------
-
     /**
      * @dev fallback function ***DO NOT OVERRIDE***
      * Note that other contracts will transfer fund with a base gas stipend
@@ -130,12 +126,9 @@ contract Crowdsale is ReentrancyGuard {
         _postValidatePurchase(beneficiary, weiAmount);
     }
 
-    // -----------------------------------------
-    // Internal interface (extensible)
-    // -----------------------------------------
-
     /**
-     * @dev Validation of an incoming purchase. Use require statements to revert state when conditions are not met. Use `super` in contracts that inherit from Crowdsale to extend their validations.
+     * @dev Validation of an incoming purchase. Use require statements to revert state when conditions are not met.
+     * Use `super` in contracts that inherit from Crowdsale to extend their validations.
      * Example from CappedCrowdsale.sol's _preValidatePurchase method:
      *     super._preValidatePurchase(beneficiary, weiAmount);
      *     require(weiRaised().add(weiAmount) <= cap);
@@ -148,16 +141,18 @@ contract Crowdsale is ReentrancyGuard {
     }
 
     /**
-     * @dev Validation of an executed purchase. Observe state and use revert statements to undo rollback when valid conditions are not met.
+     * @dev Validation of an executed purchase. Observe state and use revert statements to undo rollback when valid
+     * conditions are not met.
      * @param beneficiary Address performing the token purchase
      * @param weiAmount Value in wei involved in the purchase
      */
     function _postValidatePurchase(address beneficiary, uint256 weiAmount) internal view {
-        // optional override
+        // solhint-disable-previous-line no-empty-blocks
     }
 
     /**
-     * @dev Source of tokens. Override this method to modify the way in which the crowdsale ultimately gets and sends its tokens.
+     * @dev Source of tokens. Override this method to modify the way in which the crowdsale ultimately gets and sends
+     * its tokens.
      * @param beneficiary Address performing the token purchase
      * @param tokenAmount Number of tokens to be emitted
      */
@@ -166,7 +161,8 @@ contract Crowdsale is ReentrancyGuard {
     }
 
     /**
-     * @dev Executed when a purchase has been validated and is ready to be executed. Doesn't necessarily emit/send tokens.
+     * @dev Executed when a purchase has been validated and is ready to be executed. Doesn't necessarily emit/send
+     * tokens.
      * @param beneficiary Address receiving the tokens
      * @param tokenAmount Number of tokens to be purchased
      */
@@ -175,12 +171,13 @@ contract Crowdsale is ReentrancyGuard {
     }
 
     /**
-     * @dev Override for extensions that require an internal state to check for validity (current user contributions, etc.)
+     * @dev Override for extensions that require an internal state to check for validity (current user contributions,
+     * etc.)
      * @param beneficiary Address receiving the tokens
      * @param weiAmount Value in wei involved in the purchase
      */
     function _updatePurchasingState(address beneficiary, uint256 weiAmount) internal {
-        // optional override
+        // solhint-disable-previous-line no-empty-blocks
     }
 
     /**

.soliumrc.json

@@ -45,5 +45,7 @@ contract FinalizableCrowdsale is TimedCrowdsale {
      * should call super._finalization() to ensure the chain of finalization is
      * executed entirely.
      */
-    function _finalization() internal {}
+    function _finalization() internal {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }

contracts/crowdsale/Crowdsale.sol

@@ -59,7 +59,7 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
             return 0;
         }
 
-        // solium-disable-next-line security/no-block-members
+        // solhint-disable-next-line not-rely-on-time
         uint256 elapsedTime = block.timestamp.sub(openingTime());
         uint256 timeRange = closingTime().sub(openingTime());
         uint256 rateRange = _initialRate.sub(_finalRate);

contracts/crowdsale/distribution/FinalizableCrowdsale.sol

@@ -8,9 +8,9 @@ import "../../lifecycle/Pausable.sol";
  * @dev Extension of Crowdsale contract where purchases can be paused and unpaused by the pauser role.
  */
 contract PausableCrowdsale is Crowdsale, Pausable {
-
     /**
-     * @dev Validation of an incoming purchase. Use require statements to revert state when conditions are not met. Use super to concatenate validations.
+     * @dev Validation of an incoming purchase. Use require statements to revert state when conditions are not met.
+     * Use super to concatenate validations.
      * Adds the validation that the crowdsale must not be paused.
      * @param _beneficiary Address performing the token purchase
      * @param _weiAmount Value in wei involved in the purchase

contracts/crowdsale/price/IncreasingPriceCrowdsale.sol

@@ -27,7 +27,7 @@ contract TimedCrowdsale is Crowdsale {
      * @param closingTime Crowdsale closing time
      */
     constructor (uint256 openingTime, uint256 closingTime) public {
-        // solium-disable-next-line security/no-block-members
+        // solhint-disable-next-line not-rely-on-time
         require(openingTime >= block.timestamp);
         require(closingTime > openingTime);
 
@@ -53,7 +53,7 @@ contract TimedCrowdsale is Crowdsale {
      * @return true if the crowdsale is open, false otherwise.
      */
     function isOpen() public view returns (bool) {
-        // solium-disable-next-line security/no-block-members
+        // solhint-disable-next-line not-rely-on-time
         return block.timestamp >= _openingTime && block.timestamp <= _closingTime;
     }
 
@@ -62,7 +62,7 @@ contract TimedCrowdsale is Crowdsale {
      * @return Whether crowdsale period has elapsed
      */
     function hasClosed() public view returns (bool) {
-        // solium-disable-next-line security/no-block-members
+        // solhint-disable-next-line not-rely-on-time
         return block.timestamp > _closingTime;
     }
 

contracts/crowdsale/validation/PausableCrowdsale.sol

@@ -26,7 +26,7 @@ library ECDSA {
         // Divide the signature in r, s and v variables
         // ecrecover takes the signature parameters, and the only way to get them
         // currently is to use assembly.
-        // solium-disable-next-line security/no-inline-assembly
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             r := mload(add(signature, 0x20))
             s := mload(add(signature, 0x40))
@@ -42,7 +42,6 @@ library ECDSA {
         if (v != 27 && v != 28) {
             return (address(0));
         } else {
-            // solium-disable-next-line arg-overflow
             return ecrecover(hash, v, r, s);
         }
     }

contracts/crowdsale/validation/TimedCrowdsale.sol

@@ -6,7 +6,6 @@ import "../../token/ERC20/IERC20.sol";
  * @title ERC-1047 Token Metadata
  * @dev See https://eips.ethereum.org/EIPS/eip-1046
  * @dev tokenURI must respond with a URI that implements https://eips.ethereum.org/EIPS/eip-1047
- * @dev TODO - update https://github.com/OpenZeppelin/openzeppelin-solidity/blob/master/contracts/token/ERC721/IERC721.sol#L17 when 1046 is finalized
  */
 contract ERC20TokenMetadata is IERC20 {
     function tokenURI() external view returns (string memory);

contracts/cryptography/ECDSA.sol

@@ -43,7 +43,9 @@ contract SignatureBouncer is SignerRole {
     // Signature size is 65 bytes (tightly packed v + r + s), but gets padded to 96 bytes
     uint256 private constant _SIGNATURE_SIZE = 96;
 
-    constructor () internal {}
+    constructor () internal {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 
     /**
      * @dev requires that a valid signature of a signer was provided

contracts/drafts/ERC1046/TokenMetadata.sol

@@ -1,5 +1,3 @@
-/* solium-disable security/no-block-members */
-
 pragma solidity ^0.5.0;
 
 import "../token/ERC20/SafeERC20.sol";
@@ -13,6 +11,12 @@ import "../math/SafeMath.sol";
  * owner.
  */
 contract TokenVesting is Ownable {
+    // The vesting schedule is time-based (i.e. using block timestamps as opposed to e.g. block numbers), and is
+    // therefore sensitive to timestamp manipulation (which is something miners can do, to a certain degree). Therefore,
+    // it is recommended to avoid using short time durations (less than a minute). Typical vesting schemes, with a cliff
+    // period of a year and a duration of four years, are safe to use.
+    // solhint-disable not-rely-on-time
+
     using SafeMath for uint256;
     using SafeERC20 for IERC20;
 
@@ -22,6 +26,7 @@ contract TokenVesting is Ownable {
     // beneficiary of tokens after they are released
     address private _beneficiary;
 
+    // Durations and timestamps are expressed in UNIX time, the same units as block.timestamp.
     uint256 private _cliff;
     uint256 private _start;
     uint256 private _duration;

contracts/drafts/SignatureBouncer.sol

@@ -12,7 +12,9 @@ import "../token/ERC20/ERC20Detailed.sol";
  * It is meant to be used in a crowdsale contract.
  */
 contract SampleCrowdsaleToken is ERC20Mintable, ERC20Detailed {
-    constructor () public ERC20Detailed("Sample Crowdsale Token", "SCT", 18) {}
+    constructor () public ERC20Detailed("Sample Crowdsale Token", "SCT", 18) {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }
 
 /**
@@ -28,11 +30,6 @@ contract SampleCrowdsaleToken is ERC20Mintable, ERC20Detailed {
  * After adding multiple features it's good practice to run integration tests
  * to ensure that subcontracts works together as intended.
  */
-// XXX There doesn't seem to be a way to split this line that keeps solium
-// happy. See:
-// https://github.com/duaraghav8/Solium/issues/205
-// --elopio - 2018-05-10
-// solium-disable-next-line max-len
 contract SampleCrowdsale is CappedCrowdsale, RefundableCrowdsale, MintedCrowdsale {
     constructor (
         uint256 openingTime,

contracts/drafts/TokenVesting.sol

@@ -8,7 +8,7 @@ import "./IERC165.sol";
  * @dev Implements ERC165 using a lookup table.
  */
 contract ERC165 is IERC165 {
-    bytes4 private constant _InterfaceId_ERC165 = 0x01ffc9a7;
+    bytes4 private constant _INTERFACE_ID_ERC165 = 0x01ffc9a7;
     /**
      * 0x01ffc9a7 ===
      *     bytes4(keccak256('supportsInterface(bytes4)'))
@@ -24,7 +24,7 @@ contract ERC165 is IERC165 {
      * implement ERC165 itself
      */
     constructor () internal {
-        _registerInterface(_InterfaceId_ERC165);
+        _registerInterface(_INTERFACE_ID_ERC165);
     }
 
     /**

contracts/examples/SampleCrowdsale.sol

@@ -7,9 +7,9 @@ pragma solidity ^0.5.0;
  */
 library ERC165Checker {
     // As per the EIP-165 spec, no interface should ever match 0xffffffff
-    bytes4 private constant _InterfaceId_Invalid = 0xffffffff;
+    bytes4 private constant _INTERFACE_ID_INVALID = 0xffffffff;
 
-    bytes4 private constant _InterfaceId_ERC165 = 0x01ffc9a7;
+    bytes4 private constant _INTERFACE_ID_ERC165 = 0x01ffc9a7;
     /**
      * 0x01ffc9a7 ===
      *     bytes4(keccak256('supportsInterface(bytes4)'))
@@ -23,8 +23,8 @@ library ERC165Checker {
     function _supportsERC165(address account) internal view returns (bool) {
         // Any contract that implements ERC165 must explicitly indicate support of
         // InterfaceId_ERC165 and explicitly indicate non-support of InterfaceId_Invalid
-        return _supportsERC165Interface(account, _InterfaceId_ERC165) &&
-            !_supportsERC165Interface(account, _InterfaceId_Invalid);
+        return _supportsERC165Interface(account, _INTERFACE_ID_ERC165) &&
+            !_supportsERC165Interface(account, _INTERFACE_ID_INVALID);
     }
 
     /**
@@ -94,13 +94,13 @@ library ERC165Checker {
      * indicates support of the interface with identifier interfaceId, false otherwise
      */
     function _callERC165SupportsInterface(address account, bytes4 interfaceId)
-      private
-      view
-      returns (bool success, bool result)
+        private
+        view
+        returns (bool success, bool result)
     {
-        bytes memory encodedParams = abi.encodeWithSelector(_InterfaceId_ERC165,interfaceId);
+        bytes memory encodedParams = abi.encodeWithSelector(_INTERFACE_ID_ERC165, interfaceId);
 
-        // solium-disable-next-line security/no-inline-assembly
+        // solhint-disable-next-line no-inline-assembly
         assembly {
             let encodedParams_data := add(0x20, encodedParams)
             let encodedParams_size := mload(encodedParams)

contracts/introspection/ERC165.sol

@@ -8,5 +8,7 @@ contract AllowanceCrowdsaleImpl is AllowanceCrowdsale {
         public
         Crowdsale(rate, wallet, token)
         AllowanceCrowdsale(tokenWallet)
-    {}
+    {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }

contracts/introspection/ERC165Checker.sol

@@ -8,5 +8,7 @@ contract CappedCrowdsaleImpl is CappedCrowdsale {
         public
         Crowdsale(rate, wallet, token)
         CappedCrowdsale(cap)
-    {}
+    {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }

contracts/mocks/AllowanceCrowdsaleImpl.sol

@@ -8,6 +8,7 @@ contract CapperRoleMock is CapperRole {
     }
 
     function onlyCapperMock() public view onlyCapper {
+        // solhint-disable-previous-line no-empty-blocks
     }
 
     // Causes a compilation error if super._removeCapper is not internal

contracts/mocks/CappedCrowdsaleImpl.sol

@@ -3,5 +3,7 @@ pragma solidity ^0.5.0;
 import "../crowdsale/Crowdsale.sol";
 
 contract CrowdsaleMock is Crowdsale {
-    constructor (uint256 rate, address payable wallet, IERC20 token) public Crowdsale(rate, wallet, token) {}
+    constructor (uint256 rate, address payable wallet, IERC20 token) public Crowdsale(rate, wallet, token) {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }

contracts/mocks/CapperRoleMock.sol

@@ -4,5 +4,10 @@ import "../token/ERC20/ERC20.sol";
 import "../token/ERC20/ERC20Detailed.sol";
 
 contract ERC20DetailedMock is ERC20, ERC20Detailed {
-    constructor (string memory name, string memory symbol, uint8 decimals) ERC20Detailed(name, symbol, decimals) public {}
+    constructor (string memory name, string memory symbol, uint8 decimals)
+        public
+        ERC20Detailed(name, symbol, decimals)
+    {
+        // solhint-disable-previous-line no-empty-blocks
+    }
 }

contracts/mocks/CrowdsaleMock.sol

@@ -4,14 +4,15 @@ import "../../introspection/IERC165.sol";
 
 /**
  * https://github.com/ethereum/EIPs/blob/master/EIPS/eip-214.md#specification
- * > Any attempts to make state-changing operations inside an execution instance with STATIC set to true will instead throw an exception.
+ * > Any attempts to make state-changing operations inside an execution instance with STATIC set to true will instead
+ * throw an exception.
  * > These operations include [...], LOG0, LOG1, LOG2, [...]
  *
  * therefore, because this contract is staticcall'd we need to not emit events (which is how solidity-coverage works)
  * solidity-coverage ignores the /mocks folder, so we duplicate its implementation here to avoid instrumenting it
  */
 contract SupportsInterfaceWithLookupMock is IERC165 {
-    bytes4 public constant InterfaceId_ERC165 = 0x01ffc9a7;
+    bytes4 public constant INTERFACE_ID_ERC165 = 0x01ffc9a7;
     /**
      * 0x01ffc9a7 ===
      *     bytes4(keccak256('supportsInterface(bytes4)'))
@@ -27,7 +28,7 @@ contract SupportsInterfaceWithLookupMock is IERC165 {
      * implement ERC165 itself
      */
     constructor () public {
-        _registerInterface(InterfaceId_ERC165);
+        _registerInterface(INTERFACE_ID_ERC165);
     }
 
     /**

contracts/mocks/DetailedERC20Mock.sol

@@ -1,3 +1,5 @@
 pragma solidity ^0.5.0;
 
-contract ERC165NotSupported {}
+contract ERC165NotSupported {
+    // solhint-disable-previous-line no-empty-blocks
+}