Merge pull request #1314 from ethereum/fix/#994

loop over dynamic array length will show warning

remix-analyzer/src/solidity-analyzer/modules/forLoopIteratesOverDynamicArray.js

@@ -1,17 +1,24 @@
 var name = 'For loop iterates over dynamic array: '
 var desc = 'The number of \'for\' loop iterations depends on dynamic array\'s size'
 var categories = require('./categories')
-var common = require('./staticAnalysisCommon')
+var { isForLoop, isDynamicArrayLengthAccess, isBinaryOperation } = require('./staticAnalysisCommon')
 
 function forLoopIteratesOverDynamicArray () {
   this.relevantNodes = []
 }
 
 forLoopIteratesOverDynamicArray.prototype.visit = function (node) {
-  if (common.isForLoop(node) &&
-    node.children[1].children[1].attributes.member_name === 'length' &&
-    node.children[1].children[1].children[0].attributes.type.indexOf('[]') !== -1) {
-    this.relevantNodes.push(node)
+  if (isForLoop(node)) {
+    // Access 'condition' node of 'for' loop statement
+    let forLoopConditionNode = node.children[1]
+    // Access right side of condition as its children
+    let conditionChildrenNode = forLoopConditionNode.children[1]
+    // Check if it is a binary operation. if yes, check if its children node access length of dynamic array
+    if (isBinaryOperation(conditionChildrenNode) && isDynamicArrayLengthAccess(conditionChildrenNode.children[0])) {
+      this.relevantNodes.push(node)
+    } else if (isDynamicArrayLengthAccess(conditionChildrenNode)) { // else check if condition node itself access length of dynamic array
+      this.relevantNodes.push(node)
+    }
   }
 }
 

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

@@ -517,6 +517,18 @@ function isDynamicArrayAccess (node) {
   return node && nodeType(node, exactMatch(nodeTypes.IDENTIFIER)) && (node.attributes.type.endsWith('[] storage ref') || node.attributes.type === 'bytes storage ref' || node.attributes.type === 'string storage ref')
 }
 
+/**
+ * True if node accesses 'length' member of dynamic array
+ * @node {ASTNode} node to check for
+ * @return {bool}
+ */
+function isDynamicArrayLengthAccess (node) {
+  return node && // if node exists
+  nodeType(node, exactMatch(nodeTypes.MEMBERACCESS)) && // is memberAccess Node
+  (node.attributes.member_name === 'length') && // accessing 'length' member
+  node.children[0].attributes.type.indexOf('[]') !== -1 // member is accessed from dynamic array, notice [] without any number
+}
+
 /**
  * True if node is a delete instruction for an element from a dynamic array
  * @node {ASTNode} node to check for
@@ -1111,6 +1123,7 @@ module.exports = {
   isAbiNamespaceCall: isAbiNamespaceCall,
   isSpecialVariableAccess: isSpecialVariableAccess,
   isDynamicArrayAccess: isDynamicArrayAccess,
+  isDynamicArrayLengthAccess: isDynamicArrayLengthAccess,
   isIndexAccess: isIndexAccess,
   isMappingIndexAccess: isMappingIndexAccess,
   isSubScopeWithTopLevelUnAssignedBinOp: isSubScopeWithTopLevelUnAssignedBinOp,
@@ -1171,6 +1184,7 @@ module.exports = {
   isStatement: isStatement,
   isExpressionStatement: isExpressionStatement,
   isBlock: isBlock,
+  isBinaryOperation: isBinaryOperation,
 
   // #################### Constants
   nodeTypes: nodeTypes,