Merge pull request #3808 from ethereum/parserTests

SyntaxTests: extend syntax tests and isoltest to support parser error…

8 files changed, 113 insertions, 145 deletions

diff --git a/test/libsolidity/AnalysisFramework.cpp b/test/libsolidity/AnalysisFramework.cpp
index 4538757d..72b86767 100644
--- a/test/libsolidity/AnalysisFramework.cpp
+++ b/test/libsolidity/AnalysisFramework.cpp
@@ -56,12 +56,23 @@ AnalysisFramework::parseAnalyseAndReturnError(
 
 	m_compiler.analyze();
 
+	ErrorList errors = filterErrors(m_compiler.errors(), _reportWarnings);
+	if (errors.size() > 1 && !_allowMultipleErrors)
+		BOOST_FAIL("Multiple errors found: " + formatErrors());
+
+	return make_pair(&m_compiler.ast(""), std::move(errors));
+}
+
+ErrorList AnalysisFramework::filterErrors(ErrorList const& _errorList, bool _includeWarnings) const
+{
 	ErrorList errors;
-	for (auto const& currentError: m_compiler.errors())
+	for (auto const& currentError: _errorList)
 	{
 		solAssert(currentError->comment(), "");
 		if (currentError->type() == Error::Type::Warning)
 		{
+			if (!_includeWarnings)
+				continue;
 			bool ignoreWarning = false;
 			for (auto const& filter: m_warningsToFilter)
 				if (currentError->comment()->find(filter) == 0)
@@ -73,17 +84,10 @@ AnalysisFramework::parseAnalyseAndReturnError(
 				continue;
 		}
 
-		if (_reportWarnings || (currentError->type() != Error::Type::Warning))
-		{
-			if (!_allowMultipleErrors && !errors.empty())
-			{
-				BOOST_FAIL("Multiple errors found: " + formatErrors());
-			}
-			errors.emplace_back(std::move(currentError));
-		}
+		errors.emplace_back(currentError);
 	}
 
-	return make_pair(&m_compiler.ast(""), errors);
+	return errors;
 }
 
 SourceUnit const* AnalysisFramework::parseAndAnalyse(string const& _source)
@@ -110,7 +114,7 @@ ErrorList AnalysisFramework::expectError(std::string const& _source, bool _warni
 	return sourceAndErrors.second;
 }
 
-string AnalysisFramework::formatErrors()
+string AnalysisFramework::formatErrors() const
 {
 	string message;
 	for (auto const& error: m_compiler.errors())
@@ -118,7 +122,7 @@ string AnalysisFramework::formatErrors()
 	return message;
 }
 
-string AnalysisFramework::formatError(Error const& _error)
+string AnalysisFramework::formatError(Error const& _error) const
 {
 	return SourceReferenceFormatter::formatExceptionInformation(
 			_error,
diff --git a/test/libsolidity/AnalysisFramework.h b/test/libsolidity/AnalysisFramework.h
index 6ecf4a5a..05490a42 100644
--- a/test/libsolidity/AnalysisFramework.h
+++ b/test/libsolidity/AnalysisFramework.h
@@ -57,8 +57,8 @@ protected:
 	bool success(std::string const& _source);
 	ErrorList expectError(std::string const& _source, bool _warning = false, bool _allowMultiple = false);
 
-	std::string formatErrors();
-	std::string formatError(Error const& _error);
+	std::string formatErrors() const;
+	std::string formatError(Error const& _error) const;
 
 	static ContractDefinition const* retrieveContractByName(SourceUnit const& _source, std::string const& _name);
 	static FunctionTypePointer retrieveFunctionBySignature(
@@ -66,6 +66,9 @@ protected:
 		std::string const& _signature
 	);
 
+	// filter out the warnings in m_warningsToFilter or all warnings if _includeWarnings is false
+	ErrorList filterErrors(ErrorList const& _errorList, bool _includeWarnings) const;
+
 	std::vector<std::string> m_warningsToFilter = {"This is a pre-release compiler version"};
 	dev::solidity::CompilerStack m_compiler;
 };
diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp
index 4e862f60..93e6bcaa 100644
--- a/test/libsolidity/SolidityParser.cpp
+++ b/test/libsolidity/SolidityParser.cpp
@@ -112,26 +112,6 @@ while(0)
 
 BOOST_AUTO_TEST_SUITE(SolidityParser)
 
-BOOST_AUTO_TEST_CASE(smoke_test)
-{
-	char const* text = R"(
-		contract test {
-			uint256 stateVariable1;
-		}
-	)";
-	BOOST_CHECK(successParse(text));
-}
-
-BOOST_AUTO_TEST_CASE(missing_variable_name_in_declaration)
-{
-	char const* text = R"(
-		contract test {
-			uint256 ;
-		}
-	)";
-	CHECK_PARSE_ERROR(text, "Expected identifier");
-}
-
 BOOST_AUTO_TEST_CASE(empty_function)
 {
 	char const* text = R"(
diff --git a/test/libsolidity/SyntaxTest.cpp b/test/libsolidity/SyntaxTest.cpp
index ca051138..329543bf 100644
--- a/test/libsolidity/SyntaxTest.cpp
+++ b/test/libsolidity/SyntaxTest.cpp
@@ -16,6 +16,7 @@
 */
 
 #include <test/libsolidity/SyntaxTest.h>
+#include <test/Options.h>
 #include <boost/algorithm/string.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/throw_exception.hpp>
@@ -59,93 +60,52 @@ SyntaxTest::SyntaxTest(string const& _filename)
 
 bool SyntaxTest::run(ostream& _stream, string const& _linePrefix, bool const _formatted)
 {
-	m_errorList = parseAnalyseAndReturnError(m_source, true, true, true).second;
-	if (!matchesExpectations(m_errorList))
+	m_compiler.reset();
+	m_compiler.addSource("", "pragma solidity >=0.0;\n" + m_source);
+	m_compiler.setEVMVersion(dev::test::Options::get().evmVersion());
+
+	if (m_compiler.parse())
+		m_compiler.analyze();
+
+	for (auto const& currentError: filterErrors(m_compiler.errors(), true))
+		m_errorList.emplace_back(SyntaxTestError{currentError->typeName(), errorMessage(*currentError)});
+
+	if (m_expectations != m_errorList)
 	{
-		std::string nextIndentLevel = _linePrefix + "  ";
+		string nextIndentLevel = _linePrefix + "  ";
 		FormattedScope(_stream, _formatted, {BOLD, CYAN}) << _linePrefix << "Expected result:" << endl;
-		printExpected(_stream, nextIndentLevel, _formatted);
+		printErrorList(_stream, m_expectations, nextIndentLevel, _formatted);
 		FormattedScope(_stream, _formatted, {BOLD, CYAN}) << _linePrefix << "Obtained result:\n";
-		printErrorList(_stream, m_errorList, nextIndentLevel, false, false, _formatted);
+		printErrorList(_stream, m_errorList, nextIndentLevel, _formatted);
 		return false;
 	}
 	return true;
 }
 
-void SyntaxTest::printExpected(ostream& _stream, string const& _linePrefix, bool const _formatted) const
-{
-	if (m_expectations.empty())
-		FormattedScope(_stream, _formatted, {BOLD, GREEN}) << _linePrefix << "Success" << endl;
-	else
-		for (auto const& expectation: m_expectations)
-		{
-			FormattedScope(_stream, _formatted, {BOLD, expectation.type == "Warning" ? YELLOW : RED}) <<
-				_linePrefix << expectation.type << ": ";
-			_stream << expectation.message << endl;
-		}
-}
-
 void SyntaxTest::printErrorList(
 	ostream& _stream,
-	ErrorList const& _errorList,
+	vector<SyntaxTestError> const& _errorList,
 	string const& _linePrefix,
-	bool const _ignoreWarnings,
-	bool const _lineNumbers,
 	bool const _formatted
-) const
+)
 {
 	if (_errorList.empty())
 		FormattedScope(_stream, _formatted, {BOLD, GREEN}) << _linePrefix << "Success" << endl;
 	else
 		for (auto const& error: _errorList)
 		{
-			bool isWarning = (error->type() == Error::Type::Warning);
-			if (isWarning && _ignoreWarnings) continue;
-
 			{
-				FormattedScope scope(_stream, _formatted, {BOLD, isWarning ? YELLOW : RED});
+				FormattedScope scope(_stream, _formatted, {BOLD, (error.type == "Warning") ? YELLOW : RED});
 				_stream << _linePrefix;
-				if (_lineNumbers)
-				{
-					int line = offsetToLineNumber(
-						boost::get_error_info<errinfo_sourceLocation>(*error)->start
-					);
-					if (line >= 0)
-						_stream << "(" << line << "): ";
-				}
-				_stream << error->typeName() << ": ";
+				_stream << error.type << ": ";
 			}
-			_stream << errorMessage(*error) << endl;
+			_stream << error.message << endl;
 		}
 }
 
