Fix if statements

With tests to prove it! Hopefully the start of a long, good, habit...

instrumentSolidity.js

@@ -1,12 +1,10 @@
 // var SolidityParser = require("solidity-parser");
 var solparse = require("solparse");
-var fs = require('fs');
 
 var path = require("path");
-module.exports = function(pathToFile, instrumentingActive){
+module.exports = function(contract, fileName, instrumentingActive){
 
 	// var result = SolidityParser.parseFile("./" + pathToFile);
-	var contract = fs.readFileSync("./" + pathToFile).toString();
 	var result = solparse.parse(contract);
 	var instrumented = "";
 	const __INDENTATION__ = "    ";
@@ -19,7 +17,6 @@ module.exports = function(pathToFile, instrumentingActive){
 	var statementMap = {};
 	var statementId = 0;
 	var linecount = 1;
-	var fileName = path.basename(pathToFile);
 	var injectionPoints = {};
 
 	function createOrAppendInjectionPoint(key, value){
@@ -117,7 +114,6 @@ module.exports = function(pathToFile, instrumentingActive){
 		//what's the position of the most recent newline?
 		var startchar = expression.start
 		var endchar = expression.end
-
 		lastNewLine = contract.slice(0, startchar).lastIndexOf('\n');
 		nextNewLine = startchar + contract.slice(startchar).indexOf('\n');
 		var contractSnipped = contract.slice(lastNewLine, nextNewLine);
@@ -155,12 +151,12 @@ module.exports = function(pathToFile, instrumentingActive){
 			createOrAppendInjectionPoint(expression.consequent.start+1,{type: "callBranchEvent", branchId: branchId, locationIdx: 0} )
 		}else{
 			createOrAppendInjectionPoint(expression.consequent.start,{type: "callBranchEvent", branchId: branchId, locationIdx: 0, openBracket:true} )
-			createOrAppendInjectionPoint(expression.consequent.end, {type:"closeBracket"});
+			createOrAppendInjectionPoint(expression.consequent.end, {type:"closeBracketStart"});
 		}
 
 		if (expression.alternate && expression.alternate.type==='IfStatement'){
 			createOrAppendInjectionPoint(expression.alternate.start, {type: "callBranchEvent", branchId: branchId, locationIdx:1, openBracket: true})
-			createOrAppendInjectionPoint(expression.alternate.end, {type:"closeBracket"});
+			createOrAppendInjectionPoint(expression.alternate.end, {type:"closeBracketEnd"});
 			//It should get instrumented when we parse it
 		} else if (expression.alternate){
 			createOrAppendInjectionPoint(expression.alternate.start+1, {type: "callBranchEvent", branchId: branchId, locationIdx: 1})
@@ -371,7 +367,7 @@ module.exports = function(pathToFile, instrumentingActive){
 		injectionPoint = sortedPoints[x];
 		//Line instrumentation has to happen first
 		injectionPoints[injectionPoint].sort(function(a,b){
-			var eventTypes = ["openParen", "callBranchEvent","callEmptyBranchEvent","callEvent", "closeBracket"];
+			var eventTypes = ["openParen", "closeBracketStart", "callBranchEvent","callEmptyBranchEvent","callEvent", "closeBracketEnd"];
 			return eventTypes.indexOf(b.type) - eventTypes.indexOf(a.type);
 		});
 		for (y in injectionPoints[injectionPoint]){
@@ -390,7 +386,9 @@ module.exports = function(pathToFile, instrumentingActive){
 				contract = contract.slice(0, injectionPoint) + "(" + contract.slice(injectionPoint);
 			}else if (injection.type==='closeParen'){
 				contract = contract.slice(0, injectionPoint) + ")" + contract.slice(injectionPoint);
-			}else if (injection.type==='closeBracket'){
+			}else if (injection.type==='closeBracketStart'){
+				contract = contract.slice(0, injectionPoint) + "}" + contract.slice(injectionPoint);
+			}else if (injection.type==='closeBracketEnd'){
 				contract = contract.slice(0, injectionPoint) + "}" + contract.slice(injectionPoint);
 			}else if (injection.type==='literal'){
 				contract = contract.slice(0, injectionPoint) + injection.string + contract.slice(injectionPoint);

package.json

@@ -16,5 +16,8 @@
     "istanbul": "^0.4.5",
     "shelljs": "^0.7.4",
     "solparse": "^1.0.15"
+  },
+  "devDependencies": {
+    "mocha": "^3.1.0"
   }
 }

runCoveredTests.js

@@ -18,13 +18,20 @@ if (!shell.test('-e','./node_modules/ethereumjs-vm/lib/opFns.js.orig')){
 //Run the modified testrpc with large block limit
 var testrpcProcess = childprocess.exec('./node_modules/ethereumjs-testrpc/bin/testrpc --gasLimit 0xfffffffffff')
 
-shell.mv('./../contracts/', './../originalContracts');
+if (shell.test('-d','../originalContracts')){
+    console.log("There is already an 'originalContracts' directory in your truffle directory.\nThis is probably due to a previous solcover failure.\nPlease make sure the ./contracts/ directory contains your contracts (perhaps by copying them from originalContracts), and then delete the originalContracts directory.")
+    process.exit(1);
+}
+
+shell.mv('./../contracts/', './../originalContracts/');
 shell.mkdir('./../contracts/');
 //For each contract in originalContracts, get the instrumented version
 shell.ls('./../originalContracts/*.sol').forEach(function(file) {
     if (file !== 'originalContracts/Migrations.sol') {
         console.log("instrumenting ", file);
-        var instrumentedContractInfo = getInstrumentedVersion(file, true);
+        var contract = fs.readFileSync("./" + file).toString();
+        var fileName = path.basename(file);
+        var instrumentedContractInfo = getInstrumentedVersion(contract, fileName, true);
         fs.writeFileSync('./../contracts/' + path.basename(file), instrumentedContractInfo.contract);
         var canonicalContractPath = path.resolve('./../originalContracts/' + path.basename(file));
         coverage[canonicalContractPath] = { "l": {}, "path": canonicalContractPath, "s": {}, "b": {}, "f": {}, "fnMap": {}, "statementMap": {}, "branchMap": {} };

test/ifStatements.js

@@ -0,0 +1,49 @@
+var solc = require('solc');
+
+var getInstrumentedVersion = require('./../instrumentSolidity.js');
+
+describe('if statements', function(){
+  it('should instrument if statements with no brackets',function(){
+  	this.timeout(10000)
+  		var contract="contract Test{\n\
+  			function a(uint x){\n\
+	  			if (x==1) throw;\n\
+  			}\n\
+  		}"
+		var instrumentedContractInfo = getInstrumentedVersion(contract, "test.sol", true);
+		var output = solc.compile(instrumentedContractInfo.contract, 1); // 1 activates the optimiser
+		if (output.errors){
+			throw new Error("Instrumented solidity invalid: " + output.errors)
+		}
+    })
+  it('should instrument if statements with brackets',function(){
+  	this.timeout(10000)
+  		var contract="contract Test{\n\
+  			function a(uint x){\n\
+	  			if (x==1) { throw; }\n\
+  			}\n\
+  		}"
+		var instrumentedContractInfo = getInstrumentedVersion(contract, "test.sol", true);
+		var output = solc.compile(instrumentedContractInfo.contract, 1); // 1 activates the optimiser
+		if (output.errors){
+			throw new Error("Instrumented solidity invalid: " + output.errors)
+		}
+    })
+   it('should instrument nested if statements with missing else statements',function(){
+  	this.timeout(10000)
+  		var contract="contract Test{\n\
+  			function a(uint x,uint y, uint z){\n\
+	  			if (x==y){\n\
+			  	}else if ( x==2 ){\n\
+			  		if (y==z){\n\
+				  	}\n\
+	  			}\n\
+	  		}\n\
+  		}"
+		var instrumentedContractInfo = getInstrumentedVersion(contract, "test.sol", true);
+		var output = solc.compile(instrumentedContractInfo.contract, 1); // 1 activates the optimiser
+		if (output.errors){
+			throw new Error("Instrumented solidity invalid: " + output.errors)
+		}
+    })
+  })