Cosmetic cleanup (#352)

Upgrades the linter, turns off unnecessary `pragma 0.5.0` rules, and fixes the linting.

.soliumrc.json

@@ -4,22 +4,34 @@
     "rules": {
         "security/no-low-level-calls": "off",
         "security/no-inline-assembly": "off",
-        "function-order": "off",
-        "imports-on-top": 1,
-        "variable-declarations": 1,
-        "array-declarations": 1,
-        "operator-whitespace": 1,
-        "lbrace": 1,
-        "mixedcase": 0,
-        "camelcase": 1,
-        "uppercase": 1,
-        "no-empty-blocks": 1,
-        "no-unused-vars": 1,
-        "quotes": 1,
-        "indentation": 1,
+        "emit": "off",
+        "error-reason": "off",
+        "imports-on-top": "error",
+        "variable-declarations": "error",
+        "array-declarations": "error",
+        "operator-whitespace": "error",
+        "conditionals-whitespace": "error",
+        "comma-whitespace": "error",
+        "semicolon-whitespace": "error",
+        "function-whitespace": "error",
+        "lbrace": "error",
+        "mixedcase": "error",
+        "camelcase": "error",
+        "uppercase": "error",
+        "no-empty-blocks": "error",
+        "no-unused-vars": "error",
+        "quotes": "error",
+        "blank-lines": "error",
+        "indentation": "error",
         "arg-overflow": ["error", 8],
-        "whitespace": 1,
-        "deprecated-suicide": 1,
-        "pragma-on-top": 1
+        "whitespace": "error",
+        "deprecated-suicide": "error",
+        "pragma-on-top": "error",
+        "function-order": "error",
+        "no-constant": "error",
+        "value-in-payable": "error",
+        "max-len": "error",
+        "visibility-first": "error",
+        "linebreak-style": "error"
     }
 }

contracts/acl/ACL.sol

@@ -6,16 +6,18 @@ import "./IACL.sol";
 import "./IACLOracle.sol";
 
 
