Allow NatSpec comments for state variables #8532

aarlt · 2020-03-24T22:47:22Z

Fixes #3418.
replaces #8399

chriseth · 2020-03-25T16:53:37Z

Please squash.

libsolidity/ast/ASTJsonConverter.cpp

libsolidity/interface/Natspec.cpp

libsolidity/parsing/Parser.cpp

test/libsolidity/SolidityNatspecJSON.cpp

libsolidity/analysis/DocStringAnalyser.cpp

aarlt · 2020-03-27T16:20:40Z

@chriseth i'm not sure whether it is good to forbid notice on non-public state variables. it is used a lot by contract developers. Some examples:

colonyNetwork @ contracts/ReputationMiningCycle.sol

contract ReputationMiningCycle is ReputationMiningCycleStorage, PatriciaTreeProofs, DSMath {
  /// @notice Minimum reputation mining stake in CLNY
  uint256 constant MIN_STAKE = 2000 * WAD;

  /// @notice Size of mining window in seconds
  uint256 constant MINING_WINDOW_SIZE = 60 * 60 * 24; // 24 hours
  ...
}

OpenZeppelin @ contracts/drafts/ERC20Migrator.sol:

contract ERC20Migrator {
    using SafeERC20 for IERC20;

    /// Address of the old token contract
    IERC20 private _legacyToken;

    /// Address of the new token contract
    ERC20Mintable private _newToken;
    ...
}

Solidity Documentation

    contract Fund {
        /// Mapping of ether shares of the contract.
        mapping(address => uint) shares;
        /// Withdraw your share.
        function withdraw() public {
            (bool success,) = msg.sender.call{value: shares[msg.sender]}("");
            if (success)
                shares[msg.sender] = 0;
        }
    }

chriseth · 2020-04-01T09:24:20Z

@aarlt @notice is useless on non-public functions. We can add a warning now and disallow it in 0.7.0.

chriseth · 2020-05-14T13:34:43Z

Needs rebase.

leonardoalt · 2020-05-18T09:39:23Z

Needs rebase again. solc-js tests might be fixed then

chriseth · 2020-05-19T15:02:26Z

Needs rebase.

chriseth · 2020-05-19T15:03:08Z

docs/natspec-format.rst

@@ -82,10 +85,10 @@ Tag
 =========== =============================================================================== =============================
 ``@title``  A title that should describe the contract/interface                             contract, interface
 ``@author`` The name of the author                                                          contract, interface, function
-``@notice`` Explain to an end user what this does                                           contract, interface, function
-``@dev``    Explain to a developer any extra details                                        contract, interface, function
+``@notice`` Explain to an end user what this does                                           contract, interface, function, state variable


Suggested change

``@notice`` Explain to an end user what this does contract, interface, function, state variable

``@notice`` Explain to an end user what this does contract, interface, function, public state variable

chriseth · 2020-05-19T15:03:21Z

docs/natspec-format.rst

 ``@param``  Documents a parameter just like in doxygen (must be followed by parameter name) function
-``@return`` Documents the return variables of a contract's function                         function
+``@return`` Documents the return variables of a contract's function                         function, state variable


Suggested change

``@return`` Documents the return variables of a contract's function function, state variable

``@return`` Documents the return variables of a contract's function function, public state variable

Thats true, @return should only be allowed on public state-variables.

I added now a check: @return is now only allowed on public state-variables. I added also a test for this.

chriseth · 2020-05-19T15:05:07Z

libsolidity/analysis/DocStringAnalyser.cpp

+					if (returnTagsVisited > 1)
+						appendError(
+							_node.documentation()->location(),
+							"Documentation tag \"@" + docTag.first + "\" is only allowed once on state-variables.");


Suggested change

"Documentation tag \"@" + docTag.first + "\" is only allowed once on state-variables.");

"Documentation tag \"@" + docTag.first + "\" is only allowed once on state-variables."

);

chriseth · 2020-05-19T15:05:24Z

libsolidity/analysis/DocStringAnalyser.cpp

+		{
+			parseDocStrings(_variable, _variable.annotation(), validPublicTags, "non-public state variables");
+			if (_variable.annotation().docTags.count("notice") > 0)
+				m_errorReporter.warning(9098_error, _variable.documentation()->location(), "Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly.");


Please wrap this line.

chriseth · 2020-05-19T15:05:31Z

libsolidity/analysis/DocStringAnalyser.cpp

+				m_errorReporter.warning(9098_error, _variable.documentation()->location(), "Documentation tag on non-public state variables will be disallowed in 0.7.0. You will need to use the @dev tag explicitly.");
+		}
+		if (_variable.annotation().docTags.count("title") > 0 || _variable.annotation().docTags.count("author") > 0)
+			m_errorReporter.warning(4822_error, _variable.documentation()->location(), "Documentation tag @title and @author is only allowed on contract definitions. It will be disallowed in 0.7.0.");


Please wrap this line.

- adds natspec generation for state variables. - exports structured docs for state variables to JSON.

aarlt force-pushed the structured-docs-variables-aarlt branch 4 times, most recently from 9e6d5c4 to 84f27c4 Compare March 25, 2020 00:04

chriseth mentioned this pull request Mar 25, 2020

Allow NatSpec comments for state variables #8399

Closed

6 tasks