Merge pull request #5539 from ethereum/fix_deprecated_sha3_suicide

Report deprecation error on functions sha3 and suicide also without call

Changelog.md

@@ -22,6 +22,7 @@ Bugfixes:
  * Type Checker: Properly detect different return types when overriding an external interface function with a public contract function.
  * Type Checker: Disallow struct return types for getters of public state variables unless the new ABI encoder is active.
  * Type Checker: Fix internal compiler error when a field of a struct used as a parameter in a function type has a non-existent type.
+ * Type Checker: Disallow functions ``sha3`` and ``suicide`` also without a function call.
 
 Build System:
  * Emscripten: Upgrade to Emscripten SDK 1.37.21 and boost 1.67.

libsolidity/analysis/TypeChecker.cpp

@@ -1824,26 +1824,6 @@ void TypeChecker::typeCheckFunctionCall(
 			"\"staticcall\" is not supported by the VM version."
 		);
 
-	// Check for deprecated function names
-	if (_functionType->kind() == FunctionType::Kind::KECCAK256)
-	{
-		if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression()))
-			if (functionName->name() == "sha3")
-				m_errorReporter.typeError(
-					_functionCall.location(),
-					"\"sha3\" has been deprecated in favour of \"keccak256\""
-				);
-	}
-	else if (_functionType->kind() == FunctionType::Kind::Selfdestruct)
-	{
-		if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression()))
-			if (functionName->name() == "suicide")
-				m_errorReporter.typeError(
-					_functionCall.location(),
-					"\"suicide\" has been deprecated in favour of \"selfdestruct\""
-				);
-	}
-
 	// Check for event outside of emit statement
 	if (!m_insideEmitStatement && _functionType->kind() == FunctionType::Kind::Event)
 		m_errorReporter.typeError(
@@ -2639,6 +2619,23 @@ bool TypeChecker::visit(Identifier const& _identifier)
 	else if (dynamic_cast<MagicVariableDeclaration const*>(annotation.referencedDeclaration))
 		if (dynamic_cast<FunctionType const*>(annotation.type.get()))
 			annotation.isPure = true;
+
+	// Check for deprecated function names.
+	// The check is done here for the case without an actual function call.
+	if (FunctionType const* fType = dynamic_cast<FunctionType const*>(_identifier.annotation().type.get()))
+	{
+		if (_identifier.name() == "sha3" && fType->kind() == FunctionType::Kind::KECCAK256)
+			m_errorReporter.typeError(
+				_identifier.location(),
+				"\"sha3\" has been deprecated in favour of \"keccak256\""
+			);
+		else if (_identifier.name() == "suicide" && fType->kind() == FunctionType::Kind::Selfdestruct)
+			m_errorReporter.typeError(
+				_identifier.location(),
+				"\"suicide\" has been deprecated in favour of \"selfdestruct\""
+			);
+	}
+
 	return false;
 }
 

test/libsolidity/syntaxTests/deprecated_functions.sol

@@ -8,5 +8,5 @@ contract test {
 	}
 }
 // ----
-// TypeError: (58-66): "sha3" has been deprecated in favour of "keccak256"
+// TypeError: (58-62): "sha3" has been deprecated in favour of "keccak256"
-// TypeError: (101-152): "suicide" has been deprecated in favour of "selfdestruct"
+// TypeError: (101-108): "suicide" has been deprecated in favour of "selfdestruct"

test/libsolidity/syntaxTests/globalFunctions/sha3_no_call.sol

@@ -0,0 +1,8 @@
+contract C
+{
+	function f(bytes memory data) public pure {
+		sha3;
+	}
+}
+// ----
+// TypeError: (60-64): "sha3" has been deprecated in favour of "keccak256"

test/libsolidity/syntaxTests/globalFunctions/sha3_override.sol

@@ -0,0 +1,11 @@
+contract C
+{
+	function sha3() public pure returns (bool) {
+		return true;
+	}
+	function f() public pure returns (bool) {
+		return sha3();
+	}
+}
+// ----
+// Warning: (14-76): This declaration shadows a builtin symbol.

test/libsolidity/syntaxTests/globalFunctions/sha3_var.sol

@@ -0,0 +1,9 @@
+contract C
+{
+	function f() public pure returns (bool) {
+		bool sha3 = true;
+		return sha3;
+	}
+}
+// ----
+// Warning: (58-67): This declaration shadows a builtin symbol.

test/libsolidity/syntaxTests/globalFunctions/suicide_no_call.sol

@@ -0,0 +1,8 @@
+contract C
+{
+	function f(bytes memory data) public pure {
+		suicide;
+	}
+}
+// ----
+// TypeError: (60-67): "suicide" has been deprecated in favour of "selfdestruct"

test/libsolidity/syntaxTests/globalFunctions/suicide_override.sol

@@ -0,0 +1,11 @@
+contract C
+{
+	function suicide() public pure returns (bool) {
+		return true;
+	}
+	function f() public pure returns (bool) {
+		return suicide();
+	}
+}
+// ----
+// Warning: (14-79): This declaration shadows a builtin symbol.

test/libsolidity/syntaxTests/globalFunctions/suicide_var.sol

@@ -0,0 +1,9 @@
+contract C
+{
+	function f() public pure returns (bool) {
+		bool suicide = true;
+		return suicide;
+	}
+}
+// ----
+// Warning: (58-70): This declaration shadows a builtin symbol.