Merge pull request #55 from sc-forks/aragon-inst-bug

Instrument conditional assignment to MemberExpressions

lib/instrumenter.js

@@ -11,6 +11,13 @@ function createOrAppendInjectionPoint(contract, key, value) {
   }
 }
 
+instrumenter.prePosition = function prePosition(expression){
+  if (expression.right.type === 'ConditionalExpression' && 
+      expression.left.type === 'MemberExpression'){
+    expression.start -= 2;
+  }
+}
+
 instrumenter.instrumentAssignmentExpression = function instrumentAssignmentExpression(contract, expression) {
   // The only time we instrument an assignment expression is if there's a conditional expression on
   // the right
@@ -22,8 +29,17 @@ instrumenter.instrumentAssignmentExpression = function instrumentAssignmentExpre
         type: 'literal', string: '; (,' + expression.left.name + ')',
       });
       instrumenter.instrumentConditionalExpression(contract, expression.right);
+    } else if (expression.left.type === 'MemberExpression'){
+      createOrAppendInjectionPoint(contract, expression.left.start, {
+        type: 'literal', string: '(,',
+      });
+      createOrAppendInjectionPoint(contract, expression.left.end, {
+        type: 'literal', string: ')',
+      });
+      instrumenter.instrumentConditionalExpression(contract, expression.right);
     } else {
-      console.log(contract, expression.left);
+      const err = 'Error instrumenting assignment expression @ solidity-coverage/lib/instrumenter.js';
+      console.log(err, contract, expression.left);
       process.exit();
     }
   }

lib/parse.js

@@ -10,6 +10,7 @@ const parse = {};
 const instrumenter = require('./instrumenter');
 
 parse.AssignmentExpression = function parseAssignmentExpression(contract, expression) {
+  instrumenter.prePosition(expression);
   instrumenter.instrumentStatement(contract, expression);
   instrumenter.instrumentAssignmentExpression(contract, expression);
 };

test/conditional.js

@@ -159,6 +159,30 @@ describe('conditional statements', () => {
     }).catch(done);
   });
 
+  it('should cover an assignment to a member expression (reaches the alternate)', done => {
+    const contract = util.getCode('conditional/mapping-assignment.sol');
+    const info = getInstrumentedVersion(contract, filePath);
+    const coverage = new CoverageMap();
+    coverage.addContract(info, filePath);
+
+    vm.execute(info.contract, 'a', []).then(events => {
+      const mapping = coverage.generate(events, pathPrefix);
+      assert.deepEqual(mapping[filePath].l, {
+        '11': 1, '12': 1,
+      });
+      assert.deepEqual(mapping[filePath].b, {
+        1: [0, 1],
+      });
+      assert.deepEqual(mapping[filePath].s, {
+        '1': 1, '2': 1,
+      });
+      assert.deepEqual(mapping[filePath].f, {
+        1: 1,
+      });
+      done();
+    }).catch(done);
+  });
+
   // Solcover has trouble with this case. The conditional coverage strategy relies on being able to
   // reference the left-hand variable before its value is assigned. Solidity doesn't allow this
   // for 'var'.

test/sources/conditional/mapping-assignment.sol

@@ -0,0 +1,14 @@
+pragma solidity ^0.4.3;
+
+contract Test {
+  struct Vote {
+    mapping (address => uint) voted;
+  }
+
+  Vote vote;
+
+  function a(){
+    var isYay = false;  
+    vote.voted[msg.sender] = isYay ? 1 : 2;
+  }
+}