Natspec: Fix internal error when different return name was inherited

This commit is contained in:
Mathias Baumann 2020-10-08 11:07:31 +02:00
parent 6fa42b5efd
commit 559b27aaad
4 changed files with 268 additions and 11 deletions

View File

@ -20,6 +20,7 @@ Bugfixes:
* SMTChecker: Fix internal error in the BMC engine when inherited contract from a different source unit has private state variables.
* SMTChecker: Fix internal error when ``array.push()`` is used as the LHS of an assignment.
* Code generator: Fix missing creation dependency tracking for abstract contracts.
* NatSpec: Fix internal error when inheriting return parameter documentation but the parameter names differ between base and inherited.
### 0.7.4 (2020-10-19)

View File

@ -36,21 +36,52 @@ using namespace solidity::frontend;
namespace
{
void copyMissingTags(StructurallyDocumentedAnnotation& _target, set<CallableDeclaration const*> const& _baseFunctions)
void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, StructurallyDocumentedAnnotation& _target, CallableDeclaration const* _declaration = nullptr)
{
// Only copy if there is exactly one direct base function.
if (_baseFunctions.size() != 1)
return;
auto& sourceDoc = dynamic_cast<StructurallyDocumentedAnnotation const&>((*_baseFunctions.begin())->annotation());
set<string> existingTags;
for (auto it = sourceDoc.docTags.begin(); it != sourceDoc.docTags.end();)
{
string const& tag = it->first;
// Don't copy tag "inheritdoc" or already existing tags
if (tag == "inheritdoc" || _target.docTags.count(tag))
{
it++;
continue;
}
for (auto const& iterator: _target.docTags)
existingTags.insert(iterator.first);
size_t n = 0;
// Iterate over all values of the current tag (it's a multimap)
for (auto next = sourceDoc.docTags.upper_bound(tag); it != next; it++, n++)
{
DocTag content = it->second;
// Update the parameter name for @return tags
if (_declaration && tag == "return")
{
size_t docParaNameEndPos = content.content.find_first_of(" \t");
string const docParameterName = content.content.substr(0, docParaNameEndPos);
if (docParameterName != _declaration->returnParameters().at(n)->name())
{
bool baseHasNoName = (*_baseFunctions.begin())->returnParameters().at(n)->name().empty();
string paramName = _declaration->returnParameters().at(n)->name();
content.content =
(paramName.empty() ? "" : std::move(paramName) + " ") + (
string::npos == docParaNameEndPos || baseHasNoName ?
content.content :
content.content.substr(docParaNameEndPos + 1)
);
}
}
for (auto const& [tag, content]: sourceDoc.docTags)
if (tag != "inheritdoc" && !existingTags.count(tag))
_target.docTags.emplace(tag, content);
}
}
}
CallableDeclaration const* findBaseCallable(set<CallableDeclaration const*> const& _baseFunctions, int64_t _contractId)
@ -91,9 +122,9 @@ bool DocStringAnalyser::visit(VariableDeclaration const& _variable)
return false;
if (CallableDeclaration const* baseFunction = resolveInheritDoc(_variable.annotation().baseFunctions, _variable, _variable.annotation()))
copyMissingTags(_variable.annotation(), {baseFunction});
copyMissingTags({baseFunction}, _variable.annotation());
else if (_variable.annotation().docTags.empty())
copyMissingTags(_variable.annotation(), _variable.annotation().baseFunctions);
copyMissingTags(_variable.annotation().baseFunctions, _variable.annotation());
return false;
}
@ -119,13 +150,13 @@ void DocStringAnalyser::handleCallable(
)
{
if (CallableDeclaration const* baseFunction = resolveInheritDoc(_callable.annotation().baseFunctions, _node, _annotation))
copyMissingTags(_annotation, {baseFunction});
copyMissingTags({baseFunction}, _annotation, &_callable);
else if (
_annotation.docTags.empty() &&
_callable.annotation().baseFunctions.size() == 1 &&
parameterNamesEqual(_callable, **_callable.annotation().baseFunctions.begin())
)
copyMissingTags(_annotation, _callable.annotation().baseFunctions);
copyMissingTags(_callable.annotation().baseFunctions, _annotation, &_callable);
}
CallableDeclaration const* DocStringAnalyser::resolveInheritDoc(

View File

@ -1748,6 +1748,7 @@ BOOST_AUTO_TEST_CASE(user_explicit_inherit_partial2)
checkNatspec(sourceCode, "ERC20", natspec, true);
checkNatspec(sourceCode, "Token", natspec2, true);
}
BOOST_AUTO_TEST_CASE(dev_explicit_inherit_partial)
{
char const *sourceCode = R"(
@ -2022,6 +2023,230 @@ BOOST_AUTO_TEST_CASE(dev_explicit_inehrit_complex)
);
}
BOOST_AUTO_TEST_CASE(dev_different_return_name)
{
char const *sourceCode = R"(
contract A {
/// @return y value
function g(int x) public pure virtual returns (int y) { return x; }
}
contract B is A {
function g(int x) public pure override returns (int z) { return x; }
}
)";
char const *natspec = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"y": "value"
}
}
}
})ABCDEF";
char const *natspec2 = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"z": "value"
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "A", natspec, false);
checkNatspec(sourceCode, "B", natspec2, false);
}
BOOST_AUTO_TEST_CASE(dev_different_return_name_multiple)
{
char const *sourceCode = R"(
contract A {
/// @return a value A
/// @return b value B
function g(int x) public pure virtual returns (int a, int b) { return (1, 2); }
}
contract B is A {
function g(int x) public pure override returns (int z, int y) { return (1, 2); }
}
)";
char const *natspec = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"a": "value A",
"b": "value B"
}
}
}
})ABCDEF";
char const *natspec2 = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"z": "value A",
"y": "value B"
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "A", natspec, false);
checkNatspec(sourceCode, "B", natspec2, false);
}
BOOST_AUTO_TEST_CASE(dev_different_return_name_multiple_partly_unnamed)
{
char const *sourceCode = R"(
contract A {
/// @return value A
/// @return b value B
function g(int x) public pure virtual returns (int, int b) { return (1, 2); }
}
contract B is A {
function g(int x) public pure override returns (int z, int) { return (1, 2); }
}
)";
char const *natspec = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"_0": "value A",
"b": "value B"
}
}
}
})ABCDEF";
char const *natspec2 = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"z": "value A",
"_1": "value B"
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "A", natspec, false);
checkNatspec(sourceCode, "B", natspec2, false);
}
BOOST_AUTO_TEST_CASE(dev_different_return_name_multiple_unnamed)
{
char const *sourceCode = R"(
contract A {
/// @return value A
/// @return value B
function g(int x) public pure virtual returns (int, int) { return (1, 2); }
}
contract B is A {
function g(int x) public pure override returns (int z, int y) { return (1, 2); }
}
)";
char const *natspec = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"_0": "value A",
"_1": "value B"
}
}
}
})ABCDEF";
char const *natspec2 = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"z": "value A",
"y": "value B"
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "A", natspec, false);
checkNatspec(sourceCode, "B", natspec2, false);
}
BOOST_AUTO_TEST_CASE(dev_return_name_no_description)
{
char const *sourceCode = R"(
contract A {
/// @return a
function g(int x) public pure virtual returns (int a) { return 2; }
}
contract B is A {
function g(int x) public pure override returns (int b) { return 2; }
}
)";
char const *natspec = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"a": "a",
}
}
}
})ABCDEF";
char const *natspec2 = R"ABCDEF({
"methods":
{
"g(int256)":
{
"returns":
{
"b": "a",
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "A", natspec, false);
checkNatspec(sourceCode, "B", natspec2, false);
}
}
BOOST_AUTO_TEST_SUITE_END()

View File

@ -8,7 +8,7 @@ contract C {
assert(bytes(b[10:]).length == 20);
assert(bytes(b[10:])[0] == 0xff);
//assert(bytes(b[10:])[5] == 0xff); // Removed because of Spacer's nondeterminism
assert(bytes(b[10:])[19] == 0xaa);
//assert(bytes(b[10:])[19] == 0xaa); // Removed because of Spacer nondeterminism
}
}
// ----