Merge pull request #5630 from ethereum/warn_functions_empty_body

Supress unused var and state mut warnings for functions with empty body
ethereum · Dec 12, 2018 · 8723728 · 8723728
2 parents 7e9a290 + 8c49d86
commit 8723728
Show file tree

Hide file tree

Showing 38 changed files with 16 additions and 89 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -9,6 +9,7 @@ Compiler Features:
  * Code Generator: Use codecopy for string constants more aggressively.
  * Code Generator: Use binary search for dispatch function if more efficient. The size/speed tradeoff can be tuned using ``--optimize-runs``.
  * SMTChecker: Support mathematical and cryptographic functions in an uninterpreted way.
+ * Static Analyzer: Do not warn about unused variables or state mutability for functions with an empty body.
  * Type Checker: Add an additional reason to be displayed when type conversion fails.
  * Yul: Support object access via ``datasize``, ``dataoffset`` and ``datacopy`` in standalone assembly mode.
 

diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -63,21 +63,21 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function)
 
 void StaticAnalyzer::endVisit(FunctionDefinition const&)
 {
-	m_currentFunction = nullptr;
-	m_constructor = false;
-	for (auto const& var: m_localVarUseCount)
-		if (var.second == 0)
-		{
-			if (var.first.second->isCallableParameter())
-				m_errorReporter.warning(
-					var.first.second->location(),
-					"Unused function parameter. Remove or comment out the variable name to silence this warning."
-				);
-			else
-				m_errorReporter.warning(var.first.second->location(), "Unused local variable.");
-		}
-
+	if (m_currentFunction && !m_currentFunction->body().statements().empty())
+		for (auto const& var: m_localVarUseCount)
+			if (var.second == 0)
+			{
+				if (var.first.second->isCallableParameter())
+					m_errorReporter.warning(
+						var.first.second->location(),
+						"Unused function parameter. Remove or comment out the variable name to silence this warning."
+					);
+				else
+					m_errorReporter.warning(var.first.second->location(), "Unused local variable.");
+			}
 	m_localVarUseCount.clear();
+	m_constructor = false;
+	m_currentFunction = nullptr;
 }
 
 bool StaticAnalyzer::visit(Identifier const& _identifier)

diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp
@@ -156,6 +156,7 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef)
 		m_bestMutabilityAndLocation.mutability < _funDef.stateMutability() &&
 		_funDef.stateMutability() != StateMutability::Payable &&
 		_funDef.isImplemented() &&
+		!_funDef.body().statements().empty() &&
 		!_funDef.isConstructor() &&
 		!_funDef.isFallback() &&
 		!_funDef.annotation().superFunction

diff --git a/test/libsolidity/syntaxTests/functionTypes/valid_function_type_variables.sol b/test/libsolidity/syntaxTests/functionTypes/valid_function_type_variables.sol
@@ -18,9 +18,3 @@ contract test {
     function(uint) pure internal h = fh;
 }
 // ----
-// Warning: (20-47): Function state mutability can be restricted to pure
-// Warning: (52-81): Function state mutability can be restricted to pure
-// Warning: (86-115): Function state mutability can be restricted to pure
-// Warning: (120-149): Function state mutability can be restricted to pure
-// Warning: (154-183): Function state mutability can be restricted to pure
-// Warning: (188-217): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/004_reference_to_later_declaration.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/004_reference_to_later_declaration.sol
@@ -3,4 +3,3 @@ contract test {
     function f() public {}
 }
 // ----
-// Warning: (53-75): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/009_type_checking_function_call.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/009_type_checking_function_call.sol
@@ -3,4 +3,3 @@ contract test {
     function g(uint256, bool) public returns (uint256) { }
 }
 // ----
-// Warning: (88-142): Function state mutability can be restricted to pure
diff --git a/...idity/syntaxTests/nameAndTypeResolution/049_function_external_call_allowed_conversion.sol b/...idity/syntaxTests/nameAndTypeResolution/049_function_external_call_allowed_conversion.sol
@@ -7,5 +7,3 @@ contract Test {
     function g (C c) external {}
 }
 // ----
-// Warning: (125-128): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (113-141): Function state mutability can be restricted to pure
diff --git a/...ibsolidity/syntaxTests/nameAndTypeResolution/051_function_internal_allowed_conversion.sol b/...ibsolidity/syntaxTests/nameAndTypeResolution/051_function_internal_allowed_conversion.sol
@@ -9,5 +9,3 @@ contract Test {
     }
 }
 // ----
