From 64f205ab224a4c73fea1be1b90309f83b8672188 Mon Sep 17 00:00:00 2001 From: cgewecke Date: Mon, 7 Dec 2020 20:10:59 -0800 Subject: [PATCH] Add coverage for ternary conditionals (#587) --- lib/coverage.js | 3 +- lib/injector.js | 52 +++- lib/parse.js | 23 +- lib/registrar.js | 92 ++++++- lib/ternary/conditional.js | 185 -------------- lib/ternary/ternary.js | 121 --------- .../.solcover.js | 2 +- .../contracts/Contract_OR.sol | 6 + .../contracts/Contract_ternary.sol | 54 ++++ .../hardhat.config.js | 0 .../test/test_or.js | 4 + .../test/test_ternary.js | 16 ++ .../truffle-config.js | 0 .../contracts/conditional/and-condition.sol | 9 + .../conditional/or-always-false-condition.sol | 9 + .../contracts/conditional/or-condition.sol | 9 + .../conditional/unbracketed-condition.sol | 9 + .../conditional/unbracketed-or-condition.sol | 9 + test/units/conditional.js | 232 ++++++++++++++++++ test/units/hardhat/standard.js | 15 +- test/units/truffle/standard.js | 12 +- 21 files changed, 530 insertions(+), 332 deletions(-) delete mode 100644 lib/ternary/conditional.js delete mode 100644 lib/ternary/ternary.js rename test/integration/projects/{logical-or => ternary-and-logical-or}/.solcover.js (75%) rename test/integration/projects/{logical-or => ternary-and-logical-or}/contracts/Contract_OR.sol (87%) create mode 100644 test/integration/projects/ternary-and-logical-or/contracts/Contract_ternary.sol rename test/integration/projects/{logical-or => ternary-and-logical-or}/hardhat.config.js (100%) rename test/integration/projects/{logical-or => ternary-and-logical-or}/test/test_or.js (89%) create mode 100644 test/integration/projects/ternary-and-logical-or/test/test_ternary.js rename test/integration/projects/{logical-or => ternary-and-logical-or}/truffle-config.js (100%) create mode 100644 test/sources/solidity/contracts/conditional/and-condition.sol create mode 100644 test/sources/solidity/contracts/conditional/or-always-false-condition.sol create mode 100644 test/sources/solidity/contracts/conditional/or-condition.sol create mode 100644 test/sources/solidity/contracts/conditional/unbracketed-condition.sol create mode 100644 test/sources/solidity/contracts/conditional/unbracketed-or-condition.sol create mode 100644 test/units/conditional.js diff --git a/lib/coverage.js b/lib/coverage.js index 0fe1b71..4ad1ce8 100644 --- a/lib/coverage.js +++ b/lib/coverage.js @@ -79,7 +79,8 @@ class Coverage { case 'function': this.data[contractPath].f[id] = hits; break; case 'statement': this.data[contractPath].s[id] = hits; break; case 'branch': this.data[contractPath].b[id][data.locationIdx] = hits; break; - case 'logicalOR': this.data[contractPath].b[id][data.locationIdx] = hits / 2; break; + case 'and-true': this.data[contractPath].b[id][data.locationIdx] = hits / 2; break; + case 'or-false': this.data[contractPath].b[id][data.locationIdx] = hits / 2; break; case 'requirePre': this.requireData[contractPath][id].preEvents = hits; break; case 'requirePost': this.requireData[contractPath][id].postEvents = hits; break; } diff --git a/lib/injector.js b/lib/injector.js index c689408..5305eac 100644 --- a/lib/injector.js +++ b/lib/injector.js @@ -14,8 +14,10 @@ class Injector { _getInjectable(id, hash, type){ switch(type){ - case 'logicalOR': + case 'and-true': return ` && ${this._getTrueMethodIdentifier(id)}(${hash}))`; + case 'or-false': + return ` || ${this._getFalseMethodIdentifier(id)}(${hash}))`; default: return `${this._getDefaultMethodIdentifier(id)}(${hash}); /* ${type} */ \n`; } @@ -31,11 +33,16 @@ class Injector { return `c_${web3Utils.keccak256(id).slice(0,10)}` } - // Method returns boolean true + // Method returns boolean: true _getTrueMethodIdentifier(id){ return `c_true${web3Utils.keccak256(id).slice(0,10)}` } + // Method returns boolean: false + _getFalseMethodIdentifier(id){ + return `c_false${web3Utils.keccak256(id).slice(0,10)}` + } + _getInjectionComponents(contract, injectionPoint, id, type){ const { start, end } = this._split(contract, injectionPoint); const hash = this._getHash(id) @@ -82,7 +89,19 @@ class Injector { _getTrueMethodDefinition(id){ const hash = web3Utils.keccak256(id).slice(0,10); const method = this._getTrueMethodIdentifier(id); - return `\nfunction ${method}(bytes32 c__${hash}) public pure returns (bool){ return true; }\n`; + return `function ${method}(bytes32 c__${hash}) public pure returns (bool){ return true; }\n`; + } + + /** + * Generates a solidity statement injection defining a method + * *which returns boolean false* to pass instrumentation hash to. + * @param {String} fileName + * @return {String} ex: bytes32[1] memory _sc_82e0891 + */ + _getFalseMethodDefinition(id){ + const hash = web3Utils.keccak256(id).slice(0,10); + const method = this._getFalseMethodIdentifier(id); + return `function ${method}(bytes32 c__${hash}) public pure returns (bool){ return false; }\n`; } injectLine(contract, fileName, injectionPoint, injection, instrumentation){ @@ -247,6 +266,7 @@ class Injector { contract.instrumented = `${start}` + `${defaultMethodDefinition}` + `${this._getTrueMethodDefinition(id)}` + + `${this._getFalseMethodDefinition(id)}` + `${end}`; } @@ -256,8 +276,30 @@ class Injector { contract.instrumented = `${start}(${end}`; } - injectLogicalOR(contract, fileName, injectionPoint, injection, instrumentation){ - const type = 'logicalOR'; + injectAndTrue(contract, fileName, injectionPoint, injection, instrumentation){ + const type = 'and-true'; + const id = `${fileName}:${injection.contractName}`; + + const { + start, + end, + hash, + injectable + } = this._getInjectionComponents(contract, injectionPoint, id, type); + + instrumentation[hash] = { + id: injection.branchId, + locationIdx: injection.locationIdx, + type: type, + contractPath: fileName, + hits: 0 + } + + contract.instrumented = `${start}${injectable}${end}`; + } + + injectOrFalse(contract, fileName, injectionPoint, injection, instrumentation){ + const type = 'or-false'; const id = `${fileName}:${injection.contractName}`; const { diff --git a/lib/parse.js b/lib/parse.js index 42abdbd..65eb2e5 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -33,11 +33,21 @@ parse.Block = function(contract, expression) { }; parse.BinaryOperation = function(contract, expression, skipStatementRegistry) { + // Free-floating ternary conditional + if (expression.left && expression.left.type === 'Conditional'){ + parse[expression.left.type](contract, expression.left, true); + register.statement(contract, expression); + + // Ternary conditional assignment + } else if (expression.right && expression.right.type === 'Conditional'){ + parse[expression.right.type](contract, expression.right, true); + register.statement(contract, expression); + // Regular binary operation - if (!skipStatementRegistry){ + } else if(!skipStatementRegistry){ register.statement(contract, expression); - // LogicalOR conditional search... + // LogicalOR condition search... } else { parse[expression.left.type] && parse[expression.left.type](contract, expression.left, true); @@ -83,12 +93,10 @@ parse.FunctionCall = function(contract, expression, skipStatementRegistry) { }; parse.Conditional = function(contract, expression, skipStatementRegistry) { - if (!skipStatementRegistry){ - register.statement(contract, expression); - } - parse[expression.condition.type] && parse[expression.condition.type](contract, expression.condition, skipStatementRegistry); + + register.conditional(contract, expression); }; parse.ContractDefinition = function(contract, expression) { @@ -291,6 +299,9 @@ parse.UsingStatement = function (contract, expression) { }; parse.VariableDeclarationStatement = function (contract, expression) { + if (expression.initialValue && expression.initialValue.type === 'Conditional'){ + parse[expression.initialValue.type](contract, expression.initialValue, true) + } register.statement(contract, expression); }; diff --git a/lib/registrar.js b/lib/registrar.js index 563ec43..e6c3def 100644 --- a/lib/registrar.js +++ b/lib/registrar.js @@ -186,6 +186,54 @@ class Registrar { }; }; + addNewConditionalBranch(contract, expression){ + let start; + // Instabul HTML highlighting location data... + const trueZero = expression.trueExpression.range[0]; + const trueOne = expression.trueExpression.range[1]; + const falseZero = expression.falseExpression.range[0]; + const falseOne = expression.falseExpression.range[1]; + + start = contract.instrumented.slice(0, trueZero); + const trueStartLine = ( start.match(/\n/g) || [] ).length + 1; + const trueStartCol = trueZero - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, trueOne); + const trueEndLine = ( start.match(/\n/g) || [] ).length + 1; + const trueEndCol = trueOne - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, falseZero); + const falseStartLine = ( start.match(/\n/g) || [] ).length + 1; + const falseStartCol = falseZero - start.lastIndexOf('\n') - 1; + + start = contract.instrumented.slice(0, falseOne); + const falseEndLine = ( start.match(/\n/g) || [] ).length + 1; + const falseEndCol = falseOne - start.lastIndexOf('\n') - 1; + + contract.branchId += 1; + + contract.branchMap[contract.branchId] = { + line: trueStartLine, + type: 'if', + locations: [{ + start: { + line: trueStartLine, column: trueStartCol, + }, + end: { + line: trueEndLine, column: trueEndCol, + }, + }, { + start: { + line: falseStartLine, column: falseStartCol, + }, + end: { + line: falseEndLine, column: falseEndCol, + }, + }], + }; + + } + /** * Registers injections for branch measurements. Used by logicalOR registration method. * @param {Object} contract instrumentation target @@ -241,6 +289,46 @@ class Registrar { }; }; + conditional(contract, expression){ + this.addNewConditionalBranch(contract, expression); + + // Double open parens + this._createInjectionPoint( + contract, + expression.condition.range[0], + { + type: 'injectOpenParen', + } + ); + this._createInjectionPoint( + contract, + expression.condition.range[0], + { + type: 'injectOpenParen', + } + ); + // False condition: (these get sorted in reverse order, so create in reversed order) + this._createInjectionPoint( + contract, + expression.condition.range[1] + 1, + { + type: 'injectOrFalse', + branchId: contract.branchId, + locationIdx: 1 + } + ); + // True condition + this._createInjectionPoint( + contract, + expression.condition.range[1] + 1, + { + type: 'injectAndTrue', + branchId: contract.branchId, + locationIdx: 0 + } + ); + } + /** * Registers injections for logicalOR clause measurements (branches) * @param {Object} contract instrumentation target @@ -262,7 +350,7 @@ class Registrar { contract, expression.left.range[1] + 1, { - type: 'injectLogicalOR', + type: 'injectAndTrue', branchId: contract.branchId, locationIdx: 0 } @@ -280,7 +368,7 @@ class Registrar { contract, expression.right.range[1] + 1, { - type: 'injectLogicalOR', + type: 'injectAndTrue', branchId: contract.branchId, locationIdx: 1 } diff --git a/lib/ternary/conditional.js b/lib/ternary/conditional.js deleted file mode 100644 index bf011ea..0000000 --- a/lib/ternary/conditional.js +++ /dev/null @@ -1,185 +0,0 @@ -/* eslint-env node, mocha */ - -/*const path = require('path'); -const getInstrumentedVersion = require('./../lib/instrumentSolidity.js'); -const util = require('./util/util.js'); -const CoverageMap = require('./../lib/coverageMap'); -const vm = require('./util/vm'); -const assert = require('assert'); - -describe.skip('conditional statements', () => { - const filePath = path.resolve('./test.sol'); - const pathPrefix = './'; - - it('should cover a conditional that reaches the consequent (same-line)', done => { - const contract = util.getCode('conditional/sameline-consequent.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, { - 5: 1, 6: 1, 7: 1, - }); - assert.deepEqual(mapping[filePath].b, { - 1: [1, 0], - }); - assert.deepEqual(mapping[filePath].s, { - 1: 1, 2: 1, 3: 1, - }); - assert.deepEqual(mapping[filePath].f, { - 1: 1, - }); - done(); - }).catch(done); - }); - - it('should cover a conditional that reaches the alternate (same-line)', done => { - const contract = util.getCode('conditional/sameline-alternate.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, { - 5: 1, 6: 1, 7: 1, - }); - assert.deepEqual(mapping[filePath].b, { - 1: [0, 1], - }); - assert.deepEqual(mapping[filePath].s, { - 1: 1, 2: 1, 3: 1, - }); - assert.deepEqual(mapping[filePath].f, { - 1: 1, - }); - done(); - }).catch(done); - }); - - it('should cover a conditional that reaches the consequent (multi-line)', done => { - const contract = util.getCode('conditional/multiline-consequent.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, { - 5: 1, 6: 1, 7: 1, - }); - assert.deepEqual(mapping[filePath].b, { - 1: [1, 0], - }); - assert.deepEqual(mapping[filePath].s, { - 1: 1, 2: 1, 3: 1, - }); - assert.deepEqual(mapping[filePath].f, { - 1: 1, - }); - done(); - }).catch(done); - }); - - it('should cover a conditional that reaches the alternate (multi-line)', done => { - const contract = util.getCode('conditional/multiline-alternate.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, { - 5: 1, 6: 1, 7: 1, - }); - assert.deepEqual(mapping[filePath].b, { - 1: [0, 1], - }); - assert.deepEqual(mapping[filePath].s, { - 1: 1, 2: 1, 3: 1, - }); - assert.deepEqual(mapping[filePath].f, { - 1: 1, - }); - done(); - }).catch(done); - }); - - it('should cover a DeclarativeExpression assignment by conditional that reaches the alternate', done => { - const contract = util.getCode('conditional/declarative-exp-assignment-alternate.sol'); - const info = getInstrumentedVersion(contract, filePath); - const coverage = new CoverageMap(); - coverage.addContract(info, filePath); - - // Runs bool z = (x) ? false : true; - vm.execute(info.contract, 'a', []).then(events => { - const mapping = coverage.generate(events, pathPrefix); - assert.deepEqual(mapping[filePath].l, { - 5: 1, 6: 1, 7: 1, - }); - assert.deepEqual(mapping[filePath].b, { - 1: [0, 1], - }); - assert.deepEqual(mapping[filePath].s, { - 1: 1, 2: 1, 3: 1, - }); - assert.deepEqual(mapping[filePath].f, { - 1: 1, - }); - done(); - }).catch(done); - }); - - it('should cover an Identifier assignment by conditional that reaches the alternate', done => { - const contract = util.getCode('conditional/identifier-assignment-alternate.sol'); - const info = getInstrumentedVersion(contract, filePath); - const coverage = new CoverageMap(); - coverage.addContract(info, filePath); - - // Runs z = (x) ? false : true; - vm.execute(info.contract, 'a', []).then(events => { - const mapping = coverage.generate(events, pathPrefix); - assert.deepEqual(mapping[filePath].l, { - 5: 1, 6: 1, 7: 1, 8: 1, - }); - assert.deepEqual(mapping[filePath].b, { - 1: [0, 1], - }); - assert.deepEqual(mapping[filePath].s, { - 1: 1, 2: 1, 3: 1, 4: 1, - }); - assert.deepEqual(mapping[filePath].f, { - 1: 1, - }); - done(); - }).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); - }); - -}); -*/ diff --git a/lib/ternary/ternary.js b/lib/ternary/ternary.js deleted file mode 100644 index 82ff304..0000000 --- a/lib/ternary/ternary.js +++ /dev/null @@ -1,121 +0,0 @@ -/** - * This is logic to instrument ternary conditional assignment statements. Preserving - * here for the time being, because instrumentation of these became impossible in - * solc >= 0.5.0 - */ - -function instrumentAssignmentExpression(contract, expression) { - - // This is suspended for 0.5.0 which tries to accomodate the new `emit` keyword. - // Solc is not allowing us to use the construction `emit SomeEvent()` within the parens :/ - return; - // -------------------------------------------------------------------------------------------- - - // The only time we instrument an assignment expression is if there's a conditional expression on - // the right - /*if (expression.right.type === 'ConditionalExpression') { - if (expression.left.type === 'DeclarativeExpression' || expression.left.type === 'Identifier') { - // Then we need to go from bytes32 varname = (conditional expression) - // to bytes32 varname; (,varname) = (conditional expression) - createOrAppendInjectionPoint(contract, expression.left.range[1], { - type: 'literal', string: '; (,' + expression.left.name + ')', - }); - instrumenter.instrumentConditionalExpression(contract, expression.right); - } else if (expression.left.type === 'MemberExpression') { - createOrAppendInjectionPoint(contract, expression.left.range[0], { - type: 'literal', string: '(,', - }); - createOrAppendInjectionPoint(contract, expression.left.range[1], { - type: 'literal', string: ')', - }); - instrumenter.instrumentConditionalExpression(contract, expression.right); - } else { - const err = 'Error instrumenting assignment expression @ solidity-coverage/lib/instrumenter.js'; - console.log(err, contract, expression.left); - process.exit(); - } - }*/ -}; - -function instrumentConditionalExpression(contract, expression) { - // ---------------------------------------------------------------------------------------------- - // This is suspended for 0.5.0 which tries to accomodate the new `emit` keyword. - // Solc is not allowing us to use the construction `emit SomeEvent()` within the parens :/ - // Very sad, this is the coolest thing in here. - return; - // ---------------------------------------------------------------------------------------------- - - /*contract.branchId += 1; - - const startline = (contract.instrumented.slice(0, expression.range[0]).match(/\n/g) || []).length + 1; - const startcol = expression.range[0] - contract.instrumented.slice(0, expression.range[0]).lastIndexOf('\n') - 1; - const consequentStartCol = startcol + (contract, expression.trueBody.range[0] - expression.range[0]); - const consequentEndCol = consequentStartCol + (contract, expression.trueBody.range[1] - expression.trueBody.range[0]); - const alternateStartCol = startcol + (contract, expression.falseBody.range[0] - expression.range[0]); - const alternateEndCol = alternateStartCol + (contract, expression.falseBody.range[1] - expression.falseBody.range[0]); - // NB locations for conditional branches in istanbul are length 1 and associated with the : and ?. - contract.branchMap[contract.branchId] = { - line: startline, - type: 'cond-expr', - locations: [{ - start: { - line: startline, column: consequentStartCol, - }, - end: { - line: startline, column: consequentEndCol, - }, - }, { - start: { - line: startline, column: alternateStartCol, - }, - end: { - line: startline, column: alternateEndCol, - }, - }], - }; - // Right, this could be being used just by itself or as an assignment. In the case of the latter, because - // the comma operator doesn't exist, we're going to have to get funky. - // if we're on a line by ourselves, this is easier - // - // Now if we've got to wrap the expression it's being set equal to, do that... - - - // Wrap the consequent - createOrAppendInjectionPoint(contract, expression.trueBody.range[0], { - type: 'openParen', - }); - createOrAppendInjectionPoint(contract, expression.trueBody.range[0], { - type: 'callBranchEvent', comma: true, branchId: contract.branchId, locationIdx: 0, - }); - createOrAppendInjectionPoint(contract, expression.trueBody.range[1], { - type: 'closeParen', - }); - - // Wrap the alternate - createOrAppendInjectionPoint(contract, expression.falseBody.range[0], { - type: 'openParen', - }); - createOrAppendInjectionPoint(contract, expression.falseBody.range[0], { - type: 'callBranchEvent', comma: true, branchId: contract.branchId, locationIdx: 1, - }); - createOrAppendInjectionPoint(contract, expression.falseBody.range[1], { - type: 'closeParen', - });*/ -}; - -// Paren / Literal injectors -/* - -injector.openParen = function injectOpenParen(contract, fileName, injectionPoint, injection) { - contract.instrumented = contract.instrumented.slice(0, injectionPoint) + '(' + contract.instrumented.slice(injectionPoint); -}; - -injector.closeParen = function injectCloseParen(contract, fileName, injectionPoint, injection) { - contract.instrumented = contract.instrumented.slice(0, injectionPoint) + ')' + contract.instrumented.slice(injectionPoint); -}; - -injector.literal = function injectLiteral(contract, fileName, injectionPoint, injection) { - contract.instrumented = contract.instrumented.slice(0, injectionPoint) + injection.string + contract.instrumented.slice(injectionPoint); -}; - -*/ \ No newline at end of file diff --git a/test/integration/projects/logical-or/.solcover.js b/test/integration/projects/ternary-and-logical-or/.solcover.js similarity index 75% rename from test/integration/projects/logical-or/.solcover.js rename to test/integration/projects/ternary-and-logical-or/.solcover.js index c245304..7a3746c 100644 --- a/test/integration/projects/logical-or/.solcover.js +++ b/test/integration/projects/ternary-and-logical-or/.solcover.js @@ -4,5 +4,5 @@ const fn = (msg, config) => config.logger.log(msg); module.exports = { skipFiles: ['Migrations.sol'], silent: process.env.SILENT ? true : false, - istanbulReporter: ['json-summary', 'text'], + istanbulReporter: ['json-summary', 'text', 'html'], } diff --git a/test/integration/projects/logical-or/contracts/Contract_OR.sol b/test/integration/projects/ternary-and-logical-or/contracts/Contract_OR.sol similarity index 87% rename from test/integration/projects/logical-or/contracts/Contract_OR.sol rename to test/integration/projects/ternary-and-logical-or/contracts/Contract_OR.sol index 0745303..0c7c7c5 100644 --- a/test/integration/projects/logical-or/contracts/Contract_OR.sol +++ b/test/integration/projects/ternary-and-logical-or/contracts/Contract_OR.sol @@ -37,4 +37,10 @@ contract Contract_OR { x == 3 ); } + + function _if_neither(uint i) public { + if (i == 1 || i == 2){ + /* ignore */ + } + } } diff --git a/test/integration/projects/ternary-and-logical-or/contracts/Contract_ternary.sol b/test/integration/projects/ternary-and-logical-or/contracts/Contract_ternary.sol new file mode 100644 index 0000000..c791d4c --- /dev/null +++ b/test/integration/projects/ternary-and-logical-or/contracts/Contract_ternary.sol @@ -0,0 +1,54 @@ +pragma solidity ^0.7.0; + + +contract Contract_ternary { + + // Sameline consequent + function a() public { + bool x = true; + bool y = true; + x && y ? y = false : y = false; + } + + // Multiline consequent + function b() public { + bool x = false; + bool y = false; + (x) + ? y = false + : y = false; + } + + // Sameline w/ logicalOR + function c() public { + bool x = false; + bool y = true; + (x || y) ? y = false : y = false; + } + + // Multiline w/ logicalOR + function d() public { + bool x = false; + bool y = true; + (x || y) + ? y = false + : y = false; + } + + // Sameline alternate + function e() public { + bool x = false; + bool y = false; + (x) ? y = false : y = false; + } + + // Multiline w/ logicalOR (both false) + function f() public { + bool x = false; + bool y = false; + (x || y) + ? y = false + : y = false; + } + +} diff --git a/test/integration/projects/logical-or/hardhat.config.js b/test/integration/projects/ternary-and-logical-or/hardhat.config.js similarity index 100% rename from test/integration/projects/logical-or/hardhat.config.js rename to test/integration/projects/ternary-and-logical-or/hardhat.config.js diff --git a/test/integration/projects/logical-or/test/test_or.js b/test/integration/projects/ternary-and-logical-or/test/test_or.js similarity index 89% rename from test/integration/projects/logical-or/test/test_or.js rename to test/integration/projects/ternary-and-logical-or/test/test_or.js index 8f936d7..de2491c 100644 --- a/test/integration/projects/logical-or/test/test_or.js +++ b/test/integration/projects/ternary-and-logical-or/test/test_or.js @@ -30,4 +30,8 @@ contract("contract_or", function(accounts) { await instance._require_multi_line(1); await instance._require_multi_line(3); }) + + it('_if_neither', async function(){ + await instance._if_neither(3); + }) }); diff --git a/test/integration/projects/ternary-and-logical-or/test/test_ternary.js b/test/integration/projects/ternary-and-logical-or/test/test_ternary.js new file mode 100644 index 0000000..820f231 --- /dev/null +++ b/test/integration/projects/ternary-and-logical-or/test/test_ternary.js @@ -0,0 +1,16 @@ +const Contract_ternary = artifacts.require("Contract_ternary"); + +contract("contract_ternary", function(accounts) { + let instance; + + before(async () => instance = await Contract_ternary.new()) + + it('misc ternary conditionals', async function(){ + await instance.a(); + await instance.b(); + await instance.c(); + await instance.d(); + await instance.e(); + await instance.f(); + }); +}); diff --git a/test/integration/projects/logical-or/truffle-config.js b/test/integration/projects/ternary-and-logical-or/truffle-config.js similarity index 100% rename from test/integration/projects/logical-or/truffle-config.js rename to test/integration/projects/ternary-and-logical-or/truffle-config.js diff --git a/test/sources/solidity/contracts/conditional/and-condition.sol b/test/sources/solidity/contracts/conditional/and-condition.sol new file mode 100644 index 0000000..d3229f2 --- /dev/null +++ b/test/sources/solidity/contracts/conditional/and-condition.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.7.0; + +contract Test { + function a() public { + bool x = true; + bool y = true; + x && y ? y = false : y = false; + } +} diff --git a/test/sources/solidity/contracts/conditional/or-always-false-condition.sol b/test/sources/solidity/contracts/conditional/or-always-false-condition.sol new file mode 100644 index 0000000..84325e6 --- /dev/null +++ b/test/sources/solidity/contracts/conditional/or-always-false-condition.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.7.0; + +contract Test { + function a() public { + bool x = false; + bool y = false; + (x || y) ? y = false : y = false; + } +} diff --git a/test/sources/solidity/contracts/conditional/or-condition.sol b/test/sources/solidity/contracts/conditional/or-condition.sol new file mode 100644 index 0000000..2f2aa1c --- /dev/null +++ b/test/sources/solidity/contracts/conditional/or-condition.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.7.0; + +contract Test { + function a() public { + bool x = false; + bool y = true; + (x || y) ? y = false : y = false; + } +} diff --git a/test/sources/solidity/contracts/conditional/unbracketed-condition.sol b/test/sources/solidity/contracts/conditional/unbracketed-condition.sol new file mode 100644 index 0000000..38a7d5e --- /dev/null +++ b/test/sources/solidity/contracts/conditional/unbracketed-condition.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.7.0; + +contract Test { + function a() public { + bool x = true; + bool y = false; + x ? y = false : y = false; + } +} diff --git a/test/sources/solidity/contracts/conditional/unbracketed-or-condition.sol b/test/sources/solidity/contracts/conditional/unbracketed-or-condition.sol new file mode 100644 index 0000000..193f1db --- /dev/null +++ b/test/sources/solidity/contracts/conditional/unbracketed-or-condition.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.7.0; + +contract Test { + function a() public { + bool x = false; + bool y = true; + x || y ? y = false : y = false; + } +} diff --git a/test/units/conditional.js b/test/units/conditional.js new file mode 100644 index 0000000..81c6ed3 --- /dev/null +++ b/test/units/conditional.js @@ -0,0 +1,232 @@ +const assert = require('assert'); +const util = require('./../util/util.js'); + +const client = require('ganache-cli'); +const Coverage = require('./../../lib/coverage'); +const Api = require('./../../lib/api') + +describe('ternary conditionals', () => { + let coverage; + let api; + + before(async () => { + api = new Api({silent: true}); + await api.ganache(client); + }) + beforeEach(() => coverage = new Coverage()); + after(async() => await api.finish()); + + async function setupAndRun(solidityFile){ + const contract = await util.bootstrapCoverage(solidityFile, api); + coverage.addContract(contract.instrumented, util.filePath); + await contract.instance.a(); + return coverage.generate(contract.data, util.pathPrefix); + } + + it('should cover a conditional that reaches the consequent (same-line)', async function() { + const mapping = await setupAndRun('conditional/sameline-consequent'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover an unbracketed conditional that reaches the consequent (same-line)', async function() { + const mapping = await setupAndRun('conditional/unbracketed-condition'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover a multi-part conditional (&&) that reaches the consequent', async function() { + const mapping = await setupAndRun('conditional/and-condition'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover a multi-part conditional (||) that reaches the consequent', async function() { + const mapping = await setupAndRun('conditional/or-condition'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], 2: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover a multi-part unbracketed conditional (||) that reaches the consequent', async function() { + const mapping = await setupAndRun('conditional/unbracketed-or-condition'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], 2: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover an always-false multi-part unbracketed conditional (||)', async function() { + const mapping = await setupAndRun('conditional/or-always-false-condition'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 0], 2: [0, 1], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover a conditional that reaches the alternate (same-line)', async function() { + const mapping = await setupAndRun('conditional/sameline-alternate'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover a conditional that reaches the consequent (multi-line)', async function() { + const mapping = await setupAndRun('conditional/multiline-consequent'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [1, 0], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover a conditional that reaches the alternate (multi-line)', async function() { + const mapping = await setupAndRun('conditional/multiline-alternate'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // Runs bool z = (x) ? false : true; + it('should cover a definition assignment by conditional that reaches the alternate', async function() { + const mapping = await setupAndRun('conditional/declarative-exp-assignment-alternate'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + // Runs z = (x) ? false : true; + it('should cover an identifier assignment by conditional that reaches the alternate', async function() { + const mapping = await setupAndRun('conditional/identifier-assignment-alternate'); + + assert.deepEqual(mapping[util.filePath].l, { + 5: 1, 6: 1, 7: 1, 8: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, 3: 1, 4: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + + it('should cover an assignment to a member expression (reaches the alternate)', async function() { + const mapping = await setupAndRun('conditional/mapping-assignment'); + + assert.deepEqual(mapping[util.filePath].l, { + 11: 1, 12: 1, + }); + assert.deepEqual(mapping[util.filePath].b, { + 1: [0, 1], + }); + assert.deepEqual(mapping[util.filePath].s, { + 1: 1, 2: 1, + }); + assert.deepEqual(mapping[util.filePath].f, { + 1: 1, + }); + }); + +}); diff --git a/test/units/hardhat/standard.js b/test/units/hardhat/standard.js index 9338b6b..3c1d408 100644 --- a/test/units/hardhat/standard.js +++ b/test/units/hardhat/standard.js @@ -489,11 +489,8 @@ describe('Hardhat Plugin: standard use cases', function() { verify.lineCoverage(expected); }) - // 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('logicalOR', async function(){ - mock.installFullProject('logical-or'); + it('logicalOR & ternary conditionals', async function(){ + mock.installFullProject('ternary-and-logical-or'); mock.hardhatSetupEnv(this); await this.env.run("coverage"); @@ -501,8 +498,12 @@ describe('Hardhat Plugin: standard use cases', function() { const expected = [ { file: mock.pathToContract(hardhatConfig, 'Contract_OR.sol'), - pct: 59.09 - } + pct: 53.85 + }, + { + file: mock.pathToContract(hardhatConfig, 'Contract_ternary.sol'), + pct: 44.44 + }, ]; verify.branchCoverage(expected); diff --git a/test/units/truffle/standard.js b/test/units/truffle/standard.js index 27aff76..aa9b4c8 100644 --- a/test/units/truffle/standard.js +++ b/test/units/truffle/standard.js @@ -499,15 +499,19 @@ describe('Truffle Plugin: standard use cases', function() { verify.lineCoverage(expected); }); - it('logicalOR', async function(){ - mock.installFullProject('logical-or'); + it('logicalOR & ternary conditionals', async function(){ + mock.installFullProject('ternary-and-logical-or'); await plugin(truffleConfig); const expected = [ { file: mock.pathToContract(truffleConfig, 'Contract_OR.sol'), - pct: 59.09 - } + pct: 53.85 + }, + { + file: mock.pathToContract(truffleConfig, 'Contract_ternary.sol'), + pct: 44.44 + }, ]; verify.branchCoverage(expected);