Merge pull request #8800 from ethereum/natspec-bugfix

[BREAKING] NatSpec bugfix

Changelog.md

@@ -12,6 +12,8 @@ Compiler Features:
 Bugfixes:
  * Optimizer: Fixed a bug in BlockDeDuplicator.
  * Type Checker: Disallow assignments to storage variables of type ``mapping``.
+ * NatSpec: DocString block is terminated when encountering an empty line.
+ * Scanner: Fix bug when two empty NatSpec comments lead to scanning past EOL.
 
 
 ### 0.6.8 (2020-05-14)

liblangutil/Scanner.cpp

@@ -267,10 +267,13 @@ bool Scanner::skipWhitespace()
 	return sourcePos() != startPosition;
 }
 
-void Scanner::skipWhitespaceExceptUnicodeLinebreak()
+bool Scanner::skipWhitespaceExceptUnicodeLinebreak()
 {
+	int const startPosition = sourcePos();
 	while (isWhiteSpace(m_char) && !isUnicodeLinebreak())
 		advance();
+	// Return whether or not we skipped any characters.
+	return sourcePos() != startPosition;
 }
 
 Token Scanner::skipSingleLineComment()
@@ -321,7 +324,7 @@ int Scanner::scanSingleLineDocComment()
 		{
 			// Check if next line is also a single-line comment.
 			// If any whitespaces were skipped, use source position before.
-			if (!skipWhitespace())
+			if (!skipWhitespaceExceptUnicodeLinebreak())
 				endPosition = m_source->position();
 
 			if (!m_source->isPastEndOfInput(3) &&
@@ -329,8 +332,10 @@ int Scanner::scanSingleLineDocComment()
 				m_source->get(1) == '/' &&
 				m_source->get(2) == '/')
 			{
-				addCommentLiteralChar('\n');
 				m_char = m_source->advanceAndGet(3);
+				if (atEndOfLine())
+					continue;
+				addCommentLiteralChar('\n');
 			}
 			else
 				break; // next line is not a documentation comment, we are done
@@ -389,9 +394,11 @@ Token Scanner::scanMultiLineDocComment()
 			}
 			else if (!m_source->isPastEndOfInput(1) && m_source->get(0) == '*' && m_source->get(1) != '/')
 			{ // skip first '*' in subsequent lines
+				m_char = m_source->advanceAndGet(1);
+				if (atEndOfLine()) // ignores empty lines
+					continue;
 				if (charsAdded)
-					addCommentLiteralChar('\n');
+					addCommentLiteralChar('\n'); // corresponds to the end of previous line
-				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

liblangutil/Scanner.h

@@ -214,7 +214,7 @@ private:
 	/// Skips all whitespace and @returns true if something was skipped.
 	bool skipWhitespace();
 	/// Skips all whitespace that are neither '\r' nor '\n'.
-	void skipWhitespaceExceptUnicodeLinebreak();
+	bool skipWhitespaceExceptUnicodeLinebreak();
 	Token skipSingleLineComment();
 	Token skipMultiLineComment();
 

test/libsolidity/SolidityNatspecJSON.cpp

@@ -78,6 +78,81 @@ private:
 
 BOOST_FIXTURE_TEST_SUITE(SolidityNatspecJSON, DocumentationChecker)
 
+BOOST_AUTO_TEST_CASE(user_empty_natspec_test)
+{
+	char const* sourceCode = R"(
+		contract test {
+			///
+			///
+			function f() public {
+			}
+		}
+	)";
+
+	char const* natspec = R"(
+	{
+		"methods": {}
+	}
+	)";
+
+	checkNatspec(sourceCode, "test", natspec, true);
+}
+
+BOOST_AUTO_TEST_CASE(user_newline_break)
+{
+	char const* sourceCode = R"(
+		contract test {
+			///
+			/// @notice hello
+
+			/// @notice world
+			function f() public {
+			}
+		}
+	)";
+
+	char const* natspec = R"ABCDEF(
+	{
+		"methods": {
+			"f()":
+			{
+			"notice": "world"
+			}
+		}
+	}
+	)ABCDEF";
+
+	checkNatspec(sourceCode, "test", natspec, true);
+}
+
+BOOST_AUTO_TEST_CASE(user_multiline_empty_lines)
+{
+	char const* sourceCode = R"(
+	contract test {
+		/**
+		 *
+		 *
+		 * @notice hello world
+		 */
+		function f() public {
+		}
+	}
+	)";
+
+	char const* natspec = R"ABCDEF(
+	{
+		"methods": {
+			"f()": {
+				"notice": "hello world"
+			}
+		}
+	}
+	)ABCDEF";
+
+	checkNatspec(sourceCode, "test", natspec, true);
+}
+
+
 BOOST_AUTO_TEST_CASE(user_basic_test)
 {
 	char const* sourceCode = R"(

test/libsolidity/SolidityScanner.cpp

@@ -361,7 +361,7 @@ BOOST_AUTO_TEST_CASE(multiline_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.currentCommentLiteral(), "Send $(value / 1000) chocolates to the user");
+	BOOST_CHECK_EQUAL(scanner.currentCommentLiteral(), " Send $(value / 1000) chocolates to the user");
 }
 
 BOOST_AUTO_TEST_CASE(multiline_documentation_no_stars)
@@ -385,7 +385,7 @@ BOOST_AUTO_TEST_CASE(multiline_documentation_whitespace_hell)
 	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.currentCommentLiteral(), "Send $(value / 1000) chocolates to the user");
+	BOOST_CHECK_EQUAL(scanner.currentCommentLiteral(), " Send $(value / 1000) chocolates to the user");
 }
 
 BOOST_AUTO_TEST_CASE(comment_before_eos)

test/libsolidity/syntaxTests/natspec/docstring_double_empty.sol

@@ -0,0 +1,7 @@
+contract C {
+  ///
+  ///
+  function vote(uint id) public {
+  }
+}
+// ----