Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule request: full NatSpec documentation #298

Open
fulldecent opened this issue Jul 11, 2021 · 6 comments
Open

Rule request: full NatSpec documentation #298

fulldecent opened this issue Jul 11, 2021 · 6 comments

Comments

@fulldecent
Copy link

fulldecent commented Jul 11, 2021

Requesting a new rule and to enable it by default.

natspec-documentation

The following would trigger an error:

  • A public or external function which does not have a NatSpec comment
    • NatSpec comment does not have a @notice set
    • NatSpec comment does not have a @param for every parameter
    • NatSpec comment does not have a @return for every return
  • A public storage variable which does not have a NatSpec comment
    • NatSpec comment does not have a @notice set
    • NatSpec comment does not have a @param for every parameter
    • NatSpec comment does not have a @return for every return
  • An Error type which does not have a NatSpec comment
    • NatSpec commend does not have a @notice set

The Solidity project recommends the above. It is extremely useful. And few people do it. So it will be very helpful to add rules for it.

It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI).

https://docs.soliditylang.org/en/v0.8.6/style-guide.html?highlight=style%20guide#natspec


As a bonus I am also requesting a separate rule that is OFF by default and which is more zealous.

Perhaps name this natspec-documentation-internal.

  • An internal function which does not have a NatSpec comment
    • NatSpec comment does not have a @param for every parameter
    • NatSpec comment does not have a @return for every return

Note that @notice is not required in this circumstance because that tag applies to "end users" whereas an internal function is useful only to contract developers.

Note that private functions are not included in this rule. This is because documentation for implementation details is always less important that documentation for an objects' surface area. If you like, this could be another rule natspec-documentation-private and should be default OFF.


I am a contributor to the Solidity NatSpec specification, the style guide and the above-referenced Solidity project recommendation. Ask me anything.

@d1onys1us
Copy link

+1 for this

@coreyar
Copy link

coreyar commented Mar 17, 2023

This is an interesting feature. The hardest part about implementing this is that comments are not included in the Solidity AST
See ethereum/solidity#13865

@fulldecent
Copy link
Author

Not using the AST, but after solidity compiles it the nat spec comes through to the compiler output.

I documented this at https://docs.soliditylang.org/en/latest/natspec-format.html

@coreyar
Copy link

coreyar commented Mar 17, 2023

I mention the AST because this tool is for linting contracts using the AST.

The exact format of the comment shouldn't affect the compiled output. In my opinion, the goal for formatting should be to enforce consistent readable natspec within a codebase.

@dbale-altoros
Copy link
Collaborator

@fulldecent @d1onys1us @fedekunse @mvanmeermeck
as @coreyar mentiones
This is an interesting rule
But it is kind of out of reach of what solhint does
Solhint works as an AST analyzer and natspec comments are not being grabbed by the ast builder

This require a whole new way of working for solhint, so I doubt this can be implemented in the short/mid term... even in long seems kind of out of reach

Sorry for that...
Although I agree it could be a really nice addition

@0xGorilla
Copy link

You might want to take a look at https://github.com/defi-wonderland/natspec-smells

It aims to solve exactly what @fulldecent was looking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants