diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index c734fa38c..5b5682ff1 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -166,9 +166,12 @@ static inline std::string::const_iterator skipLineOrEOS(std::string::const_itera std::string::const_iterator InterfaceHandler::parseDocTagLine(std::string::const_iterator _pos, std::string::const_iterator _end, std::string& _tagString, - DocTagType _tagType) + DocTagType _tagType, + bool _appending) { auto nlPos = std::find(_pos, _end, '\n'); + if (_appending && _pos < _end && *_pos != ' ') + _tagString += " "; std::copy(_pos, nlPos, back_inserter(_tagString)); m_lastTag = _tagType; return skipLineOrEOS(nlPos, _end); @@ -201,7 +204,8 @@ std::string::const_iterator InterfaceHandler::appendDocTagParam(std::string::con solAssert(!m_params.empty(), "Internal: Tried to append to empty parameter"); auto pair = m_params.back(); - pair.second += " "; + if (_pos < _end && *_pos != ' ') + pair.second += " "; auto nlPos = std::find(_pos, _end, '\n'); std::copy(_pos, nlPos, back_inserter(pair.second)); @@ -221,17 +225,17 @@ std::string::const_iterator InterfaceHandler::parseDocTag(std::string::const_ite if (m_lastTag == DocTagType::NONE || _tag != "") { if (_tag == "dev") - return parseDocTagLine(_pos, _end, m_dev, DocTagType::DEV); + return parseDocTagLine(_pos, _end, m_dev, DocTagType::DEV, false); else if (_tag == "notice") - return parseDocTagLine(_pos, _end, m_notice, DocTagType::NOTICE); + return parseDocTagLine(_pos, _end, m_notice, DocTagType::NOTICE, false); else if (_tag == "return") - return parseDocTagLine(_pos, _end, m_return, DocTagType::RETURN); + return parseDocTagLine(_pos, _end, m_return, DocTagType::RETURN, false); else if (_tag == "author") { if (_owner == CommentOwner::CONTRACT) - return parseDocTagLine(_pos, _end, m_contractAuthor, DocTagType::AUTHOR); + return parseDocTagLine(_pos, _end, m_contractAuthor, DocTagType::AUTHOR, false); else if (_owner == CommentOwner::FUNCTION) - return parseDocTagLine(_pos, _end, m_author, DocTagType::AUTHOR); + return parseDocTagLine(_pos, _end, m_author, DocTagType::AUTHOR, false); else // LTODO: for now this else makes no sense but later comments will go to more language constructs BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("@author tag is legal only for contracts")); @@ -239,7 +243,7 @@ std::string::const_iterator InterfaceHandler::parseDocTag(std::string::const_ite else if (_tag == "title") { if (_owner == CommentOwner::CONTRACT) - return parseDocTagLine(_pos, _end, m_title, DocTagType::TITLE); + return parseDocTagLine(_pos, _end, m_title, DocTagType::TITLE, false); else // LTODO: Unknown tag, throw some form of warning and not just an exception BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("@title tag is legal only for contracts")); @@ -261,34 +265,22 @@ std::string::const_iterator InterfaceHandler::appendDocTag(std::string::const_it switch (m_lastTag) { case DocTagType::DEV: - m_dev += " "; - return parseDocTagLine(_pos, _end, m_dev, DocTagType::DEV); + return parseDocTagLine(_pos, _end, m_dev, DocTagType::DEV, true); case DocTagType::NOTICE: - m_notice += " "; - return parseDocTagLine(_pos, _end, m_notice, DocTagType::NOTICE); + return parseDocTagLine(_pos, _end, m_notice, DocTagType::NOTICE, true); case DocTagType::RETURN: - m_return += " "; - return parseDocTagLine(_pos, _end, m_return, DocTagType::RETURN); + return parseDocTagLine(_pos, _end, m_return, DocTagType::RETURN, true); case DocTagType::AUTHOR: if (_owner == CommentOwner::CONTRACT) - { - m_contractAuthor += " "; - return parseDocTagLine(_pos, _end, m_contractAuthor, DocTagType::AUTHOR); - } + return parseDocTagLine(_pos, _end, m_contractAuthor, DocTagType::AUTHOR, true); else if (_owner == CommentOwner::FUNCTION) - { - m_author += " "; - return parseDocTagLine(_pos, _end, m_author, DocTagType::AUTHOR); - } + return parseDocTagLine(_pos, _end, m_author, DocTagType::AUTHOR, true); else // LTODO: Unknown tag, throw some form of warning and not just an exception BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("@author tag in illegal comment")); case DocTagType::TITLE: if (_owner == CommentOwner::CONTRACT) - { - m_title += " "; - return parseDocTagLine(_pos, _end, m_title, DocTagType::TITLE); - } + return parseDocTagLine(_pos, _end, m_title, DocTagType::TITLE, true); else // LTODO: Unknown tag, throw some form of warning and not just an exception BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("@title tag in illegal comment")); @@ -329,7 +321,7 @@ void InterfaceHandler::parseDocString(std::string const& _string, CommentOwner _ currPos = parseDocTag(tagNameEndPos + 1, end, std::string(tagPos + 1, tagNameEndPos), _owner); } else if (m_lastTag != DocTagType::NONE) // continuation of the previous tag - currPos = appendDocTag(currPos + 1, end, _owner); + currPos = appendDocTag(currPos, end, _owner); else if (currPos != end) // skip the line if a newline was found currPos = nlPos + 1; } diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index d271a6697..c8399d71f 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -92,7 +92,8 @@ private: std::string::const_iterator parseDocTagLine(std::string::const_iterator _pos, std::string::const_iterator _end, std::string& _tagString, - DocTagType _tagType); + DocTagType _tagType, + bool _appending); std::string::const_iterator parseDocTagParam(std::string::const_iterator _pos, std::string::const_iterator _end); std::string::const_iterator appendDocTagParam(std::string::const_iterator _pos, diff --git a/libsolidity/Scanner.cpp b/libsolidity/Scanner.cpp index 1a21149a1..f22e69bc8 100644 --- a/libsolidity/Scanner.cpp +++ b/libsolidity/Scanner.cpp @@ -80,7 +80,7 @@ bool isLineTerminator(char c) } bool isWhiteSpace(char c) { - return c == ' ' || c == '\n' || c == '\t'; + return c == ' ' || c == '\n' || c == '\t' || c == '\r'; } bool isIdentifierStart(char c) { @@ -209,6 +209,15 @@ bool Scanner::skipWhitespace() return getSourcePos() != startPosition; } +bool Scanner::skipWhitespaceExceptLF() +{ + int const startPosition = getSourcePos(); + while (isWhiteSpace(m_char) && !isLineTerminator(m_char)) + advance(); + // Return whether or not we skipped any characters. + return getSourcePos() != startPosition; +} + Token::Value Scanner::skipSingleLineComment() { // The line terminator at the end of the line is not considered @@ -219,10 +228,11 @@ Token::Value Scanner::skipSingleLineComment() return Token::WHITESPACE; } -Token::Value Scanner::scanDocumentationComment() +Token::Value Scanner::scanSingleLineDocComment() { LiteralScope literal(this, LITERAL_TYPE_COMMENT); - advance(); //consume the last '/' + advance(); //consume the last '/' at /// + skipWhitespaceExceptLF(); while (!isSourcePastEndOfInput()) { if (isLineTerminator(m_char)) @@ -250,7 +260,6 @@ Token::Value Scanner::scanDocumentationComment() Token::Value Scanner::skipMultiLineComment() { - solAssert(m_char == '*', ""); advance(); while (!isSourcePastEndOfInput()) { @@ -270,6 +279,97 @@ Token::Value Scanner::skipMultiLineComment() return Token::ILLEGAL; } +Token::Value Scanner::scanMultiLineDocComment() +{ + LiteralScope literal(this, LITERAL_TYPE_COMMENT); + bool endFound = false; + bool charsAdded = false; + + advance(); //consume the last '*' at /** + skipWhitespaceExceptLF(); + while (!isSourcePastEndOfInput()) + { + //handle newlines in multline comments + if (isLineTerminator(m_char)) + { + skipWhitespace(); + if (!m_source.isPastEndOfInput(1) && m_source.get(0) == '*' && m_source.get(1) != '/') + { // skip first '*' in subsequent lines + if (charsAdded) + addCommentLiteralChar('\n'); + m_char = m_source.advanceAndGet(2); + } + else if (!m_source.isPastEndOfInput(1) && m_source.get(0) == '*' && m_source.get(1) == '/') + { // if after newline the comment ends, don't insert the newline + m_char = m_source.advanceAndGet(2); + endFound = true; + break; + } + else if (charsAdded) + addCommentLiteralChar('\n'); + } + + if (!m_source.isPastEndOfInput(1) && m_source.get(0) == '*' && m_source.get(1) == '/') + { + m_char = m_source.advanceAndGet(2); + endFound = true; + break; + } + addCommentLiteralChar(m_char); + charsAdded = true; + advance(); + } + literal.complete(); + if (!endFound) + return Token::ILLEGAL; + else + return Token::COMMENT_LITERAL; +} + +Token::Value Scanner::scanSlash() +{ + int firstSlashPosition = getSourcePos(); + advance(); + if (m_char == '/') + { + if (!advance()) /* double slash comment directly before EOS */ + return Token::WHITESPACE; + else if (m_char == '/') + { + // doxygen style /// comment + Token::Value comment; + m_nextSkippedComment.location.start = firstSlashPosition; + comment = scanSingleLineDocComment(); + m_nextSkippedComment.location.end = getSourcePos(); + m_nextSkippedComment.token = comment; + return Token::WHITESPACE; + } + else + return skipSingleLineComment(); + } + else if (m_char == '*') + { + // doxygen style /** natspec comment + if (!advance()) /* slash star comment before EOS */ + return Token::WHITESPACE; + else if (m_char == '*') + { + Token::Value comment; + m_nextSkippedComment.location.start = firstSlashPosition; + comment = scanMultiLineDocComment(); + m_nextSkippedComment.location.end = getSourcePos(); + m_nextSkippedComment.token = comment; + return Token::WHITESPACE; + } + else + return skipMultiLineComment(); + } + else if (m_char == '=') + return selectToken(Token::ASSIGN_DIV); + else + return Token::DIV; +} + void Scanner::scanToken() { m_nextToken.literal.clear(); @@ -372,29 +472,7 @@ void Scanner::scanToken() break; case '/': // / // /* /= - advance(); - if (m_char == '/') - { - if (!advance()) /* double slash comment directly before EOS */ - token = Token::WHITESPACE; - else if (m_char == '/') - { - Token::Value comment; - m_nextSkippedComment.location.start = getSourcePos(); - comment = scanDocumentationComment(); - m_nextSkippedComment.location.end = getSourcePos(); - m_nextSkippedComment.token = comment; - token = Token::WHITESPACE; - } - else - token = skipSingleLineComment(); - } - else if (m_char == '*') - token = skipMultiLineComment(); - else if (m_char == '=') - token = selectToken(Token::ASSIGN_DIV); - else - token = Token::DIV; + token = scanSlash(); break; case '&': // & && &= diff --git a/libsolidity/Scanner.h b/libsolidity/Scanner.h index 18b1f5d3a..5b90a94eb 100644 --- a/libsolidity/Scanner.h +++ b/libsolidity/Scanner.h @@ -182,6 +182,8 @@ private: /// Skips all whitespace and @returns true if something was skipped. bool skipWhitespace(); + /// Skips all whitespace except Line feeds and returns true if something was skipped + bool skipWhitespaceExceptLF(); Token::Value skipSingleLineComment(); Token::Value skipMultiLineComment(); @@ -190,7 +192,10 @@ private: Token::Value scanIdentifierOrKeyword(); Token::Value scanString(); - Token::Value scanDocumentationComment(); + Token::Value scanSingleLineDocComment(); + Token::Value scanMultiLineDocComment(); + /// Scans a slash '/' and depending on the characters returns the appropriate token + Token::Value scanSlash(); /// Scans an escape-sequence which is part of a string and adds the /// decoded character to the current literal. Returns true if a pattern diff --git a/test/SolidityNatspecJSON.cpp b/test/SolidityNatspecJSON.cpp index 2c3ded08d..5b48a67ce 100644 --- a/test/SolidityNatspecJSON.cpp +++ b/test/SolidityNatspecJSON.cpp @@ -394,6 +394,35 @@ BOOST_AUTO_TEST_CASE(dev_multiline_return) checkNatspec(sourceCode, natspec, false); } +BOOST_AUTO_TEST_CASE(dev_multiline_comment) +{ + char const* sourceCode = "contract test {\n" + " /**\n" + " * @dev Multiplies a number by 7 and adds second parameter\n" + " * @param a Documentation for the first parameter starts here.\n" + " * Since it's a really complicated parameter we need 2 lines\n" + " * @param second Documentation for the second parameter\n" + " * @return The result of the multiplication\n" + " * and cookies with nutella\n" + " */" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"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 and cookies with nutella\"\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + BOOST_AUTO_TEST_CASE(dev_contract_no_doc) { char const* sourceCode = "contract test {\n" diff --git a/test/SolidityScanner.cpp b/test/SolidityScanner.cpp index 573affe6d..355ea9e22 100644 --- a/test/SolidityScanner.cpp +++ b/test/SolidityScanner.cpp @@ -157,7 +157,14 @@ BOOST_AUTO_TEST_CASE(documentation_comments_parsed_begin) { Scanner scanner(CharStream("/// Send $(value / 1000) chocolates to the user")); BOOST_CHECK_EQUAL(scanner.getCurrentToken(), Token::EOS); - BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), " Send $(value / 1000) chocolates to the user"); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); +} + +BOOST_AUTO_TEST_CASE(multiline_documentation_comments_parsed_begin) +{ + Scanner scanner(CharStream("/** Send $(value / 1000) chocolates to the user*/")); + BOOST_CHECK_EQUAL(scanner.getCurrentToken(), Token::EOS); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); } BOOST_AUTO_TEST_CASE(documentation_comments_parsed) @@ -167,7 +174,43 @@ BOOST_AUTO_TEST_CASE(documentation_comments_parsed) BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); BOOST_CHECK_EQUAL(scanner.next(), Token::EOS); - BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), " Send $(value / 1000) chocolates to the user"); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); +} + +BOOST_AUTO_TEST_CASE(multiline_documentation_comments_parsed) +{ + Scanner scanner(CharStream("some other tokens /**\n" + "* Send $(value / 1000) chocolates to the user\n" + "*/")); + BOOST_CHECK_EQUAL(scanner.getCurrentToken(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::EOS); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); +} + +BOOST_AUTO_TEST_CASE(multiline_documentation_no_stars) +{ + Scanner scanner(CharStream("some other tokens /**\n" + " Send $(value / 1000) chocolates to the user\n" + "*/")); + BOOST_CHECK_EQUAL(scanner.getCurrentToken(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::EOS); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); +} + +BOOST_AUTO_TEST_CASE(multiline_documentation_whitespace_hell) +{ + Scanner scanner(CharStream("some other tokens /** \t \r \n" + "\t \r * Send $(value / 1000) chocolates to the user\n" + "*/")); + BOOST_CHECK_EQUAL(scanner.getCurrentToken(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::IDENTIFIER); + BOOST_CHECK_EQUAL(scanner.next(), Token::EOS); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), "Send $(value / 1000) chocolates to the user"); } BOOST_AUTO_TEST_CASE(comment_before_eos) @@ -184,6 +227,13 @@ BOOST_AUTO_TEST_CASE(documentation_comment_before_eos) BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), ""); } +BOOST_AUTO_TEST_CASE(empty_multiline_documentation_comment_before_eos) +{ + Scanner scanner(CharStream("/***/")); + BOOST_CHECK_EQUAL(scanner.getCurrentToken(), Token::EOS); + BOOST_CHECK_EQUAL(scanner.getCurrentCommentLiteral(), ""); +} + BOOST_AUTO_TEST_CASE(comments_mixed_in_sequence) { Scanner scanner(CharStream("hello_world ///documentation comment \n"