-int SyntaxTest::offsetToLineNumber(int _location) const
-{
-	// parseAnalyseAndReturnError(...) prepends a version pragma
-	_location -= strlen("pragma solidity >=0.0;\n");
-	if (_location < 0 || static_cast<size_t>(_location) >= m_source.size())
-		return -1;
-	else
-		return 1 + std::count(m_source.begin(), m_source.begin() + _location, '\n');
-}
-
-bool SyntaxTest::matchesExpectations(ErrorList const& _errorList) const
-{
-	if (_errorList.size() != m_expectations.size())
-		return false;
-	else
-		for (size_t i = 0; i < _errorList.size(); i++)
-			if (
-				(_errorList[i]->typeName() != m_expectations[i].type) ||
-				(errorMessage(*_errorList[i]) != m_expectations[i].message)
-			)
-				return false;
-	return true;
-}
-
-string SyntaxTest::errorMessage(Error const& _e)
+string SyntaxTest::errorMessage(Exception const& _e)
 {
-	if (_e.comment())
+	if (_e.comment() && !_e.comment()->empty())
 		return boost::replace_all_copy(*_e.comment(), "\n", "\\n");
 	else
 		return "NONE";
@@ -164,9 +124,9 @@ string SyntaxTest::parseSource(istream& _stream)
 	return source;
 }
 
