Merge pull request #7534 from gh1dra/return-multiple-natspec

Add support for multiple (named) return parameters in Natspec devdoc

Changelog.md

@@ -14,6 +14,7 @@ Breaking changes:
  * Syntax: Abstract contracts need to be marked explicitly as abstract by using the ``abstract`` keyword.
  * Inline Assembly: Only strict inline assembly is allowed.
  * Type checker: Resulting type of exponentiation is equal to the type of the base. Also allow signed types for the base.
+ * Natspec JSON Interface: Properly support multiple ``@return`` statements in ``@dev`` documentation and enforce named return parameters to be mentioned documentation.
  * Source mappings: Add "modifier depth" as a fifth field in the source mappings.
 
 
@@ -47,7 +48,6 @@ Bugfixes:
  * Type Checker: Treat magic variables as unknown identifiers in inline assembly
 
 
-
 ### 0.5.12 (2019-10-01)
 
 Language Features:

docs/060-breaking-changes.rst

@@ -66,10 +66,17 @@ This section gives detailed instructions on how to update prior code for every b
 * Change ``uint length = array.push(value)`` to ``array.push(value);``. The new length can be
   accessed via ``array.length``.
 
+* For every named return parameter in a function's ``@dev`` documentation define a ``@return``
+  entry which contains the parameter's name as the first word. E.g. if you have function ``f()`` defined
+  like ``function f() public returns (uint value)`` and a ``@dev`` annotating it, document its return
+  parameters like so: ``@return value The return value.``. You can mix named and un-named return parameters
+  documentation so long as the notices are in the order they appear in the tuple return type.
+
 New Features
 ============
 
  * The :ref:`try/catch statement <try-catch>` allows you to react on failed external calls.
+ * Natspec supports multiple return parameters in dev documentation, enforcing the same naming check as ``@param``.
  * Yul and Inline Assembly have a new statement called ``leave`` that exits the current function.
 
 

libsolidity/interface/Natspec.cpp

@@ -100,15 +100,54 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 		{
 			Json::Value method(devDocumentation(fun->annotation().docTags));
 			if (!method.empty())
+			{
 				// add the function, only if we have any documentation to add
+				Json::Value jsonReturn = extractReturnParameterDocs(fun->annotation().docTags, *fun);
+
+				if (!jsonReturn.empty())
+					method["returns"] = jsonReturn;
+
 				methods[it.second->externalSignature()] = method;
+			}
 		}
 	}
+
 	doc["methods"] = methods;
 
 	return doc;
 }
 
