search

federicobond / solidity-parser-antlr

A Solidity parser for JS built on top of a robust ANTLR4 grammar
MIT License
155 stars 56 forks source link

Building the AST sometimes fails, throwing an exception #28

Open alcuadrado opened 6 years ago

alcuadrado commented 6 years ago

I'm afraid I don't know how to fix this issue, so no PR this time :(

Some invalid sources make the parser fail when building the ast, because this method throws.

For example, this code contract { function a() return bool {} } causes that problem.

Here's a stack trace of it failing:

TypeError: Cannot read property 'getText' of null
    at ASTBuilder.StateVariableDeclaration (node_modules/solidity-parser-antlr/src/ASTBuilder.js:628:23)
    at ASTBuilder.visit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1133:40)
    at ASTBuilder.ContractPart (node_modules/solidity-parser-antlr/src/ASTBuilder.js:89:17)
    at ASTBuilder.visit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1133:40)
    at ASTBuilder.<anonymous> (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1121:19)
    at Array.map (<anonymous>)
    at ASTBuilder.visit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1120:16)
    at ASTBuilder.ContractDefinition (node_modules/solidity-parser-antlr/src/ASTBuilder.js:76:22)
    at ASTBuilder.visit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1133:40)
    at ASTBuilder.<anonymous> (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1121:19)
    at Array.map (<anonymous>)
    at ASTBuilder.visit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1120:16)
    at ASTBuilder.SourceUnit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:33:22)
    at ASTBuilder.visit (node_modules/solidity-parser-antlr/src/ASTBuilder.js:1133:40)
    at Object.parse (node_modules/solidity-parser-antlr/src/index.js:65:23)
    at getImports (src/solidity/imports.js:5:22)
    at Context.it (test/solidity/imports.js:15:21)
federicobond commented 6 years ago

Yes, this is the unfortunate result of trying to build an AST using tolerant mode. If the toleration extends beyond simple things like missing a semicolon into full parts of the tree missing (like the identifier with the contract name in this case) then we have to guard against that everywhere.

We should check if this is only a problem in the places where we are accessing getText() or requires a much bigger refactoring to handle every possible subnode not existing.

federicobond commented 6 years ago

I just wrapped all calls to getText with a function that handles nulls, that should solve your error and several others. Leaving this open to investigate if there are similar issues triggered by more obscure syntax errors.

alcuadrado commented 6 years ago

Thanks for the quick answer and fix fede 🙌🏻

I'll get back to this issue if I find this problem in other cases.