Skip to content

Report DocString error on named return paramater mismatch #7837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chriseth merged 2 commits into from
Dec 4, 2019
Merged

Report DocString error on named return paramater mismatch #7837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5cb0fe
Reports doctring error on named return mismatch.
erak Nov 28, 2019
19d466d
Fixes named return natspec errors in compilation tests.
erak Nov 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion libsolidity/analysis/DocStringAnalyser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,36 @@ void DocStringAnalyser::parseDocStrings(
m_errorOccured = true;
_annotation.docTags = parser.tags();
}

size_t returnTagsVisited = 0;
for (auto const& docTag: _annotation.docTags)
{
if (!_validTags.count(docTag.first))
appendError("Doc tag @" + docTag.first + " not valid for " + _nodeName + ".");
appendError("Documentation tag @" + docTag.first + " not valid for " + _nodeName + ".");
else
if (docTag.first == "return")
{
returnTagsVisited++;
if (auto* function = dynamic_cast<FunctionDefinition const*>(&_node))
{
string content = docTag.second.content;
string firstWord = content.substr(0, content.find_first_of(" \t"));

if (returnTagsVisited > function->returnParameters().size())
appendError("Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" +
" exceedes the number of return parameters."
);
else
{
auto parameter = function->returnParameters().at(returnTagsVisited - 1);
if (!parameter->name().empty() && parameter->name() != firstWord)
appendError("Documentation tag \"@" + docTag.first + " " + docTag.second.content + "\"" +
" does not contain the name of its return parameter."
);
}
}
}
}
}

void DocStringAnalyser::appendError(string const& _description)
Expand Down
1 change: 0 additions & 1 deletion test/compilationTests/gnosis/Markets/Campaign.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ contract Campaign {
}

/// @dev Allows to withdraw fees from market contract to campaign contract
/// @return Fee amount

@erak erak Nov 28, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function does not return anything.