+Json::Value Natspec::extractReturnParameterDocs(std::multimap<std::string, DocTag> const& _tags, FunctionDefinition const& _functionDef)
+{
+	Json::Value jsonReturn{Json::objectValue};
+	auto returnDocs = _tags.equal_range("return");
+
+	if (!_functionDef.returnParameters().empty())
+	{
+		size_t n = 0;
+		for (auto i = returnDocs.first; i != returnDocs.second; i++)
+		{
+			string paramName = _functionDef.returnParameters().at(n)->name();
+			string content = i->second.content;
+
+			if (paramName.empty())
+				paramName = "_" + std::to_string(n);
+			else
+			{
+				//check to make sure the first word of the doc str is the same as the return name
+				auto nameEndPos = content.find_first_of(" \t");
+				solAssert(content.substr(0, nameEndPos) == paramName, "No return param name given: " + paramName);
+				content = content.substr(nameEndPos+1);
+			}
+
+			jsonReturn[paramName] = Json::Value(content);
+			n++;
+		}
+	}
+
+	return jsonReturn;
+}
+
 string Natspec::extractDoc(multimap<string, DocTag> const& _tags, string const& _name)
 {
 	string value;
@@ -129,12 +168,6 @@ Json::Value Natspec::devDocumentation(std::multimap<std::string, DocTag> const&
 	if (!author.empty())
 		json["author"] = author;
 
-	// for constructors, the "return" node will never exist. invalid tags
-	// will already generate an error within dev::solidity::DocStringAnalyzer.
-	auto ret = extractDoc(_tags, "return");
-	if (!ret.empty())
-		json["return"] = ret;
-
 	Json::Value params(Json::objectValue);
 	auto paramRange = _tags.equal_range("param");
 	for (auto i = paramRange.first; i != paramRange.second; ++i)

libsolidity/interface/Natspec.h

@@ -28,6 +28,7 @@
 #include <json/json.h>
 #include <memory>
 #include <string>
+#include <libsolidity/ast/AST.h>
 
 namespace dev
 {
@@ -60,6 +61,13 @@ class Natspec
 	/// @return      A JSON representation
 	///              of the contract's developer documentation
 	static Json::Value devDocumentation(std::multimap<std::string, DocTag> const& _tags);
+
+	/// Helper-function that will create a json object for the "returns" field for a given function definition.
+	/// @param _tags docTags that are used.
+	/// @param _functionDef functionDefinition that is used to determine which return parameters are named.
+	/// @return      A JSON representation
+	///              of a method's return notice documentation
+	static Json::Value extractReturnParameterDocs(std::multimap<std::string, DocTag> const& _tags, FunctionDefinition const& _functionDef);
 };
 
 } //solidity NS

test/compilationTests/MultiSigWallet/MultiSigWallet.sol

@@ -184,7 +184,7 @@ contract MultiSigWallet {
     /// @param destination Transaction target address.
     /// @param value Transaction ether value.
     /// @param data Transaction data payload.
-    /// @return Returns transaction ID.
+    /// @return transactionId Returns transaction ID.
     function submitTransaction(address destination, uint value, bytes memory data)
         public
         returns (uint transactionId)
@@ -258,7 +258,7 @@ contract MultiSigWallet {
     /// @param destination Transaction target address.
     /// @param value Transaction ether value.
     /// @param data Transaction data payload.
-    /// @return Returns transaction ID.
+    /// @return transactionId Returns transaction ID.
     function addTransaction(address destination, uint value, bytes memory data)
         internal
         notNull(destination)
@@ -280,7 +280,7 @@ contract MultiSigWallet {
      */
     /// @dev Returns number of confirmations of a transaction.
     /// @param transactionId Transaction ID.
-    /// @return Number of confirmations.
+    /// @return count Number of confirmations.
     function getConfirmationCount(uint transactionId)
         public
         view
@@ -294,7 +294,7 @@ contract MultiSigWallet {
     /// @dev Returns total number of transactions after filers are applied.
     /// @param pending Include pending transactions.
     /// @param executed Include executed transactions.
-    /// @return Total number of transactions after filters are applied.
+    /// @return count Total number of transactions after filters are applied.
     function getTransactionCount(bool pending, bool executed)
         public
         view
@@ -318,7 +318,7 @@ contract MultiSigWallet {
 
     /// @dev Returns array with owner addresses, which confirmed transaction.
     /// @param transactionId Transaction ID.
-    /// @return Returns array of owner addresses.
+    /// @return _confirmations Returns array of owner addresses.
     function getConfirmations(uint transactionId)
         public
         view
@@ -342,7 +342,7 @@ contract MultiSigWallet {
     /// @param to Index end position of transaction array.
     /// @param pending Include pending transactions.
     /// @param executed Include executed transactions.
-    /// @return Returns array of transaction IDs.
+    /// @return _transactionIds Returns array of transaction IDs.
     function getTransactionIds(uint from, uint to, bool pending, bool executed)
         public
         view

test/compilationTests/MultiSigWallet/MultiSigWalletFactory.sol

@@ -10,7 +10,7 @@ contract MultiSigWalletFactory is Factory {
     /// @dev Allows verified creation of multisignature wallet.
     /// @param _owners List of initial owners.
     /// @param _required Number of required confirmations.
-    /// @return Returns wallet address.
+    /// @return wallet Returns wallet address.
     function create(address[] memory _owners, uint _required)
         public
         returns (address wallet)

test/compilationTests/MultiSigWallet/MultiSigWalletWithDailyLimitFactory.sol

@@ -11,7 +11,7 @@ contract MultiSigWalletWithDailyLimitFactory is Factory {
     /// @param _owners List of initial owners.
     /// @param _required Number of required confirmations.
     /// @param _dailyLimit Amount in wei, which can be withdrawn without confirmations on a daily basis.
-    /// @return Returns wallet address.
+    /// @return wallet Returns wallet address.
     function create(address[] memory _owners, uint _required, uint _dailyLimit)
         public
         returns (address wallet)

test/compilationTests/corion/premium.sol

@@ -84,7 +84,7 @@ contract premium is module, safeMath {
      * @param spender The address of the account able to transfer the tokens
      * @param amount The amount of tokens to be approved for transfer
      * @param nonce The transaction count of the authorised address
-     * @return True if the approval was successful
+     * @return success True if the approval was successful
      */
     function approve(address spender, uint256 amount, uint256 nonce) isReady external returns (bool success) {
         /*
@@ -106,7 +106,7 @@ contract premium is module, safeMath {
      * @param amount The amount of tokens to be approved for transfer
      * @param nonce The transaction count of the authorised address
      * @param extraData Data to give forward to the receiver
-     * @return True if the approval was successful
+     * @return success True if the approval was successful
      */
     function approveAndCall(address spender, uint256 amount, uint256 nonce, bytes calldata extraData) isReady external returns (bool success) {
         /*
@@ -159,7 +159,7 @@ contract premium is module, safeMath {
      * @notice Send `amount` Corion tokens to `to` from `msg.sender`
      * @param to The address of the recipient
      * @param amount The amount of tokens to be transferred
-     * @return Whether the transfer was successful or not
+     * @return success Whether the transfer was successful or not
      */
     function transfer(address to, uint256 amount) isReady external returns (bool success) {
         /*
@@ -187,7 +187,7 @@ contract premium is module, safeMath {
      * @param from The address holding the tokens being transferred
      * @param to The address of the recipient
      * @param amount The amount of tokens to be transferred
-     * @return True if the transfer was successful
+     * @return success True if the transfer was successful
      */
     function transferFrom(address from, address to, uint256 amount) isReady external returns (bool success) {
         /*
@@ -224,7 +224,7 @@ contract premium is module, safeMath {
      * @param to The contract address of the recipient
      * @param amount The amount of tokens to be transferred
      * @param extraData Data to give forward to the receiver
-     * @return Whether the transfer was successful or not
+     * @return success Whether the transfer was successful or not
      */
     function transfer(address to, uint256 amount, bytes calldata extraData) isReady external returns (bool success) {
         /*

test/compilationTests/corion/token.sol

@@ -99,7 +99,7 @@ contract token is safeMath, module, announcementTypes {
      * @param spender The address of the account able to transfer the tokens
      * @param amount The amount of tokens to be approved for transfer
      * @param nonce The transaction count of the authorised address
-     * @return True if the approval was successful
+     * @return success True if the approval was successful
      */
     function approve(address spender, uint256 amount, uint256 nonce) isReady external returns (bool success) {
         /*
@@ -121,7 +121,7 @@ contract token is safeMath, module, announcementTypes {
      * @param amount The amount of tokens to be approved for transfer
      * @param nonce The transaction count of the authorised address
      * @param extraData Data to give forward to the receiver
-     * @return True if the approval was successful
+     * @return success True if the approval was successful
      */
     function approveAndCall(address spender, uint256 amount, uint256 nonce, bytes calldata extraData) isReady external returns (bool success) {
         /*
@@ -174,7 +174,7 @@ contract token is safeMath, module, announcementTypes {
      * @notice Send `amount` Corion tokens to `to` from `msg.sender`
      * @param to The address of the recipient
      * @param amount The amount of tokens to be transferred
-     * @return Whether the transfer was successful or not
+     * @return success Whether the transfer was successful or not
      */
     function transfer(address to, uint256 amount) isReady external returns (bool success) {
         /*
@@ -202,7 +202,7 @@ contract token is safeMath, module, announcementTypes {
      * @param from The address holding the tokens being transferred
      * @param to The address of the recipient
      * @param amount The amount of tokens to be transferred
-     * @return True if the transfer was successful
+     * @return success True if the transfer was successful
      */
     function transferFrom(address from, address to, uint256 amount) isReady external returns (bool success) {
         /*
@@ -239,7 +239,7 @@ contract token is safeMath, module, announcementTypes {
      * @param from The address holding the tokens being transferred
      * @param to The address of the recipient
      * @param amount The amount of tokens to be transferred
-     * @return True if the transfer was successful
+     * @return success True if the transfer was successful
      */
     function transferFromByModule(address from, address to, uint256 amount, bool fee) isReady external returns (bool success) {
         /*
@@ -265,7 +265,7 @@ contract token is safeMath, module, announcementTypes {
      * @param to The contract address of the recipient
      * @param amount The amount of tokens to be transferred
      * @param extraData Data to give forward to the receiver
-     * @return Whether the transfer was successful or not
+     * @return success Whether the transfer was successful or not
      */
     function transfer(address to, uint256 amount, bytes calldata extraData) isReady external returns (bool success) {
         /*
@@ -341,7 +341,7 @@ contract token is safeMath, module, announcementTypes {
      * @notice Transaction fee will be deduced from `owner` for transacting `value`
      * @param owner The address where will the transaction fee deduced
      * @param value The base for calculating the fee
-     * @return True if the transfer was successful
+     * @return success True if the transfer was successful
      */
     function processTransactionFee(address owner, uint256 value) isReady external returns (bool success) {
         /*

test/compilationTests/gnosis/Events/CategoricalEvent.sol

@@ -25,7 +25,7 @@ contract CategoricalEvent is Event {
     }
 
     /// @dev Exchanges sender's winning outcome tokens for collateral tokens
-    /// @return Sender's winnings
+    /// @return winnings Sender's winnings
     function redeemWinnings()
         public
         override

test/compilationTests/gnosis/Events/Event.sol

@@ -107,7 +107,7 @@ abstract contract Event {
     }
 
     /// @dev Returns the amount of outcome tokens held by owner
-    /// @return Outcome token distribution
+    /// @return outcomeTokenDistribution Outcome token distribution
     function getOutcomeTokenDistribution(address owner)
         public
         view

test/compilationTests/gnosis/Events/EventFactory.sol

@@ -26,7 +26,7 @@ contract EventFactory {
     /// @param collateralToken Tokens used as collateral in exchange for outcome tokens
     /// @param oracle Oracle contract used to resolve the event
     /// @param outcomeCount Number of event outcomes
-    /// @return Event contract
+    /// @return eventContract Event contract
     function createCategoricalEvent(
         Token collateralToken,
         Oracle oracle,
@@ -53,7 +53,7 @@ contract EventFactory {
     /// @param oracle Oracle contract used to resolve the event
     /// @param lowerBound Lower bound for event outcome
     /// @param upperBound Lower bound for event outcome
-    /// @return Event contract
+    /// @return eventContract Event contract
     function createScalarEvent(
         Token collateralToken,
         Oracle oracle,

test/compilationTests/gnosis/Events/ScalarEvent.sol

@@ -44,7 +44,7 @@ contract ScalarEvent is Event {
     }
 
     /// @dev Exchanges sender's winning outcome tokens for collateral tokens
-    /// @return Sender's winnings
+    /// @return winnings Sender's winnings
     function redeemWinnings()
         public
         override