From f76d4d59197db344613de18d1bfcead34a5e42ba Mon Sep 17 00:00:00 2001 From: Alexander Arlt Date: Tue, 6 Mar 2018 19:26:49 +0100 Subject: Fix: natspec annotations on constructors - natspec annotations on constructore where ignored. --- libsolidity/analysis/DocStringAnalyser.cpp | 34 ++++++-- libsolidity/analysis/DocStringAnalyser.h | 11 +++ libsolidity/interface/Natspec.cpp | 67 +++++++++++----- libsolidity/interface/Natspec.h | 6 ++ test/libsolidity/SolidityNatspecJSON.cpp | 125 +++++++++++++++++++++++++++++ 5 files changed, 216 insertions(+), 27 deletions(-) diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index b3fb5258..c1b97def 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -48,7 +48,10 @@ bool DocStringAnalyser::visit(ContractDefinition const& _contract) bool DocStringAnalyser::visit(FunctionDefinition const& _function) { - handleCallable(_function, _function, _function.annotation()); + if (_function.isConstructor()) + handleConstructor(_function, _function, _function.annotation()); + else + handleCallable(_function, _function, _function.annotation()); return true; } @@ -66,15 +69,11 @@ bool DocStringAnalyser::visit(EventDefinition const& _event) return true; } -void DocStringAnalyser::handleCallable( +void DocStringAnalyser::checkParameters( CallableDeclaration const& _callable, - Documented const& _node, DocumentedAnnotation& _annotation ) { - static const set validTags = set{"author", "dev", "notice", "return", "param"}; - parseDocStrings(_node, _annotation, validTags, "functions"); - set validParams; for (auto const& p: _callable.parameters()) validParams.insert(p->name()); @@ -89,6 +88,29 @@ void DocStringAnalyser::handleCallable( i->second.paramName + "\" not found in the parameter list of the function." ); + +} + +void DocStringAnalyser::handleConstructor( + CallableDeclaration const& _callable, + Documented const& _node, + DocumentedAnnotation& _annotation +) +{ + static const set validTags = set{"author", "dev", "notice", "param"}; + parseDocStrings(_node, _annotation, validTags, "constructor"); + checkParameters(_callable, _annotation); +} + +void DocStringAnalyser::handleCallable( + CallableDeclaration const& _callable, + Documented const& _node, + DocumentedAnnotation& _annotation +) +{ + static const set validTags = set{"author", "dev", "notice", "return", "param"}; + parseDocStrings(_node, _annotation, validTags, "functions"); + checkParameters(_callable, _annotation); } void DocStringAnalyser::parseDocStrings( diff --git a/libsolidity/analysis/DocStringAnalyser.h b/libsolidity/analysis/DocStringAnalyser.h index 158b4060..5d339428 100644 --- a/libsolidity/analysis/DocStringAnalyser.h +++ b/libsolidity/analysis/DocStringAnalyser.h @@ -48,6 +48,17 @@ private: virtual bool visit(ModifierDefinition const& _modifier) override; virtual bool visit(EventDefinition const& _event) override; + void checkParameters( + CallableDeclaration const& _callable, + DocumentedAnnotation& _annotation + ); + + void handleConstructor( + CallableDeclaration const& _callable, + Documented const& _node, + DocumentedAnnotation& _annotation + ); + void handleCallable( CallableDeclaration const& _callable, Documented const& _node, diff --git a/libsolidity/interface/Natspec.cpp b/libsolidity/interface/Natspec.cpp index 29a5b798..a8716862 100644 --- a/libsolidity/interface/Natspec.cpp +++ b/libsolidity/interface/Natspec.cpp @@ -36,6 +36,15 @@ Json::Value Natspec::userDocumentation(ContractDefinition const& _contractDef) Json::Value doc; Json::Value methods(Json::objectValue); + auto constructorDefinition(_contractDef.constructor()); + if (constructorDefinition) + { + string value = extractDoc(constructorDefinition->annotation().docTags, "notice"); + if (!value.empty()) + // add the constructor, only if we have any documentation to add + methods["constructor"] = Json::Value(value); + } + string notice = extractDoc(_contractDef.annotation().docTags, "notice"); if (!notice.empty()) doc["notice"] = Json::Value(notice); @@ -73,33 +82,21 @@ Json::Value Natspec::devDocumentation(ContractDefinition const& _contractDef) if (!dev.empty()) doc["details"] = Json::Value(dev); + auto constructorDefinition(_contractDef.constructor()); + if (constructorDefinition) { + Json::Value constructor(devDocumentation(constructorDefinition->annotation().docTags)); + if (!constructor.empty()) + // add the constructor, only if we have any documentation to add + methods["constructor"] = constructor; + } + for (auto const& it: _contractDef.interfaceFunctions()) { if (!it.second->hasDeclaration()) continue; - Json::Value method; if (auto fun = dynamic_cast(&it.second->declaration())) { - auto dev = extractDoc(fun->annotation().docTags, "dev"); - if (!dev.empty()) - method["details"] = Json::Value(dev); - - auto author = extractDoc(fun->annotation().docTags, "author"); - if (!author.empty()) - method["author"] = author; - - auto ret = extractDoc(fun->annotation().docTags, "return"); - if (!ret.empty()) - method["return"] = ret; - - Json::Value params(Json::objectValue); - auto paramRange = fun->annotation().docTags.equal_range("param"); - for (auto i = paramRange.first; i != paramRange.second; ++i) - params[i->second.paramName] = Json::Value(i->second.content); - - if (!params.empty()) - method["params"] = params; - + Json::Value method(devDocumentation(fun->annotation().docTags)); if (!method.empty()) // add the function, only if we have any documentation to add methods[it.second->externalSignature()] = method; @@ -118,3 +115,31 @@ string Natspec::extractDoc(multimap const& _tags, string const& value += i->second.content; return value; } + +Json::Value Natspec::devDocumentation(std::multimap const &_tags) +{ + Json::Value json(Json::objectValue); + auto dev = extractDoc(_tags, "dev"); + if (!dev.empty()) + json["details"] = Json::Value(dev); + + auto author = extractDoc(_tags, "author"); + 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) + params[i->second.paramName] = Json::Value(i->second.content); + + if (!params.empty()) + json["params"] = params; + + return json; +} diff --git a/libsolidity/interface/Natspec.h b/libsolidity/interface/Natspec.h index 6a827d3b..0be4dda2 100644 --- a/libsolidity/interface/Natspec.h +++ b/libsolidity/interface/Natspec.h @@ -54,6 +54,12 @@ public: private: /// @returns concatenation of all content under the given tag name. static std::string extractDoc(std::multimap const& _tags, std::string const& _name); + + /// Helper-function that will create a json object with dev specific annotations, if present. + /// @param _tags docTags that are used. + /// @return A JSON representation + /// of the contract's developer documentation + static Json::Value devDocumentation(std::multimap const &_tags); }; } //solidity NS diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index cc44b578..b97df972 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -683,6 +683,131 @@ BOOST_AUTO_TEST_CASE(dev_documenting_no_param_description) expectNatspecError(sourceCode); } +BOOST_AUTO_TEST_CASE(user_constructor) +{ + char const *sourceCode = R"( + contract test { + /// @notice this is a really nice constructor + constructor(uint a, uint second) public { } + } + )"; + + char const *natspec = R"ABCDEF({ + "methods" : { + "constructor" : "this is a really nice constructor" + } + })ABCDEF"; + + checkNatspec(sourceCode, "test", natspec, true); +} + +BOOST_AUTO_TEST_CASE(user_constructor_and_function) +{ + char const *sourceCode = R"( + contract test { + /// @notice this is a really nice constructor + constructor(uint a, uint second) public { } + /// another multiplier + function mul(uint a, uint second) public returns(uint d) { return a * 7 + second; } + } + )"; + + char const *natspec = R"ABCDEF({ + "methods" : { + "mul(uint256,uint256)" : { + "notice" : "another multiplier" + }, + "constructor" : "this is a really nice constructor" + } + })ABCDEF"; + + checkNatspec(sourceCode, "test", natspec, true); +} + +BOOST_AUTO_TEST_CASE(dev_constructor) +{ + char const *sourceCode = R"( + contract test { + /// @author Alex + /// @param a the parameter a is really nice and very useful + /// @param second the second parameter is not very useful, it just provides additional confusion + constructor(uint a, uint second) public { } + } + )"; + + char const *natspec = R"ABCDEF({ + "methods" : { + "constructor" : { + "author" : "Alex", + "params" : { + "a" : "the parameter a is really nice and very useful", + "second" : "the second parameter is not very useful, it just provides additional confusion" + } + } + } + })ABCDEF"; + + checkNatspec(sourceCode, "test", natspec, false); +} + +BOOST_AUTO_TEST_CASE(dev_constructor_return) +{ + char const* sourceCode = R"( + contract test { + /// @author Alex + /// @param a the parameter a is really nice and very useful + /// @param second the second parameter is not very useful, it just provides additional confusion + /// @return return should not work within constructors + constructor(uint a, uint second) public { } + } + )"; + + expectNatspecError(sourceCode); +} + +BOOST_AUTO_TEST_CASE(dev_constructor_and_function) +{ + char const *sourceCode = R"( + contract test { + /// @author Alex + /// @param a the parameter a is really nice and very useful + /// @param second the second parameter is not very useful, it just provides additional confusion + constructor(uint a, uint second) public { } + /// @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 + /// and cookies with nutella + function mul(uint a, uint second) public returns(uint d) { + return a * 7 + second; + } + } + )"; + + char const *natspec = R"ABCDEF({ + "methods" : { + "mul(uint256,uint256)" : { + "details" : "Multiplies a number by 7 and adds second parameter", + "params" : { + "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" + }, + "constructor" : { + "author" : "Alex", + "params" : { + "a" : "the parameter a is really nice and very useful", + "second" : "the second parameter is not very useful, it just provides additional confusion" + } + } + } + })ABCDEF"; + + checkNatspec(sourceCode, "test", natspec, false); +} + BOOST_AUTO_TEST_SUITE_END() } -- cgit