function closeMarket()
public
atStage(Stages.MarketCreated)
Expand Down
30 changes: 15 additions & 15 deletions test/compilationTests/milestonetracker/RLP.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ library RLP {

/// @dev Check if the RLP item is null.
/// @param self The RLP item.
/// @return 'true' if the item is null.
/// @return ret 'true' if the item is null.
function isNull(RLPItem memory self) internal view returns (bool ret) {
return self._unsafe_length == 0;
}

/// @dev Check if the RLP item is a list.
/// @param self The RLP item.
/// @return 'true' if the item is a list.
/// @return ret 'true' if the item is a list.
function isList(RLPItem memory self) internal view returns (bool ret) {
if (self._unsafe_length == 0)
return false;
Expand All @@ -109,7 +109,7 @@ library RLP {

/// @dev Check if the RLP item is data.
/// @param self The RLP item.
/// @return 'true' if the item is data.
/// @return ret 'true' if the item is data.
function isData(RLPItem memory self) internal view returns (bool ret) {
if (self._unsafe_length == 0)
return false;
Expand All @@ -121,7 +121,7 @@ library RLP {

/// @dev Check if the RLP item is empty (string or list).
/// @param self The RLP item.
/// @return 'true' if the item is null.
/// @return ret 'true' if the item is null.
function isEmpty(RLPItem memory self) internal view returns (bool ret) {
if(isNull(self))
return false;
Expand Down Expand Up @@ -156,7 +156,7 @@ library RLP {

/// @dev Create an iterator.
/// @param self The RLP item.
/// @return An 'Iterator' over the item.
/// @return it An 'Iterator' over the item.
function iterator(RLPItem memory self) internal view returns (Iterator memory it) {
if (!isList(self))
revert();
Expand All @@ -167,7 +167,7 @@ library RLP {

/// @dev Return the RLP encoded bytes.
/// @param self The RLPItem.
/// @return The bytes.
/// @return bts The bytes.
function toBytes(RLPItem memory self) internal returns (bytes memory bts) {
uint len = self._unsafe_length;
if (len != 0)
Expand All @@ -180,7 +180,7 @@ library RLP {
/// @dev Decode an RLPItem into bytes. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return bts The decoded string.
function toData(RLPItem memory self) internal returns (bytes memory bts) {
if(!isData(self))
revert();
Expand All @@ -192,7 +192,7 @@ library RLP {
/// @dev Get the list of sub-items from an RLP encoded list.
/// Warning: This is inefficient, as it requires that the list is read twice.
/// @param self The RLP item.
/// @return Array of RLPItems.
/// @return list Array of RLPItems.
function toList(RLPItem memory self) internal view returns (RLPItem[] memory list) {
if(!isList(self))
revert();
Expand All @@ -209,7 +209,7 @@ library RLP {
/// @dev Decode an RLPItem into an ascii string. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return str The decoded string.
function toAscii(RLPItem memory self) internal returns (string memory str) {
if(!isData(self))
revert();
Expand All @@ -222,7 +222,7 @@ library RLP {
/// @dev Decode an RLPItem into a uint. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return data The decoded string.
function toUint(RLPItem memory self) internal view returns (uint data) {
if(!isData(self))
revert();
Expand All @@ -237,7 +237,7 @@ library RLP {
/// @dev Decode an RLPItem into a boolean. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return data The decoded string.
function toBool(RLPItem memory self) internal view returns (bool data) {
if(!isData(self))
revert();
Expand All @@ -256,7 +256,7 @@ library RLP {
/// @dev Decode an RLPItem into a byte. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return data The decoded string.
function toByte(RLPItem memory self) internal view returns (byte data) {
if(!isData(self))
revert();
Expand All @@ -273,23 +273,23 @@ library RLP {
/// @dev Decode an RLPItem into an int. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return data The decoded string.
function toInt(RLPItem memory self) internal view returns (int data) {
return int(toUint(self));
}

/// @dev Decode an RLPItem into a bytes32. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return data The decoded string.
function toBytes32(RLPItem memory self) internal view returns (bytes32 data) {
return bytes32(toUint(self));
}

/// @dev Decode an RLPItem into an address. This will not work if the
/// RLPItem is a list.
/// @param self The RLPItem.
/// @return The decoded string.
/// @return data The decoded string.
function toAddress(RLPItem memory self) internal view returns (address data) {
if(!isData(self))
revert();
Expand Down
4 changes: 2 additions & 2 deletions test/libsolidity/SolidityEndToEndTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12831,7 +12831,7 @@ BOOST_AUTO_TEST_CASE(snark)
return G1Point(p.X, q - (p.Y % q));
}

/// @return the sum of two points of G1
/// @return r the sum of two points of G1
function add(G1Point memory p1, G1Point memory p2) internal returns (G1Point memory r) {
uint[4] memory input;
input[0] = p1.X;
Expand All @@ -12847,7 +12847,7 @@ BOOST_AUTO_TEST_CASE(snark)
require(success);
}

/// @return the product of a point on G1 and a scalar, i.e.
/// @return r the product of a point on G1 and a scalar, i.e.
/// p == p.mul(1) and p.add(p) == p.mul(2) for all points p.
function mul(G1Point memory p, uint s) internal returns (G1Point memory r) {
uint[3] memory input;
Expand Down
5 changes: 5 additions & 0 deletions test/libsolidity/syntaxTests/natspec/dosctring_named_return_parameter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
abstract contract C {
/// @return value The value returned by this function.
function vote() public virtual returns (uint value);
}
// ----
7 changes: 7 additions & 0 deletions test/libsolidity/syntaxTests/natspec/invalid/dosctring_named_return_param_mismatch.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
abstract contract C {
/// @param id Some identifier
/// @return No value returned
function vote(uint id) public virtual returns (uint value);
}
// ----
// DocstringParsingError: Documentation tag "@return No value returned" does not contain the name of its return parameter.
7 changes: 7 additions & 0 deletions test/libsolidity/syntaxTests/natspec/invalid/dosctring_return_size_mismatch.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
abstract contract C {
/// @param id Some identifier
/// @return No value returned
function vote(uint id) public virtual returns (uint value);
}
// ----
// DocstringParsingError: Documentation tag "@return No value returned" does not contain the name of its return parameter.