-vector<SyntaxTestExpectation> SyntaxTest::parseExpectations(istream& _stream)
+vector<SyntaxTestError> SyntaxTest::parseExpectations(istream& _stream)
 {
-	vector<SyntaxTestExpectation> expectations;
+	vector<SyntaxTestError> expectations;
 	string line;
 	while (getline(_stream, line))
 	{
@@ -188,7 +148,7 @@ vector<SyntaxTestExpectation> SyntaxTest::parseExpectations(istream& _stream)
 		skipWhitespace(it, line.end());
 
 		string errorMessage(it, line.end());
-		expectations.emplace_back(SyntaxTestExpectation{move(errorType), move(errorMessage)});
+		expectations.emplace_back(SyntaxTestError{move(errorType), move(errorMessage)});
 	}
 	return expectations;
 }
@@ -239,9 +199,11 @@ int SyntaxTest::registerTests(
 		_suite.add(make_test_case(
 			[fullpath]
 			{
-				std::stringstream errorStream;
-				if (!SyntaxTest(fullpath.string()).run(errorStream))
-					BOOST_ERROR("Test expectation mismatch.\n" + errorStream.str());
+				BOOST_REQUIRE_NO_THROW({
+					stringstream errorStream;
+					if (!SyntaxTest(fullpath.string()).run(errorStream))
+						BOOST_ERROR("Test expectation mismatch.\n" + errorStream.str());
+				});
 			},
 			_path.stem().string(),
 			*filenames.back(),
diff --git a/test/libsolidity/SyntaxTest.h b/test/libsolidity/SyntaxTest.h
index cb6ee05c..dddd86ef 100644
--- a/test/libsolidity/SyntaxTest.h
+++ b/test/libsolidity/SyntaxTest.h
@@ -36,10 +36,14 @@ namespace solidity
 namespace test
 {
 
-struct SyntaxTestExpectation
+struct SyntaxTestError
 {
 	std::string type;
 	std::string message;
+	bool operator==(SyntaxTestError const& _rhs) const
+	{
+		return type == _rhs.type && message == _rhs.message;
+	}
 };
 
 
@@ -50,21 +54,16 @@ public:
 
 	bool run(std::ostream& _stream, std::string const& _linePrefix = "", bool const _formatted = false);
 
-	std::vector<SyntaxTestExpectation> const& expectations() const { return m_expectations; }
+	std::vector<SyntaxTestError> const& expectations() const { return m_expectations; }
 	std::string const& source() const { return m_source; }
-	ErrorList const& errorList() const { return m_errorList; }
-	ErrorList const& compilerErrors() const { return m_compiler.errors(); }
+	std::vector<SyntaxTestError> const& errorList() const { return m_errorList; }
 
-	void printExpected(std::ostream& _stream, std::string const& _linePrefix, bool const _formatted = false) const;
-
-	void printErrorList(
+	static void printErrorList(
 		std::ostream& _stream,
-		ErrorList const& _errors,
+		std::vector<SyntaxTestError> const& _errors,
 		std::string const& _linePrefix,
-		bool const _ignoreWarnings,
-		bool const _lineNumbers,
 		bool const _formatted = false
-	) const;
+	);
 
 	static int registerTests(
 		boost::unit_test::test_suite& _suite,
@@ -72,16 +71,14 @@ public:
 		boost::filesystem::path const& _path
 	);
 	static bool isTestFilename(boost::filesystem::path const& _filename);
+	static std::string errorMessage(Exception const& _e);
 private:
-	bool matchesExpectations(ErrorList const& _errors) const;
-	static std::string errorMessage(Error const& _e);
 	static std::string parseSource(std::istream& _stream);
-	static std::vector<SyntaxTestExpectation> parseExpectations(std::istream& _stream);
-	int offsetToLineNumber(int _location) const;
+	static std::vector<SyntaxTestError> parseExpectations(std::istream& _stream);
 
 	std::string m_source;
-	std::vector<SyntaxTestExpectation> m_expectations;
-	ErrorList m_errorList;
+	std::vector<SyntaxTestError> m_expectations;
+	std::vector<SyntaxTestError> m_errorList;
 };
 
 }
diff --git a/test/libsolidity/syntaxTests/parsing/missing_variable_name_in_declaration.sol b/test/libsolidity/syntaxTests/parsing/missing_variable_name_in_declaration.sol
new file mode 100644
index 00000000..c03fd97d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/parsing/missing_variable_name_in_declaration.sol
@@ -0,0 +1,5 @@
+contract test {
+    uint256 ;
+}
+// ----
+// ParserError: Expected identifier, got 'Semicolon'
diff --git a/test/libsolidity/syntaxTests/parsing/smoke_test.sol b/test/libsolidity/syntaxTests/parsing/smoke_test.sol
new file mode 100644
index 00000000..d328b167
--- /dev/null
+++ b/test/libsolidity/syntaxTests/parsing/smoke_test.sol
@@ -0,0 +1,4 @@
+contract test {
+    uint256 stateVariable1;
+}
+// ----
diff --git a/test/tools/isoltest.cpp b/test/tools/isoltest.cpp
index 5efec421..07df8f60 100644
--- a/test/tools/isoltest.cpp
+++ b/test/tools/isoltest.cpp
@@ -55,8 +55,8 @@ public:
 	{
 		Success,
 		Failure,
-		ParserError,
-		InputOutputError
+		InputOutputError,
+		Exception
 	};
 
 	Result process();
@@ -76,7 +76,7 @@ private:
 		Quit
 	};
 
-	Request handleResponse(bool const _parserError);
+	Request handleResponse(bool const _exception);
 
 	void printContract() const;
 
@@ -100,7 +100,6 @@ void SyntaxTestTool::printContract() const
 SyntaxTestTool::Result SyntaxTestTool::process()
 {
 	bool success;
-	bool parserError = false;
 	std::stringstream outputMessages;
 
 	(FormattedScope(cout, m_formatted, {BOLD}) << m_name << ": ").flush();
@@ -119,10 +118,35 @@ SyntaxTestTool::Result SyntaxTestTool::process()
 	{
 		success = m_test->run(outputMessages, "  ", m_formatted);
 	}
-	catch (...)
+	catch(CompilerError const& _e)
 	{
-		success = false;
-		parserError = true;
+		FormattedScope(cout, m_formatted, {BOLD, RED}) <<
+			"Exception: " << SyntaxTest::errorMessage(_e) << endl;
+		return Result::Exception;
+	}
+	catch(InternalCompilerError const& _e)
+	{
+		FormattedScope(cout, m_formatted, {BOLD, RED}) <<
+			"InternalCompilerError: " << SyntaxTest::errorMessage(_e) << endl;
+		return Result::Exception;
+	}
+	catch(FatalError const& _e)
+	{
+		FormattedScope(cout, m_formatted, {BOLD, RED}) <<
+			"FatalError: " << SyntaxTest::errorMessage(_e) << endl;
+		return Result::Exception;
+	}
+	catch(UnimplementedFeatureError const& _e)
+	{
+		FormattedScope(cout, m_formatted, {BOLD, RED}) <<
+			"UnimplementedFeatureError: " << SyntaxTest::errorMessage(_e) << endl;
+		return Result::Exception;
+	}
+	catch(...)
+	{
+		FormattedScope(cout, m_formatted, {BOLD, RED}) <<
+			"Unknown Exception" << endl;
+		return Result::Exception;
 	}
 
 	if (success)
@@ -137,25 +161,14 @@ SyntaxTestTool::Result SyntaxTestTool::process()
 		FormattedScope(cout, m_formatted, {BOLD, CYAN}) << "  Contract:" << endl;
 		printContract();
 
-		if (parserError)
-		{
-			cout << "  ";
-			FormattedScope(cout, m_formatted, {INVERSE, RED}) << "Parsing failed:" << endl;
-			m_test->printErrorList(cout, m_test->compilerErrors(), "    ", true, true, m_formatted);
-			cout << endl;
-			return Result::ParserError;
-		}
-		else
-		{
-			cout << outputMessages.str() << endl;
-			return Result::Failure;
-		}
+		cout << outputMessages.str() << endl;
+		return Result::Failure;
 	}
 }
 
