`forge fmt` parsing errors are unhelpful

[sambacha]

trafficstars

Component

Forge

Have you ensured that all of these are up to date?

• [X] Foundry
• [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (cce3a44 2022-07-20T00:14:55.494548Z)

What command(s) is the bug in?

forge fmt

Operating System

macOS (Intel)

Describe the bug

The error message from forge fmt

Failed to parse Solidity code for src/immutable/as_function_param.sol. Leaving source unchanged.
Debug info: [Diagnostic { loc: File(168, 33, 42), level: Error, ty: ParserError, message: "unrecognised token 'immutable', 

expected \"!=\", \"%\", \"%=\", \"&\", \"&&\", \"&=\", \"(\", \")\", \"*\", \"**\", \"*=\", \"+\", \"++\", \"+=\", \",\", \"-\", \"--\", \"-=\", \".\", \"/\", \"/=\", \":\", \";\", \"<\", \"<<\", \"<<=\", \"<=\", \"=\", \"==\", \"=>\", \">\", \">=\", \">>\", \">>=\", \"?\", \"[\", \"]\", \"^\", \"^=\", \"calldata\", \"case\", \"days\", \"default\", \"error\", \"ether\", \"gwei\", \"hours\", \"indexed\", \"leave\", \"memory\", \"minutes\", \"revert\", \"seconds\", \"storage\", \"switch\", \"weeks\", \"wei\", \"{\", \"|\", \"|=\", \"||\", \"}\", identifier", notes: [] }]

Contrasted with solc:

contract C {
    function f(uint immutable) public pure {}
}
// ----
// DeclarationError 8297: (28-42): The "immutable" keyword can only be used for state variables.

There are more errors like this, e.g.

unhelpful:~ $ Debug info: [Diagnostic { loc: File(47, 9, 16), level: Error, ty: ParserError, message: "unrecognised token 'library', expected \"contract\"", notes: [] }]

helpful:~ $ TypeError 9571: (0-22): Libraries cannot be abstract.

[onbjerg]

These are not necessarily incorrect, they are just more opaque because the formatter uses solang-parser to parse the code, it does not use solc in any capacity, so the error is from solang-parser. Will probably need to do a general error message pass on the formatter where possible, so I'll change this issue to that instead

[sambacha]

These are not necessarily incorrect, they are just more opaque because the formatter uses solang-parser to parse the code, it does not use solc in any capacity, so the error is from solang-parser. Will probably need to do a general error message pass on the formatter where possible, so I'll change this issue to that instead

yup, though should there be a check post fmt that ensures it compiles as a sort of sanity check?

[onbjerg]

The parser would error before that anyway, so my point re: error messages still stands :)

[sambacha]

The parser would error before that anyway, so my point re: error messages still stands :)

Ok, for reference the parser used by prettier solidity is relaxed wrt how strict it enforces reading, so your saying that the parser is infact as strict as solc gotcha

[onbjerg]

Doesn't really matter if it's as strict as solc in this context, it won't make solc/solang-parser interchangeable and it won't make it possible to invoke solc for a sanity check before we do the formatting. =

Can't speak to the parser used by prettier-solidity other than that we obviously can't use it :) =

Don't know much about how solang-parser works, it might be an option to make it more lax (cc @seanyoung) but it would likely require some work. To be clear: the error is from solang-parser itself, we do not control this specific part so we cannot make it more lax in our codebase

[seanyoung]

We have a hyperledger mentorship mentee who will work on making the parser more lax: https://wiki.hyperledger.org/display/INTERN/Solang+Solidity+Compiler+optimizations+and+error+handling

That doesn't deal with the quality of the error message though. The way solang works through the following stages:

• parse: this based on parser generated from grammar, this is solang-parser
• sema: semantic analysis. This checks that the solidity file is entirely valid, and produces a validated ast.
• codegen: generate CFG style code
• emit: generate llvm IR

Currently forge fmt uses the raw parse tree from solang-parser. Ideally it would use the ast produced by sema, but sema is not complete yet (especially for evm solidity, it was written for non-evm chains). Once we get sema in order, forge fmt can switch to the ast from solang and generate source code based on a fully validated parse tree.

In the mean time, errors like The "immutable" keyword can only be used for state variables. is not something the parser can produce, that is something that sema does.

[sambacha]

We have a hyperledger mentorship mentee who will work on making the parser more lax: https://wiki.hyperledger.org/display/INTERN/Solang+Solidity+Compiler+optimizations+and+error+handling

That doesn't deal with the quality of the error message though. The way solang works through the following stages:

• parse: this based on parser generated from grammar, this is solang-parser
• sema: semantic analysis. This checks that the solidity file is entirely valid, and produces a validated ast.
• codegen: generate CFG style code
• emit: generate llvm IR

Currently forge fmt uses the raw parse tree from solang-parser. Ideally it would use the ast produced by sema, but sema is not complete yet (especially for evm solidity, it was written for non-evm chains). Once we get sema in order, forge fmt can switch to the ast from solang and generate source code based on a fully validated parse tree.

In the mean time, errors like The "immutable" keyword can only be used for state variables. is not something the parser can produce, that is something that sema does.

When do you think it likely sema will be usable in the way you described? Also thank you for the detailed response, this would be helpful to document potentially in the handbook as a reference!

[rkrasiuk]

summary: considering that the desired error output format is not smth we can produce based on the parse tree, we'll wait for the sema to become more feature complete. closing this issue, feel free to reopen when relevant

[seanyoung]

When do you think it likely sema will be usable in the way you described?

Not sure. sema would require a new EVM target and running all the same tests through sema and fixing up all the issues. We would all love to see EVM support in solang, and that starts with EVM support in sema. Something we would be definitely be working on at some point.