Merge pull request #2749 from ethereum/require-visibility

Warn if no visibility is specified on contract functions.

Changelog.md

@@ -4,6 +4,7 @@ Features:
  * Support ``pragma experimental v0.5.0;`` to turn on upcoming breaking changes.
  * Code Generator: Added ``.selector`` member on external function types to retrieve their signature.
  * Optimizer: Add new optimization step to remove unused ``JUMPDEST``s.
+ * Syntax Checker: Warn if no visibility is specified on contract functions.
  * Type Checker: Display helpful warning for unused function arguments/return parameters.
  * Type Checker: Do not show the same error multiple times for events.
  * Type Checker: Greatly reduce the number of duplicate errors shown for duplicate constructors and functions.

libsolidity/analysis/SyntaxChecker.cpp

@@ -138,7 +138,7 @@ bool SyntaxChecker::visit(WhileStatement const&)
 	return true;
 }
 
-void SyntaxChecker::endVisit(WhileStatement const&	)
+void SyntaxChecker::endVisit(WhileStatement const&)
 {
 	m_inLoopDepth--;
 }
@@ -193,6 +193,18 @@ bool SyntaxChecker::visit(PlaceholderStatement const&)
 	return true;
 }
 
+bool SyntaxChecker::visit(FunctionDefinition const& _function)
+{
+	if (_function.noVisibilitySpecified())
+		m_errorReporter.warning(
+			_function.location(),
+			"No visibility specified. Defaulting to \"" +
+			Declaration::visibilityToString(_function.visibility()) +
+			"\"."
+		);
+	return true;
+}
+
 bool SyntaxChecker::visit(FunctionTypeName const& _node)
 {
 	for (auto const& decl: _node.parameterTypeList()->parameters())

libsolidity/analysis/SyntaxChecker.h

@@ -66,6 +66,7 @@ class SyntaxChecker: private ASTConstVisitor
 
 	virtual bool visit(PlaceholderStatement const& _placeholderStatement) override;
 
+	virtual bool visit(FunctionDefinition const& _function) override;
 	virtual bool visit(FunctionTypeName const& _node) override;
 
 	ErrorReporter& m_errorReporter;

libsolidity/ast/AST.h

@@ -180,6 +180,7 @@ class Declaration: public ASTNode
 
 	/// @returns the declared name.
 	ASTString const& name() const { return *m_name; }
+	bool noVisibilitySpecified() const { return m_visibility == Visibility::Default; }
 	Visibility visibility() const { return m_visibility == Visibility::Default ? defaultVisibility() : m_visibility; }
 	bool isPublic() const { return visibility() >= Visibility::Public; }
 	virtual bool isVisibleInContract() const { return visibility() != Visibility::External; }

std/StandardToken.sol

@@ -8,24 +8,24 @@ contract StandardToken is Token {
 	mapping (address =>
 		mapping (address => uint256)) m_allowance;
 
-	function StandardToken(address _initialOwner, uint256 _supply) {
+	function StandardToken(address _initialOwner, uint256 _supply) public {
 		supply = _supply;
 		balance[_initialOwner] = _supply;
 	}
 
-	function balanceOf(address _account) constant returns (uint) {
+	function balanceOf(address _account) constant public returns (uint) {
 		return balance[_account];
 	}
 
-	function totalSupply() constant returns (uint) {
+	function totalSupply() constant public returns (uint) {
 		return supply;
 	}
 
-	function transfer(address _to, uint256 _value) returns (bool success) {
+	function transfer(address _to, uint256 _value) public returns (bool success) {
 		return doTransfer(msg.sender, _to, _value);
 	}
 
-	function transferFrom(address _from, address _to, uint256 _value) returns (bool) {
+	function transferFrom(address _from, address _to, uint256 _value) public returns (bool) {
 		if (m_allowance[_from][msg.sender] >= _value) {
 			if (doTransfer(_from, _to, _value)) {
 				m_allowance[_from][msg.sender] -= _value;
@@ -47,13 +47,13 @@ contract StandardToken is Token {
 		}
 	}
 
-	function approve(address _spender, uint256 _value) returns (bool success) {
+	function approve(address _spender, uint256 _value) public returns (bool success) {
 		m_allowance[msg.sender][_spender] = _value;
 		Approval(msg.sender, _spender, _value);
 		return true;
 	}
 
-	function allowance(address _owner, address _spender) constant returns (uint256) {
+	function allowance(address _owner, address _spender) constant public returns (uint256) {
 		return m_allowance[_owner][_spender];
 	}
 }

std/Token.sol

@@ -4,10 +4,10 @@ contract Token {
 	event Transfer(address indexed _from, address indexed _to, uint256 _value);
 	event Approval(address indexed _owner, address indexed _spender, uint256 _value);
 
-	function totalSupply() constant returns (uint256 supply);
-	function balanceOf(address _owner) constant returns (uint256 balance);
-	function transfer(address _to, uint256 _value) returns (bool success);
-	function transferFrom(address _from, address _to, uint256 _value) returns (bool success);
-	function approve(address _spender, uint256 _value) returns (bool success);
-	function allowance(address _owner, address _spender) constant returns (uint256 remaining);
+	function totalSupply() constant public returns (uint256 supply);
+	function balanceOf(address _owner) constant public returns (uint256 balance);
+	function transfer(address _to, uint256 _value) public returns (bool success);
+	function transferFrom(address _from, address _to, uint256 _value) public returns (bool success);
+	function approve(address _spender, uint256 _value) public returns (bool success);
+	function allowance(address _owner, address _spender) constant public returns (uint256 remaining);
 }

std/mortal.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.4.0;
 import "./owned.sol";
 
 contract mortal is owned {
-    function kill() {
+    function kill() public {
         if (msg.sender == owner)
             selfdestruct(owner);
     }

std/owned.sol

@@ -9,7 +9,7 @@ contract owned {
         }
     }
 
-    function owned() {
+    function owned() public {
         owner = msg.sender;
     }
 }