-SyntaxTestTool::Request SyntaxTestTool::handleResponse(bool const _parserError)
+SyntaxTestTool::Request SyntaxTestTool::handleResponse(bool const _exception)
 {
-	if (_parserError)
+	if (_exception)
 		cout << "(e)dit/(s)kip/(q)uit? ";
 	else
 		cout << "(e)dit/(u)pdate expectations/(s)kip/(q)uit? ";
@@ -169,7 +182,7 @@ SyntaxTestTool::Request SyntaxTestTool::handleResponse(bool const _parserError)
 			cout << endl;
 			return Request::Skip;
 		case 'u':
-			if (_parserError)
+			if (_exception)
 				break;
 			else
 			{
@@ -178,7 +191,7 @@ SyntaxTestTool::Request SyntaxTestTool::handleResponse(bool const _parserError)
 				file << m_test->source();
 				file << "// ----" << endl;
 				if (!m_test->errorList().empty())
-					m_test->printErrorList(file, m_test->errorList(), "// ", false, false, false);
+					m_test->printErrorList(file, m_test->errorList(), "// ", false);
 				return Request::Rerun;
 			}
 		case 'e':
@@ -231,8 +244,8 @@ SyntaxTestStats SyntaxTestTool::processPath(
 			switch(result)
 			{
 			case Result::Failure:
-			case Result::ParserError:
-				switch(testTool.handleResponse(result == Result::ParserError))
+			case Result::Exception:
+				switch(testTool.handleResponse(result == Result::Exception))
 				{
 				case Request::Quit:
 					return { successCount, runCount };