Merge pull request #5416 from ethereum/develop

Merge develop into release for 0.5.0

1 files changed, 101 insertions, 69 deletions

diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp
index 396058f4..3f9f8373 100644
--- a/libsolidity/analysis/SyntaxChecker.cpp
+++ b/libsolidity/analysis/SyntaxChecker.cpp
@@ -22,6 +22,10 @@
 #include <libsolidity/analysis/SemVerHandler.h>
 #include <libsolidity/interface/ErrorReporter.h>
 #include <libsolidity/interface/Version.h>
+#include <boost/algorithm/cxx11/all_of.hpp>
+
+#include <boost/algorithm/string.hpp>
+#include <string>
 
 using namespace std;
 using namespace dev;
@@ -49,7 +53,7 @@ void SyntaxChecker::endVisit(SourceUnit const& _sourceUnit)
 		SemVerVersion recommendedVersion{string(VersionString)};
 		if (!recommendedVersion.isPrerelease())
 			errorString +=
-				"Consider adding \"pragma solidity ^" +
+				" Consider adding \"pragma solidity ^" +
 				to_string(recommendedVersion.major()) +
 				string(".") +
 				to_string(recommendedVersion.minor()) +
@@ -72,7 +76,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma)
 	{
 		solAssert(m_sourceUnit, "");
 		vector<string> literals(_pragma.literals().begin() + 1, _pragma.literals().end());
-		if (literals.size() == 0)
+		if (literals.empty())
 			m_errorReporter.syntaxError(
 				_pragma.location(),
 				"Experimental feature name is missing."
@@ -102,7 +106,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma)
 	}
 	else if (_pragma.literals()[0] == "solidity")
 	{
-		vector<Token::Value> tokens(_pragma.tokens().begin() + 1, _pragma.tokens().end());
+		vector<Token> tokens(_pragma.tokens().begin() + 1, _pragma.tokens().end());
 		vector<string> literals(_pragma.literals().begin() + 1, _pragma.literals().end());
 		SemVerMatchExpressionParser parser(tokens, literals);
 		auto matchExpression = parser.parse();
@@ -134,9 +138,25 @@ void SyntaxChecker::endVisit(ModifierDefinition const& _modifier)
 	m_placeholderFound = false;
 }
 
-bool SyntaxChecker::visit(WhileStatement const&)
+void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const& _statement)
+{
+	auto varDecl = dynamic_cast<VariableDeclarationStatement const*>(&_statement);
+	if (varDecl)
+		m_errorReporter.syntaxError(_statement.location(), "Variable declarations can only be used inside blocks.");
+}
+
+bool SyntaxChecker::visit(IfStatement const& _ifStatement)
+{
+	checkSingleStatementVariableDeclaration(_ifStatement.trueStatement());
+	if (Statement const* _statement = _ifStatement.falseStatement())
+		checkSingleStatementVariableDeclaration(*_statement);
+	return true;
+}
+
+bool SyntaxChecker::visit(WhileStatement const& _whileStatement)
 {
 	m_inLoopDepth++;
+	checkSingleStatementVariableDeclaration(_whileStatement.body());
 	return true;
 }
 
@@ -145,9 +165,10 @@ void SyntaxChecker::endVisit(WhileStatement const&)
 	m_inLoopDepth--;
 }
 
-bool SyntaxChecker::visit(ForStatement const&)
+bool SyntaxChecker::visit(ForStatement const& _forStatement)
 {
 	m_inLoopDepth++;
+	checkSingleStatementVariableDeclaration(_forStatement.body());
 	return true;
 }
 
@@ -174,33 +195,58 @@ bool SyntaxChecker::visit(Break const& _breakStatement)
 
 bool SyntaxChecker::visit(Throw const& _throwStatement)
 {
-	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
-
-	if (v050)
-		m_errorReporter.syntaxError(
-			_throwStatement.location(),
-			"\"throw\" is deprecated in favour of \"revert()\", \"require()\" and \"assert()\"."
-		);
-	else
-		m_errorReporter.warning(
-			_throwStatement.location(),
-			"\"throw\" is deprecated in favour of \"revert()\", \"require()\" and \"assert()\"."
-		);
+	m_errorReporter.syntaxError(
+		_throwStatement.location(),
+		"\"throw\" is deprecated in favour of \"revert()\", \"require()\" and \"assert()\"."
+	);
 
 	return true;
 }
 
