Added NatSpec support for enum value definitions in the AST

[veniger]

Adds NatSpec field to AST node for enum value definitions
Partially fixes: #12295

[github-actions]

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

[r0qs]

@veniger just a note about the failing external tests (i.e. all the ones with *_test_ext_*). Please ignore them for now, since I just noticed that due to the changes in #14242 the tests are failing for external contributions due to lack of access to the github token.

The errors in the soltests still need to be fixed in this PR though: https://app.circleci.com/pipelines/github/ethereum/solidity/29881/workflows/47cd436e-67ed-46b8-abe4-102c5f331b1d/jobs/1327332

[nikola-matic]

Changelog and a rebase (to fix the build), and should be good.

[cameel]

Same as in #14166, looks ok at a glance and is very similar to one of your PRs we have already merged so we don't have to be too strict about reviewing this. If @nikola-matic approves, you have my approval too.

The only thing we might potentially add here is is to make sure that the @custom tag works and maybe update the table at https://docs.soliditylang.org/en/latest/natspec-format.html#tags.

[nikola-matic]

Hey @veniger, as @cameel mentioned, please add usage of the @custom tag to one of the tests, and it should be good to go.

Same as in #14166, looks ok at a glance and is very similar to one of your PRs we have already merged so we don't have to be too strict about reviewing this. If @nikola-matic approves, you have my approval too.

The only thing we might potentially add here is is to make sure that the @custom tag works and maybe update the table at https://docs.soliditylang.org/en/latest/natspec-format.html#tags.

Another external contributor opened a PR for this already - #14267

[ekpyron]

@nikola-matic @veniger What's the state of this?

[nikola-matic]

@nikola-matic @veniger What's the state of this?

I can take a another more detailed look, but should be good enough to merge after a rebase.

[zerosnacks]

Hi all, would be great to get this reviewed and merged, cc @cameel / @nikola-matic

If a rebase is required, @veniger would you be able to do so?

Users are proposing to implement workarounds as seen here: foundry-rs/foundry#9905 but I think it is much better to resolve the issue here.

[nikola-matic]

Hi all, would be great to get this reviewed and merged, cc @cameel / @nikola-matic

If a rebase is require, @veniger would you be able to do so?

Users are proposing to implement workarounds as seen here: foundry-rs/foundry#9905 but I think it is much better to resolve the issue here.

Sure, we have a release coming up soon, so I'll check with the team on Monday whether we have enough time to get this in for the upcoming release; if not, we'll get it in for the next one.

[zerosnacks]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

[nikola-matic]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

I discussed it with the team and we've decided for this to go in, however, not in this release. We're currently almost exclusively focused on merging all of the storage layout (account abstraction for EOAs) PRs and will have no time for this one, especially since we're planning on releasing this week. I marked this issue (and PR) for the next release.

edit: you won't have to wait long for the (next) 8.30 release since we have some other stuff we're working for tooling as well, it's just that our priorities at the moment are on compatibility for the next hard fork.

[zerosnacks]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

I discussed it with the team and we've decided for this to go in, however, not in this release. We're currently almost exclusively focused on merging all of the storage layout (account abstraction for EOAs) PRs and will have no time for this one, especially since we're planning on releasing this week. I marked this issue (and PR) for the next release.

edit: you won't have to wait long for the (next) 8.30 release since we have some other stuff we're working for tooling as well, it's just that our priorities at the moment are on compatibility for the next hard fork.

Understood, not a problem - looking forward to the next releases 👍

[nikola-matic]

Hi @nikola-matic, any updates on this? Would be great to get this in - thanks!

I discussed it with the team and we've decided for this to go in, however, not in this release. We're currently almost exclusively focused on merging all of the storage layout (account abstraction for EOAs) PRs and will have no time for this one, especially since we're planning on releasing this week. I marked this issue (and PR) for the next release.
edit: you won't have to wait long for the (next) 8.30 release since we have some other stuff we're working for tooling as well, it's just that our priorities at the moment are on compatibility for the next hard fork.

Understood, not a problem - looking forward to the next releases 👍

I closed this PR as it was old, and opened a new one, which was just merged. The enum support for Natspec will be available in the upcoming release.