From b3ae601e88844a79fdaf340cc89702eaca2f3a90 Mon Sep 17 00:00:00 2001 From: cd10012 Date: Sat, 12 Oct 2019 21:11:56 -0400 Subject: [PATCH 1/6] Refactor to have multiple return params --- libsolidity/interface/Natspec.cpp | 48 ++++++++++++++++++++---- test/libsolidity/SolidityNatspecJSON.cpp | 35 +++++++++++++++++ 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index e28d8703e..e6cdad0af 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/libsolidity/interface/Natspec.cpp @@ -99,16 +99,54 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) if (auto fun = dynamic_cast(&it.second->declaration())) { Json::Value method(devDocumentation(fun->annotation().docTags)); + + // add the function, only if we have any documentation to add if (!method.empty()) - // add the function, only if we have any documentation to add + { + Json::Value ret(Json::objectValue); + + // for constructors, the "return" node will never exist. invalid tags + // will already generate an error within dev::solidity::DocStringAnalyzer. + auto returnParams = fun->returnParameters(); + auto returnDoc = fun->annotation().docTags.equal_range("return"); + + if (!returnParams.empty()) + { + // if there is no name then append subsequent return notices like previous behavior + string value; + unsigned int n = 0; + for (auto i = returnDoc.first; i != returnDoc.second; i++) + { + string paramName = returnParams.at(n)->name(); + + if (!paramName.empty()) + { + ret[paramName] = Json::Value(i->second.content); + n++; + } + + else + { + value += i->second.content; + method["return"] = Json::Value(value); + } + } + } + + if (!ret.empty()) + method["return"] = ret; + methods[it.second->externalSignature()] = method; - } + } + } } + doc["methods"] = methods; return doc; } + string Natspec::extractDoc(multimap const& _tags, string const& _name) { string value; @@ -129,12 +167,6 @@ Json::Value Natspec::devDocumentation(std::multimap 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) diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index faaf20da5..61eab1bd9 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -416,6 +416,41 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl) } +BOOST_AUTO_TEST_CASE(dev_return_desc_multiple) +{ + 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 And cookies with nutella + function mul(uint a, uint second) public returns (uint d, uint f) { + 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" + " \"return\": {\n" + " \"d\": \"The result of the multiplication\",\n" + " \"f\": \"And cookies with nutella\"\n" + " }\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, "test", natspec, false); +} + BOOST_AUTO_TEST_CASE(dev_multiline_return) { char const* sourceCode = R"( From 18fe693fdd2246339262c3a58e93bb7ee5d79ed1 Mon Sep 17 00:00:00 2001 From: cd10012 Date: Sun, 13 Oct 2019 05:52:13 -0400 Subject: [PATCH 2/6] Add unamed return param test and check size in conditional --- libsolidity/interface/Natspec.cpp | 2 +- test/libsolidity/SolidityNatspecJSON.cpp | 31 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index e6cdad0af..8966bca35 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/libsolidity/interface/Natspec.cpp @@ -119,7 +119,7 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) { string paramName = returnParams.at(n)->name(); - if (!paramName.empty()) + if (!paramName.empty() && returnParams.size() != 1) { ret[paramName] = Json::Value(i->second.content); n++; diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index 61eab1bd9..66690ca8d 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -415,6 +415,37 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl) checkNatspec(sourceCode, "test", natspec, false); } +BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed) +{ + 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 And cookies with nutella + function mul(uint a, uint second) public returns (uint, uint) { + 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" + " \"return\": \"The result of the multiplicationAnd cookies with nutella\"\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, "test", natspec, false); +} BOOST_AUTO_TEST_CASE(dev_return_desc_multiple) { From f598b1515fb5cbd084a0c51bd4c03df51c811059 Mon Sep 17 00:00:00 2001 From: cd10012 Date: Mon, 21 Oct 2019 18:05:34 -0400 Subject: [PATCH 3/6] Give unamed parameters unique keys and update tests to new spec Fix whitespace --- Changelog.md | 1 + libsolidity/interface/Natspec.cpp | 25 +++++------ .../MultiSigWallet/MultiSigWallet.sol | 12 ++--- .../MultiSigWallet/MultiSigWalletFactory.sol | 2 +- .../MultiSigWalletWithDailyLimitFactory.sol | 2 +- test/compilationTests/corion/premium.sol | 10 ++--- test/compilationTests/corion/token.sol | 14 +++--- .../gnosis/Events/CategoricalEvent.sol | 2 +- test/compilationTests/gnosis/Events/Event.sol | 2 +- .../gnosis/Events/EventFactory.sol | 4 +- .../gnosis/Events/ScalarEvent.sol | 2 +- .../gnosis/MarketMakers/LMSRMarketMaker.sol | 14 +++--- .../gnosis/Markets/Campaign.sol | 4 +- .../gnosis/Markets/CampaignFactory.sol | 2 +- .../gnosis/Markets/StandardMarket.sol | 8 ++-- .../gnosis/Markets/StandardMarketFactory.sol | 2 +- .../Oracles/CentralizedOracleFactory.sol | 2 +- .../Oracles/DifficultyOracleFactory.sol | 2 +- .../gnosis/Oracles/FutarchyOracleFactory.sol | 2 +- .../gnosis/Oracles/MajorityOracle.sol | 4 +- .../gnosis/Oracles/MajorityOracleFactory.sol | 2 +- .../Oracles/SignedMessageOracleFactory.sol | 2 +- .../gnosis/Oracles/UltimateOracle.sol | 2 +- .../gnosis/Oracles/UltimateOracleFactory.sol | 2 +- test/compilationTests/gnosis/Utils/Math.sol | 4 +- test/libsolidity/SolidityNatspecJSON.cpp | 44 ++++++++++++------- 26 files changed, 94 insertions(+), 78 deletions(-) diff --git a/Changelog.md b/Changelog.md index 40ff714ab..746887d3c 100644 --- a/Changelog.md +++ b/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: Support multiple ``@return`` statements in dev documentation to behave like named parameters. * Source mappings: Add "modifier depth" as a fifth field in the source mappings. diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index 8966bca35..0da229a7f 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/libsolidity/interface/Natspec.cpp @@ -99,37 +99,36 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) if (auto fun = dynamic_cast(&it.second->declaration())) { Json::Value method(devDocumentation(fun->annotation().docTags)); - - // add the function, only if we have any documentation to add if (!method.empty()) { + // add the function, only if we have any documentation to add Json::Value ret(Json::objectValue); - // for constructors, the "return" node will never exist. invalid tags - // will already generate an error within dev::solidity::DocStringAnalyzer. auto returnParams = fun->returnParameters(); auto returnDoc = fun->annotation().docTags.equal_range("return"); if (!returnParams.empty()) { - // if there is no name then append subsequent return notices like previous behavior - string value; 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() && returnParams.size() != 1) + if (paramName.empty()) { - ret[paramName] = Json::Value(i->second.content); - n++; + paramName = "_" + std::to_string(n+1); } - else { - value += i->second.content; - method["return"] = Json::Value(value); + //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++; } } @@ -138,7 +137,7 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) methods[it.second->externalSignature()] = method; } - } + } } doc["methods"] = methods; diff --git a/test/compilationTests/MultiSigWallet/MultiSigWallet.sol b/test/compilationTests/MultiSigWallet/MultiSigWallet.sol index 1ee2e5756..046895a06 100644 --- a/test/compilationTests/MultiSigWallet/MultiSigWallet.sol +++ b/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 diff --git a/test/compilationTests/MultiSigWallet/MultiSigWalletFactory.sol b/test/compilationTests/MultiSigWallet/MultiSigWalletFactory.sol index 00c165ea1..4b3c23728 100644 --- a/test/compilationTests/MultiSigWallet/MultiSigWalletFactory.sol +++ b/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) diff --git a/test/compilationTests/MultiSigWallet/MultiSigWalletWithDailyLimitFactory.sol b/test/compilationTests/MultiSigWallet/MultiSigWalletWithDailyLimitFactory.sol index 4a038864b..362399092 100644 --- a/test/compilationTests/MultiSigWallet/MultiSigWalletWithDailyLimitFactory.sol +++ b/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) diff --git a/test/compilationTests/corion/premium.sol b/test/compilationTests/corion/premium.sol index 66f3282a2..d3b842608 100644 --- a/test/compilationTests/corion/premium.sol +++ b/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) { /* diff --git a/test/compilationTests/corion/token.sol b/test/compilationTests/corion/token.sol index b966d6833..0d67feaa9 100644 --- a/test/compilationTests/corion/token.sol +++ b/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) { /* diff --git a/test/compilationTests/gnosis/Events/CategoricalEvent.sol b/test/compilationTests/gnosis/Events/CategoricalEvent.sol index 4f564166f..6b04b73d9 100644 --- a/test/compilationTests/gnosis/Events/CategoricalEvent.sol +++ b/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 diff --git a/test/compilationTests/gnosis/Events/Event.sol b/test/compilationTests/gnosis/Events/Event.sol index b6d8bc779..6df85039e 100644 --- a/test/compilationTests/gnosis/Events/Event.sol +++ b/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 diff --git a/test/compilationTests/gnosis/Events/EventFactory.sol b/test/compilationTests/gnosis/Events/EventFactory.sol index 4f4efe35e..9c3f9e8cd 100644 --- a/test/compilationTests/gnosis/Events/EventFactory.sol +++ b/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, diff --git a/test/compilationTests/gnosis/Events/ScalarEvent.sol b/test/compilationTests/gnosis/Events/ScalarEvent.sol index ad9bc3ad6..6df1dc072 100644 --- a/test/compilationTests/gnosis/Events/ScalarEvent.sol +++ b/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 diff --git a/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol b/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol index deea3fc44..2642a6e8e 100644 --- a/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol +++ b/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol @@ -21,7 +21,7 @@ contract LMSRMarketMaker is MarketMaker { /// @param market Market contract /// @param outcomeTokenIndex Index of outcome to buy /// @param outcomeTokenCount Number of outcome tokens to buy - /// @return Cost + /// @return cost Cost function calcCost(Market market, uint8 outcomeTokenIndex, uint outcomeTokenCount) public override @@ -57,7 +57,7 @@ contract LMSRMarketMaker is MarketMaker { /// @param market Market contract /// @param outcomeTokenIndex Index of outcome to sell /// @param outcomeTokenCount Number of outcome tokens to sell - /// @return Profit + /// @return profit Profit function calcProfit(Market market, uint8 outcomeTokenIndex, uint outcomeTokenCount) public override @@ -84,7 +84,7 @@ contract LMSRMarketMaker is MarketMaker { /// @dev Returns marginal price of an outcome /// @param market Market contract /// @param outcomeTokenIndex Index of outcome to determine marginal price of - /// @return Marginal price of an outcome as a fixed point number + /// @return price Marginal price of an outcome as a fixed point number function calcMarginalPrice(Market market, uint8 outcomeTokenIndex) public override @@ -110,7 +110,7 @@ contract LMSRMarketMaker is MarketMaker { /// @param logN Logarithm of the number of outcomes /// @param netOutcomeTokensSold Net outcome tokens sold by market /// @param funding Initial funding for market - /// @return Cost level + /// @return costLevel Cost level function calcCostLevel(int logN, int[] memory netOutcomeTokensSold, uint funding) private view @@ -131,7 +131,9 @@ contract LMSRMarketMaker is MarketMaker { /// @param netOutcomeTokensSold Net outcome tokens sold by market /// @param funding Initial funding for market /// @param outcomeIndex Index of exponential term to extract (for use by marginal price function) - /// @return A result structure composed of the sum, the offset used, and the summand associated with the supplied index + /// @return sum of the outcomes + /// @return offset that is used for all + /// @return outcomeExpTerm the summand associated with the supplied index function sumExpOffset(int logN, int[] memory netOutcomeTokensSold, uint funding, uint8 outcomeIndex) private view @@ -170,7 +172,7 @@ contract LMSRMarketMaker is MarketMaker { /// number of collateral tokens (which is the same as the number of outcome tokens the /// market created) subtracted by the quantity of that token held by the market. /// @param market Market contract - /// @return Net outcome tokens sold by market + /// @return quantities Net outcome tokens sold by market function getNetOutcomeTokensSold(Market market) private view diff --git a/test/compilationTests/gnosis/Markets/Campaign.sol b/test/compilationTests/gnosis/Markets/Campaign.sol index b16d102c6..a678e7a43 100644 --- a/test/compilationTests/gnosis/Markets/Campaign.sol +++ b/test/compilationTests/gnosis/Markets/Campaign.sol @@ -115,7 +115,7 @@ contract Campaign { } /// @dev Withdraws refund amount - /// @return Refund amount + /// @return refundAmount Refund amount function refund() public timedTransitions @@ -162,7 +162,7 @@ contract Campaign { } /// @dev Allows to withdraw fees from campaign contract to contributor - /// @return Fee amount + /// @return fees Fee amount function withdrawFees() public atStage(Stages.MarketClosed) diff --git a/test/compilationTests/gnosis/Markets/CampaignFactory.sol b/test/compilationTests/gnosis/Markets/CampaignFactory.sol index c7afeded3..56cf94ddb 100644 --- a/test/compilationTests/gnosis/Markets/CampaignFactory.sol +++ b/test/compilationTests/gnosis/Markets/CampaignFactory.sol @@ -21,7 +21,7 @@ contract CampaignFactory { /// @param fee Market fee /// @param funding Initial funding for market /// @param deadline Campaign deadline - /// @return Market contract + /// @return campaign Market contract function createCampaigns( Event eventContract, MarketFactory marketFactory, diff --git a/test/compilationTests/gnosis/Markets/StandardMarket.sol b/test/compilationTests/gnosis/Markets/StandardMarket.sol index 9437202e6..d77fadaca 100644 --- a/test/compilationTests/gnosis/Markets/StandardMarket.sol +++ b/test/compilationTests/gnosis/Markets/StandardMarket.sol @@ -84,7 +84,7 @@ contract StandardMarket is Market { } /// @dev Allows market creator to withdraw fees generated by trades - /// @return Fee amount + /// @return fees Fee amount function withdrawFees() public override @@ -101,7 +101,7 @@ contract StandardMarket is Market { /// @param outcomeTokenIndex Index of the outcome token to buy /// @param outcomeTokenCount Amount of outcome tokens to buy /// @param maxCost The maximum cost in collateral tokens to pay for outcome tokens - /// @return Cost in collateral tokens + /// @return cost Cost in collateral tokens function buy(uint8 outcomeTokenIndex, uint outcomeTokenCount, uint maxCost) public override @@ -132,7 +132,7 @@ contract StandardMarket is Market { /// @param outcomeTokenIndex Index of the outcome token to sell /// @param outcomeTokenCount Amount of outcome tokens to sell /// @param minProfit The minimum profit in collateral tokens to earn for outcome tokens - /// @return Profit in collateral tokens + /// @return profit Profit in collateral tokens function sell(uint8 outcomeTokenIndex, uint outcomeTokenCount, uint minProfit) public override @@ -163,7 +163,7 @@ contract StandardMarket is Market { /// @param outcomeTokenIndex Index of the outcome token to short sell /// @param outcomeTokenCount Amount of outcome tokens to short sell /// @param minProfit The minimum profit in collateral tokens to earn for short sold outcome tokens - /// @return Cost to short sell outcome in collateral tokens + /// @return cost Cost to short sell outcome in collateral tokens function shortSell(uint8 outcomeTokenIndex, uint outcomeTokenCount, uint minProfit) public override diff --git a/test/compilationTests/gnosis/Markets/StandardMarketFactory.sol b/test/compilationTests/gnosis/Markets/StandardMarketFactory.sol index a542a2573..e2d93336c 100644 --- a/test/compilationTests/gnosis/Markets/StandardMarketFactory.sol +++ b/test/compilationTests/gnosis/Markets/StandardMarketFactory.sol @@ -14,7 +14,7 @@ contract StandardMarketFactory is MarketFactory { /// @param eventContract Event contract /// @param marketMaker Market maker contract /// @param fee Market fee - /// @return Market contract + /// @return market Market contract function createMarket(Event eventContract, MarketMaker marketMaker, uint24 fee) public override diff --git a/test/compilationTests/gnosis/Oracles/CentralizedOracleFactory.sol b/test/compilationTests/gnosis/Oracles/CentralizedOracleFactory.sol index 649aaee55..be8be8276 100644 --- a/test/compilationTests/gnosis/Oracles/CentralizedOracleFactory.sol +++ b/test/compilationTests/gnosis/Oracles/CentralizedOracleFactory.sol @@ -16,7 +16,7 @@ contract CentralizedOracleFactory { */ /// @dev Creates a new centralized oracle contract /// @param ipfsHash Hash identifying off chain event description - /// @return Oracle contract + /// @return centralizedOracle Oracle contract function createCentralizedOracle(bytes memory ipfsHash) public returns (CentralizedOracle centralizedOracle) diff --git a/test/compilationTests/gnosis/Oracles/DifficultyOracleFactory.sol b/test/compilationTests/gnosis/Oracles/DifficultyOracleFactory.sol index e04f904a1..d2215fc8c 100644 --- a/test/compilationTests/gnosis/Oracles/DifficultyOracleFactory.sol +++ b/test/compilationTests/gnosis/Oracles/DifficultyOracleFactory.sol @@ -16,7 +16,7 @@ contract DifficultyOracleFactory { */ /// @dev Creates a new difficulty oracle contract /// @param blockNumber Target block number - /// @return Oracle contract + /// @return difficultyOracle Oracle contract function createDifficultyOracle(uint blockNumber) public returns (DifficultyOracle difficultyOracle) diff --git a/test/compilationTests/gnosis/Oracles/FutarchyOracleFactory.sol b/test/compilationTests/gnosis/Oracles/FutarchyOracleFactory.sol index 87bdf54b9..4a440c58c 100644 --- a/test/compilationTests/gnosis/Oracles/FutarchyOracleFactory.sol +++ b/test/compilationTests/gnosis/Oracles/FutarchyOracleFactory.sol @@ -50,7 +50,7 @@ contract FutarchyOracleFactory { /// @param marketMaker Market maker contract /// @param fee Market fee /// @param deadline Decision deadline - /// @return Oracle contract + /// @return futarchyOracle Oracle contract function createFutarchyOracle( Token collateralToken, Oracle oracle, diff --git a/test/compilationTests/gnosis/Oracles/MajorityOracle.sol b/test/compilationTests/gnosis/Oracles/MajorityOracle.sol index c10ff8a42..d57c89429 100644 --- a/test/compilationTests/gnosis/Oracles/MajorityOracle.sol +++ b/test/compilationTests/gnosis/Oracles/MajorityOracle.sol @@ -28,8 +28,8 @@ contract MajorityOracle is Oracle { } /// @dev Allows to registers oracles for a majority vote - /// @return Is outcome set? - /// @return Outcome + /// @return outcomeSet Is outcome set? + /// @return outcome Outcome function getStatusAndOutcome() public view diff --git a/test/compilationTests/gnosis/Oracles/MajorityOracleFactory.sol b/test/compilationTests/gnosis/Oracles/MajorityOracleFactory.sol index a4845a366..22e3c43a6 100644 --- a/test/compilationTests/gnosis/Oracles/MajorityOracleFactory.sol +++ b/test/compilationTests/gnosis/Oracles/MajorityOracleFactory.sol @@ -16,7 +16,7 @@ contract MajorityOracleFactory { */ /// @dev Creates a new majority oracle contract /// @param oracles List of oracles taking part in the majority vote - /// @return Oracle contract + /// @return majorityOracle Oracle contract function createMajorityOracle(Oracle[] memory oracles) public returns (MajorityOracle majorityOracle) diff --git a/test/compilationTests/gnosis/Oracles/SignedMessageOracleFactory.sol b/test/compilationTests/gnosis/Oracles/SignedMessageOracleFactory.sol index 18617faaf..33bffef03 100644 --- a/test/compilationTests/gnosis/Oracles/SignedMessageOracleFactory.sol +++ b/test/compilationTests/gnosis/Oracles/SignedMessageOracleFactory.sol @@ -19,7 +19,7 @@ contract SignedMessageOracleFactory { /// @param v Signature parameter /// @param r Signature parameter /// @param s Signature parameter - /// @return Oracle contract + /// @return signedMessageOracle Oracle contract function createSignedMessageOracle(bytes32 descriptionHash, uint8 v, bytes32 r, bytes32 s) public returns (SignedMessageOracle signedMessageOracle) diff --git a/test/compilationTests/gnosis/Oracles/UltimateOracle.sol b/test/compilationTests/gnosis/Oracles/UltimateOracle.sol index 4bbd6673d..94b57ca81 100644 --- a/test/compilationTests/gnosis/Oracles/UltimateOracle.sol +++ b/test/compilationTests/gnosis/Oracles/UltimateOracle.sol @@ -126,7 +126,7 @@ contract UltimateOracle is Oracle { } /// @dev Withdraws winnings for user - /// @return Winnings + /// @return amount Winnings function withdraw() public returns (uint amount) diff --git a/test/compilationTests/gnosis/Oracles/UltimateOracleFactory.sol b/test/compilationTests/gnosis/Oracles/UltimateOracleFactory.sol index 352872ace..7c1e16d53 100644 --- a/test/compilationTests/gnosis/Oracles/UltimateOracleFactory.sol +++ b/test/compilationTests/gnosis/Oracles/UltimateOracleFactory.sol @@ -30,7 +30,7 @@ contract UltimateOracleFactory { /// @param challengePeriod Time to challenge oracle outcome /// @param challengeAmount Amount to challenge the outcome /// @param frontRunnerPeriod Time to overbid the front-runner - /// @return Oracle contract + /// @return ultimateOracle Oracle contract function createUltimateOracle( Oracle oracle, Token collateralToken, diff --git a/test/compilationTests/gnosis/Utils/Math.sol b/test/compilationTests/gnosis/Utils/Math.sol index 2a741e53d..090c935ec 100644 --- a/test/compilationTests/gnosis/Utils/Math.sol +++ b/test/compilationTests/gnosis/Utils/Math.sol @@ -154,7 +154,7 @@ library Math { /// @dev Returns base 2 logarithm value of given x /// @param x x - /// @return logarithmic value + /// @return lo logarithmic value function floorLog2(uint x) public pure @@ -175,7 +175,7 @@ library Math { /// @dev Returns maximum of an array /// @param nums Numbers to look through - /// @return Maximum number + /// @return max Maximum number function max(int[] memory nums) public pure diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index 66690ca8d..d553c313e 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -365,7 +365,7 @@ BOOST_AUTO_TEST_CASE(dev_return) /// @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 d The result of the multiplication function mul(uint a, uint second) public returns (uint d) { return a * 7 + second; } } )"; @@ -378,12 +378,15 @@ 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\": \"The result of the multiplication\"\n" + " \"return\": {\n" + " \"d\": \"The result of the multiplication\"\n" + " }\n" " }\n" "}}"; checkNatspec(sourceCode, "test", natspec, false); } + BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl) { char const* sourceCode = R"( @@ -393,7 +396,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl) /// Since it's a really complicated parameter we need 2 lines /// @param second Documentation for the second parameter /// @return - /// The result of the multiplication + /// d The result of the multiplication function mul(uint a, uint second) public returns (uint d) { return a * 7 + second; } @@ -408,7 +411,9 @@ 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\": \"The result of the multiplication\"\n" + " \"return\": {\n" + " \"d\": \"The result of the multiplication\"\n" + " }\n" " }\n" "}}"; @@ -426,7 +431,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed) /// @return The result of the multiplication /// @return And cookies with nutella function mul(uint a, uint second) public returns (uint, uint) { - uint mul = a * 7; + uint mul = a * 7; return (mul, second); } } @@ -440,7 +445,10 @@ 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\": \"The result of the multiplicationAnd cookies with nutella\"\n" + " \"return\": {\n" + " \"_1\": \"The result of the multiplication\",\n" + " \"_2\": \"And cookies with nutella\"\n" + " }\n" " }\n" "}}"; @@ -455,10 +463,10 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_multiple) /// @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 And cookies with nutella + /// @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); } } @@ -490,7 +498,7 @@ BOOST_AUTO_TEST_CASE(dev_multiline_return) /// @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 d The result of the multiplication /// and cookies with nutella function mul(uint a, uint second) public returns (uint d) { return a * 7 + second; @@ -506,7 +514,9 @@ 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\": \"The result of the multiplication and cookies with nutella\"\n" + " \"return\": {\n" + " \"d\": \"The result of the multiplication and cookies with nutella\",\n" + " }\n" " }\n" "}}"; @@ -522,7 +532,7 @@ BOOST_AUTO_TEST_CASE(dev_multiline_comment) * @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 d The result of the multiplication * and cookies with nutella */ function mul(uint a, uint second) public returns (uint d) { @@ -539,7 +549,9 @@ 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\": \"The result of the multiplication and cookies with nutella\"\n" + " \"return\": {\n" + " \"d\": \"The result of the multiplication and cookies with nutella\",\n" + " }\n" " }\n" "}}"; @@ -843,7 +855,7 @@ BOOST_AUTO_TEST_CASE(dev_constructor_and_function) /// @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 d The result of the multiplication /// and cookies with nutella function mul(uint a, uint second) public returns(uint d) { return a * 7 + second; @@ -859,7 +871,9 @@ 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" : "The result of the multiplication and cookies with nutella" + "return" : { + "d": "The result of the multiplication and cookies with nutella" + } }, "constructor" : { "author" : "Alex", From 16fe59b7b4b6c0452cbc338cfaf113b514b52beb Mon Sep 17 00:00:00 2001 From: cd10012 Date: Sun, 27 Oct 2019 10:27:47 -0400 Subject: [PATCH 4/6] 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 | 3 +- docs/060-breaking-changes.rst | 7 ++ libsolidity/interface/Natspec.cpp | 64 +++++++------- libsolidity/interface/Natspec.h | 2 + test/externalTests/solc-js/DAO/Token.sol | 10 +-- test/libsolidity/SolidityNatspecJSON.cpp | 108 ++++++++++++++++++++--- 6 files changed, 146 insertions(+), 48 deletions(-) diff --git a/Changelog.md b/Changelog.md index 746887d3c..51f716657 100644 --- a/Changelog.md +++ b/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: diff --git a/docs/060-breaking-changes.rst b/docs/060-breaking-changes.rst index cbe9178a6..ef0ad9e6e 100644 --- a/docs/060-breaking-changes.rst +++ b/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 ` 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. diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index 0da229a7f..2c8c68694 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/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(); - auto returnDoc = fun->annotation().docTags.equal_range("return"); - - 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; + if (!jsonReturn.empty()) + method["returns"] = jsonReturn; methods[it.second->externalSignature()] = method; } @@ -145,6 +117,36 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) return doc; } +Json::Value Natspec::extractReturnParameterDocs(std::multimap 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 const& _tags, string const& _name) { diff --git a/libsolidity/interface/Natspec.h b/libsolidity/interface/Natspec.h index 7a0c40a1a..fee38c133 100644 --- a/libsolidity/interface/Natspec.h +++ b/libsolidity/interface/Natspec.h @@ -28,6 +28,7 @@ #include #include #include +#include namespace dev { @@ -60,6 +61,7 @@ private: /// @return A JSON representation /// of the contract's developer documentation static Json::Value devDocumentation(std::multimap const& _tags); + static Json::Value extractReturnParameterDocs(std::multimap const& _tags, FunctionDefinition const& _functionDef); }; } //solidity NS diff --git a/test/externalTests/solc-js/DAO/Token.sol b/test/externalTests/solc-js/DAO/Token.sol index 149e61c5e..0daa76ec9 100644 --- a/test/externalTests/solc-js/DAO/Token.sol +++ b/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, diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index d553c313e..5aa15f1ed 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/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" - " \"_1\": \"The result of the multiplication\",\n" - " \"_2\": \"And cookies with nutella\"\n" + " \"returns\": {\n" + " \"_0\": \"The result of the multiplication\",\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" } }, From a94d22e5fef578dff1a9c1456e577958a9354503 Mon Sep 17 00:00:00 2001 From: cd10012 Date: Mon, 28 Oct 2019 19:09:18 -0400 Subject: [PATCH 5/6] Add documentation for extractReturnParameterDocs --- libsolidity/interface/Natspec.h | 6 ++++++ .../gnosis/MarketMakers/LMSRMarketMaker.sol | 6 +++--- test/libsolidity/SolidityNatspecJSON.cpp | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libsolidity/interface/Natspec.h b/libsolidity/interface/Natspec.h index fee38c133..c37a4ac0e 100644 --- a/libsolidity/interface/Natspec.h +++ b/libsolidity/interface/Natspec.h @@ -61,6 +61,12 @@ private: /// @return A JSON representation /// of the contract's developer documentation static Json::Value devDocumentation(std::multimap 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 const& _tags, FunctionDefinition const& _functionDef); }; diff --git a/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol b/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol index 2642a6e8e..b64392b0d 100644 --- a/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol +++ b/test/compilationTests/gnosis/MarketMakers/LMSRMarketMaker.sol @@ -131,9 +131,9 @@ contract LMSRMarketMaker is MarketMaker { /// @param netOutcomeTokensSold Net outcome tokens sold by market /// @param funding Initial funding for market /// @param outcomeIndex Index of exponential term to extract (for use by marginal price function) - /// @return sum of the outcomes - /// @return offset that is used for all - /// @return outcomeExpTerm the summand associated with the supplied index + /// @return sum The sum of the outcomes + /// @return offset The offset that is used for all + /// @return outcomeExpTerm The summand associated with the supplied index function sumExpOffset(int logN, int[] memory netOutcomeTokensSold, uint funding, uint8 outcomeIndex) private view diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index 5aa15f1ed..435ff7f08 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -468,7 +468,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_multiple_unamed_mixed_2) /// @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; + uint milk = 4; return (mul, second, milk); } } From f9603521a431f47364ce7222debfb81a662893bc Mon Sep 17 00:00:00 2001 From: Cory Dickson <7246942+gh1dra@users.noreply.github.com> Date: Mon, 28 Oct 2019 19:16:04 -0400 Subject: [PATCH 6/6] Update Changelog.md author:@erak Co-Authored-By: Erik K --- Changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 51f716657..88618b7b1 100644 --- a/Changelog.md +++ b/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 parameters where the first word in the entry must contain the name. + * 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.