Implement @erak review notes by creating function and adding constructor test

Update 060 doc with natspec change

Add two more tests with mixed usage

Fix solc-js

fix changelog

Changelog.md

@@ -14,7 +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: Support multiple ``@return`` statements in dev documentation to behave like named parameters.
+ * Natspec JSON Interface: Support multiple ``@return`` statements in dev documentation to behave like parameters where the first word in the entry must contain the name.
  * Source mappings: Add "modifier depth" as a fifth field in the source mappings.
 
 
@@ -48,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

@@ -102,38 +102,10 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 			if (!method.empty())
 			{
 				// add the function, only if we have any documentation to add
-				Json::Value ret(Json::objectValue);
+				Json::Value jsonReturn = extractReturnParameterDocs(fun->annotation().docTags, *fun);
 
-				auto returnParams = fun->returnParameters();
+				if (!jsonReturn.empty())
-				auto returnDoc = fun->annotation().docTags.equal_range("return");
+					method["returns"] = jsonReturn;
-
-				if (!returnParams.empty())
-				{
-					unsigned int n = 0;
-					for (auto i = returnDoc.first; i != returnDoc.second; i++)
-					{
-						string paramName = returnParams.at(n)->name();
-						string content = i->second.content;
-
-						if (paramName.empty())
-						{
-							paramName = "_" + std::to_string(n+1);
-						}
-						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);
-						}
-
-						ret[paramName] = Json::Value(content);
-						n++;
-					}
-				}
-
-				if (!ret.empty())
-					method["return"] = ret;
 
 				methods[it.second->externalSignature()] = method;
 			}
@@ -145,6 +117,36 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
 	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)
 {

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,7 @@ private:
 	/// @return      A JSON representation
 	///              of the contract's developer documentation
 	static Json::Value devDocumentation(std::multimap<std::string, DocTag> const& _tags);
+	static Json::Value extractReturnParameterDocs(std::multimap<std::string, DocTag> const& _tags, FunctionDefinition const& _functionDef);
 };
 
 } //solidity NS

test/externalTests/solc-js/DAO/Token.sol

@@ -44,13 +44,13 @@ abstract contract TokenInterface {
     uint256 public totalSupply;
 
     /// @param _owner The address from which the balance will be retrieved
-    /// @return The balance
+    /// @return balance The balance
     function balanceOf(address _owner) public view returns (uint256 balance);
 
     /// @notice Send `_amount` 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) public returns (bool success);
 
     /// @notice Send `_amount` tokens to `_to` from `_from` on the condition it
@@ -58,19 +58,19 @@ abstract contract TokenInterface {
     /// @param _from The address of the origin of the transfer
     /// @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 transferFrom(address _from, address _to, uint256 _amount) public returns (bool success);
 
     /// @notice `msg.sender` approves `_spender` to spend `_amount` tokens on
     /// its behalf
     /// @param _spender The address of the account able to transfer the tokens
     /// @param _amount The amount of tokens to be approved for transfer
-    /// @return Whether the approval was successful or not
+    /// @return success Whether the approval was successful or not
     function approve(address _spender, uint256 _amount) public returns (bool success);
 
     /// @param _owner The address of the account owning tokens
     /// @param _spender The address of the account able to transfer the tokens
-    /// @return Amount of remaining tokens of _owner that _spender is allowed
+    /// @return remaining Amount of remaining tokens of _owner that _spender is allowed
     /// to spend
     function allowance(
         address _owner,

test/libsolidity/SolidityNatspecJSON.cpp

@@ -378,7 +378,7 @@ BOOST_AUTO_TEST_CASE(dev_return)
 	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
 	"            \"second\": \"Documentation for the second parameter\"\n"
 	"        },\n"
-	"        \"return\": {\n"
+	"        \"returns\": {\n"
 	"            \"d\": \"The result of the multiplication\"\n"
 	"        }\n"
 	"    }\n"
@@ -411,7 +411,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl)
 	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
 	"            \"second\": \"Documentation for the second parameter\"\n"
 	"        },\n"
-	"        \"return\": {\n"
+	"        \"returns\": {\n"
 	"            \"d\": \"The result of the multiplication\"\n"
 	"        }\n"
 	"    }\n"
@@ -420,6 +420,79 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl)
 	checkNatspec(sourceCode, "test", natspec, false);
 }
 
+BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed_mixed)
+{
+	char const* sourceCode = R"(
+		contract test {
+			/// @dev Multiplies a number by 7 and adds second parameter
+			/// @param a Documentation for the first parameter starts here.
+			/// Since it's a really complicated parameter we need 2 lines
+			/// @param second Documentation for the second parameter
+			/// @return The result of the multiplication
+			/// @return _cookies And cookies with nutella
+			function mul(uint a, uint second) public returns (uint, uint _cookies) {
+				uint mul = a * 7;
+				return (mul, second);
+			}
+		}
+	)";
+
+	char const* natspec = "{"
+	"\"methods\":{"
+	"    \"mul(uint256,uint256)\":{ \n"
+	"        \"details\": \"Multiplies a number by 7 and adds second parameter\",\n"
+	"        \"params\": {\n"
+	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
+	"            \"second\": \"Documentation for the second parameter\"\n"
+	"        },\n"
+	"        \"returns\": {\n"
+	"            \"_0\": \"The result of the multiplication\",\n"
+	"            \"_cookies\": \"And cookies with nutella\"\n"
+	"        }\n"
+	"    }\n"
+	"}}";
+
+	checkNatspec(sourceCode, "test", natspec, false);
+}
+
+BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed_mixed_2)
+{
+	char const* sourceCode = R"(
+		contract test {
+			/// @dev Multiplies a number by 7 and adds second parameter
+			/// @param a Documentation for the first parameter starts here.
+			/// Since it's a really complicated parameter we need 2 lines
+			/// @param second Documentation for the second parameter
+			/// @return _cookies And cookies with nutella
+			/// @return The result of the multiplication
+			/// @return _milk And milk with nutella
+			function mul(uint a, uint second) public returns (uint _cookies, uint, uint _milk) {
+				uint mul = a * 7;
+        uint milk = 4;
+				return (mul, second, milk);
+			}
+		}
+	)";
+
+	char const* natspec = "{"
+	"\"methods\":{"
+	"    \"mul(uint256,uint256)\":{ \n"
+	"        \"details\": \"Multiplies a number by 7 and adds second parameter\",\n"
+	"        \"params\": {\n"
+	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
+	"            \"second\": \"Documentation for the second parameter\"\n"
+	"        },\n"
+	"        \"returns\": {\n"
+	"            \"_cookies\": \"And cookies with nutella\",\n"
+	"            \"_1\": \"The result of the multiplication\",\n"
+	"            \"_milk\": \"And milk with nutella\"\n"
+	"        }\n"
+	"    }\n"
+	"}}";
+
+	checkNatspec(sourceCode, "test", natspec, false);
+}
+
 BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed)
 {
 	char const* sourceCode = R"(
@@ -445,9 +518,9 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed)
 	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
 	"            \"second\": \"Documentation for the second parameter\"\n"
 	"        },\n"
-	"        \"return\": {\n"
+	"        \"returns\": {\n"
-	"            \"_1\": \"The result of the multiplication\",\n"
+	"            \"_0\": \"The result of the multiplication\",\n"
-	"            \"_2\": \"And cookies with nutella\"\n"
+	"            \"_1\": \"And cookies with nutella\"\n"
 	"        }\n"
 	"    }\n"
 	"}}";
@@ -466,7 +539,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_multiple)
 			/// @return d The result of the multiplication
 			/// @return f And cookies with nutella
 			function mul(uint a, uint second) public returns (uint d, uint f) {
-        uint mul = a * 7;
+				uint mul = a * 7;
 				return (mul, second);
 			}
 		}
@@ -480,7 +553,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_multiple)
 	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
 	"            \"second\": \"Documentation for the second parameter\"\n"
 	"        },\n"
-	"        \"return\": {\n"
+	"        \"returns\": {\n"
 	"            \"d\": \"The result of the multiplication\",\n"
 	"            \"f\": \"And cookies with nutella\"\n"
 	"        }\n"
@@ -514,7 +587,7 @@ BOOST_AUTO_TEST_CASE(dev_multiline_return)
 	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
 	"            \"second\": \"Documentation for the second parameter\"\n"
 	"        },\n"
-	"        \"return\": {\n"
+	"        \"returns\": {\n"
 	"            \"d\": \"The result of the multiplication and cookies with nutella\",\n"
 	"        }\n"
 	"    }\n"
@@ -549,7 +622,7 @@ BOOST_AUTO_TEST_CASE(dev_multiline_comment)
 	"            \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n"
 	"            \"second\": \"Documentation for the second parameter\"\n"
 	"        },\n"
-	"        \"return\": {\n"
+	"        \"returns\": {\n"
 	"            \"d\": \"The result of the multiplication and cookies with nutella\",\n"
 	"        }\n"
 	"    }\n"
@@ -558,6 +631,21 @@ BOOST_AUTO_TEST_CASE(dev_multiline_comment)
 	checkNatspec(sourceCode, "test", natspec, false);
 }
 
+BOOST_AUTO_TEST_CASE(dev_documenting_no_return_paramname)
+{
+	char const* sourceCode = R"(
+		contract test {
+			/// @dev Multiplies a number by 7 and adds second parameter
+			/// @param a Documentation for the first parameter
+			/// @param second Documentation for the second parameter
+			/// @return
+			function mul(uint a, uint second) public returns (uint d) { return a * 7 + second; }
+		}
+	)";
+
+	expectNatspecError(sourceCode);
+}
+
 BOOST_AUTO_TEST_CASE(dev_contract_no_doc)
 {
 	char const* sourceCode = R"(
@@ -871,7 +959,7 @@ BOOST_AUTO_TEST_CASE(dev_constructor_and_function)
 				"a" : "Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines",
 				"second" : "Documentation for the second parameter"
 			},
-			"return" : {
+			"returns" : {
 				"d": "The result of the multiplication and cookies with nutella"
 			}
 		},