Warn on unused local variables

Changelog.md

@@ -10,6 +10,7 @@ Features:
  * Inline Assembly: Storage variable access using ``_slot`` and ``_offset`` suffixes.
  * Inline Assembly: Disallow blocks with unbalanced stack.
  * Static analyzer: Warn about statements without effects.
+ * Static analyzer: Warn about unused local variables, parameters, and return parameters.
  * Syntax checker: issue deprecation warning for unary '+'
 
 Bugfixes:

libsolidity/analysis/StaticAnalyzer.cpp

@@ -48,13 +48,58 @@ void StaticAnalyzer::endVisit(ContractDefinition const&)
 
 bool StaticAnalyzer::visit(FunctionDefinition const& _function)
 {
+	if (_function.isImplemented())
+		m_currentFunction = &_function;
+	else
+		solAssert(!m_currentFunction, "");
+	solAssert(m_localVarUseCount.empty(), "");
 	m_nonPayablePublic = _function.isPublic() && !_function.isPayable();
 	return true;
 }
 
 void StaticAnalyzer::endVisit(FunctionDefinition const&)
 {
+	m_currentFunction = nullptr;
 	m_nonPayablePublic = false;
+	for (auto const& var: m_localVarUseCount)
+		if (var.second == 0)
+			warning(var.first->location(), "Unused local variable");
+	m_localVarUseCount.clear();
+}
+
+bool StaticAnalyzer::visit(Identifier const& _identifier)
+{
+	if (m_currentFunction)
+		if (auto var = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration))
+		{
+			solAssert(!var->name().empty(), "");
+			if (var->isLocalVariable())
+				m_localVarUseCount[var] += 1;
+		}
+	return true;
+}
+
+bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
+{
+	if (m_currentFunction)
+	{
+		solAssert(_variable.isLocalVariable(), "");
+		if (_variable.name() != "")
+			// This is not a no-op, the entry might pre-exist.
+			m_localVarUseCount[&_variable] += 0;
+	}
+	return true;
+}
+
+bool StaticAnalyzer::visit(Return const& _return)
+{
+	// If the return has an expression, it counts as
+	// a "use" of the return parameters.
+	if (m_currentFunction && _return.expression())
+		for (auto const& var: m_currentFunction->returnParameters())
+			if (!var->name().empty())
+				m_localVarUseCount[var.get()] += 1;
+	return true;
 }
 
 bool StaticAnalyzer::visit(ExpressionStatement const& _statement)

libsolidity/analysis/StaticAnalyzer.h

@@ -61,7 +61,9 @@ class StaticAnalyzer: private ASTConstVisitor
 	virtual void endVisit(FunctionDefinition const& _function) override;
 
 	virtual bool visit(ExpressionStatement const& _statement) override;
-
+	virtual bool visit(VariableDeclaration const& _variable) override;
+	virtual bool visit(Identifier const& _identifier) override;
+	virtual bool visit(Return const& _return) override;
 	virtual bool visit(MemberAccess const& _memberAccess) override;
 
 	ErrorList& m_errors;
@@ -71,6 +73,11 @@ class StaticAnalyzer: private ASTConstVisitor
 
 	/// Flag that indicates whether a public function does not contain the "payable" modifier.
 	bool m_nonPayablePublic = false;
+
+	/// Number of uses of each (named) local variable in a function, counter is initialized with zero.
+	std::map<VariableDeclaration const*, int> m_localVarUseCount;
+
+	FunctionDefinition const* m_currentFunction = nullptr;
 };
 
 }

std/StandardToken.sol

@@ -25,7 +25,7 @@ contract StandardToken is Token {
 		return doTransfer(msg.sender, _to, _value);
 	}
 
-	function transferFrom(address _from, address _to, uint256 _value) returns (bool success) {
+	function transferFrom(address _from, address _to, uint256 _value) returns (bool) {
 		if (m_allowance[_from][msg.sender] >= _value) {
 			if (doTransfer(_from, _to, _value)) {
 				m_allowance[_from][msg.sender] -= _value;
@@ -53,7 +53,7 @@ contract StandardToken is Token {
 		return true;
 	}
 
-	function allowance(address _owner, address _spender) constant returns (uint256 remaining) {
+	function allowance(address _owner, address _spender) constant returns (uint256) {
 		return m_allowance[_owner][_spender];
 	}
 }