+/* solium-disable function-order */
+// Allow public initialize() to be first
 contract ACL is IACL, AragonApp, ACLHelpers {
     // Hardcoded constant to save gas
     //bytes32 constant public CREATE_PERMISSIONS_ROLE = keccak256("CREATE_PERMISSIONS_ROLE");
     bytes32 constant public CREATE_PERMISSIONS_ROLE = 0x0b719b33c83b8e5d300c521cb8b54ae9bd933996a14bef8c2f4e0285d2d2400a;
 
-    // whether a certain entity has a permission
-    mapping (bytes32 => bytes32) internal permissions; // 0 for no permission, or parameters id
-    mapping (bytes32 => Param[]) internal permissionParams;
+    // Whether someone has a permission
+    mapping (bytes32 => bytes32) internal permissions; // permissions hash => params hash
+    mapping (bytes32 => Param[]) internal permissionParams; // params hash => params
 
-    // who is the manager of a permission
+    // Who is the manager of a permission
     mapping (bytes32 => address) internal permissionManager;
 
     enum Op { NONE, EQ, NEQ, GT, LT, GTE, LTE, RET, NOT, AND, OR, XOR, IF_ELSE } // op types
@@ -54,7 +56,7 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     * @notice Initializes an ACL instance and sets `_permissionsCreator` as the entity that can create other permissions
     * @param _permissionsCreator Entity that will be given permission over createPermission
     */
-    function initialize(address _permissionsCreator) onlyInit public {
+    function initialize(address _permissionsCreator) public onlyInit {
         initialized();
         require(msg.sender == address(kernel));
 
@@ -98,8 +100,8 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     * @param _params Permission parameters
     */
     function grantPermissionP(address _entity, address _app, bytes32 _role, uint256[] _params)
-        onlyPermissionManager(_app, _role)
         public
+        onlyPermissionManager(_app, _role)
     {
         bytes32 paramsHash = _params.length > 0 ? _saveParams(_params) : EMPTY_PARAM_HASH;
         _setPermission(_entity, _app, _role, paramsHash);
@@ -113,8 +115,8 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     * @param _role Identifier for the group of actions in app being revoked
     */
     function revokePermission(address _entity, address _app, bytes32 _role)
-        onlyPermissionManager(_app, _role)
         external
+        onlyPermissionManager(_app, _role)
     {
         _setPermission(_entity, _app, _role, bytes32(0));
     }
@@ -126,8 +128,8 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     * @param _role Identifier for the group of actions being transferred
     */
     function setPermissionManager(address _newManager, address _app, bytes32 _role)
-        onlyPermissionManager(_app, _role)
         external
+        onlyPermissionManager(_app, _role)
     {
         _setPermissionManager(_newManager, _app, _role);
     }
@@ -138,8 +140,8 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     * @param _role Identifier for the group of actions being unmanaged
     */
     function removePermissionManager(address _app, bytes32 _role)
-        onlyPermissionManager(_app, _role)
         external
+        onlyPermissionManager(_app, _role)
     {
         _setPermissionManager(address(0), _app, _role);
     }
@@ -163,7 +165,11 @@ contract ACL is IACL, AragonApp, ACLHelpers {
     * @param _index Index of parameter in the array
     * @return Parameter (id, op, value)
     */
-    function getPermissionParam(address _entity, address _app, bytes32 _role, uint _index) external view returns (uint8 id, uint8 op, uint240 value) {
+    function getPermissionParam(address _entity, address _app, bytes32 _role, uint _index)
+        external
+        view
+        returns (uint8 id, uint8 op, uint240 value)
+    {
         Param storage param = permissionParams[permissions[permissionHash(_entity, _app, _role)]][_index];
         id = param.id;
         op = param.op;
@@ -316,7 +322,11 @@ contract ACL is IACL, AragonApp, ACLHelpers {
         return compare(value, Op(param.op), comparedTo);
     }
 
-    function evalLogic(Param _param, bytes32 _paramsHash, address _who, address _where, bytes32 _what, uint256[] _how) internal view returns (bool) {
+    function evalLogic(Param _param, bytes32 _paramsHash, address _who, address _where, bytes32 _what, uint256[] _how)
+        internal
+        view
+        returns (bool)
+    {
         if (Op(_param.op) == Op.IF_ELSE) {
             var (condition, success, failure) = decodeParamsList(uint256(_param.value));
             bool result = evalParam(_paramsHash, condition, _who, _where, _what, _how);
@@ -394,11 +404,11 @@ contract ACL is IACL, AragonApp, ACLHelpers {
         ChangePermissionManager(_app, _role, _newManager);
     }
 
-    function roleHash(address _where, bytes32 _what) pure internal returns (bytes32) {
+    function roleHash(address _where, bytes32 _what) internal pure returns (bytes32) {
         return keccak256(uint256(1), _where, _what);
     }
 
-    function permissionHash(address _who, address _where, bytes32 _what) pure internal returns (bytes32) {
+    function permissionHash(address _who, address _where, bytes32 _what) internal pure returns (bytes32) {
         return keccak256(uint256(2), _who, _where, _what);
     }
 

contracts/apm/APMNamehash.sol

@@ -1,3 +1,7 @@
+/*
+ * SPDX-License-Identitifer:    MIT
+ */
+
 pragma solidity ^0.4.18;
 
 import "../ens/ENSConstants.sol";

contracts/apm/APMRegistry.sol

@@ -30,7 +30,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
     * NEEDS CREATE_NAME_ROLE and POINT_ROOTNODE_ROLE permissions on registrar
     * @param _registrar ENSSubdomainRegistrar instance that holds registry root node ownership
     */
-    function initialize(ENSSubdomainRegistrar _registrar) onlyInit public {
+    function initialize(ENSSubdomainRegistrar _registrar) public onlyInit {
         initialized();
 
         registrar = _registrar;
@@ -49,7 +49,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
     * @param _name Repo name, must be ununsed
     * @param _dev Address that will be given permission to create versions
     */
-    function newRepo(string _name, address _dev) auth(CREATE_REPO_ROLE) public returns (Repo) {
+    function newRepo(string _name, address _dev) public auth(CREATE_REPO_ROLE) returns (Repo) {
         return _newRepo(_name, _dev);
     }
 
@@ -67,7 +67,7 @@ contract APMRegistry is AragonApp, AppProxyFactory, APMRegistryConstants {
         uint16[3] _initialSemanticVersion,
         address _contractAddress,
         bytes _contentURI
-    ) auth(CREATE_REPO_ROLE) public returns (Repo)
+    ) public auth(CREATE_REPO_ROLE) returns (Repo)
     {
         Repo repo = _newRepo(_name, this); // need to have permissions to create version
         repo.newVersion(_initialSemanticVersion, _contractAddress, _contentURI);

contracts/apm/Repo.sol

@@ -29,7 +29,7 @@ contract Repo is AragonApp {
         uint16[3] _newSemanticVersion,
         address _contractAddress,
         bytes _contentURI
-    ) auth(CREATE_VERSION_ROLE) public
+    ) public auth(CREATE_VERSION_ROLE)
     {
         address contractAddress = _contractAddress;
         if (versions.length > 0) {
@@ -57,11 +57,19 @@ contract Repo is AragonApp {
         return getByVersionId(versions.length - 1);
     }
 
-    function getLatestForContractAddress(address _contractAddress) public view returns (uint16[3] semanticVersion, address contractAddress, bytes contentURI) {
+    function getLatestForContractAddress(address _contractAddress)
+        public
+        view
+        returns (uint16[3] semanticVersion, address contractAddress, bytes contentURI)
+    {
         return getByVersionId(latestVersionIdForContract[_contractAddress]);
     }
 
-    function getBySemanticVersion(uint16[3] _semanticVersion) public view returns (uint16[3] semanticVersion, address contractAddress, bytes contentURI) {
+    function getBySemanticVersion(uint16[3] _semanticVersion)
+        public
+        view
+        returns (uint16[3] semanticVersion, address contractAddress, bytes contentURI)
+    {
         return getByVersionId(versionIdForSemantic[semanticVersionHash(_semanticVersion)]);
     }
 

contracts/apps/AppProxyPinned.sol

@@ -11,7 +11,8 @@ contract AppProxyPinned is AppProxyBase {
     * @param _initializePayload Payload for call to be made after setup to initialize
     */
     function AppProxyPinned(IKernel _kernel, bytes32 _appId, bytes _initializePayload)
-             AppProxyBase(_kernel, _appId, _initializePayload) public
+        AppProxyBase(_kernel, _appId, _initializePayload)
+        public // solium-disable-line visibility-first
     {
         pinnedCode = getAppBase(appId);
         require(pinnedCode != address(0));

contracts/apps/AppProxyUpgradeable.sol

@@ -11,7 +11,8 @@ contract AppProxyUpgradeable is AppProxyBase {
     * @param _initializePayload Payload for call to be made after setup to initialize
     */
     function AppProxyUpgradeable(IKernel _kernel, bytes32 _appId, bytes _initializePayload)
-             AppProxyBase(_kernel, _appId, _initializePayload) public
+        AppProxyBase(_kernel, _appId, _initializePayload)
+        public // solium-disable-line visibility-first
     {
 
     }

contracts/apps/AragonApp.sol

@@ -19,23 +19,34 @@ contract AragonApp is AppStorage, Initializable, ACLSyntaxSugar, VaultRecoverabl
         _;
     }
 
-    modifier authP(bytes32 _role, uint256[] params) {
-        require(canPerform(msg.sender, _role, params));
+    modifier authP(bytes32 _role, uint256[] _params) {
+        require(canPerform(msg.sender, _role, _params));
         _;
     }
 
-    function canPerform(address _sender, bytes32 _role, uint256[] params) public view returns (bool) {
+    /**
+    * @dev Check whether an action can be performed by a sender for a particular role on this app
+    * @param _sender Sender of the call
+    * @param _role Role on this app
+    * @param _params Permission params for the role
+    * @return Boolean indicating whether the sender has the permissions to perform the action
+    */
+    function canPerform(address _sender, bytes32 _role, uint256[] _params) public view returns (bool) {
         bytes memory how; // no need to init memory as it is never used
-        if (params.length > 0) {
-            uint256 byteLength = params.length * 32;
+        if (_params.length > 0) {
+            uint256 byteLength = _params.length * 32;
             assembly {
-                how := params // forced casting
+                how := _params // forced casting
                 mstore(how, byteLength)
             }
         }
         return address(kernel) == 0 || kernel.hasPermission(_sender, address(this), _role, how);
     }
 
+    /**
+    * @dev Get the recovery vault for the app
+    * @return Recovery vault address for the app
+    */
     function getRecoveryVault() public view returns (address) {
         // Funds recovery via a vault is only available when used with a kernel
         require(address(kernel) != 0);

contracts/common/DelegateProxy.sol

@@ -14,10 +14,10 @@ contract DelegateProxy is ERCProxy, IsContract {
     */
     function delegatedFwd(address _dst, bytes _calldata) internal {
         require(isContract(_dst));
-        uint256 fwd_gas_limit = FWD_GAS_LIMIT;
+        uint256 fwdGasLimit = FWD_GAS_LIMIT;
 
         assembly {
-            let result := delegatecall(sub(gas, fwd_gas_limit), _dst, add(_calldata, 0x20), mload(_calldata), 0, 0)
+            let result := delegatecall(sub(gas, fwdGasLimit), _dst, add(_calldata, 0x20), mload(_calldata), 0, 0)
             let size := returndatasize
             let ptr := mload(0x40)
             returndatacopy(ptr, 0, size)

contracts/ens/ENSSubdomainRegistrar.sol

@@ -7,6 +7,8 @@ import "./ENSConstants.sol";
 import "../apps/AragonApp.sol";
 
 
+/* solium-disable function-order */
+// Allow public initialize() to be first
 contract ENSSubdomainRegistrar is AragonApp, ENSConstants {
     // bytes32 constant public CREATE_NAME_ROLE = keccak256("CREATE_NAME_ROLE");
     // bytes32 constant public DELETE_NAME_ROLE = keccak256("DELETE_NAME_ROLE");
@@ -21,7 +23,7 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {
     event NewName(bytes32 indexed node, bytes32 indexed label);
     event DeleteName(bytes32 indexed node, bytes32 indexed label);
 
-    function initialize(AbstractENS _ens, bytes32 _rootNode) onlyInit public {
+    function initialize(AbstractENS _ens, bytes32 _rootNode) public onlyInit {
         initialized();
 
         // We need ownership to create subnodes
@@ -31,16 +33,16 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {
         rootNode = _rootNode;
     }
 
-    function createName(bytes32 _label, address _owner) auth(CREATE_NAME_ROLE) external returns (bytes32 node) {
+    function createName(bytes32 _label, address _owner) external auth(CREATE_NAME_ROLE) returns (bytes32 node) {
         return _createName(_label, _owner);
     }
 
-    function createNameAndPoint(bytes32 _label, address _target) auth(CREATE_NAME_ROLE) external returns (bytes32 node) {
+    function createNameAndPoint(bytes32 _label, address _target) external auth(CREATE_NAME_ROLE) returns (bytes32 node) {
         node = _createName(_label, this);
         _pointToResolverAndResolve(node, _target);
     }
 
-    function deleteName(bytes32 _label) auth(DELETE_NAME_ROLE) external {
+    function deleteName(bytes32 _label) external auth(DELETE_NAME_ROLE) {
         bytes32 node = keccak256(rootNode, _label);
 
         address currentOwner = ens.owner(node);
@@ -57,7 +59,7 @@ contract ENSSubdomainRegistrar is AragonApp, ENSConstants {
         DeleteName(node, _label);
     }
 
-    function pointRootNode(address _target) auth(POINT_ROOTNODE_ROLE) external {
+    function pointRootNode(address _target) external auth(POINT_ROOTNODE_ROLE) {
         _pointToResolverAndResolve(rootNode, _target);
     }
 

contracts/evmscript/EVMScriptRegistry.sol

@@ -7,6 +7,8 @@ import "./IEVMScriptRegistry.sol";
 import "../apps/AragonApp.sol";
 
 
+/* solium-disable function-order */
+// Allow public initialize() to be first
 contract EVMScriptRegistry is IEVMScriptRegistry, EVMScriptRegistryConstants, AragonApp {
     using ScriptHelpers for bytes;
 
@@ -21,7 +23,7 @@ contract EVMScriptRegistry is IEVMScriptRegistry, EVMScriptRegistryConstants, Ar
 
     ExecutorEntry[] public executors;
 
-    function initialize() onlyInit public {
+    function initialize() public onlyInit {
         initialized();
         // Create empty record to begin executor IDs at 1
         executors.push(ExecutorEntry(IEVMScriptExecutor(0), false));

contracts/evmscript/EVMScriptRunner.sol

@@ -16,6 +16,10 @@ contract EVMScriptRunner is AppStorage, EVMScriptRegistryConstants {
 
     event ScriptResult(address indexed executor, bytes script, bytes input, bytes returnData);
 
+    function getExecutor(bytes _script) public view returns (IEVMScriptExecutor) {
+        return IEVMScriptExecutor(getExecutorRegistry().getScriptExecutor(_script));
+    }
+
     function runScript(bytes _script, bytes _input, address[] _blacklist) internal protectState returns (bytes output) {
         // TODO: Too much data flying around, maybe extracting spec id here is cheaper
         IEVMScriptExecutor executor = getExecutor(_script);
@@ -31,11 +35,6 @@ contract EVMScriptRunner is AppStorage, EVMScriptRegistryConstants {
         ScriptResult(address(executor), _script, _input, output);
     }
 
-    function getExecutor(bytes _script) public view returns (IEVMScriptExecutor) {
-        return IEVMScriptExecutor(getExecutorRegistry().getScriptExecutor(_script));
-    }
-
-    // TODO: Internal
     function getExecutorRegistry() internal view returns (IEVMScriptRegistry) {
         address registryAddr = kernel.getApp(EVMSCRIPT_REGISTRY_APP);
         return IEVMScriptRegistry(registryAddr);