cerc-io/solidity
16
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Natspec: Fix ICE when overriding a struct getter with a Natspec-documented return value and the name in the struct is different.

Browse Source
This commit is contained in:
Marenz 2022-01-17 17:09:14 +01:00
parent 57d84c8bfb
commit 7c0a121e45
4 changed files with 73 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Changelog.md
View File

@ -12,6 +12,7 @@ Compiler Features:
Bugfixes:
* Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis.
* Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the parent contract contains immutable variables.
* Natspec: Fix ICE when overriding a struct getter with a Natspec-documented return value and the name in the struct is different.
* TypeChecker: Fix ICE when a constant variable declaration forward references a struct.

35
libsolidity/analysis/DocStringAnalyser.cpp
View File

@ -25,6 +25,7 @@
#include <libsolidity/analysis/DocStringAnalyser.h>
#include <libsolidity/ast/AST.h>
#include <libsolidity/ast/TypeProvider.h>
#include <liblangutil/ErrorReporter.h>
#include <boost/algorithm/string.hpp>
@ -37,7 +38,7 @@ using namespace solidity::frontend;
namespace
{
void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, StructurallyDocumentedAnnotation& _target, CallableDeclaration const* _declaration = nullptr)
void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, StructurallyDocumentedAnnotation& _target, FunctionType const* _functionType = nullptr)
{
// Only copy if there is exactly one direct base function.
if (_baseFunctions.size() != 1)
@ -45,12 +46,6 @@ void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, Stru
CallableDeclaration const& baseFunction = **_baseFunctions.begin();
auto hasReturnParameter = [](CallableDeclaration const& declaration, size_t _n)
{
return declaration.returnParameterList() &&
declaration.returnParameters().size() > _n;
};
auto& sourceDoc = dynamic_cast<StructurallyDocumentedAnnotation const&>(baseFunction.annotation());
for (auto it = sourceDoc.docTags.begin(); it != sourceDoc.docTags.end();)
@ -70,21 +65,22 @@ void copyMissingTags(set<CallableDeclaration const*> const& _baseFunctions, Stru
DocTag content = it->second;
// Update the parameter name for @return tags
if (_declaration && tag == "return")
if (_functionType && tag == "return")
{
size_t docParaNameEndPos = content.content.find_first_of(" \t");
string const docParameterName = content.content.substr(0, docParaNameEndPos);
if (
hasReturnParameter(*_declaration, n) &&
docParameterName != _declaration->returnParameters().at(n)->name()
_functionType->returnParameterNames().size() > n &&
docParameterName != _functionType->returnParameterNames().at(n)
)
{
bool baseHasNoName =
hasReturnParameter(baseFunction, n) &&
baseFunction.returnParameterList() &&
baseFunction.returnParameters().size() > n &&
baseFunction.returnParameters().at(n)->name().empty();
string paramName = _declaration->returnParameters().at(n)->name();
string paramName = _functionType->returnParameterNames().at(n);
content.content =
(paramName.empty() ? "" : std::move(paramName) + " ") + (
string::npos == docParaNameEndPos || baseHasNoName ?
@ -127,7 +123,7 @@ bool DocStringAnalyser::analyseDocStrings(SourceUnit const& _sourceUnit)
bool DocStringAnalyser::visit(FunctionDefinition const& _function)
{
if (!_function.isConstructor())
handleCallable(_function, _function, _function.annotation());
handleCallable(_function, _function, _function.annotation(), TypeProvider::function(_function));
return true;
}
@ -136,10 +132,12 @@ bool DocStringAnalyser::visit(VariableDeclaration const& _variable)
if (!_variable.isStateVariable() && !_variable.isFileLevelVariable())
return false;
auto const* getterType = TypeProvider::function(_variable);
if (CallableDeclaration const* baseFunction = resolveInheritDoc(_variable.annotation().baseFunctions, _variable, _variable.annotation()))
copyMissingTags({baseFunction}, _variable.annotation());
copyMissingTags({baseFunction}, _variable.annotation(), getterType);
else if (_variable.annotation().docTags.empty())
copyMissingTags(_variable.annotation().baseFunctions, _variable.annotation());
copyMissingTags(_variable.annotation().baseFunctions, _variable.annotation(), getterType);
return false;
}
@ -168,17 +166,18 @@ bool DocStringAnalyser::visit(ErrorDefinition const& _error)
void DocStringAnalyser::handleCallable(
CallableDeclaration const& _callable,
StructurallyDocumented const& _node,
StructurallyDocumentedAnnotation& _annotation
StructurallyDocumentedAnnotation& _annotation,
FunctionType const* _functionType
)
{
if (CallableDeclaration const* baseFunction = resolveInheritDoc(_callable.annotation().baseFunctions, _node, _annotation))
copyMissingTags({baseFunction}, _annotation, &_callable);
copyMissingTags({baseFunction}, _annotation, _functionType);
else if (
_annotation.docTags.empty() &&
_callable.annotation().baseFunctions.size() == 1 &&
parameterNamesEqual(_callable, **_callable.annotation().baseFunctions.begin())
)
copyMissingTags(_callable.annotation().baseFunctions, _annotation, &_callable);
copyMissingTags(_callable.annotation().baseFunctions, _annotation, _functionType);
}
CallableDeclaration const* DocStringAnalyser::resolveInheritDoc(

3
libsolidity/analysis/DocStringAnalyser.h
View File

@ -54,7 +54,8 @@ private:
void handleCallable(
CallableDeclaration const& _callable,
StructurallyDocumented const& _node,
StructurallyDocumentedAnnotation& _annotation
StructurallyDocumentedAnnotation& _annotation,
FunctionType const* _functionType = nullptr
);
langutil::ErrorReporter& m_errorReporter;

54
test/libsolidity/SolidityNatspecJSON.cpp
View File

@ -2482,7 +2482,7 @@ BOOST_AUTO_TEST_CASE(custom_inheritance)
checkNatspec(sourceCode, "B", natspecB, false);
}
BOOST_AUTO_TEST_CASE(dev_different_amount_return_parameters)
BOOST_AUTO_TEST_CASE(dev_struct_getter_override)
{
char const *sourceCode = R"(
interface IThing {
@ -2534,6 +2534,58 @@ BOOST_AUTO_TEST_CASE(dev_different_amount_return_parameters)
checkNatspec(sourceCode, "Thing", natspec2, false);
}
BOOST_AUTO_TEST_CASE(dev_struct_getter_override_different_return_parameter_names)
{
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 a;
uint128 b;
}
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":
{
"a": "a number",
"b": "another number"
}
}
}
})ABCDEF";
checkNatspec(sourceCode, "IThing", natspec, false);
checkNatspec(sourceCode, "Thing", natspec2, false);
}
}
BOOST_AUTO_TEST_SUITE_END()