diff options
| -rw-r--r-- | Changelog.md | 1 | ||||
| -rw-r--r-- | libsolidity/ast/Types.cpp | 18 | ||||
| -rw-r--r-- | libsolidity/ast/Types.h | 2 | ||||
| -rw-r--r-- | libsolidity/interface/InterfaceHandler.cpp | 48 | ||||
| -rw-r--r-- | libsolidity/interface/InterfaceHandler.h | 10 | ||||
| -rw-r--r-- | libsolidity/parsing/DocStringParser.cpp | 70 | ||||
| -rwxr-xr-x | scripts/install_deps.sh | 2 | ||||
| -rw-r--r-- | test/libsolidity/SolidityNameAndTypeResolution.cpp | 22 | ||||
| -rw-r--r-- | test/libsolidity/SolidityNatspecJSON.cpp | 71 |
9 files changed, 174 insertions, 70 deletions
diff --git a/Changelog.md b/Changelog.md index c87d1647..8dd1b89c 100644 --- a/Changelog.md +++ b/Changelog.md @@ -16,6 +16,7 @@ Bugfixes: * Code generator: Allow recursive structs. * Inline assembly: Disallow variables named like opcodes. * Type checker: Allow multiple events of the same name (but with different arities or argument types) + * Natspec parser: Fix error with ``@param`` parsing and whitespace. ### 0.4.8 (2017-01-13) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 971e1f18..dbabc8db 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2512,24 +2512,6 @@ FunctionTypePointer FunctionType::asMemberFunction(bool _inLibrary, bool _bound) ); } -vector<string> const FunctionType::parameterTypeNames(bool _addDataLocation) const -{ - vector<string> names; - for (TypePointer const& t: parameterTypes()) - names.push_back(t->canonicalName(_addDataLocation)); - - return names; -} - -vector<string> const FunctionType::returnParameterTypeNames(bool _addDataLocation) const -{ - vector<string> names; - for (TypePointer const& t: m_returnParameterTypes) - names.push_back(t->canonicalName(_addDataLocation)); - - return names; -} - TypePointer const& FunctionType::selfType() const { solAssert(bound(), "Function is not bound."); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 770cbb30..a5147f17 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -915,10 +915,8 @@ public: TypePointers parameterTypes() const; std::vector<std::string> parameterNames() const; - std::vector<std::string> const parameterTypeNames(bool _addDataLocation) const; TypePointers const& returnParameterTypes() const { return m_returnParameterTypes; } std::vector<std::string> const& returnParameterNames() const { return m_returnParameterNames; } - std::vector<std::string> const returnParameterTypeNames(bool _addDataLocation) const; /// @returns the "self" parameter type for a bound function TypePointer const& selfType() const; diff --git a/libsolidity/interface/InterfaceHandler.cpp b/libsolidity/interface/InterfaceHandler.cpp index 9944bb22..6c1bb0c4 100644 --- a/libsolidity/interface/InterfaceHandler.cpp +++ b/libsolidity/interface/InterfaceHandler.cpp @@ -30,20 +30,6 @@ Json::Value InterfaceHandler::abiInterface(ContractDefinition const& _contractDe { Json::Value abi(Json::arrayValue); - auto populateParameters = [](vector<string> const& _paramNames, vector<string> const& _paramTypes) - { - Json::Value params(Json::arrayValue); - solAssert(_paramNames.size() == _paramTypes.size(), "Names and types vector size does not match"); - for (unsigned i = 0; i < _paramNames.size(); ++i) - { - Json::Value param; - param["name"] = _paramNames[i]; - param["type"] = _paramTypes[i]; - params.append(param); - } - return params; - }; - for (auto it: _contractDef.interfaceFunctions()) { auto externalFunctionType = it.second->interfaceFunctionType(); @@ -52,13 +38,15 @@ Json::Value InterfaceHandler::abiInterface(ContractDefinition const& _contractDe method["name"] = it.second->declaration().name(); method["constant"] = it.second->isConstant(); method["payable"] = it.second->isPayable(); - method["inputs"] = populateParameters( + method["inputs"] = formatTypeList( externalFunctionType->parameterNames(), - externalFunctionType->parameterTypeNames(_contractDef.isLibrary()) + externalFunctionType->parameterTypes(), + _contractDef.isLibrary() ); - method["outputs"] = populateParameters( + method["outputs"] = formatTypeList( externalFunctionType->returnParameterNames(), - externalFunctionType->returnParameterTypeNames(_contractDef.isLibrary()) + externalFunctionType->returnParameterTypes(), + _contractDef.isLibrary() ); abi.append(method); } @@ -69,9 +57,10 @@ Json::Value InterfaceHandler::abiInterface(ContractDefinition const& _contractDe auto externalFunction = FunctionType(*_contractDef.constructor(), false).interfaceFunctionType(); solAssert(!!externalFunction, ""); method["payable"] = externalFunction->isPayable(); - method["inputs"] = populateParameters( + method["inputs"] = formatTypeList( externalFunction->parameterNames(), - externalFunction->parameterTypeNames(_contractDef.isLibrary()) + externalFunction->parameterTypes(), + _contractDef.isLibrary() ); abi.append(method); } @@ -179,6 +168,25 @@ Json::Value InterfaceHandler::devDocumentation(ContractDefinition const& _contra return doc; } +Json::Value InterfaceHandler::formatTypeList( + vector<string> const& _names, + vector<TypePointer> const& _types, + bool _forLibrary +) +{ + Json::Value params(Json::arrayValue); + solAssert(_names.size() == _types.size(), "Names and types vector size does not match"); + for (unsigned i = 0; i < _names.size(); ++i) + { + solAssert(_types[i], ""); + Json::Value param; + param["name"] = _names[i]; + param["type"] = _types[i]->canonicalName(_forLibrary); + params.append(param); + } + return params; +} + string InterfaceHandler::extractDoc(multimap<string, DocTag> const& _tags, string const& _name) { string value; diff --git a/libsolidity/interface/InterfaceHandler.h b/libsolidity/interface/InterfaceHandler.h index b7e1bb00..56927d44 100644 --- a/libsolidity/interface/InterfaceHandler.h +++ b/libsolidity/interface/InterfaceHandler.h @@ -37,6 +37,8 @@ namespace solidity // Forward declarations class ContractDefinition; +class Type; +using TypePointer = std::shared_ptr<Type const>; struct DocTag; enum class DocumentationType: uint8_t; @@ -84,6 +86,14 @@ public: static Json::Value devDocumentation(ContractDefinition const& _contractDef); private: + /// @returns a json value suitable for a list of types in function input or output + /// parameters or other places. If @a _forLibrary is true, complex types are referenced + /// by name, otherwise they are anonymously expanded. + static Json::Value formatTypeList( + std::vector<std::string> const& _names, + std::vector<TypePointer> const& _types, + bool _forLibrary + ); /// @returns concatenation of all content under the given tag name. static std::string extractDoc(std::multimap<std::string, DocTag> const& _tags, std::string const& _name); }; diff --git a/libsolidity/parsing/DocStringParser.cpp b/libsolidity/parsing/DocStringParser.cpp index bbee35f5..8e912126 100644 --- a/libsolidity/parsing/DocStringParser.cpp +++ b/libsolidity/parsing/DocStringParser.cpp @@ -1,14 +1,19 @@ #include <libsolidity/parsing/DocStringParser.h> -#include <boost/range/irange.hpp> #include <libsolidity/interface/Utils.h> +#include <boost/range/irange.hpp> +#include <boost/range/algorithm.hpp> + using namespace std; using namespace dev; using namespace dev::solidity; -static inline string::const_iterator skipLineOrEOS( +namespace +{ + +string::const_iterator skipLineOrEOS( string::const_iterator _nlPos, string::const_iterator _end ) @@ -16,14 +21,34 @@ static inline string::const_iterator skipLineOrEOS( return (_nlPos == _end) ? _end : ++_nlPos; } -static inline string::const_iterator firstSpaceOrNl( +string::const_iterator firstSpaceOrTab( string::const_iterator _pos, string::const_iterator _end ) { - auto spacePos = find(_pos, _end, ' '); - auto nlPos = find(_pos, _end, '\n'); - return (spacePos < nlPos) ? spacePos : nlPos; + return boost::range::find_first_of(make_pair(_pos, _end), " \t"); +} + +string::const_iterator firstWhitespaceOrNewline( + string::const_iterator _pos, + string::const_iterator _end +) +{ + return boost::range::find_first_of(make_pair(_pos, _end), " \t\n"); +} + + +string::const_iterator skipWhitespace( + string::const_iterator _pos, + string::const_iterator _end +) +{ + auto currPos = _pos; + while (currPos != _end && (*currPos == ' ' || *currPos == '\t')) + currPos += 1; + return currPos; +} + } bool DocStringParser::parse(string const& _docString, ErrorList& _errors) @@ -43,7 +68,7 @@ bool DocStringParser::parse(string const& _docString, ErrorList& _errors) if (tagPos != end && tagPos < nlPos) { // we found a tag - auto tagNameEndPos = firstSpaceOrNl(tagPos, end); + auto tagNameEndPos = firstWhitespaceOrNewline(tagPos, end); if (tagNameEndPos == end) { appendError("End of tag " + string(tagPos, tagNameEndPos) + "not found"); @@ -75,27 +100,40 @@ DocStringParser::iter DocStringParser::parseDocTagLine(iter _pos, iter _end, boo { solAssert(!!m_lastTag, ""); auto nlPos = find(_pos, _end, '\n'); - if (_appending && _pos < _end && *_pos != ' ') + if (_appending && _pos < _end && *_pos != ' ' && *_pos != '\t') m_lastTag->content += " "; + else if (!_appending) + _pos = skipWhitespace(_pos, _end); copy(_pos, nlPos, back_inserter(m_lastTag->content)); return skipLineOrEOS(nlPos, _end); } DocStringParser::iter DocStringParser::parseDocTagParam(iter _pos, iter _end) { - // find param name - auto currPos = find(_pos, _end, ' '); - if (currPos == _end) + // find param name start + auto nameStartPos = skipWhitespace(_pos, _end); + if (nameStartPos == _end) { - appendError("End of param name not found" + string(_pos, _end)); + appendError("No param name given"); return _end; } + auto nameEndPos = firstSpaceOrTab(nameStartPos, _end); + if (nameEndPos == _end) + { + appendError("End of param name not found: " + string(nameStartPos, _end)); + return _end; + } + auto paramName = string(nameStartPos, nameEndPos); - auto paramName = string(_pos, currPos); + auto descStartPos = skipWhitespace(nameEndPos, _end); + if (descStartPos == _end) + { + appendError("No description given for param " + paramName); + return _end; + } - currPos += 1; - auto nlPos = find(currPos, _end, '\n'); - auto paramDesc = string(currPos, nlPos); + auto nlPos = find(descStartPos, _end, '\n'); + auto paramDesc = string(descStartPos, nlPos); newTag("param"); m_lastTag->paramName = paramName; m_lastTag->content = paramDesc; diff --git a/scripts/install_deps.sh b/scripts/install_deps.sh index 255176ab..f21c48d0 100755 --- a/scripts/install_deps.sh +++ b/scripts/install_deps.sh @@ -138,11 +138,13 @@ case $(uname -s) in # All our dependencies can be found in the Arch Linux official repositories. # See https://wiki.archlinux.org/index.php/Official_repositories + # Also adding ethereum-git to allow for testing with the `eth` client sudo pacman -Sy \ base-devel \ boost \ cmake \ git \ + ethereum-git \ ;; #------------------------------------------------------------------------------ diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index b6067ea5..472067ec 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1102,25 +1102,25 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) BOOST_REQUIRE((contract = retrieveContract(source, 0)) != nullptr); FunctionTypePointer function = retrieveFunctionBySignature(*contract, "foo()"); BOOST_REQUIRE(function && function->hasDeclaration()); - auto returnParams = function->returnParameterTypeNames(false); - BOOST_CHECK_EQUAL(returnParams.at(0), "uint256"); + auto returnParams = function->returnParameterTypes(); + BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(false), "uint256"); BOOST_CHECK(function->isConstant()); function = retrieveFunctionBySignature(*contract, "map(uint256)"); BOOST_REQUIRE(function && function->hasDeclaration()); - auto params = function->parameterTypeNames(false); - BOOST_CHECK_EQUAL(params.at(0), "uint256"); - returnParams = function->returnParameterTypeNames(false); - BOOST_CHECK_EQUAL(returnParams.at(0), "bytes4"); + auto params = function->parameterTypes(); + BOOST_CHECK_EQUAL(params.at(0)->canonicalName(false), "uint256"); + returnParams = function->returnParameterTypes(); + BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(false), "bytes4"); BOOST_CHECK(function->isConstant()); function = retrieveFunctionBySignature(*contract, "multiple_map(uint256,uint256)"); BOOST_REQUIRE(function && function->hasDeclaration()); - params = function->parameterTypeNames(false); - BOOST_CHECK_EQUAL(params.at(0), "uint256"); - BOOST_CHECK_EQUAL(params.at(1), "uint256"); - returnParams = function->returnParameterTypeNames(false); - BOOST_CHECK_EQUAL(returnParams.at(0), "bytes4"); + params = function->parameterTypes(); + BOOST_CHECK_EQUAL(params.at(0)->canonicalName(false), "uint256"); + BOOST_CHECK_EQUAL(params.at(1)->canonicalName(false), "uint256"); + returnParams = function->returnParameterTypes(); + BOOST_CHECK_EQUAL(returnParams.at(0)->canonicalName(false), "bytes4"); BOOST_CHECK(function->isConstant()); } diff --git a/test/libsolidity/SolidityNatspecJSON.cpp b/test/libsolidity/SolidityNatspecJSON.cpp index e32264c4..ac55382b 100644 --- a/test/libsolidity/SolidityNatspecJSON.cpp +++ b/test/libsolidity/SolidityNatspecJSON.cpp @@ -56,7 +56,7 @@ public: m_reader.parse(_expectedDocumentationString, expectedDocumentation); BOOST_CHECK_MESSAGE( expectedDocumentation == generatedDocumentation, - "Expected " << expectedDocumentation.toStyledString() << + "Expected:\n" << expectedDocumentation.toStyledString() << "\n but got:\n" << generatedDocumentation.toStyledString() ); } @@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(dev_desc_after_nl) char const* natspec = "{" "\"methods\":{" " \"mul(uint256,uint256)\":{ \n" - " \"details\": \" Multiplies a number by 7 and adds second parameter\",\n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" " \"params\": {\n" " \"a\": \"Documentation for the first parameter\",\n" " \"second\": \"Documentation for the second parameter\"\n" @@ -251,6 +251,29 @@ BOOST_AUTO_TEST_CASE(dev_multiple_params) checkNatspec(sourceCode, natspec, false); } +BOOST_AUTO_TEST_CASE(dev_multiple_params_mixed_whitespace) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter\n" + " /// @param second Documentation for the second parameter\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul(uint256,uint256)\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " }\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + BOOST_AUTO_TEST_CASE(dev_mutiline_param_description) { char const* sourceCode = R"( @@ -379,7 +402,7 @@ BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl) " \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n" " \"second\": \"Documentation for the second parameter\"\n" " },\n" - " \"return\": \" The result of the multiplication\"\n" + " \"return\": \"The result of the multiplication\"\n" " }\n" "}}"; @@ -612,6 +635,48 @@ BOOST_AUTO_TEST_CASE(dev_documenting_nonexistent_param) expectNatspecError(sourceCode); } +BOOST_AUTO_TEST_CASE(dev_documenting_no_paramname) +{ + char const* sourceCode = R"( + contract test { + /// @dev Multiplies a number by 7 and adds second parameter + /// @param a Documentation for the first parameter + /// @param + function mul(uint a, uint second) returns(uint d) { return a * 7 + second; } + } + )"; + + expectNatspecError(sourceCode); +} + +BOOST_AUTO_TEST_CASE(dev_documenting_no_paramname_end) +{ + char const* sourceCode = R"( + contract test { + /// @dev Multiplies a number by 7 and adds second parameter + /// @param a Documentation for the first parameter + /// @param se + function mul(uint a, uint second) returns(uint d) { return a * 7 + second; } + } + )"; + + expectNatspecError(sourceCode); +} + +BOOST_AUTO_TEST_CASE(dev_documenting_no_param_description) +{ + char const* sourceCode = R"( + contract test { + /// @dev Multiplies a number by 7 and adds second parameter + /// @param a Documentation for the first parameter + /// @param second + function mul(uint a, uint second) returns(uint d) { return a * 7 + second; } + } + )"; + + expectNatspecError(sourceCode); +} + BOOST_AUTO_TEST_SUITE_END() } |