From bbd414bdcd3bcbe3a00ff38664cd9c6e488a7d6a Mon Sep 17 00:00:00 2001 From: perissology Date: Fri, 22 Jun 2018 15:47:44 -0700 Subject: [PATCH 1/4] add ability to ignore covering specific code blocks --- packages/sol-cov/src/ast_visitor.ts | 29 ++++++++++++++++++- .../sol-cov/src/collect_coverage_entries.ts | 21 +++++++++++++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/sol-cov/src/ast_visitor.ts b/packages/sol-cov/src/ast_visitor.ts index 564f0f7d2b..49b7ac4bd7 100644 --- a/packages/sol-cov/src/ast_visitor.ts +++ b/packages/sol-cov/src/ast_visitor.ts @@ -23,8 +23,11 @@ export class ASTVisitor { private _modifiersStatementIds: number[] = []; private _statementMap: StatementMap = {}; private _locationByOffset: LocationByOffset; - constructor(locationByOffset: LocationByOffset) { + private _ignoreRangesBeginingAt: number[]; + private _ignoreRangesWithin: Array<[number, number]> = []; + constructor(locationByOffset: LocationByOffset, ignoreRangesBeginingAt: number[] = []) { this._locationByOffset = locationByOffset; + this._ignoreRangesBeginingAt = ignoreRangesBeginingAt; } public getCollectedCoverageEntries(): CoverageEntriesDescription { const coverageEntriesDescription = { @@ -96,6 +99,9 @@ export class ASTVisitor { public ModifierInvocation(ast: Parser.ModifierInvocation): void { const BUILTIN_MODIFIERS = ['public', 'view', 'payable', 'external', 'internal', 'pure', 'constant']; if (!_.includes(BUILTIN_MODIFIERS, ast.name)) { + if (this._ignoreExpression(ast)) { + return; + } this._modifiersStatementIds.push(this._entryId); this._visitStatement(ast); } @@ -106,6 +112,9 @@ export class ASTVisitor { right: Parser.ASTNode, type: BranchType, ): void { + if (this._ignoreExpression(ast)) { + return; + } this._branchMap[this._entryId++] = { line: this._getExpressionRange(ast).start.line, type, @@ -113,6 +122,9 @@ export class ASTVisitor { }; } private _visitStatement(ast: Parser.ASTNode): void { + if (this._ignoreExpression(ast)) { + return; + } this._statementMap[this._entryId++] = this._getExpressionRange(ast); } private _getExpressionRange(ast: Parser.ASTNode): SingleFileSourceRange { @@ -125,7 +137,22 @@ export class ASTVisitor { }; return range; } + private _ignoreExpression(ast: Parser.ASTNode): boolean { + const [astStart, astEnd] = ast.range as [number, number]; + const isRangeIgnored = _.some( + this._ignoreRangesWithin, + ([rangeStart, rangeEnd]: [number, number]) => astStart >= rangeStart && astEnd <= rangeEnd, + ); + return this._ignoreRangesBeginingAt.includes(astStart) || isRangeIgnored; + } private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void { + if (this._ignoreExpression(ast)) { + // we want to ignore everything within this function + // add this nodes range to the ignoreRangesWithin array + // so we can ignore any later nodes that are children of this node + this._ignoreRangesWithin.push(ast.range as [number, number]); + return; + } const loc = this._getExpressionRange(ast); this._fnMap[this._entryId++] = { name: ast.name, diff --git a/packages/sol-cov/src/collect_coverage_entries.ts b/packages/sol-cov/src/collect_coverage_entries.ts index 3fc85008c7..703af30999 100644 --- a/packages/sol-cov/src/collect_coverage_entries.ts +++ b/packages/sol-cov/src/collect_coverage_entries.ts @@ -13,10 +13,29 @@ export const collectCoverageEntries = (contractSource: string) => { if (_.isUndefined(coverageEntriesBySourceHash[sourceHash]) && !_.isUndefined(contractSource)) { const ast = parser.parse(contractSource, { range: true }); const locationByOffset = getLocationByOffset(contractSource); - const visitor = new ASTVisitor(locationByOffset); + const ignoreRangesBegingingAt = gatherRangesToIgnore(contractSource); + const visitor = new ASTVisitor(locationByOffset, ignoreRangesBegingingAt); parser.visit(ast, visitor); coverageEntriesBySourceHash[sourceHash] = visitor.getCollectedCoverageEntries(); } const coverageEntriesDescription = coverageEntriesBySourceHash[sourceHash]; return coverageEntriesDescription; }; + +const IGNORE_RE = /\/\*\s*solcov\s+ignore\s+next\s*\*\/\s*/gm; + +// Gather the start index of all code blocks preceeded by "/* solcov ignore next */" +function gatherRangesToIgnore(contractSource: string): number[] { + const ignoreRangesStart = []; + + let match; + do { + match = IGNORE_RE.exec(contractSource); + if (match) { + const matchLen = match[0].length; + ignoreRangesStart.push(match.index + matchLen); + } + } while (match); + + return ignoreRangesStart; +} From 1a4e99431bf0cfc8c5db2235a935d62ebb8469b2 Mon Sep 17 00:00:00 2001 From: perissology Date: Mon, 25 Jun 2018 07:55:19 -0700 Subject: [PATCH 2/4] support ignoring entire contracts --- packages/sol-cov/src/ast_visitor.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/sol-cov/src/ast_visitor.ts b/packages/sol-cov/src/ast_visitor.ts index 49b7ac4bd7..166cf13bd7 100644 --- a/packages/sol-cov/src/ast_visitor.ts +++ b/packages/sol-cov/src/ast_visitor.ts @@ -23,11 +23,13 @@ export class ASTVisitor { private _modifiersStatementIds: number[] = []; private _statementMap: StatementMap = {}; private _locationByOffset: LocationByOffset; - private _ignoreRangesBeginingAt: number[]; + private _ignoreRangesBeginningAt: number[]; + // keep track of contract/function ranges that are to be ignored + // so we can also ignore any children nodes within the contract/function private _ignoreRangesWithin: Array<[number, number]> = []; - constructor(locationByOffset: LocationByOffset, ignoreRangesBeginingAt: number[] = []) { + constructor(locationByOffset: LocationByOffset, ignoreRangesBeginningAt: number[] = []) { this._locationByOffset = locationByOffset; - this._ignoreRangesBeginingAt = ignoreRangesBeginingAt; + this._ignoreRangesBeginningAt = ignoreRangesBeginningAt; } public getCollectedCoverageEntries(): CoverageEntriesDescription { const coverageEntriesDescription = { @@ -45,6 +47,11 @@ export class ASTVisitor { public FunctionDefinition(ast: Parser.FunctionDefinition): void { this._visitFunctionLikeDefinition(ast); } + public ContractDefinition(ast: Parser.ContractDefinition): void { + if (this._ignoreExpression(ast)) { + this._ignoreRangesWithin.push(ast.range as [number, number]); + } + } public ModifierDefinition(ast: Parser.ModifierDefinition): void { this._visitFunctionLikeDefinition(ast); } @@ -143,13 +150,10 @@ export class ASTVisitor { this._ignoreRangesWithin, ([rangeStart, rangeEnd]: [number, number]) => astStart >= rangeStart && astEnd <= rangeEnd, ); - return this._ignoreRangesBeginingAt.includes(astStart) || isRangeIgnored; + return this._ignoreRangesBeginningAt.includes(astStart) || isRangeIgnored; } private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void { if (this._ignoreExpression(ast)) { - // we want to ignore everything within this function - // add this nodes range to the ignoreRangesWithin array - // so we can ignore any later nodes that are children of this node this._ignoreRangesWithin.push(ast.range as [number, number]); return; } From 92cb9c3807917049b392d86160e7d88f73a5c7b4 Mon Sep 17 00:00:00 2001 From: perissology Date: Mon, 25 Jun 2018 08:18:02 -0700 Subject: [PATCH 3/4] add /*solcov ignore next*/ tests --- .../test/collect_coverage_entries_test.ts | 30 +++++++++++++++++++ .../test/fixtures/contracts/SolcovIgnore.sol | 22 ++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 packages/sol-cov/test/fixtures/contracts/SolcovIgnore.sol diff --git a/packages/sol-cov/test/collect_coverage_entries_test.ts b/packages/sol-cov/test/collect_coverage_entries_test.ts index f88f3b3c35..7832ec3164 100644 --- a/packages/sol-cov/test/collect_coverage_entries_test.ts +++ b/packages/sol-cov/test/collect_coverage_entries_test.ts @@ -121,5 +121,35 @@ describe('Collect coverage entries', () => { const branchTypes = _.map(branchDescriptions, branchDescription => branchDescription.type); expect(branchTypes).to.be.deep.equal(['if', 'if', 'if', 'if', 'binary-expr', 'if']); }); + + it('correctly ignores all coverage entries for Ignore contract', () => { + const solcovIgnoreContractBaseName = 'SolcovIgnore.sol'; + const solcovIgnoreContractFileName = path.resolve( + __dirname, + 'fixtures/contracts', + solcovIgnoreContractBaseName, + ); + const solcovIgnoreContract = fs.readFileSync(solcovIgnoreContractFileName).toString(); + const coverageEntries = collectCoverageEntries(solcovIgnoreContract); + const fnIds = _.keys(coverageEntries.fnMap); + + expect(fnIds.length).to.be.equal(1); + expect(coverageEntries.fnMap[fnIds[0]].name).to.be.equal('set'); + // tslint:disable-next-line:custom-no-magic-numbers + expect(coverageEntries.fnMap[fnIds[0]].line).to.be.equal(6); + const setFunction = `function set(uint x) public { + /* solcov ignore next */ + storedData = x; + }`; + expect(utils.getRange(solcovIgnoreContract, coverageEntries.fnMap[fnIds[0]].loc)).to.be.equal(setFunction); + + expect(coverageEntries.branchMap).to.be.deep.equal({}); + const statementIds = _.keys(coverageEntries.statementMap); + expect(utils.getRange(solcovIgnoreContract, coverageEntries.statementMap[statementIds[0]])).to.be.equal( + setFunction, + ); + expect(statementIds.length).to.be.equal(1); + expect(coverageEntries.modifiersStatementIds.length).to.be.equal(0); + }); }); }); diff --git a/packages/sol-cov/test/fixtures/contracts/SolcovIgnore.sol b/packages/sol-cov/test/fixtures/contracts/SolcovIgnore.sol new file mode 100644 index 0000000000..a7977ffb4d --- /dev/null +++ b/packages/sol-cov/test/fixtures/contracts/SolcovIgnore.sol @@ -0,0 +1,22 @@ +pragma solidity ^0.4.21; + +contract SolcovIgnore { + uint public storedData; + + function set(uint x) public { + /* solcov ignore next */ + storedData = x; + } + + /* solcov ignore next */ + function get() constant public returns (uint retVal) { + return storedData; + } +} + +/* solcov ignore next */ +contract Ignore { + function ignored() public returns (bool) { + return false; + } +} From e0a2afc068a48f58786bc3a3952ffe024edc037c Mon Sep 17 00:00:00 2001 From: perissology Date: Wed, 27 Jun 2018 07:26:12 -0700 Subject: [PATCH 4/4] rename function --- packages/sol-cov/src/ast_visitor.ts | 12 ++++++------ packages/sol-cov/src/collect_coverage_entries.ts | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/sol-cov/src/ast_visitor.ts b/packages/sol-cov/src/ast_visitor.ts index 166cf13bd7..a6bca4704a 100644 --- a/packages/sol-cov/src/ast_visitor.ts +++ b/packages/sol-cov/src/ast_visitor.ts @@ -48,7 +48,7 @@ export class ASTVisitor { this._visitFunctionLikeDefinition(ast); } public ContractDefinition(ast: Parser.ContractDefinition): void { - if (this._ignoreExpression(ast)) { + if (this._shouldIgnoreExpression(ast)) { this._ignoreRangesWithin.push(ast.range as [number, number]); } } @@ -106,7 +106,7 @@ export class ASTVisitor { public ModifierInvocation(ast: Parser.ModifierInvocation): void { const BUILTIN_MODIFIERS = ['public', 'view', 'payable', 'external', 'internal', 'pure', 'constant']; if (!_.includes(BUILTIN_MODIFIERS, ast.name)) { - if (this._ignoreExpression(ast)) { + if (this._shouldIgnoreExpression(ast)) { return; } this._modifiersStatementIds.push(this._entryId); @@ -119,7 +119,7 @@ export class ASTVisitor { right: Parser.ASTNode, type: BranchType, ): void { - if (this._ignoreExpression(ast)) { + if (this._shouldIgnoreExpression(ast)) { return; } this._branchMap[this._entryId++] = { @@ -129,7 +129,7 @@ export class ASTVisitor { }; } private _visitStatement(ast: Parser.ASTNode): void { - if (this._ignoreExpression(ast)) { + if (this._shouldIgnoreExpression(ast)) { return; } this._statementMap[this._entryId++] = this._getExpressionRange(ast); @@ -144,7 +144,7 @@ export class ASTVisitor { }; return range; } - private _ignoreExpression(ast: Parser.ASTNode): boolean { + private _shouldIgnoreExpression(ast: Parser.ASTNode): boolean { const [astStart, astEnd] = ast.range as [number, number]; const isRangeIgnored = _.some( this._ignoreRangesWithin, @@ -153,7 +153,7 @@ export class ASTVisitor { return this._ignoreRangesBeginningAt.includes(astStart) || isRangeIgnored; } private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void { - if (this._ignoreExpression(ast)) { + if (this._shouldIgnoreExpression(ast)) { this._ignoreRangesWithin.push(ast.range as [number, number]); return; } diff --git a/packages/sol-cov/src/collect_coverage_entries.ts b/packages/sol-cov/src/collect_coverage_entries.ts index 703af30999..bdbcd613e2 100644 --- a/packages/sol-cov/src/collect_coverage_entries.ts +++ b/packages/sol-cov/src/collect_coverage_entries.ts @@ -5,6 +5,8 @@ import * as parser from 'solidity-parser-antlr'; import { ASTVisitor, CoverageEntriesDescription } from './ast_visitor'; import { getLocationByOffset } from './source_maps'; +const IGNORE_RE = /\/\*\s*solcov\s+ignore\s+next\s*\*\/\s*/gm; + // Parsing source code for each transaction/code is slow and therefore we cache it const coverageEntriesBySourceHash: { [sourceHash: string]: CoverageEntriesDescription } = {}; @@ -22,8 +24,6 @@ export const collectCoverageEntries = (contractSource: string) => { return coverageEntriesDescription; }; -const IGNORE_RE = /\/\*\s*solcov\s+ignore\s+next\s*\*\/\s*/gm; - // Gather the start index of all code blocks preceeded by "/* solcov ignore next */" function gatherRangesToIgnore(contractSource: string): number[] { const ignoreRangesStart = [];