"{" in base contract constructor parameter string confuses instumentation

[KaiRo-at]

I have a contract using my patches in OpenZeppelin/openzeppelin-contracts#2029 to add a URL to an ERC-1155 token, with code like this:

import ".../ERC1155MetadataURICatchAll.sol";
contract MYToken is ERC1155MetadataURICatchAll("https://example.com/meta/{id}"), {
    // Some contract code...
}

This compiles and works fine in general, but when running coverage, the instrumentation gets confused by the curly braces in the string and I get an error like this:

.../MYToken.sol:15:51: ParserError: Expected string end-quote.
 ... oken is ERC1155MetadataURICatchAll("https://example.com/meta/{event __CoverageMYToken(string fileName, uint256 lineNumber);
                                        ^--------------------------------------------------------------------------------------^

Compilation failed. See above.
Truffle v5.1.12 (core: 5.1.12)
Node v10.18.1
Cleaning up...
Event trace could not be read.
Error: ENOENT: no such file or directory, open './allFiredEvents'
Exiting without generating coverage...

When I remove the curly braces from the string before running coverage, it works correctly.

[KaiRo-at]

FWIW, if I move to a pattern where the parameter is noted with the constructor, e.g.

import ".../ERC1155MetadataURICatchAll.sol";
contract MYToken is ERC1155MetadataURICatchAll {
    constructor()
    ERC1155MetadataURICatchAll("https://example.com/CS2PS/meta/{id}")
    public
    {
    }
}

I get the same kind of error (message cropping not entirely correct as I had to edit out our actual URL given the project will not be public for some time):

.../MYToken.sol:56:32: ParserError: Expected string end-quote.
    ERC1155MetadataURICatchAll("https://example.com/meta ... yproject/contracts/MYToken.sol',1);
                               ^---------------------------------------------------------------^

[cgewecke]

@KaiRo-at Thanks for reporting! Nice catch.

This should be fixed by #479. We just published yesterday - it could be a week (+) before another patch is released.

I think Zeppelin is migrating to solidity-coverage 0.7.x in the near term but at the moment they are using an older fork which is set up differently.

There might be a bit of delay getting these changes to percolate through to your PR...

[KaiRo-at]

Awesome!
Just out of interest, does this cover the similar case in #478 (comment) (i.e. the additional comment where I moved that base constructor call into the constructor declaration instead of the contract declaration) as well?
(And FWIW, I'm using solidity-coverage 0.6.7 directly in our projects, I just in addition also contributed that ERC-1155 URL support in OpenZeppelin.)

[cgewecke]

@KaiRo-at Ah! Thanks for double-checking that...I didn't look at the second example closely enough. That will probably need it's own fix...

[cgewecke]

@KaiRo-at The second one is fixed in #480...

[cgewecke]

Published with 0.7.3