Merge pull request #629 from soad003/staticanalysis_warn_assert_require

Staticanalysis warn assert require

remix-solidity/package.json

@@ -31,7 +31,7 @@
   "scripts": {
     "postinstall": "npm-link-local ../remix-lib && npm-link-local ../remix-core",
     "test": "standard && npm run downloadsolc && tape ./test/tests.js",
-    "downloadsolc": "wget https://ethereum.github.io/solc-bin/soljson.js"
+    "downloadsolc": "test -e soljson.js || wget https://ethereum.github.io/solc-bin/soljson.js"
   },
   "standard": {
     "ignore": [

remix-solidity/src/analysis/modules/guardConditions.js

@@ -0,0 +1,29 @@
+var name = 'Guard Conditions: '
+var desc = 'Use require and appropriately'
+var categories = require('./categories')
+var common = require('./staticAnalysisCommon')
+
+function guardConditions () {
+  this.guards = []
+}
+
+guardConditions.prototype.visit = function (node) {
+  if (common.isRequireCall(node) || common.isAssertCall(node)) this.guards.push(node)
+}
+
+guardConditions.prototype.report = function (compilationResults) {
+  if (this.guards.length > 0) {
+    return [{
+      warning: 'Use <i>assert(x)</i> if you never ever want <i>x</i> to be false, not in any circumstance (apart from a bug in your code). Use <i>require(x)</i> if <i>x</i> can be false, due to e.g. invalid input or a failing external component.',
+      more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#error-handling-assert-require-revert-and-exceptions'
+    }]
+  }
+  return []
+}
+
+module.exports = {
+  name: name,
+  description: desc,
+  category: categories.MISC,
+  Module: guardConditions
+}

remix-solidity/src/analysis/modules/list.js

@@ -10,5 +10,6 @@ module.exports = [
   require('./lowLevelCalls'),
   require('./blockBlockhash'),
   require('./noReturn'),
-  require('./selfdestruct')
+  require('./selfdestruct'),
+  require('./guardConditions')
 ]

remix-solidity/src/analysis/modules/staticAnalysisCommon.js

@@ -416,6 +416,24 @@ function isSelfdestructCall (node) {
   return isBuiltinFunctionCall(node) && getLocalCallName(node) === 'selfdestruct'
 }
 
+/**
+ * True if node is a call to builtin assert(bool)
+ * @node {ASTNode} some AstNode
+ * @return {bool}
+ */
+function isAssertCall (node) {
+  return isBuiltinFunctionCall(node) && getLocalCallName(node) === 'assert'
+}
+
+/**
+ * True if node is a call to builtin require(bool)
+ * @node {ASTNode} some AstNode
+ * @return {bool}
+ */
+function isRequireCall (node) {
+  return isBuiltinFunctionCall(node) && getLocalCallName(node) === 'require'
+}
+
 /**
  * True if is storage variable declaration
  * @node {ASTNode} some AstNode
@@ -810,6 +828,8 @@ module.exports = {
   isMinusMinusUnaryOperation: isMinusMinusUnaryOperation,
   isBuiltinFunctionCall: isBuiltinFunctionCall,
   isSelfdestructCall: isSelfdestructCall,
+  isAssertCall: isAssertCall,
+  isRequireCall: isRequireCall,
 
   // #################### Trivial Node Identification
   isFunctionDefinition: isFunctionDefinition,

remix-solidity/test/analysis/staticAnalysisIntegration-test.js

@@ -441,6 +441,37 @@ test('Integration test selfdestruct.js', function (t) {
   })
 })
 
+test('Integration test guardConditions.js', function (t) {
+  t.plan(testFiles.length)
+
+  var module = require('../../src/analysis/modules/guardConditions')
+
+  var lengthCheck = {
+    'KingOfTheEtherThrone.sol': 0,
+    'assembly.sol': 1,
+    'ballot.sol': 0,
+    'ballot_reentrant.sol': 0,
+    'ballot_withoutWarnings.sol': 0,
+    'cross_contract.sol': 0,
+    'inheritance.sol': 0,
+    'modifier1.sol': 0,
+    'modifier2.sol': 0,
+    'notReentrant.sol': 0,
+    'structReentrant.sol': 0,
+    'thisLocal.sol': 0,
+    'globals.sol': 1,
+    'library.sol': 0,
+    'transfer.sol': 0,
+    'ctor.sol': 0,
+    'forgottenReturn.sol': 0,
+    'selfdestruct.sol': 0
+  }
+
+  runModuleOnFiles(module, t, (file, report) => {
+    t.equal(report.length, lengthCheck[file], `${file} has right amount of guardCondition warnings`)
+  })
+})
+
 // #################### Helpers
 function runModuleOnFiles (module, t, cb) {
   var statRunner = new StatRunner()

remix-solidity/test/analysis/test-contracts/assembly.sol

@@ -4,6 +4,7 @@
      address owner;
      
     function at(address _addr) returns (bytes o_code) {
+        assert(_addr != 0x0);
         assembly {
             // retrieve the size of the code, this needs assembly
             let size := extcodesize(_addr)
@@ -20,7 +21,7 @@
     }
      
     function bla() {
-        if(tx.origin == owner)
+        require(tx.origin == owner);
         msg.sender.send(19);
         assembly {
              

remix-solidity/test/analysis/test-contracts/globals.sol

@@ -32,8 +32,8 @@ contract a is bla {
         now;
         tx.gasprice;
         tx.origin;
-        //assert(now == block.timestamp);
+        // assert(1 == 2);
-        //require(now == block.timestamp);
+        // require(1 == 1);
         keccak256(a);
         sha3(a);
         sha256(a);
@@ -52,6 +52,7 @@ contract a is bla {
         //a.transfer(a.balance);
         selfdestruct(a);
         //revert();
+        assert(a.balance == 0);
     }
     
 }