[AST] Consider expanding the use of NatSpec documentation to all declarations.

[christianparpart]

The motivation behind is, to be able to display additional information about a declaration, not just its type signature.

This might also help documentation.

The relevant AST nodes I'd at least expand to are:

• struct definition
• enum definition
• enum value defintion
• UDVTs
• ASTJSON tests for custom errors

[frangio]

Would really like to see this. On top of the ones mentioned in the issue, I'd add custom errors and user defined value types.

It's currently weird because it's possible to add NatSpec in the source code but it doesn't show up in the AST.

[chriseth]

Sounds good!

[PaulRBerg]

An interim solution is to apply a @custom NatSpec tag to document struct fields and enum values. Though I am not sure if I should do it like this:

/// @custom: var Description for my var
struct Foo {
    uint256 var;
}

Or like this:

/// @custom:field var Description for my var
struct Foo {
    uint256 var;
}

What would you recommend, @chriseth? I can't quite tell if there's any meaningful difference between the two, i.e. if one of the two variants produces a more informative ABI.

[PaulRBerg]

Any chance this could be included in Solidity v0.8.19?

Seems like an easy fix but with lopsided benefits for end users - e.g. OpenZeppelin is currently working on their V5. It would be super helpful to have NatSpec documentation in all OpenZeppelin structs!

At a minimum, could this be implemented for structs?

[cameel]

Probably not 0.8.19 because we want to release that pretty soon (our initial plan was to aim for 2 weeks after 0.8.18 and that may still be possible).

But as for including it in some release in the near future, I'll defer to @NunoFilipeSantos here. Unfortunately this is nowhere close to our current roadmap. This is not a big task so we might be able to fit it in somewhere between bigger tasks, but I can't really promise anything.

Incidentally, we have a pending PR that adds support for NatSpec on enums: #13626. It's stuck on some trivial things (needs more tests), mostly because everyone on the team is busy with other tasks so there and there was no one to take it over so far. If some contributor would pick that up, we might have at least that pretty soon.

[PaulRBerg]

I see, thanks anyway @cameel - I do appreciate that you are tied up nowadays.

[frangio]

This definitely isn't world-ending priority but I think we should consider it a bug with medium rather than low impact.

[cameel]

I'm fine with increasing the impact - this is only an estimate and the fact that it's of that much interest to libraries shows that it perhaps should be higher. I wouldn't really consider it a bug though because AFAIK it's was just designed that way and not originally meant for internal documentation. Of course it does not mean that we should not change it. It's just more of a minor feature at this point.

[nikola-matic]

@frangio, @PaulRBerg structs and enums are definitely going into 0.8.20 since they're merged, and @veniger is continuing work on custom errors.

[nikola-matic]

Would really like to see this. On top of the ones mentioned in the issue, I'd add custom errors and user defined value types.

It's currently weird because it's possible to add NatSpec in the source code but it doesn't show up in the AST.

@frangio, replying a tad bit late, but anyway, are you sure that custom error NatSpec is not exported to the JSON AST?
Given the following minimal *.sol:

/// @notice example of notice
/// @dev example of dev
error CustomError();

running with solc v0.8.19 I get the following output when using --ast-compact-json --pretty-json:

{
  "absolutePath": "error.sol",
  "exportedSymbols":
  {
    "CustomError":
    [
      3
    ]
  },
  "id": 4,
  "nodeType": "SourceUnit",
  "nodes":
  [
    {
      "documentation":
      {
        "id": 1,
        "nodeType": "StructuredDocumentation",
        "src": "0:54:0",
        "text": "@notice example of notice\n @dev example of dev"
      },
      "errorSelector": "09caebf3",
      "id": 3,
      "name": "CustomError",
      "nameLocation": "60:11:0",
      "nodeType": "ErrorDefinition",
      "parameters":
      {
        "id": 2,
        "nodeType": "ParameterList",
        "parameters": [],
        "src": "71:2:0"
      },
      "src": "54:20:0"
    }
  ],
  "src": "54:21:0"
}

[frangio]

@nikola-matic That seems right, I misspoke. For user defined types I don't see the documentation in the AST.

[veniger]

@frangio I will start implementing this for the UDVT's today, you can expect a PR tommorow or monday

[PaulRBerg]

Am I understanding correctly that #14119 did not add support for documenting struct params/ fields? Based on this example, only the @title, @author, @notice, and @dev seem to be supported.

Why not let users document struct fields with the @param tag? Or create a new tag @field to make a distinction between struct fields and function params?

[nikola-matic]

Am I understanding correctly that #14119 did not add support for documenting struct params/ fields? Based on this example, only the @title, @author, @notice, and @dev seem to be supported.

Why not let users document struct fields with the @param tag? Or create a new tag @field to make a distinction between struct fields and function params?

Yup, you are correct; at the moment, only top level documentation for structs is exported. NatSpec is Doxygen inspired, which generally serves to document the interface, rather than the implementation itself, so I am personally not a big fan of having custom tags for contract/struct internals, however, I do agree that having a specific tag for such scenarios (e.g. @field that you suggested) is preferable than misusing @param for that purpose.

PRs for UDVTs and enum values are open right now and will be part of the next release; beyond that, we have no plans to work on NatSpec for the foreseeable future (as far as I'm aware). You are of course welcome in the Wednesday language design meetings to convince us otherwise :)

[frangio]

Struct fields become a part of the interface if they are used as a function argument or return value though?

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

[gabrielkrell]

PRs for UDVTs and enum values are open right now and will be part of the next release; beyond that, we have no plans to work on NatSpec for the foreseeable future (as far as I'm aware).

@nikola-matic What happened to this? Next release has come and gone. Yesterday I had to implement some horrible local bodge to inject UDVT docstrings into my company's generated documentation. I would really like to see #14166 merged. The JSON output for UDVTs has no tags at all. At least we can document enum values with @custom while we wait for #14193.

---

Like frangio I also think struct fields should get first-class support, not the custom DIY stuff everyone is doing right now. Consider the following:

struct User {
	uint assets;
	uint debt;
	uint64 age;
	...
}

mapping (bytes32 userId => User) private users;

function registerUser(User newUser) external onlyAdmin() returns (bytes32 userId);
function updateUser(bytes32 userId, User updatedUser) external onlyAdmin();

function getUser(bytes32 userId) external view returns (User selectedUser);
function getAverageUser() external view returns (User averageUser);
function getMedianUser() external view returns (User medianUser);

Clearly User is part of the interface and people calling these functions need to know what it is. But functions that work with shared structs shouldn't each be responsible for documenting every field of the struct. You'd have tons of duplicate documentation, and each function would have to have many field docstrings crammed into a single @param or @return. Those functions should simply say "this takes/returns a User which represents blah" and then you click on User to see how it's constructed. If you are going to support enum values, the same logic should lead you to support struct fields.

[nikola-matic]

@gabrielkrell we're working on Cancun support (transient storage, mcopy, etc.) at the moment, hoping to release in Jan 2024 - so this is pretty low priority; in other words, no one on the team is going to be working on this for the next 1-2 months. Would you like to give it a shot? There are a few other PRs in this issue that address the same problem for structs, enums, etc.

[gabrielkrell]

@nikola-matic Unfortunately my team probably can't spare the bandwidth either. I'm glad to know it's still on your radar.