diff options
Diffstat (limited to 'packages/sol-cov')
-rw-r--r-- | packages/sol-cov/CHANGELOG.json | 32 | ||||
-rw-r--r-- | packages/sol-cov/package.json | 2 | ||||
-rw-r--r-- | packages/sol-cov/src/ast_visitor.ts | 5 | ||||
-rw-r--r-- | packages/sol-cov/src/get_source_range_snippet.ts | 180 | ||||
-rw-r--r-- | packages/sol-cov/src/revert_trace_subprovider.ts | 63 | ||||
-rw-r--r-- | packages/sol-cov/src/types.ts | 10 | ||||
-rw-r--r-- | packages/sol-cov/src/utils.ts | 6 | ||||
-rw-r--r-- | packages/sol-cov/test/collect_coverage_entries_test.ts | 17 |
8 files changed, 279 insertions, 36 deletions
diff --git a/packages/sol-cov/CHANGELOG.json b/packages/sol-cov/CHANGELOG.json index e61201a42..3b6071801 100644 --- a/packages/sol-cov/CHANGELOG.json +++ b/packages/sol-cov/CHANGELOG.json @@ -3,6 +3,22 @@ "version": "0.2.0", "changes": [ { + "note": "Add artifact adapter as a parameter for CoverageSubprovider. Export AbstractArtifactAdapter", + "pr": 589 + }, + { + "note": "Implement SolCompilerArtifactAdapter and TruffleArtifactAdapter", + "pr": 589 + }, + { + "note": "Properly parse multi-level traces", + "pr": 589 + }, + { + "note": "Add support for solidity libraries", + "pr": 589 + }, + { "note": "Fixed a bug causing RegExp to crash if contract code is longer that 32767 characters", "pr": 675 }, @@ -51,20 +67,12 @@ "pr": 690 }, { - "note": "Add artifact adapter as a parameter for CoverageSubprovider. Export AbstractArtifactAdapter", - "pr": 589 - }, - { - "note": "Implement SolCompilerArtifactAdapter and TruffleArtifactAdapter", - "pr": 589 - }, - { - "note": "Properly parse multi-level traces", - "pr": 589 + "note": "Create `RevertTraceSubprovider` which prints a stack trace when a REVERT is detected", + "pr": 705 }, { - "note": "Add support for solidity libraries", - "pr": 589 + "note": "Add source code snippets to stack traces printed by `RevertTraceSubprovider`", + "pr": 725 } ] }, diff --git a/packages/sol-cov/package.json b/packages/sol-cov/package.json index 39618eacf..f489edc97 100644 --- a/packages/sol-cov/package.json +++ b/packages/sol-cov/package.json @@ -73,7 +73,7 @@ "@types/istanbul": "^0.4.30", "@types/loglevel": "^1.5.3", "@types/mkdirp": "^0.5.1", - "@types/solidity-parser-antlr": "^0.2.0", + "@types/solidity-parser-antlr": "^0.2.1", "@types/mocha": "^2.2.42", "@types/node": "^8.0.53", "@types/rimraf": "^2.0.2", diff --git a/packages/sol-cov/src/ast_visitor.ts b/packages/sol-cov/src/ast_visitor.ts index 16984b5ec..564f0f7d2 100644 --- a/packages/sol-cov/src/ast_visitor.ts +++ b/packages/sol-cov/src/ast_visitor.ts @@ -116,8 +116,9 @@ export class ASTVisitor { this._statementMap[this._entryId++] = this._getExpressionRange(ast); } private _getExpressionRange(ast: Parser.ASTNode): SingleFileSourceRange { - const start = this._locationByOffset[ast.range[0]]; - const end = this._locationByOffset[ast.range[1] + 1]; + const astRange = ast.range as [number, number]; + const start = this._locationByOffset[astRange[0]]; + const end = this._locationByOffset[astRange[1] + 1]; const range = { start, end, diff --git a/packages/sol-cov/src/get_source_range_snippet.ts b/packages/sol-cov/src/get_source_range_snippet.ts new file mode 100644 index 000000000..30d6ec802 --- /dev/null +++ b/packages/sol-cov/src/get_source_range_snippet.ts @@ -0,0 +1,180 @@ +import * as ethUtil from 'ethereumjs-util'; +import * as _ from 'lodash'; +import * as Parser from 'solidity-parser-antlr'; + +import { SingleFileSourceRange, SourceRange, SourceSnippet } from './types'; +import { utils } from './utils'; + +interface ASTInfo { + type: string; + node: Parser.ASTNode; + name: string | null; + range?: SingleFileSourceRange; +} + +// Parsing source code for each transaction/code is slow and therefore we cache it +const parsedSourceByHash: { [sourceHash: string]: Parser.ASTNode } = {}; + +export function getSourceRangeSnippet(sourceRange: SourceRange, sourceCode: string): SourceSnippet | null { + const sourceHash = ethUtil.sha3(sourceCode).toString('hex'); + if (_.isUndefined(parsedSourceByHash[sourceHash])) { + parsedSourceByHash[sourceHash] = Parser.parse(sourceCode, { loc: true }); + } + const astNode = parsedSourceByHash[sourceHash]; + const visitor = new ASTInfoVisitor(); + Parser.visit(astNode, visitor); + const astInfo = visitor.getASTInfoForRange(sourceRange); + if (astInfo === null) { + return null; + } + const sourceCodeInRange = utils.getRange(sourceCode, sourceRange.location); + return { + ...astInfo, + range: astInfo.range as SingleFileSourceRange, + source: sourceCodeInRange, + fileName: sourceRange.fileName, + }; +} + +// A visitor which collects ASTInfo for most nodes in the AST. +class ASTInfoVisitor { + private _astInfos: ASTInfo[] = []; + public getASTInfoForRange(sourceRange: SourceRange): ASTInfo | null { + // HACK(albrow): Sometimes the source range doesn't exactly match that + // of astInfo. To work around that we try with a +/-1 offset on + // end.column. If nothing matches even with the offset, we return null. + const offset = { + start: { + line: 0, + column: 0, + }, + end: { + line: 0, + column: 0, + }, + }; + let astInfo = this._getASTInfoForRange(sourceRange, offset); + if (astInfo !== null) { + return astInfo; + } + offset.end.column += 1; + astInfo = this._getASTInfoForRange(sourceRange, offset); + if (astInfo !== null) { + return astInfo; + } + offset.end.column -= 2; + astInfo = this._getASTInfoForRange(sourceRange, offset); + if (astInfo !== null) { + return astInfo; + } + return null; + } + public ContractDefinition(ast: Parser.ContractDefinition): void { + this._visitContractDefinition(ast); + } + public IfStatement(ast: Parser.IfStatement): void { + this._visitStatement(ast); + } + public FunctionDefinition(ast: Parser.FunctionDefinition): void { + this._visitFunctionLikeDefinition(ast); + } + public ModifierDefinition(ast: Parser.ModifierDefinition): void { + this._visitFunctionLikeDefinition(ast); + } + public ForStatement(ast: Parser.ForStatement): void { + this._visitStatement(ast); + } + public ReturnStatement(ast: Parser.ReturnStatement): void { + this._visitStatement(ast); + } + public BreakStatement(ast: Parser.BreakStatement): void { + this._visitStatement(ast); + } + public ContinueStatement(ast: Parser.ContinueStatement): void { + this._visitStatement(ast); + } + public EmitStatement(ast: any /* TODO: Parser.EmitStatement */): void { + this._visitStatement(ast); + } + public VariableDeclarationStatement(ast: Parser.VariableDeclarationStatement): void { + this._visitStatement(ast); + } + public Statement(ast: Parser.Statement): void { + this._visitStatement(ast); + } + public WhileStatement(ast: Parser.WhileStatement): void { + this._visitStatement(ast); + } + public SimpleStatement(ast: Parser.SimpleStatement): void { + this._visitStatement(ast); + } + public ThrowStatement(ast: Parser.ThrowStatement): void { + this._visitStatement(ast); + } + public DoWhileStatement(ast: Parser.DoWhileStatement): void { + this._visitStatement(ast); + } + public ExpressionStatement(ast: Parser.ExpressionStatement): void { + this._visitStatement(ast.expression); + } + public InlineAssemblyStatement(ast: Parser.InlineAssemblyStatement): void { + this._visitStatement(ast); + } + public ModifierInvocation(ast: Parser.ModifierInvocation): void { + const BUILTIN_MODIFIERS = ['public', 'view', 'payable', 'external', 'internal', 'pure', 'constant']; + if (!_.includes(BUILTIN_MODIFIERS, ast.name)) { + this._visitStatement(ast); + } + } + private _visitStatement(ast: Parser.ASTNode): void { + this._astInfos.push({ + type: ast.type, + node: ast, + name: null, + range: ast.loc, + }); + } + private _visitFunctionLikeDefinition(ast: Parser.ModifierDefinition | Parser.FunctionDefinition): void { + this._astInfos.push({ + type: ast.type, + node: ast, + name: ast.name, + range: ast.loc, + }); + } + private _visitContractDefinition(ast: Parser.ContractDefinition): void { + this._astInfos.push({ + type: ast.type, + node: ast, + name: ast.name, + range: ast.loc, + }); + } + private _getASTInfoForRange(sourceRange: SourceRange, offset: SingleFileSourceRange): ASTInfo | null { + const offsetSourceRange = { + ...sourceRange, + location: { + start: { + line: sourceRange.location.start.line + offset.start.line, + column: sourceRange.location.start.column + offset.start.column, + }, + end: { + line: sourceRange.location.end.line + offset.end.line, + column: sourceRange.location.end.column + offset.end.column, + }, + }, + }; + for (const astInfo of this._astInfos) { + const astInfoRange = astInfo.range as SingleFileSourceRange; + if ( + astInfoRange.start.column === offsetSourceRange.location.start.column && + astInfoRange.start.line === offsetSourceRange.location.start.line && + astInfoRange.end.column === offsetSourceRange.location.end.column && + astInfoRange.end.line === offsetSourceRange.location.end.line + ) { + return astInfo; + } + } + return null; + } +} diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts index b1d4da10c..fb2215eaa 100644 --- a/packages/sol-cov/src/revert_trace_subprovider.ts +++ b/packages/sol-cov/src/revert_trace_subprovider.ts @@ -4,10 +4,11 @@ import { getLogger, levels, Logger } from 'loglevel'; import { AbstractArtifactAdapter } from './artifact_adapters/abstract_artifact_adapter'; import { constants } from './constants'; +import { getSourceRangeSnippet } from './get_source_range_snippet'; import { getRevertTrace } from './revert_trace'; import { parseSourceMap } from './source_maps'; import { TraceCollectionSubprovider } from './trace_collection_subprovider'; -import { ContractData, EvmCallStack, SourceRange } from './types'; +import { ContractData, EvmCallStack, SourceRange, SourceSnippet } from './types'; import { utils } from './utils'; /** @@ -53,7 +54,7 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider { } } private async _printStackTraceAsync(evmCallStack: EvmCallStack): Promise<void> { - const sourceRanges: SourceRange[] = []; + const sourceSnippets: SourceSnippet[] = []; if (_.isUndefined(this._contractsData)) { this._contractsData = await this._artifactAdapter.collectContractsDataAsync(); } @@ -97,18 +98,62 @@ export class RevertTraceSubprovider extends TraceCollectionSubprovider { continue; } } - sourceRanges.push(sourceRange); + const fileIndex = contractData.sources.indexOf(sourceRange.fileName); + const sourceSnippet = getSourceRangeSnippet(sourceRange, contractData.sourceCodes[fileIndex]); + if (sourceSnippet !== null) { + sourceSnippets.push(sourceSnippet); + } } - if (sourceRanges.length > 0) { + const filteredSnippets = filterSnippets(sourceSnippets); + if (filteredSnippets.length > 0) { this._logger.error('\n\nStack trace for REVERT:\n'); - _.forEach(_.reverse(sourceRanges), sourceRange => { - this._logger.error( - `${sourceRange.fileName}:${sourceRange.location.start.line}:${sourceRange.location.start.column}`, - ); + _.forEach(_.reverse(filteredSnippets), snippet => { + const traceString = getStackTraceString(snippet); + this._logger.error(traceString); }); this._logger.error('\n'); } else { - this._logger.error('Could not determine stack trace'); + this._logger.error('REVERT detected but could not determine stack trace'); + } + } +} + +// removes duplicates and if statements +function filterSnippets(sourceSnippets: SourceSnippet[]): SourceSnippet[] { + if (sourceSnippets.length === 0) { + return []; + } + const results: SourceSnippet[] = [sourceSnippets[0]]; + let prev = sourceSnippets[0]; + for (const sourceSnippet of sourceSnippets) { + if (sourceSnippet.type === 'IfStatement') { + continue; + } else if (sourceSnippet.source === prev.source) { + prev = sourceSnippet; + continue; } + results.push(sourceSnippet); + prev = sourceSnippet; + } + return results; +} + +function getStackTraceString(sourceSnippet: SourceSnippet): string { + let result = `${sourceSnippet.fileName}:${sourceSnippet.range.start.line}:${sourceSnippet.range.start.column}`; + const snippetString = getSourceSnippetString(sourceSnippet); + if (snippetString !== '') { + result += `:\n ${snippetString}`; + } + return result; +} + +function getSourceSnippetString(sourceSnippet: SourceSnippet): string { + switch (sourceSnippet.type) { + case 'ContractDefinition': + return `contract ${sourceSnippet.name}`; + case 'FunctionDefinition': + return `function ${sourceSnippet.name}`; + default: + return `${sourceSnippet.source}`; } } diff --git a/packages/sol-cov/src/types.ts b/packages/sol-cov/src/types.ts index cef7141cb..54ade0400 100644 --- a/packages/sol-cov/src/types.ts +++ b/packages/sol-cov/src/types.ts @@ -1,4 +1,5 @@ import { StructLog } from 'ethereum-types'; +import * as Parser from 'solidity-parser-antlr'; export interface LineColumn { line: number; @@ -114,3 +115,12 @@ export interface EvmCallStackEntry { } export type EvmCallStack = EvmCallStackEntry[]; + +export interface SourceSnippet { + source: string; + fileName: string; + type: string; + node: Parser.ASTNode; + name: string | null; + range: SingleFileSourceRange; +} diff --git a/packages/sol-cov/src/utils.ts b/packages/sol-cov/src/utils.ts index 4f16a1cda..d31696636 100644 --- a/packages/sol-cov/src/utils.ts +++ b/packages/sol-cov/src/utils.ts @@ -66,4 +66,10 @@ export const utils = { } return structLogs; }, + getRange(sourceCode: string, range: SingleFileSourceRange): string { + const lines = sourceCode.split('\n').slice(range.start.line - 1, range.end.line); + lines[lines.length - 1] = lines[lines.length - 1].slice(0, range.end.column); + lines[0] = lines[0].slice(range.start.column); + return lines.join('\n'); + }, }; diff --git a/packages/sol-cov/test/collect_coverage_entries_test.ts b/packages/sol-cov/test/collect_coverage_entries_test.ts index a03be19cd..5eedfd55c 100644 --- a/packages/sol-cov/test/collect_coverage_entries_test.ts +++ b/packages/sol-cov/test/collect_coverage_entries_test.ts @@ -6,17 +6,10 @@ import 'mocha'; import * as path from 'path'; import { collectCoverageEntries } from '../src/collect_coverage_entries'; -import { SingleFileSourceRange } from '../src/types'; +import { utils } from '../src/utils'; const expect = chai.expect; -const getRange = (sourceCode: string, range: SingleFileSourceRange) => { - const lines = sourceCode.split('\n').slice(range.start.line - 1, range.end.line); - lines[lines.length - 1] = lines[lines.length - 1].slice(0, range.end.column); - lines[0] = lines[0].slice(range.start.column); - return lines.join('\n'); -}; - describe('Collect coverage entries', () => { describe('#collectCoverageEntries', () => { it('correctly collects coverage entries for Simplest contract', () => { @@ -45,20 +38,20 @@ describe('Collect coverage entries', () => { const setFunction = `function set(uint x) { storedData = x; }`; - expect(getRange(simpleStorageContract, coverageEntries.fnMap[fnIds[0]].loc)).to.be.equal(setFunction); + expect(utils.getRange(simpleStorageContract, coverageEntries.fnMap[fnIds[0]].loc)).to.be.equal(setFunction); expect(coverageEntries.fnMap[fnIds[1]].name).to.be.equal('get'); // tslint:disable-next-line:custom-no-magic-numbers expect(coverageEntries.fnMap[fnIds[1]].line).to.be.equal(8); const getFunction = `function get() constant returns (uint retVal) { return storedData; }`; - expect(getRange(simpleStorageContract, coverageEntries.fnMap[fnIds[1]].loc)).to.be.equal(getFunction); + expect(utils.getRange(simpleStorageContract, coverageEntries.fnMap[fnIds[1]].loc)).to.be.equal(getFunction); expect(coverageEntries.branchMap).to.be.deep.equal({}); const statementIds = _.keys(coverageEntries.statementMap); - expect(getRange(simpleStorageContract, coverageEntries.statementMap[statementIds[1]])).to.be.equal( + expect(utils.getRange(simpleStorageContract, coverageEntries.statementMap[statementIds[1]])).to.be.equal( 'storedData = x', ); - expect(getRange(simpleStorageContract, coverageEntries.statementMap[statementIds[3]])).to.be.equal( + expect(utils.getRange(simpleStorageContract, coverageEntries.statementMap[statementIds[3]])).to.be.equal( 'return storedData;', ); expect(coverageEntries.modifiersStatementIds).to.be.deep.equal([]); |