From 70b39e890c300c7da5bd6cda54cca80be8bd1c7e Mon Sep 17 00:00:00 2001
From: soad003 <michael.froewis@gmx.at>
Date: Wed, 2 May 2018 11:17:41 +0200
Subject: [PATCH] Static Analysis: unassigned binops, bug fix, wording

---
 .../src/analysis/modules/assignAndCompare.js        | 11 +++++------
 .../src/analysis/modules/staticAnalysisCommon.js    | 13 +++++++++++--
 .../test/analysis/staticAnalysisIntegration-test.js |  2 +-
 .../analysis/test-contracts/blockLevelCompare.sol   |  2 ++
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/remix-solidity/src/analysis/modules/assignAndCompare.js b/remix-solidity/src/analysis/modules/assignAndCompare.js
index e9c465a457..3d82ec7102 100644
--- a/remix-solidity/src/analysis/modules/assignAndCompare.js
+++ b/remix-solidity/src/analysis/modules/assignAndCompare.js
@@ -1,5 +1,5 @@
-var name = 'Assign or Compare: '
-var desc = 'Assign and compare operators can be confusing.'
+var name = 'Result not used: '
+var desc = 'The result of an operation was not used.'
 var categories = require('./categories')
 var common = require('./staticAnalysisCommon')
 
@@ -8,15 +8,14 @@ function assignAndCompare () {
 }
 
 assignAndCompare.prototype.visit = function (node) {
-  if (common.isBlockWithTopLevelUnAssignedBinOp(node)) this.warningNodes.push(node)
+  if (common.isBlockWithTopLevelUnAssignedBinOp(node)) common.getUnAssignedTopLevelBinOps(node).forEach((n) => this.warningNodes.push(n))
 }
 
 assignAndCompare.prototype.report = function (compilationResults) {
   return this.warningNodes.map(function (item, i) {
     return {
-      warning: 'Use of "this" for local functions: Never use this to call functions in the same contract, it only consumes more gas than normal local calls.',
-      location: item.src,
-      more: 'http://solidity.readthedocs.io/en/develop/control-structures.html#external-function-calls'
+      warning: 'A binary operation yields a value that is not used in the following. This is often caused by confusing assignment (=) and comparison (==).',
+      location: item.src
     }
   })
 }
diff --git a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js
index 277224e4f3..571b822e3d 100644
--- a/remix-solidity/src/analysis/modules/staticAnalysisCommon.js
+++ b/remix-solidity/src/analysis/modules/staticAnalysisCommon.js
@@ -24,7 +24,8 @@ var nodeTypes = {
   RETURN: 'Return',
   IFSTATEMENT: 'IfStatement',
   FORSTATEMENT: 'ForStatement',
-  WHILESTATEMENT: 'WhileStatement'
+  WHILESTATEMENT: 'WhileStatement',
+  DOWHILESTATEMENT: 'DoWhileStatement'
 }
 
 var basicTypes = {
@@ -385,6 +386,10 @@ function getFullQuallyfiedFuncDefinitionIdent (contract, func, paramTypes) {
   return getContractName(contract) + '.' + getFunctionDefinitionName(func) + '(' + util.concatWithSeperator(paramTypes, ',') + ')'
 }
 
+function getUnAssignedTopLevelBinOps (blocklike) {
+  return blocklike.children.filter(isBinaryOpInExpression)
+}
+
 // #################### Trivial Node Identification
 
 function isFunctionDefinition (node) {
@@ -621,7 +626,10 @@ function isBlockWithTopLevelUnAssignedBinOp (node) {
 }
 
 function isBlockLikeStatement (node) {
-  return (nodeType(node, exactMatch(nodeTypes.IFSTATEMENT)) || nodeType(node, exactMatch(nodeTypes.FORSTATEMENT)) || nodeType(node, exactMatch(nodeTypes.WHILESTATEMENT))) &&
+  return (nodeType(node, exactMatch(nodeTypes.IFSTATEMENT)) ||
+            nodeType(node, exactMatch(nodeTypes.FORSTATEMENT)) ||
+            nodeType(node, exactMatch(nodeTypes.WHILESTATEMENT)) ||
+            nodeType(node, exactMatch(nodeTypes.DOWHILESTATEMENT))) &&
           minNrOfChildren(node, 2) && !nodeType(node.children[1], exactMatch(nodeTypes.BLOCK))
 }
 
@@ -937,6 +945,7 @@ module.exports = {
   getStateVariableDeclarationsFormContractNode: getStateVariableDeclarationsFormContractNode,
   getFunctionOrModifierDefinitionParameterPart: getFunctionOrModifierDefinitionParameterPart,
   getFunctionOrModifierDefinitionReturnParameterPart: getFunctionOrModifierDefinitionReturnParameterPart,
+  getUnAssignedTopLevelBinOps: getUnAssignedTopLevelBinOps,
 
   // #################### Complex Node Identification
   isDeleteOfDynamicArray: isDeleteOfDynamicArray,
diff --git a/remix-solidity/test/analysis/staticAnalysisIntegration-test.js b/remix-solidity/test/analysis/staticAnalysisIntegration-test.js
index 09423109fb..b03df5e05b 100644
--- a/remix-solidity/test/analysis/staticAnalysisIntegration-test.js
+++ b/remix-solidity/test/analysis/staticAnalysisIntegration-test.js
@@ -559,7 +559,7 @@ test('Integration test assignAndCompare.js', function (t) {
     'forgottenReturn.sol': 0,
     'selfdestruct.sol': 0,
     'deleteDynamicArray.sol': 0,
-    'blockLevelCompare.sol': 6
+    'blockLevelCompare.sol': 8
   }
 
   runModuleOnFiles(module, t, (file, report) => {
diff --git a/remix-solidity/test/analysis/test-contracts/blockLevelCompare.sol b/remix-solidity/test/analysis/test-contracts/blockLevelCompare.sol
index d51ad108c3..14c4eb9d7a 100644
--- a/remix-solidity/test/analysis/test-contracts/blockLevelCompare.sol
+++ b/remix-solidity/test/analysis/test-contracts/blockLevelCompare.sol
@@ -32,11 +32,13 @@ contract grr {
 
         while(false) {
             int c = 3;
+            uint(c) + a;
 
             c == 5;
 
         }
 
+        a + b;
         breaker = false;
     }