Merge pull request #2687 from ethereum/show-unimplemented-funcs

Show unimplemented function if trying to instantiate an abstract class

6 files changed, 37 insertions, 16 deletions

diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 5fe03aa6..ce4baa3f 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -112,8 +112,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
 					m_errorReporter.typeError(fallbackFunction->returnParameterList()->location(), "Fallback function cannot return values.");
 			}
 		}
-		if (!function->isImplemented())
-			_contract.annotation().isFullyImplemented = false;
 	}
 
 	for (auto const& n: _contract.subNodes())
@@ -188,7 +186,6 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 	using FunTypeAndFlag = std::pair<FunctionTypePointer, bool>;
 	map<string, vector<FunTypeAndFlag>> functions;
 
-	bool allBaseConstructorsImplemented = true;
 	// Search from base to derived
 	for (ContractDefinition const* contract: boost::adaptors::reverse(_contract.annotation().linearizedBaseContracts))
 		for (FunctionDefinition const* function: contract->definedFunctions())
@@ -199,7 +196,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 				if (!function->isImplemented())
 					// Base contract's constructor is not fully implemented, no way to get
 					// out of this.
-					allBaseConstructorsImplemented = false;
+					_contract.annotation().unimplementedFunctions.push_back(function);
 				continue;
 			}
 			auto& overloads = functions[function->name()];
@@ -219,16 +216,15 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 				it->second = true;
 		}
 
-	if (!allBaseConstructorsImplemented)
-		_contract.annotation().isFullyImplemented = false;
-
 	// Set to not fully implemented if at least one flag is false.
 	for (auto const& it: functions)
 		for (auto const& funAndFlag: it.second)
 			if (!funAndFlag.second)
 			{
-				_contract.annotation().isFullyImplemented = false;
-				return;
+				FunctionDefinition const* function = dynamic_cast<FunctionDefinition const*>(&funAndFlag.first->declaration());
+				solAssert(function, "");
+				_contract.annotation().unimplementedFunctions.push_back(function);
+				break;
 			}
 }
 
@@ -266,7 +262,8 @@ void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _c
 		}
 	}
 	if (!argumentsNeeded.empty())
-		_contract.annotation().isFullyImplemented = false;
+		for (ContractDefinition const* contract: argumentsNeeded)
+			_contract.annotation().unimplementedFunctions.push_back(contract->constructor());
 }
 
 void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract)
@@ -1523,8 +1520,15 @@ void TypeChecker::endVisit(NewExpression const& _newExpression)
 
 		if (!contract)
 			m_errorReporter.fatalTypeError(_newExpression.location(), "Identifier is not a contract.");
-		if (!contract->annotation().isFullyImplemented)
-			m_errorReporter.typeError(_newExpression.location(), "Trying to create an instance of an abstract contract.");
+		if (!contract->annotation().unimplementedFunctions.empty())
+			m_errorReporter.typeError(
+				_newExpression.location(),
+				SecondarySourceLocation().append(
+					"Missing implementation:",
+					contract->annotation().unimplementedFunctions.front()->location()
+				),
+				"Trying to create an instance of an abstract contract."
+			);
 		if (!contract->constructorIsPublic())
 			m_errorReporter.typeError(_newExpression.location(), "Contract with internal constructor cannot be created directly.");
 
diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h
index 45a6dd1a..ecddbe21 100644
--- a/libsolidity/ast/ASTAnnotations.h
+++ b/libsolidity/ast/ASTAnnotations.h
@@ -79,8 +79,9 @@ struct TypeDeclarationAnnotation: ASTAnnotation
 
 struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnotation
 {
-	/// Whether all functions are implemented.
-	bool isFullyImplemented = true;
+	/// List of functions without a body. Can also contain functions from base classes,
+	/// especially constructors.
+	std::vector<FunctionDefinition const*> unimplementedFunctions;
 	/// List of all (direct and indirect) base contracts in order from derived to
 	/// base, including the contract itself.
 	std::vector<ContractDefinition const*> linearizedBaseContracts;
diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp
index eda70b63..e4a602cb 100644
--- a/libsolidity/ast/ASTJsonConverter.cpp
+++ b/libsolidity/ast/ASTJsonConverter.cpp
@@ -253,7 +253,7 @@ bool ASTJsonConverter::visit(ContractDefinition const& _node)
 		make_pair("name", _node.name()),
 		make_pair("documentation", _node.documentation() ? Json::Value(*_node.documentation()) : Json::nullValue),
 		make_pair("contractKind", contractKind(_node.contractKind())),
-		make_pair("fullyImplemented", _node.annotation().isFullyImplemented),
+		make_pair("fullyImplemented", _node.annotation().unimplementedFunctions.empty()),
 		make_pair("linearizedBaseContracts", getContainerIds(_node.annotation().linearizedBaseContracts)),
 		make_pair("baseContracts", toJson(_node.baseContracts())),
 		make_pair("contractDependencies", getContainerIds(_node.annotation().contractDependencies)),
diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp
index 9689b700..7c66a843 100644
--- a/libsolidity/interface/CompilerStack.cpp
+++ b/libsolidity/interface/CompilerStack.cpp
@@ -639,7 +639,7 @@ void CompilerStack::compileContract(
 {
 	if (
 		_compiledContracts.count(&_contract) ||
-		!_contract.annotation().isFullyImplemented ||
+		!_contract.annotation().unimplementedFunctions.empty() ||
 		!_contract.constructorIsPublic()
 	)
 		return;
diff --git a/libsolidity/interface/ErrorReporter.cpp b/libsolidity/interface/ErrorReporter.cpp
index f9ef4ceb..e6171756 100644
--- a/libsolidity/interface/ErrorReporter.cpp
+++ b/libsolidity/interface/ErrorReporter.cpp
@@ -151,6 +151,16 @@ void ErrorReporter::syntaxError(SourceLocation const& _location, string const& _
 	);
 }
 
+void ErrorReporter::typeError(SourceLocation const& _location, SecondarySourceLocation const& _secondaryLocation, string const& _description)
+{
+	error(
+		Error::Type::TypeError,
+		_location,
+		_secondaryLocation,
+		_description
+	);
+}
+
 void ErrorReporter::typeError(SourceLocation const& _location, string const& _description)
 {
 	error(
diff --git a/libsolidity/interface/ErrorReporter.h b/libsolidity/interface/ErrorReporter.h
index 8b066a3e..42b0c8b6 100644
--- a/libsolidity/interface/ErrorReporter.h
+++ b/libsolidity/interface/ErrorReporter.h
@@ -73,6 +73,12 @@ public:
 
 	void syntaxError(SourceLocation const& _location, std::string const& _description);
 
+	void typeError(
+		SourceLocation const& _location,
+		SecondarySourceLocation const& _secondaryLocation,
+		std::string const& _description
+	);
+
 	void typeError(SourceLocation const& _location, std::string const& _description);
 
 	void fatalTypeError(SourceLocation const& _location, std::string const& _description);