Merge pull request #1451 from LefterisJP/fix_build

Accept any kind of whitespace after natspec tags

3 files changed, 123 insertions, 19 deletions

diff --git a/Changelog.md b/Changelog.md
index 71a1e096..7c09407a 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -14,6 +14,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/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/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()
 
 }