search

JoinColony / solcover

Code coverage for solidity
MIT License
64 stars 8 forks source link

Lint: refactor parse table in instrumentSolidity.js #48

Closed cgewecke closed 7 years ago

cgewecke commented 7 years ago

The CI linter complains that instrumentSolidity.js is too long. Perhaps it is.

It might be nice to restructure the parse table in that file to excise the entries for nodes that aren't actively managed. This could be done by creating a file that exports an alphabetically listed array of all the node type names and then initializing the parse table with these values and stub functions via a one-line array.map at the top of instrumentSolidity.

area commented 7 years ago

I think the natural first step towards making BitHound happier with that file should be to move the injector and parse objects into their own files that get imported into instrumentSolidity. I can easily believe that the parse file would still be too long though and we'd need to do what you suggest. I think it's likely the instrument* functions should be in their own file too, come to think of it.

It's not as easy as just moving everything across to new files, because of my (despicable) use of global variables when I wrote this initially, but getting rid of those would be a benefit too!

cgewecke commented 7 years ago

Yes, agreed - it's looking pretty close to do-able in there though. You've basically done injector already. Maybe if the injection logic that's currently in parse[ContractStatement] and parse[LibraryStatement] is moved out to something like instrumentContract? That would separate all the concerns. And break out parse, injector, and instrument leaving instrumentSolidity as a few lines knitting everything together?