Repository: protofire/solhint (js)

Issue:
check-send-result is triggered when using require
Code:
```solidity
contract Foo {
    function sendEth(address _receiver, uint256 _value) public {
        require(_receiver.send(_value));
    }
}
```

the rule `check-send-result` is not triggered when using an `if` statement or assigning the result of `send` to a variable but it is triggered when using `require` or `assert`. I think the last two are also valid ways of checking the result.

Pull request:
Don't emit 'check-send-result' when used inside a require or assert
Closes #204.

Patch:
diff --git a/lib/common/tree-traversing.js b/lib/common/tree-traversing.js
index a56e3d4..6487f5f 100644
--- a/lib/common/tree-traversing.js
+++ b/lib/common/tree-traversing.js
@@ -45,6 +45,23 @@ class TreeTraversing {
     }
   }
 
+  /**
+   * Traverses the tree up and checks `predicate` in each node.
+   *
+   * @returns {boolean}
+   */
+  someParent(node, predicate) {
+    let parent = node.parent
+    while (parent) {
+      if (predicate(parent)) {
+        return true
+      }
+      parent = parent.parent
+    }
+
+    return false
+  }
+
   *findIdentifier(ctx) {
     const children = ctx.children
 
diff --git a/lib/rules/security/check-send-result.js b/lib/rules/security/check-send-result.js
index 7d14399..508e535 100644
--- a/lib/rules/security/check-send-result.js
+++ b/lib/rules/security/check-send-result.js
@@ -46,12 +46,22 @@ class CheckSendResultChecker extends BaseChecker {
     if (node.memberName === 'send') {
       const hasVarDeclaration = traversing.statementNotContains(node, 'VariableDeclaration')
       const hasIfStatement = traversing.statementNotContains(node, 'IfStatement')
+      const hasRequire = traversing.someParent(node, this.isRequire)
+      const hasAssert = traversing.someParent(node, this.isAssert)
 
-      if (!hasIfStatement && !hasVarDeclaration) {
+      if (!hasIfStatement && !hasVarDeclaration && !hasRequire && !hasAssert) {
         this.error(node, 'Check result of "send" call')
       }
     }
   }
+
+  isRequire(node) {
+    return node.type === 'FunctionCall' && node.expression.name === 'require'
+  }
+
+  isAssert(node) {
+    return node.type === 'FunctionCall' && node.expression.name === 'assert'
+  }
 }
 
 module.exports = CheckSendResultChecker


Test patch:
diff --git a/test/rules/security/check-send-result.js b/test/rules/security/check-send-result.js
index b624b23..90e8db7 100644
--- a/test/rules/security/check-send-result.js
+++ b/test/rules/security/check-send-result.js
@@ -23,4 +23,54 @@ describe('Linter - check-send-result', () => {
 
     assert.equal(report.errorCount, 0)
   })
+
+  it('do not emit error when a require is used', () => {
+    const code = funcWith('require(x.send(1));')
+
+    const report = linter.processStr(code, {
+      rules: { 'check-send-result': 'error' }
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('do not emit error when a require is used upper in the tree', () => {
+    const code = funcWith('require(!x.send(1));')
+
+    const report = linter.processStr(code, {
+      rules: { 'check-send-result': 'error' }
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('do not emit error when an assert is used', () => {
+    const code = funcWith('assert(x.send(1));')
+
+    const report = linter.processStr(code, {
+      rules: { 'check-send-result': 'error' }
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('do not emit error when an assert is used upper in the tree', () => {
+    const code = funcWith('assert(x.send(1) || something);')
+
+    const report = linter.processStr(code, {
+      rules: { 'check-send-result': 'error' }
+    })
+
+    assert.equal(report.errorCount, 0)
+  })
+
+  it('emit error when an arbitrary function surrounds the expression', () => {
+    const code = funcWith('f(x.send(1));')
+
+    const report = linter.processStr(code, {
+      rules: { 'check-send-result': 'error' }
+    })
+
+    assert.equal(report.errorCount, 1)
+  })
 })