Skip to content

Conversation

samooyo
Copy link
Contributor

@samooyo samooyo commented Mar 6, 2025

Motivation

Solution

The forge doc comment parser now supports parsing Enum variants in Natspecs and displaying them in a table. To declare these variants, you can use either the @param or @custom:variant tag.

To implement this feature:

  • Comments containing the @custom:variant tag are filtered out to prevent them from being displayed in the Note field.
  • Both the variant and param tags are parsed to properly display the table.

Additionally:

  • The From trait was implemented for the Comments struct to enable the construction of the returned vector from the filtered comments.
  • A dedicated table header and separator were added specifically for variants.

2
1

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @samooyo, thanks for your PR!

Given that Solidity has not prioritised adding support for this in the current release I think it is reasonable to add this ahead of time and make adjustments where needed afterwards.

Would you mind adding a test case for this?

The proposed syntax of Solidity (argotorg/solidity#14193) is as follows:

    enum Color {
        Red,
        /// @notice example of notice
        /// @dev example of dev
        Green,
        /// @dev example of dev
        Blue
    }

Once Solidity has implement support for this in-line syntax we can expand it to also cover this case.

Personally I think the @param syntax as proposed makes most sense here

@zerosnacks zerosnacks added the T-to-discuss Type: requires discussion label Mar 7, 2025
Copy link
Contributor

@srdtrk srdtrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it seems like CommentTag::Custom("variant".to_string()) is commonly used in this PR. Would it be ok to add the following method to CommentTag in parser.

    /// Create a new instance of [CommentTag::Custom] with the `variant` tag.
    pub fn variant() -> Self {
        Self::Custom("variant".to_string())
    }

On a side note @zerosnacks, I'm using the parser crate in an external project for a natspec linter I'm building (it's private at the moment) in my free time. I also have to maintain a copy of the parser module since it is not published on crates.io. Would you be open to refactor and publish the parser as a crate if I create an issue for this?

@samooyo
Copy link
Contributor Author

samooyo commented Apr 22, 2025

Hi @samooyo, thanks for your PR!

Given that Solidity has not prioritised adding support for this in the current release I think it is reasonable to add this ahead of time and make adjustments where needed afterwards.

Would you mind adding a test case for this?

The proposed syntax of Solidity (ethereum/solidity#14193) is as follows:

    enum Color {
        Red,
        /// @notice example of notice
        /// @dev example of dev
        Green,
        /// @dev example of dev
        Blue
    }

Once Solidity has implement support for this in-line syntax we can expand it to also cover this case.

Personally I think the @param syntax as proposed makes most sense here

Hi @zerosnacks, thanks for the feedback!

From what I can see, there currently aren’t any existing tests for the writer part of the doc generation. Could you point me to where those tests should go, or how you'd like them structured? I'd be happy to add them accordingly.

@zerosnacks
Copy link
Member

zerosnacks commented May 7, 2025

0.8.30 adds NatSpec support for enum value definitions: #10455

https://github.com/ethereum/solidity/releases/tag/v0.8.30

@samooyo samooyo force-pushed the feature/forge-doc-enums branch from da74ec7 to 7f359b4 Compare May 12, 2025 07:36
Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @samooyo, could you take a look at updating this implementation for the new feature as released in solc 0.8.30?:)

@samooyo samooyo requested a review from 0xrusowsky as a code owner August 25, 2025 11:35
@onbjerg onbjerg self-assigned this Aug 25, 2025
@onbjerg
Copy link
Collaborator

onbjerg commented Sep 1, 2025

Hi @samooyo, let me know if you are blocked on supporting variant docs from argotorg/solidity#15956

@samooyo
Copy link
Contributor Author

samooyo commented Sep 2, 2025

Hi @samooyo, let me know if you are blocked on supporting variant docs from argotorg/solidity#15956

Hi @onbjerg, from my understanding, Solang has not yet implemented this new Solidity feature. So I’ve just removed the @custom:variant to prioritize the @param. Is there anything else I should add?

@samooyo samooyo requested a review from onbjerg September 2, 2025 12:37
@onbjerg
Copy link
Collaborator

onbjerg commented Sep 2, 2025

no this is ok, ty

@DaniPopes DaniPopes enabled auto-merge (squash) September 3, 2025 12:04
@DaniPopes DaniPopes merged commit 3d47818 into foundry-rs:master Sep 3, 2025
23 checks passed
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-to-discuss Type: requires discussion
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Added NatSpec support for enum value definitions
5 participants