Slang

[Janther]

This is a proposal for a version 2 of this plugin.

It has been rewritten leveraging the power of Nomic Foundation's Slang to provide a much more scalable and better supported parser.

While we are still in beta we have disabled browser support until we can make sure we can fully provide this functionality.

The coverage has been reduced as well since slang's AST is much more detailed than solidity-parser's and while all our tests still pass, they don't provide every possible scenario.

Using with the old parser is still possible but a deprecated warning will be logged.

[fvictorio]

I didn't do an in-depth review of the PR; this is just the result of manually testing it and skimming the code for a couple of hours. If there are things you would like me to review in depth, please point them out to me, because I don't think I'll be able to do a line-by-line review of the whole PR.

Also, leaving inline comments in a PR this big is impossible (GitHub's UI sucks for big diffs), so I will just list my comments here.

Pushed commits

I added a couple of commits. The first one clears the current line when printing a warning. Without this, if you do prettier Foo.sol and get a warning, the line says Foo.sol[prettier-solidity] 'solidity-parse' has been deprecated, please use 'slang-solidity'. The Foo.sol part is a temporary write that prettier does to stdout, and the warning messes with the cleanup of that temporary warning.

The other commit just makes src/index.ts clearer, by using explicit names for antlr and slang variables.

Comments.sol tests

These tests are only run with the ANTLR parser, and the run-format-test.js file has a comment that says // TODO: finish Comments. Does this mean that there's something pending around comments when using the Slang parser?

And, as an aside, all the Slang-related code in run-format-test.js doesn't seem to make sense? shouldTestSlang will always return false, so all the code used in that scenario seems like dead code.

Defaulting to the latest supported version

I mentioned this already before, but defaulting to the latest Solidity version supported by slang won't work for projects with multiple versions of Solidity, which is a common thing for projects that have been around for some years.

I don't know what's the right solution here. I kind of feel that Slang should export some helper utility to let you infer the version to use for a given code (by getting the pragmas doing what solidity-analyzer does, and then returning the latest supported version that matches the pragmas). But there might be better solutions.

Other comments

• There are some commented out lines in CI.yml
• The options.parentParser trick used in the SourceUnit nodes is, if I remember correctly, done to avoid printing empty trailing lines in Solidity snippets within a markdown file. Can we add a comment explaining that? Otherwise it's a bit obscure.
• Out of scope for the Slang refactor, but I think we should use tsx instead of ts-node.

[Janther]

I didn't do an in-depth review of the PR; this is just the result of manually testing it and skimming the code for a couple of hours. If there are things you would like me to review in depth, please point them out to me, because I don't think I'll be able to do a line-by-line review of the whole PR.

The only thing that pops up is the slang-comments section which is new.

Comments.sol tests

These tests are only run with the ANTLR parser, and the run-format-test.js file has a comment that says // TODO: finish Comments. Does this mean that there's something pending around comments when using the Slang parser?

These were some extremely edge cases with the positioning of comments where honestly comments should not go. I pushed a commit making these scenarios, not the same as with the antlr parser but closer to what prettier does with JS in a similar situation.

And, as an aside, all the Slang-related code in run-format-test.js doesn't seem to make sense? shouldTestSlang will always return false, so all the code used in that scenario seems like dead code.

yeah this was dead code (just removed it) that was helpful when I was slowly implementing new printers and had a proper list of tests that needed to be tested with both parsers.

Defaulting to the latest supported version

I mentioned this already before, but defaulting to the latest Solidity version supported by slang won't work for projects with multiple versions of Solidity, which is a common thing for projects that have been around for some years.

I don't know what's the right solution here. I kind of feel that Slang should export some helper utility to let you infer the version to use for a given code (by getting the pragmas doing what solidity-analyzer does, and then returning the latest supported version that matches the pragmas). But there might be better solutions.

This is planned but not in the scope of this PR.

Other comments

• There are some commented out lines in CI.yml

While we slang doesn't support WASM, the build process is broken and the CI step for browser should be skipped. Once they start supporting it, the CI comes right back.

• The options.parentParser trick used in the SourceUnit nodes is, if I remember correctly, done to avoid printing empty trailing lines in Solidity snippets within a markdown file. Can we add a comment explaining that? Otherwise it's a bit obscure.

Done.

• Out of scope for the Slang refactor, but I think we should use tsx instead of ts-node.

Will look into this. The main thing would be that it would need to be compatible with jest-light-runner