prettier-plugin-solidity icon indicating copy to clipboard operation
prettier-plugin-solidity copied to clipboard

Unbalanced parenthesis cause unclear error message

Open axic opened this issue 4 years ago • 4 comments

contract C {
  function f() {
    require(balanceOf();
  }
}

results in

[error] contracts/A.sol: TypeError: Cannot read property 'length' of undefined
[error]     at ASTBuilder.visitExpression (/node_modules/@solidity-parser/parser/dist/index.cjs.js:36002:26)
[error]     at ASTBuilder.visitExpressionStatement (/node_modules/@solidity-parser/parser/dist/index.cjs.js:35932:24)
[error]     at ExpressionStatementContext.accept (/node_modules/@solidity-parser/parser/dist/index.cjs.js:33418:22)
[error]     at ASTBuilder.visit (/node_modules/@solidity-parser/parser/dist/index.cjs.js:17669:19)
[error]     at ASTBuilder.visitSimpleStatement (/node_modules/@solidity-parser/parser/dist/index.cjs.js:35449:17)
[error]     at SimpleStatementContext.accept (/node_modules/@solidity-parser/parser/dist/index.cjs.js:33590:22)
[error]     at ASTBuilder.visit (/node_modules/@solidity-parser/parser/dist/index.cjs.js:17669:19)
[error]     at ASTBuilder.visitStatement (/node_modules/@solidity-parser/parser/dist/index.cjs.js:35446:17)
[error]     at /node_modules/@solidity-parser/parser/dist/index.cjs.js:35482:51
[error]     at Array.map (<anonymous>)

My reasoning for including this here is the same as in #585, let me know if these reports should not be made here, or if they are not useful.

axic avatar Sep 24 '21 12:09 axic

@axic isn't this an issue that a linter should catch?

mattiaerre avatar Sep 24 '21 21:09 mattiaerre

Yes, incorrect syntax is not something in the scope of this tool.

Janther avatar Sep 25 '21 01:09 Janther

The main question with these issues is a clear goal/expectation detailed in the README:

  1. Is prettier only supposed to work on code the compiler (which version?) is not complaining about? In this case the current non-user-friendly failures are expected.

(Please note that with the compiler has changed syntax numerous times, and most tools try to support the superset of syntax it ever supported.)

  1. Is prettier supposed to gracefully work (report an error) on invalid code?

It would be nice to have this decision documented in the README 🙂

Disclaimer: I work on the Solidity compiler.

axic avatar Sep 25 '21 10:09 axic

Same comment as here, but I agree with this:

It would be nice to have this decision documented in the README

Clarifying in the README that invalid code (that is, code that cannot be parsed) doesn't work makes sense. Let's keep this one open until we do that.

fvictorio avatar Sep 30 '21 11:09 fvictorio