Natspec feature throwing errors in natspec not attached to functions

[Janther]

when running the latest code of master against openzepellin contracts there are many errors because of the natspec feature.

for example when running it against contracts/mocks/ERC165/ERC165InterfacesSupported.sol I get these errors.

ParserError: no viable alternative at input '/**\n     * @dev A mapping of interface id to whether or not it's supported.\n     */mapping' (24:4)
    at Object.parse (/Users/klaushottvidal/DAPPS/solidity-parser-antlr/dist/index.js:79:11) {
  message: "no viable alternative at input '/**\\n     * @dev A mapping of interface id to whether or not it's supported.\\n     */mapping' (24:4)",
  errors: [
    {
      message: "no viable alternative at input '/**\\n     * @dev A mapping of interface id to whether or not it's supported.\\n     */mapping'",
      line: 24,
      column: 4
    },
    {
      message: "no viable alternative at input '/**\\n     * @dev A contract implementing SupportsInterfaceWithLookup\\n     * implement ERC165 itself.\\n     */constructor'",
      line: 30,
      column: 4
    }
  ]
}

my guess is that probably the current rules only consider natspec for functions, contracts, libraries, and interfaces (notice that the constructor is a valid place for natspec and also throws an error in the example shown).

While the solc only considers these cases, people use natspec for variable declaration and modifier among other nodes so perhaps ignore these cases without throwing errors or also parse them (I like the latter better but it's up to you).

[federicobond]

Cool, this is exactly the kind of feedback I wanted to get before publishing 0.5.0. Thank you @Janther!

I'll try to look into this in the next days, but I don't have much time. Pinging @andremedeiros if he wants to check this out too.

[Janther]

on the top of my mind, the nodes that are direct child of ContractDefinition, and therefor very likely to get an @dev or an @notice (or a cheeky @author if a dev want to take full credit of a feature but it's unlikely), are:

• EnumDefinition
• EventDefinition
• FunctionDefinition (constructors also fall here)
• ModifierDefinition (can also get @param)
• StateVariableDeclaration
• StructDefinition
• UsingForDeclaration

I hope this helps.

[alcuadrado]

Hi Fede 👋🏻

Was this released in the latest version?

I've been contacted by one of Aragon's engineers to tell me that their CI broke because solidity-parser-antlr fails to parse this file. The error message in their CI looks like an instance of this issue.

btw, thanks for maintaining this project! ❤️

[federicobond]

Yes, I messed up the latest release, sorry! I created the stable branch here to continue the 0.4.x series without having to worry about NatSpec problems, but I did not choose the right solidity-antlr tip. There should be a stable branch there too without NatSpec changes.

I don't have much time today, but if you can help me out we should be able to get this solved by rebuilding the stable branch here with the fixed stable branch tip from solidity-antlr.

[federicobond]

Well, I think I got it fixed in 0.4.11. Can you check with the Aragon folks?

[alcuadrado]

It works now :) Thanks @federicobond !

[vibern0]

Hey,
Can you guys extract the natspec? It seems like it's not working. I took an example from a test and does not work

const ast = parser.parse(
    `/**
        * @dev This is the Sum contract.
        * @title Sum Contract
        * @author username
        */
    contract Sum { }`
);
console.log(ast);

I'm using 0.4.11

[Janther]

As far as I know, the Natspec feature was put on hold until this issue is solved.
The comments about this getting fixed in 0.4.11 were because the feature was published by mistake and removed with 0.4.11.

[vibern0]

hmm,
Well, I coded most of it when it was first merged. But I think, the problem is because the wrong ANTLR code was merged. The code merged wasn't mine, so it didn't work together. @federicobond
As a reference:

• 2273972
• Add natspec comments parser #71
• Add natspec comments parser solidityj/solidity-antlr4#19

When I did this, I didn't parse variables natspec, and now I was planning to do so.