-bool SyntaxChecker::visit(UnaryOperation const& _operation)
+bool SyntaxChecker::visit(Literal const& _literal)
 {
-	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
+	if (_literal.token() != Token::Number)
+		return true;
 
-	if (_operation.getOperator() == Token::Add)
+	ASTString const& value = _literal.value();
+	solAssert(!value.empty(), "");
+
+	// Generic checks no matter what base this number literal is of:
+	if (value.back() == '_')
 	{
-		if (v050)
-			m_errorReporter.syntaxError(_operation.location(), "Use of unary + is deprecated.");
-		else
-			m_errorReporter.warning(_operation.location(), "Use of unary + is deprecated.");
+		m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No trailing underscores allowed.");
+		return true;
+	}
+
+	if (value.find("__") != ASTString::npos)
+	{
+		m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. Only one consecutive underscores between digits allowed.");
+		return true;
+	}
+
+	if (!_literal.isHexNumber()) // decimal literal
+	{
+		if (value.find("._") != ASTString::npos)
+			m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscores in front of the fraction part allowed.");
+
+		if (value.find("_.") != ASTString::npos)
+			m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscores in front of the fraction part allowed.");
+
+		if (value.find("_e") != ASTString::npos)
+			m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscore at the end of the mantissa allowed.");
+
+		if (value.find("e_") != ASTString::npos)
+			m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscore in front of exponent allowed.");
 	}
+
+	return true;
+}
+
+bool SyntaxChecker::visit(UnaryOperation const& _operation)
+{
+	if (_operation.getOperator() == Token::Add)
+		m_errorReporter.syntaxError(_operation.location(), "Use of unary + is disallowed.");
+
 	return true;
 }
 
@@ -210,40 +256,34 @@ bool SyntaxChecker::visit(PlaceholderStatement const&)
 	return true;
 }
 
-bool SyntaxChecker::visit(FunctionDefinition const& _function)
+bool SyntaxChecker::visit(ContractDefinition const& _contract)
 {
-	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
-
-	if (v050 && _function.noVisibilitySpecified())
-		m_errorReporter.syntaxError(_function.location(), "No visibility specified.");
+	m_isInterface = _contract.contractKind() == ContractDefinition::ContractKind::Interface;
 
-	if (_function.isOldStyleConstructor())
-	{
-		if (v050)
-			m_errorReporter.syntaxError(
-				_function.location(),
+	ASTString const& contractName = _contract.name();
+	for (FunctionDefinition const* function: _contract.definedFunctions())
+		if (function->name() == contractName)
+			m_errorReporter.syntaxError(function->location(),
 				"Functions are not allowed to have the same name as the contract. "
 				"If you intend this to be a constructor, use \"constructor(...) { ... }\" to define it."
 			);
-		else
-			m_errorReporter.warning(
-				_function.location(),
-				"Defining constructors as functions with the same name as the contract is deprecated. "
-				"Use \"constructor(...) { ... }\" instead."
-			);
-	}
-	if (!_function.isImplemented() && !_function.modifiers().empty())
+	return true;
+}
+
+bool SyntaxChecker::visit(FunctionDefinition const& _function)
+{
+	if (_function.noVisibilitySpecified())
 	{
-		if (v050)
-			m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers.");
-		else
-			m_errorReporter.warning(_function.location(), "Modifiers of functions without implementation are ignored." );
-	}
-	if (_function.name() == "constructor")
-		m_errorReporter.warning(_function.location(),
-			"This function is named \"constructor\" but is not the constructor of the contract. "
-			"If you intend this to be a constructor, use \"constructor(...) { ... }\" without the \"function\" keyword to define it."
+		string suggestedVisibility = _function.isFallback() || m_isInterface ? "external" : "public";
+		m_errorReporter.syntaxError(
+			_function.location(),
+			"No visibility specified. Did you intend to add \"" + suggestedVisibility + "\"?"
 		);
+	}
+
+	if (!_function.isImplemented() && !_function.modifiers().empty())
+		m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers.");
+
 	return true;
 }
 
@@ -255,35 +295,27 @@ bool SyntaxChecker::visit(FunctionTypeName const& _node)
 
 	for (auto const& decl: _node.returnParameterTypeList()->parameters())
 		if (!decl->name().empty())
-			m_errorReporter.warning(decl->location(), "Naming function type return parameters is deprecated.");
+			m_errorReporter.syntaxError(decl->location(), "Return parameters in function types may not be named.");
 
 	return true;
 }
 
-bool SyntaxChecker::visit(VariableDeclaration const& _declaration)
+bool SyntaxChecker::visit(VariableDeclarationStatement const& _statement)
 {
-	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
+	// Report if none of the variable components in the tuple have a name (only possible via deprecated "var")
+	if (boost::algorithm::all_of_equal(_statement.declarations(), nullptr))
+		m_errorReporter.syntaxError(
+			_statement.location(),
+			"The use of the \"var\" keyword is disallowed. The declaration part of the statement can be removed, since it is empty."
+		);
 
-	if (!_declaration.typeName())
-	{
-		if (v050)
-			m_errorReporter.syntaxError(_declaration.location(), "Use of the \"var\" keyword is deprecated.");
-		else
-			m_errorReporter.warning(_declaration.location(), "Use of the \"var\" keyword is deprecated.");
-	}
 	return true;
 }
 
 bool SyntaxChecker::visit(StructDefinition const& _struct)
 {
-	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
-
 	if (_struct.members().empty())
-	{
-		if (v050)
-			m_errorReporter.syntaxError(_struct.location(), "Defining empty structs is disallowed.");
-		else
-			m_errorReporter.warning(_struct.location(), "Defining empty structs is deprecated.");
-	}
+		m_errorReporter.syntaxError(_struct.location(), "Defining empty structs is disallowed.");
+
 	return true;
 }