Improvements

lib/modules/coverage/contract_source.js

@@ -101,7 +101,7 @@ class ContractSource {
   generateCodeCoverage(trace) {
     if(!this.ast || !this.contractBytecode) throw new Error('Error generating coverage: solc output was not assigned');
 
-    var coverage = {
+    let coverage = {
       code: this.body.trim().split("\n"),
       l: {},
       path: this.file,
@@ -117,11 +117,12 @@ class ContractSource {
     var sourceMapToNodeType = {};
 
     do {
-      var node = nodesRequiringVisiting.pop();
+      let node = nodesRequiringVisiting.pop();
       if(!node) continue;
 
-      var children = [];
-      var markLocations = [];
+      let children = [];
+      let markLocations = [];
+      let location;
 
       switch(node.nodeType) {
         case 'Assignment':
@@ -132,10 +133,11 @@ class ContractSource {
           break;
 
         case 'IfStatement':
-          coverage.b[node.id] = [0,0];
+          location = this.sourceMapToLocations(node.src);
+          let trueBranchLocation = this.sourceMapToLocations(node.trueBody.src);
 
-          var location = this.sourceMapToLocations(node.src);
-          var trueBranchLocation = this.sourceMapToLocations(node.trueBody.src);
+          let declarationSourceMap = new SourceMap(node.src).subtract(new SourceMap(node.trueBody.src));
+          let declarationLocation = this.sourceMapToLocations(declarationSourceMap.toString());
 
           var falseBranchLocation;
           if(node.falseBody) {
@@ -144,6 +146,7 @@ class ContractSource {
             falseBranchLocation = trueBranchLocation;
           }
 
+          coverage.b[node.id] = [0,0];
           coverage.branchMap[node.id] = {
             type: 'if',
             locations: [trueBranchLocation, falseBranchLocation],
@@ -155,7 +158,7 @@ class ContractSource {
 
           if(node.falseBody) children = children.concat(node.falseBody.statements);
 
-          markLocations = [location, trueBranchLocation, falseBranchLocation];
+          markLocations = [declarationLocation];
 
           if(node.trueBody.statements[0]) {
             node.trueBody.statements[0]._parent = {type: 'b', id: node.id, idx: 0};
@@ -169,18 +172,9 @@ class ContractSource {
           break;
 
         case 'BinaryOperation':
-          children = [node.leftExpression, node.rightExpression];
-          break;
-
-        case 'Return':
-          // Return is a bit of a special case, because the statement src is the
-          // return "body". We don't `break` here on purpose so it can share the
-          // same logic below.
-          node.src = node.expression.src;
-          // falls through
-
         case 'ExpressionStatement':
         case 'Identifier':
+        case 'Return':
         case 'UnaryOperation':
           coverage.s[node.id] = 0;
 
@@ -196,7 +190,7 @@ class ContractSource {
           });
 
 
-          children = node.expression ? [node.expression] : [];
+          //children = node.expression ? [node.expression] : [];
           markLocations = [location];
           break;
 
@@ -250,8 +244,6 @@ class ContractSource {
           if(!sourceMapToNodeType[node.initializationExpression.src]) sourceMapToNodeType[node.initializationExpression.src] = [];
           sourceMapToNodeType[node.initializationExpression.src].push({type: 's', id: node.id, body: {loc: location}});
 
-          console.dir(node);
-
           children = node.body.statements;
           markLocations = [initializationLocation];
           break;
@@ -293,22 +285,21 @@ class ContractSource {
       if(!contractMatches) break;
 
       console.log(sourceMapToNodeType);
-
+      console.log('---------------------------------------------------');
       trace.structLogs.forEach((step) => {
         step = bytecode[step.pc];
-        if(!step.sourceMap || step.sourceMap == '') return;
-
+        if(!step.sourceMap || step.sourceMap == '' || step.sourceMap == SourceMap.empty()) return;
+        console.dir(step.sourceMap);
         let sourceMapString = step.sourceMap.toString(this.id);
         var nodes = sourceMapToNodeType[sourceMapString];
 
-        if(step.sourceMap.offset == 189) console.log(nodes);
         if(!nodes) return;
 
         nodes.forEach((node) => {
           // Skip duplicate function reports by only reporting when there is a jump.
           if(node.type == 'f' && step.jump) return;
 
-          if(node.body && node.body.loc) {
+          if(node.type != 'b' && node.body && node.body.loc) {
             for(var line = node.body.loc.start.line; line <= node.body.loc.end.line; line++) {
               coverage.l[line]++;
             }

test_apps/coverage_app/contracts/branches.sol

@@ -0,0 +1,35 @@
+pragma solidity ^0.4.23;
+
+contract Branches {
+  uint public lastComparison;
+  uint public storedData;
+
+  constructor(uint initialValue) public {
+    storedData = initialValue;
+  }
+
+  function bigger(uint x) public returns (uint biggest) {
+    lastComparison = x;
+
+    if(x > storedData) {
+      storedData = x;
+      return x;
+    } else {
+      return storedData;
+    }
+  }
+
+  function smaller(uint x) public returns (uint smallest) {
+    lastComparison = x;
+
+    if(x < storedData) {
+      return x;
+    } else {
+      return storedData;
+    }
+  }
+
+  function get() public view returns (uint retVal) {
+    return storedData;
+  }
+}

test_apps/coverage_app/test/branches_spec.js

@@ -0,0 +1,31 @@
+/*global contract, config, it, assert*/
+const Branches = require('Embark/contracts/Branches');
+
+config({
+  contracts: {
+    "Branches": {
+      args: [5]
+    }
+  }
+});
+
+contract("Branches", function() {
+  it("should return the bigger value and set it", function(done) {
+    Branches.methods.bigger(10).send().then(() => {
+      Branches.methods.get().call().then((result) => {
+        assert.strictEqual(parseInt(result, 10), 10);
+        done();
+      });
+    });
+  });
+
+  it("should return the set number if it's bigger", function(done) {
+    Branches.methods.bigger(1).send().then(() => {
+      done();
+    });
+  });
+
+  it("should return the smaller number", function(done) {
+    Branches.methods.smaller(10).send().then(() => { done() });
+  });
+});