-// Warning: (68-71): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (56-82): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/055_inheritance_diamond_basic.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/055_inheritance_diamond_basic.sol
@@ -5,5 +5,3 @@ contract derived is root, inter2, inter1 {
     function g() public { f(); rootFunction(); }
 }
 // ----
-// Warning: (16-49): Function state mutability can be restricted to pure
-// Warning: (129-151): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/057_legal_override_direct.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/057_legal_override_direct.sol
@@ -1,6 +1,3 @@
 contract B { function f() public {} }
 contract C is B { function f(uint i) public {} }
 // ----
-// Warning: (67-73): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (13-35): Function state mutability can be restricted to pure
-// Warning: (56-84): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/058_legal_override_indirect.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/058_legal_override_indirect.sol
@@ -2,6 +2,3 @@ contract A { function f(uint a) public {} }
 contract B { function f() public {} }
 contract C is A, B { }
 // ----
-// Warning: (24-30): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (13-41): Function state mutability can be restricted to pure
-// Warning: (57-79): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/060_complex_inheritance.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/060_complex_inheritance.sol
@@ -3,4 +3,3 @@ contract B { function f() public {} function g() public returns (uint8) {} }
 contract C is A, B { }
 // ----
 // Warning: (35-42): Unused local variable.
-// Warning: (95-133): Function state mutability can be restricted to pure
diff --git a/.../libsolidity/syntaxTests/nameAndTypeResolution/061_missing_base_constructor_arguments.sol b/.../libsolidity/syntaxTests/nameAndTypeResolution/061_missing_base_constructor_arguments.sol
@@ -1,4 +1,3 @@
 contract A { constructor(uint a) public { } }
 contract B is A { }
 // ----
-// Warning: (25-31): Unused function parameter. Remove or comment out the variable name to silence this warning.
diff --git a/...libsolidity/syntaxTests/nameAndTypeResolution/062_base_constructor_arguments_override.sol b/...libsolidity/syntaxTests/nameAndTypeResolution/062_base_constructor_arguments_override.sol
@@ -1,4 +1,3 @@
 contract A { constructor(uint a) public { } }
 contract B is A { }
 // ----
-// Warning: (25-31): Unused function parameter. Remove or comment out the variable name to silence this warning.
diff --git a/...bsolidity/syntaxTests/nameAndTypeResolution/096_access_to_default_function_visibility.sol b/...bsolidity/syntaxTests/nameAndTypeResolution/096_access_to_default_function_visibility.sol
@@ -5,4 +5,3 @@ contract d {
     function g() public { c(0).f(); }
 }
 // ----
-// Warning: (17-39): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/104_empty_name_input_parameter.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/104_empty_name_input_parameter.sol
@@ -2,4 +2,3 @@ contract test {
     function f(uint) public { }
 }
 // ----
-// Warning: (20-47): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/106_empty_name_return_parameter.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/106_empty_name_return_parameter.sol
@@ -2,4 +2,3 @@ contract test {
     function f() public returns (bool) { }
 }
 // ----
-// Warning: (20-58): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/216_function_argument_storage_to_mem.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/216_function_argument_storage_to_mem.sol
@@ -6,5 +6,3 @@ contract C {
     }
 }
 // ----
-// Warning: (91-106): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (80-122): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/256_using_for_overload.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/256_using_for_overload.sol
@@ -11,4 +11,3 @@ contract C {
     }
 }
 // ----
-// Warning: (128-189): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/260_library_memory_struct.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/260_library_memory_struct.sol
@@ -5,4 +5,3 @@ library c {
 }
 // ----
 // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
-// Warning: (75-116): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/288_conditional_with_all_types.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/288_conditional_with_all_types.sol
@@ -85,7 +85,5 @@ contract C {
 }
 // ----
 // Warning: (1005-1019): This declaration shadows an existing declaration.
-// Warning: (90-116): Function state mutability can be restricted to pure
-// Warning: (121-147): Function state mutability can be restricted to pure
 // Warning: (257-642): Function state mutability can be restricted to pure
 // Warning: (647-1227): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/345_unused_return_value.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/345_unused_return_value.sol
@@ -5,4 +5,3 @@ contract test {
     }
 }
 // ----
-// Warning: (20-57): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/376_inline_assembly_in_modifier.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/376_inline_assembly_in_modifier.sol
@@ -10,4 +10,3 @@ contract test {
     }
 }
 // ----
-// Warning: (122-151): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/403_return_structs.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/403_return_structs.sol
@@ -7,4 +7,3 @@ contract C {
 }
 // ----
 // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
-// Warning: (112-164): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/437_warn_unused_function_parameter.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/437_warn_unused_function_parameter.sol
@@ -3,4 +3,3 @@ contract C {
     }
 }
 // ----
