Merge pull request #10921 from ethereum/issue-10881

Natspec: Don't copy from base function if return parameters differ
This commit is contained in:
chriseth 2021-04-20 15:33:48 +02:00 committed by GitHub
commit cf7f814a4e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 109 additions and 9 deletions

View File

@ -26,6 +26,7 @@ Compiler Features:
Bugfixes:
* Natspec: Fix internal error related to the `@returns` documentation for a public state variable overriding a function.
* Antlr Grammar: Fix parsing of import paths involving properly distinguishing between empty and non-empty string literals in general.
* AST Output: Fix ``kind`` field of ``ModifierInvocation`` for base constructor calls.
* Commandline interface: Fix standard input bypassing allowed path checks.

View File

@ -134,7 +134,10 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
{
Json::Value method(devDocumentation(fun->annotation().docTags));
// add the function, only if we have any documentation to add
Json::Value jsonReturn = extractReturnParameterDocs(fun->annotation().docTags, *fun);
Json::Value jsonReturn = extractReturnParameterDocs(
fun->annotation().docTags,
fun->functionType(false)->returnParameterNames()
);
if (!jsonReturn.empty())
method["returns"] = move(jsonReturn);
@ -149,9 +152,20 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef)
if (auto devDoc = devDocumentation(varDecl->annotation().docTags); !devDoc.empty())
doc["stateVariables"][varDecl->name()] = devDoc;
solAssert(varDecl->annotation().docTags.count("return") <= 1, "");
auto const assignIfNotEmpty = [&](string const& _name, Json::Value const& _content)
{
if (!_content.empty())
doc["stateVariables"][varDecl->name()][_name] = _content;
};
if (varDecl->annotation().docTags.count("return") == 1)
doc["stateVariables"][varDecl->name()]["return"] = extractDoc(varDecl->annotation().docTags, "return");
assignIfNotEmpty("return", extractDoc(varDecl->annotation().docTags, "return"));
if (FunctionTypePointer functionType = varDecl->functionType(false))
assignIfNotEmpty("returns", extractReturnParameterDocs(
varDecl->annotation().docTags,
functionType->returnParameterNames()
));
}
for (auto const& event: _contractDef.events())
@ -165,17 +179,17 @@ 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 Natspec::extractReturnParameterDocs(std::multimap<std::string, DocTag> const& _tags, vector<string> const& _returnParameterNames)
{
Json::Value jsonReturn{Json::objectValue};
auto returnDocs = _tags.equal_range("return");
if (!_functionDef.returnParameters().empty())
if (!_returnParameterNames.empty())
{
size_t n = 0;
for (auto i = returnDocs.first; i != returnDocs.second; i++)
{
string paramName = _functionDef.returnParameters().at(n)->name();
string paramName = _returnParameterNames.at(n);
string content = i->second.content;
if (paramName.empty())

View File

@ -68,10 +68,10 @@ private:
/// 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.
/// @param _returnParameterNames names of the return parameters
/// @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);
static Json::Value extractReturnParameterDocs(std::multimap<std::string, DocTag> const& _tags, std::vector<std::string> const& _returnParameterNames);
};
}

View File

@ -303,7 +303,11 @@ BOOST_AUTO_TEST_CASE(public_state_variable)
"state" :
{
"details" : "example of dev",
"return" : "returns state"
"return" : "returns state",
"returns" :
{
"_0" : "returns state"
}
}
}
}
@ -2411,6 +2415,58 @@ BOOST_AUTO_TEST_CASE(custom_inheritance)
checkNatspec(sourceCode, "B", natspecB, false);
}
BOOST_AUTO_TEST_CASE(dev_different_amount_return_parameters)
{
char const *sourceCode = R"(
interface IThing {
/// @return x a number
/// @return y another number
function value() external view returns (uint128 x, uint128 y);
}
contract Thing is IThing {
struct Value {
uint128 x;
uint128 y;
}
Value public override value;
}
)";
char const *natspec = R"ABCDEF({
"methods":
{
"value()":
{
"returns":
{
"x": "a number",
"y": "another number"
}
}
}
})ABCDEF";
char const *natspec2 = R"ABCDEF({
"methods": {},
"stateVariables":
{
"value":
{
"returns":
{
"x": "a number",
"y": "another number"
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "IThing", natspec, false);
checkNatspec(sourceCode, "Thing", natspec2, false);
}
}
BOOST_AUTO_TEST_SUITE_END()

View File

@ -0,0 +1,14 @@
interface IThing {
/// @return x a number
/// @return y another number
function value() external view returns (uint128 x, uint128 y);
}
contract Thing is IThing {
struct Value {
uint128 x;
uint128 y;
}
Value public override value;
}

View File

@ -0,0 +1,15 @@
interface IThing {
/// @param v value to search for
/// @return x a number
/// @return y another number
function value(uint256 v) external view returns (uint128 x, uint128 y);
}
contract Thing is IThing {
struct Value {
uint128 x;
uint128 y;
}
mapping(uint256=>Value) public override value;
}