[natspec] Use @inheritdoc instead of "See {***}"

[GrandSchtroumpf]

When compiling a contract, the devdoc doesn't extends from the parent. For example when compiling the ERC20 contract, the devdoc of method balanceOf is

See {IERC20-balanceOf}.

Instead of

Returns the amount of tokens owned by `account`.

💻 Environment Hardhat

📝 Details

npx hardhat compile

Check the build-info with the compile output : JSON.parse(output.contracts[sourceName][contractName].metadata).output.devdoc. The methods don't inherit the parent class.

🛠️ Solution Use @inheritdoc <contractName> instead of See {<contractName>-<method>}. Here are two example from ERC20, with full inherited documentation & extended documentation

  /**
   * @inheritdoc IERC20
   */
   function balanceOf(...) ... {...)

  /**
   * NOTE: If `amount` is the maximum `uint256`, the allowance is not updated on
   * `transferFrom`. This is semantically equivalent to an infinite approval.
   *
   * Requirements:
   *
   * - `spender` cannot be the zero address.
   * @inheritdoc IERC20
   */
    function approve(...) ... { ... }

I can make a draft PR to discuss the implementation if you want. Thanks 🙂

[Amxx]

Hello @GrandSchtroumpf

This would definitely be a nice change, but we need to check that all the documentation generation will support that.

[GrandSchtroumpf]

Hey @Amxx, is it the documentation generator you're referring to : https://github.com/OpenZeppelin/solidity-docgen ? I can take a look if you want. Or simply keep both See {IERC20-balanceOf}. and @inheritdoc IERC20

[Amxx]

I'd love to remove See {IERC20-balanceOf} in favor of @inheritdoc IERC20

But from my early tests, https://github.com/OpenZeppelin/solidity-docgen doesn't support it :/

[Amxx]

Check the build-info with the compile output : JSON.parse(output.contracts[sourceName][contractName].metadata).output.devdoc. The methods don't inherit the parent class.

I just confirmed that. How are we supposed to get the inherited doc?

[GrandSchtroumpf]

Concerning docgen

From what I could read in the docgen, the issue comes from here : https://github.com/OpenZeppelin/solidity-docgen/blob/d3ce9e26c470c5eb8ceb10ae9345bff930967c42/src/utils/natspec.ts#L35 docgen parse the AST to get, amongst other things, the StructuredDocumentation nodeType. But the inheritance comes from devdoc.

Concerning inherited doc

How are we supposed to get the inherited doc?

The natspec documentation mentioned that inheritance happens automatically for the methods, except in these cases

• When the parameter names are different.
• When there is more than one base function.
• When there is an explicit @inheritdoc tag which specifies which contract should be used to inherit.

For me the best practice would be to

• not add comment by default
• use @inheritdoc to add more specific documentation (see approve method in the my example above)

then you can get the inherited doc from devdoc

JSON.parse(output.contracts[sourceName][contractName].metadata).output.devdoc.methods['approve(address,uint256)']

[frangio]

inheritdoc is supported out of the box in solidity-docgen (here it is in 0.5 which is what we're using currently).

Note that the way it works is it "copies all missing tags from the base function" (as defned by Solidity). This means if we want the docs to be inherited we have to remove @dev See {...}.

There are some cases where we extend the parent docs with additional notes, and then add "See ...". These can't be replaced by inheritdoc. In the snippet you shared, we can use inheritdoc for balanceOf, but not for approve.

We can take a PR if you want to do this!

[GrandSchtroumpf]

@frangio thx for the feedback. I opened a draft PRs removing the @dev See {... ...} comment that were blocking the inheritance, leaving the rest as it. I checked, and normally there is no naming conflict that would require to specify the inherited contract documentation with @inheritdoc ContractName.

For the rest, maybe using @param in specific cases would help have better inheritance. This would require a case by case refactoring though...

  /**
   * @param spender Requirements: cannot be the zero address.
   * @param amount if it is this maximum `uint256`, the allowance is not updated on `transferFrom`. 
   * This is semantically equivalent to an infinite approval.
   * @inheritdoc IERC20
   */
    function approve(...) ... { ... }

[frangio]

Yes we want to use @param but it's a lot of work.