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

Unbalanced parenthesis cause unclear error message #586

Closed
axic opened this issue Sep 24, 2021 · 4 comments · Fixed by #760
Closed

Unbalanced parenthesis cause unclear error message #586

axic opened this issue Sep 24, 2021 · 4 comments · Fixed by #760
Labels
help wanted Extra attention is needed

Comments

@axic
Copy link

axic commented Sep 24, 2021

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.

@mattiaerre
Copy link
Member

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

@Janther
Copy link
Contributor

Janther commented Sep 25, 2021

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

@axic
Copy link
Author

axic commented Sep 25, 2021

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.

@fvictorio
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants