Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
drgorillamd opened this issue Mar 14, 2022 · 6 comments
Closed

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

drgorillamd opened this issue Mar 14, 2022 · 6 comments

Comments

@drgorillamd
Copy link

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
Copy link
Member

Will take a look at this...

@drgorillamd
Copy link
Author

@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
Copy link
Member

@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.

@drgorillamd
Copy link
Author

@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
Copy link
Member

@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;

@drgorillamd
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants