diff --git a/lib/registrar.js b/lib/registrar.js index 4c6b6c4..bdad33f 100644 --- a/lib/registrar.js +++ b/lib/registrar.js @@ -182,29 +182,50 @@ class Registrar { * @param {Object} expression AST node */ addNewLogicalORBranch(contract, expression) { - const startContract = contract.instrumented.slice(0, expression.range[0]); - const startline = ( startContract.match(/\n/g) || [] ).length + 1; + let start; + + // Instabul HTML highlighting location data... + const leftZero = expression.left.range[0]; + const leftOne = expression.left.range[1]; + const rightZero = expression.right.range[0]; + const rightOne = expression.right.range[1]; + + start = contract.instrumented.slice(0, leftZero); + const leftStartLine = ( start.match(/\n/g) || [] ).length + 1; + const leftStartCol = leftZero - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, leftOne); + const leftEndLine = ( start.match(/\n/g) || [] ).length + 1; + const leftEndCol = leftOne - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, rightZero); + const rightStartLine = ( start.match(/\n/g) || [] ).length + 1; + const rightStartCol = rightZero - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, rightOne); + const rightEndLine = ( start.match(/\n/g) || [] ).length + 1; + const rightEndCol = rightOne - start.lastIndexOf('\n') - 1; contract.branchId += 1; // NB locations for if branches in istanbul are zero // length and associated with the start of the if. contract.branchMap[contract.branchId] = { - line: startline, + line: leftStartLine, type: 'cond-expr', locations: [{ start: { - line: startline, column: expression.left.range[0], + line: leftStartLine, column: leftStartCol, }, end: { - line: startline, column: expression.left.range[1], + line: leftEndLine, column: leftEndCol, }, }, { start: { - line: startline, column: expression.right.range[0], + line: rightStartLine, column: rightStartCol, }, end: { - line: startline, column: expression.right.range[1], + line: rightEndLine, column: rightEndCol, }, }], }; diff --git a/test/integration/projects/logical-or/.solcover.js b/test/integration/projects/logical-or/.solcover.js new file mode 100644 index 0000000..c245304 --- /dev/null +++ b/test/integration/projects/logical-or/.solcover.js @@ -0,0 +1,8 @@ +// Testing hooks +const fn = (msg, config) => config.logger.log(msg); + +module.exports = { + skipFiles: ['Migrations.sol'], + silent: process.env.SILENT ? true : false, + istanbulReporter: ['json-summary', 'text'], +} diff --git a/test/integration/projects/logical-or/buidler.config.js b/test/integration/projects/logical-or/buidler.config.js new file mode 100644 index 0000000..fcdac9a --- /dev/null +++ b/test/integration/projects/logical-or/buidler.config.js @@ -0,0 +1,7 @@ +const { loadPluginFile } = require("@nomiclabs/buidler/plugins-testing"); +loadPluginFile(__dirname + "/../plugins/buidler.plugin"); +usePlugin("@nomiclabs/buidler-truffle5"); + +module.exports={ + defaultNetwork: "buidlerevm" +}; diff --git a/test/integration/projects/logical-or/contracts/ContractA.sol b/test/integration/projects/logical-or/contracts/ContractA.sol new file mode 100644 index 0000000..8c20a73 --- /dev/null +++ b/test/integration/projects/logical-or/contracts/ContractA.sol @@ -0,0 +1,40 @@ +pragma solidity ^0.5.0; + + +contract ContractA { + + function _if(uint i) public pure { + if (i == 0 || i > 5){ + /* ignore */ + } + } + + function _if_and(uint i) public pure { + if (i != 0 && (i < 2 || i > 5)){ + /* ignore */ + } + } + + function _return(uint i) public pure returns (bool){ + return (i != 0 && i != 1 ) || + ((i + 1) == 2); + } + + function _while(uint i) public pure returns (bool){ + uint counter; + while( (i == 1 || i == 2) && counter < 2 ){ + counter++; + } + } + + function _require(uint x) public { + require(x == 1 || x == 2); + } + + function _require_multi_line(uint x) public { + require( + (x == 1 || x == 2) || + x == 3 + ); + } +} diff --git a/test/integration/projects/logical-or/test/test.js b/test/integration/projects/logical-or/test/test.js new file mode 100644 index 0000000..a9a864b --- /dev/null +++ b/test/integration/projects/logical-or/test/test.js @@ -0,0 +1,33 @@ +const ContractA = artifacts.require("ContractA"); + +contract("contracta", function(accounts) { + let instance; + + before(async () => instance = await ContractA.new()) + + it('_if', async function(){ + await instance._if(0); + await instance._if(7); + }); + + it('_if_and', async function(){ + await instance._if_and(1); + }); + + it('_return', async function(){ + await instance._return(4); + }); + + it('_while', async function(){ + await instance._while(1); + }); + + it('_require', async function(){ + await instance._require(2); + }) + + it('_require_multi_line', async function(){ + await instance._require_multi_line(1); + await instance._require_multi_line(3); + }) +}); diff --git a/test/integration/projects/logical-or/truffle-config.js b/test/integration/projects/logical-or/truffle-config.js new file mode 100644 index 0000000..b398b07 --- /dev/null +++ b/test/integration/projects/logical-or/truffle-config.js @@ -0,0 +1,7 @@ +module.exports = { + networks: {}, + mocha: {}, + compilers: { + solc: {} + } +} diff --git a/test/sources/solidity/contracts/or/require-multiline-or.sol b/test/sources/solidity/contracts/or/require-multiline-or.sol new file mode 100644 index 0000000..04d195b --- /dev/null +++ b/test/sources/solidity/contracts/or/require-multiline-or.sol @@ -0,0 +1,11 @@ +pragma solidity ^0.5.0; + +contract Test { + function a(uint x) public { + require( + x == 1 || + x == 2 || + x == 3 + ); + } +} diff --git a/test/units/buidler/standard.js b/test/units/buidler/standard.js index 485af9a..e12e73b 100644 --- a/test/units/buidler/standard.js +++ b/test/units/buidler/standard.js @@ -275,6 +275,25 @@ describe('Buidler Plugin: standard use cases', function() { ); }); + // This works on it's own but there's some kind of weird interaction with + // the solc 6 test which causes subsequent cov measurements to be zero. + // Have tried re-ordering etc...???. Truffle tests this & should be the same anyway... + it.skip('logicalOR', async function(){ + mock.installFullProject('logical-or'); + mock.buidlerSetupEnv(this); + + await this.env.run("coverage"); + + const expected = [ + { + file: mock.pathToContract(buidlerConfig, 'ContractA.sol'), + pct: 59.09 + } + ]; + + verify.branchCoverage(expected); + }) + it('solc 0.6.x', async function(){ mock.installFullProject('solc-6'); mock.buidlerSetupEnv(this); @@ -294,4 +313,4 @@ describe('Buidler Plugin: standard use cases', function() { verify.lineCoverage(expected); }) -}) \ No newline at end of file +}) diff --git a/test/units/or.js b/test/units/or.js index 74a9a22..e20e279 100644 --- a/test/units/or.js +++ b/test/units/or.js @@ -79,6 +79,33 @@ describe('logical OR branches', () => { }); }); + // require( + // x == 1 || + // x == 2 || + // x == 3 + // ) + it('should cover a require statement with multiline OR condition (two branches)', async function() { + const contract = await util.bootstrapCoverage('or/require-multiline-or', provider, collector); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(1); + await contract.instance.a(3); + + const mapping = coverage.generate(contract.data, util.pathPrefix); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 2, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [2, 0], 2: [1, 0], 3: [0, 1] + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 2, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 2, + }); + }); + // require(x == 1 || x == 2) it('should cover a require statement with a simple OR condition (both branches)', async function() { const contract = await util.bootstrapCoverage('or/require-or', provider, collector); diff --git a/test/units/truffle/standard.js b/test/units/truffle/standard.js index 00da625..5c1f53c 100644 --- a/test/units/truffle/standard.js +++ b/test/units/truffle/standard.js @@ -433,4 +433,18 @@ describe('Truffle Plugin: standard use cases', function() { verify.lineCoverage(expected); }) + + it('logicalOR', async function(){ + mock.installFullProject('logical-or'); + await plugin(truffleConfig); + + const expected = [ + { + file: mock.pathToContract(truffleConfig, 'ContractA.sol'), + pct: 59.09 + } + ]; + + verify.branchCoverage(expected); + }) }) diff --git a/test/util/verifiers.js b/test/util/verifiers.js index b5a25d7..4c5eb73 100644 --- a/test/util/verifiers.js +++ b/test/util/verifiers.js @@ -18,6 +18,19 @@ function lineCoverage(expected=[]){ }); } +function branchCoverage(expected=[]){ + let summary = JSON.parse(fs.readFileSync('coverage/coverage-summary.json')); + + expected.forEach((item, idx) => { + assert( + summary[item.file].branches.pct === item.pct, + + `For condition ${idx} - expected ${item.pct} %,` + + `saw - ${summary[item.file].branches.pct} %` + ) + }); +} + function coverageMissing(expected=[]){ let summary = JSON.parse(fs.readFileSync('coverage/coverage-summary.json')); expected.forEach(item => assert(summary[item.file] === undefined)) @@ -47,6 +60,7 @@ function coverageNotGenerated(config){ module.exports = { pathExists: pathExists, lineCoverage: lineCoverage, + branchCoverage: branchCoverage, coverageMissing: coverageMissing, cleanInitialState: cleanInitialState, coverageGenerated: coverageGenerated,