-// Warning: (28-34): Unused function parameter. Remove or comment out the variable name to silence this warning.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/439_warn_unused_return_parameter.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/439_warn_unused_return_parameter.sol
@@ -3,4 +3,3 @@ contract C {
     }
 }
 // ----
-// Warning: (51-57): Unused function parameter. Remove or comment out the variable name to silence this warning.
diff --git a/test/libsolidity/syntaxTests/parsing/empty_function.sol b/test/libsolidity/syntaxTests/parsing/empty_function.sol
@@ -3,7 +3,3 @@ contract test {
 	function functionName(bytes20 arg1, address addr) public view returns (int id) { }
 }
 // ----
-// Warning: (58-70): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (72-84): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (107-113): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (36-118): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/external_function.sol b/test/libsolidity/syntaxTests/parsing/external_function.sol
@@ -2,4 +2,3 @@ contract c {
     function x() external {}
 }
 // ----
-// Warning: (17-41): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/function_normal_comments.sol b/test/libsolidity/syntaxTests/parsing/function_normal_comments.sol
@@ -4,6 +4,3 @@ contract test {
     function functionName(bytes32 input) public returns (bytes32 out) {}
 }
 // ----
-// Warning: (97-110): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (128-139): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (75-143): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/function_type_as_storage_variable_with_assignment.sol b/test/libsolidity/syntaxTests/parsing/function_type_as_storage_variable_with_assignment.sol
@@ -3,7 +3,3 @@ contract test {
     function (uint, uint) internal returns (uint) f1 = f;
 }
 // ----
-// Warning: (31-37): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (39-45): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (63-69): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (20-73): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/function_type_in_expression.sol b/test/libsolidity/syntaxTests/parsing/function_type_in_expression.sol
@@ -5,9 +5,5 @@ contract test {
     }
 }
 // ----
-// Warning: (31-37): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (39-45): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (63-69): Unused function parameter. Remove or comment out the variable name to silence this warning.
 // Warning: (108-156): Unused local variable.
-// Warning: (20-73): Function state mutability can be restricted to pure
 // Warning: (78-167): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/library_simple.sol b/test/libsolidity/syntaxTests/parsing/library_simple.sol
@@ -2,4 +2,3 @@ library Lib {
     function f() public { }
 }
 // ----
-// Warning: (18-41): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/modifier_invocation.sol b/test/libsolidity/syntaxTests/parsing/modifier_invocation.sol
@@ -4,4 +4,3 @@ contract c {
     function f() public mod1(7) mod2 { }
 }
 // ----
-// Warning: (135-171): Function state mutability can be restricted to view
diff --git a/test/libsolidity/syntaxTests/parsing/multiple_functions_natspec_documentation.sol b/test/libsolidity/syntaxTests/parsing/multiple_functions_natspec_documentation.sol
@@ -10,15 +10,3 @@ contract test {
     function functionName4(bytes32 input) public returns (bytes32 out) {}
 }
 // ----
-// Warning: (97-110): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (128-139): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (203-216): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (234-245): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (304-317): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (335-346): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (410-423): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (441-452): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (74-143): Function state mutability can be restricted to pure
-// Warning: (180-249): Function state mutability can be restricted to pure
-// Warning: (281-350): Function state mutability can be restricted to pure
-// Warning: (387-456): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/no_function_params.sol b/test/libsolidity/syntaxTests/parsing/no_function_params.sol
@@ -3,4 +3,3 @@ contract test {
 	function functionName() public {}
 }
 // ----
-// Warning: (36-69): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/single_function_param.sol b/test/libsolidity/syntaxTests/parsing/single_function_param.sol
@@ -3,6 +3,3 @@ contract test {
 	function functionName(bytes32 input) public returns (bytes32 out) {}
 }
 // ----
-// Warning: (58-71): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (89-100): Unused function parameter. Remove or comment out the variable name to silence this warning.
-// Warning: (36-104): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/parsing/visibility_specifiers.sol b/test/libsolidity/syntaxTests/parsing/visibility_specifiers.sol
@@ -9,6 +9,3 @@ contract c {
 }
 // ----
 // Warning: (58-71): This declaration shadows an existing declaration.
-// Warning: (89-111): Function state mutability can be restricted to pure
-// Warning: (116-144): Function state mutability can be restricted to pure
-// Warning: (149-182): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/viewPureChecker/suggest_pure.sol b/test/libsolidity/syntaxTests/viewPureChecker/suggest_pure.sol
@@ -2,4 +2,3 @@ contract C {
     function g() view public { }
 }
 // ----
-// Warning: (17-45): Function state mutability can be restricted to pure