Merge pull request #2451 from ethereum/warnVarInLoop

Warn var in loop

4 files changed, 100 insertions, 12 deletions

diff --git a/Changelog.md b/Changelog.md
index 3d8701ca..c12afcd2 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -5,6 +5,7 @@ Features:
  * Assembly: Display auxiliary data in the assembly output.
  * Assembly: Add ``CREATE2`` (EIP86), ``STATICCALL`` (EIP214), ``RETURNDATASIZE`` and ``RETURNDATACOPY`` (EIP211) instructions.
  * AST: export all attributes to JSON format.
+ * Type Checker: Warn about type inference from literal numbers.
  * C API (``jsonCompiler``): Use the Standard JSON I/O internally.
  * Inline Assembly: Present proper error message when not supplying enough arguments to a functional
    instruction.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index ef8a9345..b276a2d4 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -956,6 +956,38 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 					var.location(),
 					"Cannot declare variable with void (empty tuple) type."
 				);
+			else if (valueComponentType->category() == Type::Category::RationalNumber)
+			{
+				string typeName = var.annotation().type->toString(true);
+				string extension;
+				if (auto type = dynamic_cast<IntegerType const*>(var.annotation().type.get()))
+				{
+					int numBits = type->numBits();
+					bool isSigned = type->isSigned();
+					string minValue;
+					string maxValue;
+					if (isSigned)
+					{
+						numBits--;
+						minValue = "-" + bigint(bigint(1) << numBits).str();
+					}
+					else
+						minValue = "0";
+					maxValue = bigint((bigint(1) << numBits) - 1).str();
+					extension = ", which can hold values between " + minValue + " and " + maxValue;
+				}
+				else
+					solAssert(dynamic_cast<FixedPointType const*>(var.annotation().type.get()), "Unknown type.");
+
+				m_errorReporter.warning(
+					_statement.location(),
+					"The type of this variable was inferred as " +
+					typeName +
+					extension +
+					". This is probably not desired. Use an explicit type to silence this warning."
+				);
+			}
+
 			var.accept(*this);
 		}
 		else
diff --git a/test/libsolidity/ErrorCheck.cpp b/test/libsolidity/ErrorCheck.cpp
index 75555c9b..9b0f9fb7 100644
--- a/test/libsolidity/ErrorCheck.cpp
+++ b/test/libsolidity/ErrorCheck.cpp
@@ -29,6 +29,15 @@ using namespace std;
 bool dev::solidity::searchErrorMessage(Error const& _err, std::string const& _substr)
 {
 	if (string const* errorMessage = boost::get_error_info<dev::errinfo_comment>(_err))
-		return errorMessage->find(_substr) != std::string::npos;
+	{
+		if (errorMessage->find(_substr) == std::string::npos)
+		{
+			cout << "Expected message \"" << _substr << "\" but found" << *errorMessage << endl;
+			return false;
+		}
+		return true;
+	}
+	else
+		cout << "Expected error message but found none." << endl;
 	return _substr.empty();
 }
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index c8a04539..169b33d1 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -103,16 +103,22 @@ parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false,
 		if (success)
 			if (!StaticAnalyzer(errorReporter).analyze(*sourceUnit))
 				success = false;
-		if (errorReporter.errors().size() > 1 && !_allowMultipleErrors)
-			BOOST_FAIL("Multiple errors found");
+		std::shared_ptr<Error const> error;
 		for (auto const& currentError: errorReporter.errors())
 		{
 			if (
 				(_reportWarnings && currentError->type() == Error::Type::Warning) ||
 				(!_reportWarnings && currentError->type() != Error::Type::Warning)
 			)
-				return make_pair(sourceUnit, currentError);
+			{
+				if (error && !_allowMultipleErrors)
+					BOOST_FAIL("Multiple errors found");
+				if (!error)
+					error = currentError;
+			}
 		}
+		if (error)
+			return make_pair(sourceUnit, error);
 	}
 	catch (InternalCompilerError const& _e)
 	{
@@ -1142,7 +1148,7 @@ BOOST_AUTO_TEST_CASE(modifier_overrides_function)
 	)";
 	// Error: Identifier already declared.
 	// Error: Override changes modifier to function.
-	CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, "");
+	CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, "Identifier already declared");
 }
 
 BOOST_AUTO_TEST_CASE(function_overrides_modifier)
@@ -1781,6 +1787,46 @@ BOOST_AUTO_TEST_CASE(exp_warn_literal_base)
 	CHECK_SUCCESS(sourceCode);
 }
 
+
+BOOST_AUTO_TEST_CASE(warn_var_from_zero)
+{
+	char const* sourceCode = R"(
+		contract test {
+			function f() returns (uint) {
+				var i = 1;
+				return i;
+			}
+		}
+	)";
+	CHECK_WARNING(sourceCode, "uint8, which can hold values between 0 and 255");
+	sourceCode = R"(
+		contract test {
+			function f() {
+				var i = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
+				i;
+			}
+		}
+	)";
+	CHECK_WARNING(sourceCode, "uint256, which can hold values between 0 and 115792089237316195423570985008687907853269984665640564039457584007913129639935");
+	sourceCode = R"(
+		contract test {
+			function f() {
+				var i = -2;
+				i;
+			}
+		}
+	)";
+	CHECK_WARNING(sourceCode, "int8, which can hold values between -128 and 127");
+	sourceCode = R"(
+		 contract test {
+			 function f() {
+				 for (var i = 0; i < msg.data.length; i++) { }
+			 }
+		 }
+	)";
+	CHECK_WARNING(sourceCode, "uint8, which can hold");
+}
+
 BOOST_AUTO_TEST_CASE(enum_member_access)
 {
 	char const* text = R"(
@@ -3175,9 +3221,9 @@ BOOST_AUTO_TEST_CASE(tuples)
 		contract C {
 			function f() {
 				uint a = (1);
-				var (b,) = (1,);
-				var (c,d) = (1, 2 + a);
-				var (e,) = (1, 2, b);
+				var (b,) = (uint8(1),);
+				var (c,d) = (uint32(1), 2 + a);
+				var (e,) = (uint64(1), 2, b);
 				a;b;c;d;e;
 			}
 		}
@@ -5518,7 +5564,7 @@ BOOST_AUTO_TEST_CASE(invalid_address_checksum)
 	char const* text = R"(
 		contract C {
 			function f() {
-				var x = 0xFA0bFc97E48458494Ccd857e1A85DC91F7F0046E;
+				address x = 0xFA0bFc97E48458494Ccd857e1A85DC91F7F0046E;
 				x;
 			}
 		}
@@ -5531,7 +5577,7 @@ BOOST_AUTO_TEST_CASE(invalid_address_no_checksum)
 	char const* text = R"(
 		contract C {
 			function f() {
-				var x = 0xfa0bfc97e48458494ccd857e1a85dc91f7f0046e;
+				address x = 0xfa0bfc97e48458494ccd857e1a85dc91f7f0046e;
 				x;
 			}
 		}
@@ -5544,7 +5590,7 @@ BOOST_AUTO_TEST_CASE(invalid_address_length)
 	char const* text = R"(
 		contract C {
 			function f() {
-				var x = 0xA0bFc97E48458494Ccd857e1A85DC91F7F0046E;
+				address x = 0xA0bFc97E48458494Ccd857e1A85DC91F7F0046E;
 				x;
 			}
 		}
@@ -5927,7 +5973,7 @@ BOOST_AUTO_TEST_CASE(warn_unused_local_assigned)
 	char const* text = R"(
 		contract C {
 			function f() {
-				var a = 1;
+				uint a = 1;
 			}
 		}
 	)";