openzeppelin-contracts icon indicating copy to clipboard operation
openzeppelin-contracts copied to clipboard

Published 20 hours ago • verified publisher icon OpenZeppelin

Support @deprecated tags in Natspec

Open 0xmichalis opened this issue 2 years ago • 3 comments

It would be great to have a @deprecated Natspec tag that we can use for storage fields that are not used anymore and furhtermore leverage a compiler plugin to fail compilation in case these deprecated fields are actually used somewhere in a contract.

0xmichalis avatar Aug 03 '22 12:08 0xmichalis

Thanks @kargakis. This is a good suggestion. There are a couple of caveats though.

@deprecated is not valid NatSpec so we would have to use @custom:deprecated.

Solidity 0.8.0 does not allow NatSpec tags on non-public variables, so this would be a breaking change (most of our contracts currently compile with 0.8.0). We can adopt this for our 5.0 release.

frangio avatar Aug 23 '22 01:08 frangio

@frangio maybe there are two parts into this: one is Natspec and the other one is inline comments like /// @custom:oz-upgrades-unsafe-allow constructor which is not a breaking change. It may also be that the Natspec suggestion is redundant given the inline comments and the compiler being able to fail on them accordingly.

0xmichalis avatar Aug 23 '22 19:08 0xmichalis

I'm not sure I see the distinction. /// @custom:oz-upgrades-... is NatSpec!

It is a breaking change because if we add a comment like that to a private variable Solidity 0.8.0 (and a few later versions as well) will refuse to compile the contract. :slightly_frowning_face:

frangio avatar Aug 23 '22 21:08 frangio