Merge pull request #1637 from ethereum/warn-shadowing-globals

Warn if shadowing built-ins

2 files changed, 146 insertions, 26 deletions

diff --git a/test/libsolidity/Imports.cpp b/test/libsolidity/Imports.cpp
index 6aa96fb8..00f093b7 100644
--- a/test/libsolidity/Imports.cpp
+++ b/test/libsolidity/Imports.cpp
@@ -20,11 +20,15 @@
  * Tests for high level features like import.
  */
 
-#include <string>
-#include <boost/test/unit_test.hpp>
+#include <test/libsolidity/ErrorCheck.h>
+
 #include <libsolidity/interface/Exceptions.h>
 #include <libsolidity/interface/CompilerStack.h>
 
+#include <boost/test/unit_test.hpp>
+
+#include <string>
+
 using namespace std;
 
 namespace dev
@@ -202,6 +206,67 @@ BOOST_AUTO_TEST_CASE(context_dependent_remappings_order_independent)
 	BOOST_CHECK(d.compile());
 }
 
+BOOST_AUTO_TEST_CASE(shadowing_via_import)
+{
+	CompilerStack c;
+	c.addSource("a", "library A {} pragma solidity >=0.0;");
+	c.addSource("b", "library A {} pragma solidity >=0.0;");
+	c.addSource("c", "import {A} from \"./a\"; import {A} from \"./b\";");
+	BOOST_CHECK(!c.compile());
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_imports)
+{
+	CompilerStack c;
+	c.addSource("B.sol", "contract X {} pragma solidity >=0.0;");
+	c.addSource("b", R"(
+		pragma solidity >=0.0;
+		import * as msg from "B.sol";
+		contract C {
+		}
+	)");
+	BOOST_CHECK(c.compile());
+	auto numErrors = c.errors().size();
+	// Sometimes we get the prerelease warning, sometimes not.
+	BOOST_CHECK(2 <= numErrors && numErrors <= 3);
+	for (auto const& e: c.errors())
+	{
+		string const* msg = e->comment();
+		BOOST_REQUIRE(msg);
+		BOOST_CHECK(
+			msg->find("pre-release") != string::npos ||
+			msg->find("shadows a builtin symbol") != string::npos
+		);
+	}
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_multiple_imports)
+{
+	CompilerStack c;
+	c.addSource("B.sol", "contract msg {} contract block{} pragma solidity >=0.0;");
+	c.addSource("b", R"(
+		pragma solidity >=0.0;
+		import {msg, block} from "B.sol";
+		contract C {
+		}
+	)");
+	BOOST_CHECK(c.compile());
+	auto numErrors = c.errors().size();
+	// Sometimes we get the prerelease warning, sometimes not.
+	BOOST_CHECK(4 <= numErrors && numErrors <= 5);
+	for (auto const& e: c.errors())
+	{
+		string const* msg = e->comment();
+		BOOST_REQUIRE(msg);
+		BOOST_CHECK(
+			msg->find("pre-release") != string::npos ||
+			msg->find("shadows a builtin symbol") != string::npos
+		);
+	}
+}
+
+
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 4b29243a..48fe4d24 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -5498,22 +5498,6 @@ BOOST_AUTO_TEST_CASE(does_not_warn_msg_value_in_library)
 	CHECK_SUCCESS_NO_WARNINGS(text);
 }
 
-BOOST_AUTO_TEST_CASE(does_not_warn_non_magic_msg_value)
-{
-	char const* text = R"(
-		contract C {
-			struct msg {
-				uint256 value;
-			}
-
-			function f() {
-				msg.value;
-			}
-		}
-	)";
-	CHECK_SUCCESS_NO_WARNINGS(text);
-}
-
 BOOST_AUTO_TEST_CASE(does_not_warn_msg_value_in_modifier_following_non_payable_public_function)
 {
 	char const* text = R"(
@@ -6127,6 +6111,85 @@ BOOST_AUTO_TEST_CASE(no_unused_inline_asm)
 	CHECK_SUCCESS_NO_WARNINGS(text);
 }
 
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_functions)
+{
+	char const* text = R"(
+		contract C {
+			function keccak256() {}
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_variables)
+{
+	char const* text = R"(
+		contract C {
+			function f() {
+				uint msg;
+				msg;
+			}
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_parameters)
+{
+	char const* text = R"(
+		contract C {
+			function f(uint require) {
+				require = 2;
+			}
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_return_parameters)
+{
+	char const* text = R"(
+		contract C {
+			function f() returns (uint require) {
+				require = 2;
+			}
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_events)
+{
+	char const* text = R"(
+		contract C {
+			event keccak256();
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_ignores_struct)
+{
+	char const* text = R"(
+		contract C {
+			struct a {
+				uint msg;
+			}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtins_ignores_constructor)
+{
+	char const* text = R"(
+		contract C {
+			function C() {}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
 BOOST_AUTO_TEST_CASE(callable_crash)
 {
 	char const* text = R"(
@@ -6245,14 +6308,6 @@ BOOST_AUTO_TEST_CASE(create2_as_variable)
 	CHECK_WARNING_ALLOW_MULTI(text, "Variable is shadowed in inline assembly by an instruction of the same name");
 }
 
-BOOST_AUTO_TEST_CASE(shadowing_warning_can_be_removed)
-{
-	char const* text = R"(
-		contract C {function f() {assembly {}}}
-	)";
-	CHECK_SUCCESS_NO_WARNINGS(text);
-}
-
 BOOST_AUTO_TEST_CASE(warn_unspecified_storage)
 {
 	char const* text = R"(