search

sc-forks / solidity-coverage

Code coverage for Solidity smart-contracts
MIT License
977 stars 264 forks source link

ParserError in solidity-coverage 0.8.0 (solc 0.8.6) #700

Closed simon-something closed 2 years ago

simon-something commented 2 years ago

Hi, we're running into this error while trying to run coverage on Juicebox-V2 codebase (https://github.com/jbx-protocol/juice-contracts-v2) - v0.7.0 runs without issue while 0.8.0 returns the following error (I tried resolving solidity-parser to the latest version, without success neither). All our codebase is in solidity 0.8.6.

ParserError: Expected ',' but got ';'
  --> contracts/JB18DecimalERC20PaymentTerminal.sol:63:37:
   |
63 |     ((c_c24adfcc(0x5c1a3ceb145f412e); /* statement */ 
   |                                     ^

ParserError: Expected ',' but got ';'
    --> contracts/abstract/JBPaymentTerminal.sol:1199:37:
     |
1199 |     ((c_d41e2bbe(0xf9757d8e289ec41e); /* statement */ 
     |                                     ^

Error in plugin solidity-coverage: HardhatError: HH600: Compilation failed

Anyone faced the same issue? Any clue on how to handle it? If not, we mostly want to use sol-cov0.8.0 to cover ternary operators, any alternative? Thank you:)

cgewecke commented 2 years ago

Will take a look at this...

simon-something commented 2 years ago

@cgewecke is there a grant for solidity-coverage somewhere? We've been using it quite a lot while working on Juicebox and I'd be happy to submit a proposal to help fund you!

cgewecke commented 2 years ago

@drgorillamd That's very kind of you. I should be able to look at this tomorrow and publish a patch to the beta as a short term solution.

RE grant funding...this project has lots of possible funding tbh.

simon-something commented 2 years ago

@cgewecke I'm sure it has! Still, if something is created at some point, I think it would make sense for us to help

cgewecke commented 2 years ago

@drgorillamd This is fixed on the beta. You should be able to install and run (without needing yarn resolutions for anything) with:

yarn add solidity-coverage@beta

There is a sample report generated for juice-box here: https://lyrical-horn.surge.sh/contracts/index.html

If you see anything odd in the coverage - things reported as missing hits which you're certain get hit - just lmk and will check.

To fix the bug you ran into it was necessary to stop covering statements in some conditional expressions (due to solc 0.8.x's parsing strictness). This does not impact line or branch coverage - it's only relevant if you have multiple statements on a single line and there are very few circumstances where the first two measurements won't catch a missing execution path. And example might be:

return x; y = 5;
simon-something commented 2 years ago

Great, working like a charm! Will sure let you know if we encounter strange behaviors, thank you very much