Merge pull request #3613 from ethereum/require-visibility

Issue error if no visibility is specified (on 0.5.0)

Changelog.md

@@ -8,6 +8,7 @@ Features:
  * Support and recommend using ``emit EventName();`` to call events explicitly.
  * Syntax Analyser: Do not warn about experimental features if they do not concern code generation.
  * Syntax Checker: Mark ``throw`` as an error as experimental 0.5.0 feature.
+ * Syntax Checker: Issue error if no visibility is specified on contract functions as experimental 0.5.0 feature.
 
 Bugfixes:
  * Assembly: Raise error on oversized number literals in assembly.

libsolidity/analysis/SyntaxChecker.cpp

@@ -212,13 +212,20 @@ bool SyntaxChecker::visit(PlaceholderStatement const&)
 
 bool SyntaxChecker::visit(FunctionDefinition const& _function)
 {
+	bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050);
+
 	if (_function.noVisibilitySpecified())
-		m_errorReporter.warning(
-			_function.location(),
-			"No visibility specified. Defaulting to \"" +
-			Declaration::visibilityToString(_function.visibility()) +
-			"\"."
-		);
+	{
+		if (v050)
+			m_errorReporter.syntaxError(_function.location(), "No visibility specified.");
+		else
+			m_errorReporter.warning(
+				_function.location(),
+				"No visibility specified. Defaulting to \"" +
+				Declaration::visibilityToString(_function.visibility()) +
+				"\"."
+			);
+	}
 	return true;
 }
 

test/libsolidity/SolidityNameAndTypeResolution.cpp

@@ -7974,7 +7974,7 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract)
 	char const* text = R"(
 		pragma experimental "v0.5.0";
 		contract C {
-			function f() {
+			function f() public {
 				this.balance;
 			}
 		}
@@ -7983,7 +7983,7 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract)
 	text = R"(
 		pragma experimental "v0.5.0";
 		contract C {
-			function f() {
+			function f() public {
 				this.transfer;
 			}
 		}
@@ -7992,7 +7992,7 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract)
 	text = R"(
 		pragma experimental "v0.5.0";
 		contract C {
-			function f() {
+			function f() public {
 				this.send;
 			}
 		}
@@ -8001,7 +8001,7 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract)
 	text = R"(
 		pragma experimental "v0.5.0";
 		contract C {
-			function f() {
+			function f() public {
 				this.call;
 			}
 		}
@@ -8010,7 +8010,7 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract)
 	text = R"(
 		pragma experimental "v0.5.0";
 		contract C {
-			function f() {
+			function f() public {
 				this.callcode;
 			}
 		}
@@ -8019,7 +8019,7 @@ BOOST_AUTO_TEST_CASE(no_address_members_on_contract)
 	text = R"(
 		pragma experimental "v0.5.0";
 		contract C {
-			function f() {
+			function f() public {
 				this.delegatecall;
 			}
 		}
@@ -8103,6 +8103,23 @@ BOOST_AUTO_TEST_CASE(getter_is_memory_type)
 	}
 }
 
+BOOST_AUTO_TEST_CASE(require_visibility_specifiers)
+{
+	char const* text = R"(
+		contract C {
+			function f() pure { }
+		}
+	)";
+	CHECK_WARNING(text, "No visibility specified. Defaulting to");
+	text = R"(
+		pragma experimental "v0.5.0";
+		contract C {
+			function f() pure { }
+		}
+	)";
+	CHECK_ERROR(text, SyntaxError, "No visibility specified.");
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }

test/libsolidity/ViewPureChecker.cpp

@@ -148,7 +148,7 @@ BOOST_AUTO_TEST_CASE(environment_access)
 BOOST_AUTO_TEST_CASE(view_error_for_050)
 {
 	CHECK_ERROR(
-		"pragma experimental \"v0.5.0\"; contract C { uint x; function f() view { x = 2; } }",
+		"pragma experimental \"v0.5.0\"; contract C { uint x; function f() view public { x = 2; } }",
 		TypeError,
 		"Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable."
 	);