Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

[sol-cov] add ability to ignore covering specific code blocks #766

Merged
merged 4 commits into from
Jun 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion packages/sol-cov/src/ast_visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@ export class ASTVisitor {
private _modifiersStatementIds: number[] = [];
private _statementMap: StatementMap = {};
private _locationByOffset: LocationByOffset;
constructor(locationByOffset: LocationByOffset) {
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, ignoreRangesBeginningAt: number[] = []) {
this._locationByOffset = locationByOffset;
this._ignoreRangesBeginningAt = ignoreRangesBeginningAt;
}
public getCollectedCoverageEntries(): CoverageEntriesDescription {
const coverageEntriesDescription = {
Expand All @@ -42,6 +47,11 @@ export class ASTVisitor {
public FunctionDefinition(ast: Parser.FunctionDefinition): void {
this._visitFunctionLikeDefinition(ast);
}
public ContractDefinition(ast: Parser.ContractDefinition): void {
if (this._shouldIgnoreExpression(ast)) {
this._ignoreRangesWithin.push(ast.range as [number, number]);
}
}
public ModifierDefinition(ast: Parser.ModifierDefinition): void {
this._visitFunctionLikeDefinition(ast);
}
Expand Down Expand Up @@ -96,6 +106,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._shouldIgnoreExpression(ast)) {
return;
}
this._modifiersStatementIds.push(this._entryId);
this._visitStatement(ast);
}
Expand All @@ -106,13 +119,19 @@ export class ASTVisitor {
right: Parser.ASTNode,
type: BranchType,
): void {
if (this._shouldIgnoreExpression(ast)) {
return;
}
this._branchMap[this._entryId++] = {
line: this._getExpressionRange(ast).start.line,
type,
locations: [this._getExpressionRange(left), this._getExpressionRange(right)],
};
}
private _visitStatement(ast: Parser.ASTNode): void {
if (this._shouldIgnoreExpression(ast)) {
return;
}
this._statementMap[this._entryId++] = this._getExpressionRange(ast);
}
private _getExpressionRange(ast: Parser.ASTNode): SingleFileSourceRange {
Expand All @@ -125,7 +144,19 @@ export class ASTVisitor {
};
return range;
}
private _shouldIgnoreExpression(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._ignoreRangesBeginningAt.includes(astStart) || isRangeIgnored;
}
private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void {
if (this._shouldIgnoreExpression(ast)) {
this._ignoreRangesWithin.push(ast.range as [number, number]);
return;
}
const loc = this._getExpressionRange(ast);
this._fnMap[this._entryId++] = {
name: ast.name,
Expand Down
21 changes: 20 additions & 1 deletion packages/sol-cov/src/collect_coverage_entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } = {};

Expand All @@ -13,10 +15,27 @@ 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;
};

// 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;
}
30 changes: 30 additions & 0 deletions packages/sol-cov/test/collect_coverage_entries_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove excess new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier adds that line

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;
}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If indentation doesn't matter here, let's indent this further so that it's easier to read.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does matter here

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);
});
});
});
22 changes: 22 additions & 0 deletions packages/sol-cov/test/fixtures/contracts/SolcovIgnore.sol
Original file line number Diff line number Diff line change
@@ -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;
}
}