diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 1736469eb..e3c9bc964 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -21,6 +21,7 @@ */ #include +#include #include #include #include @@ -52,6 +53,7 @@ void ContractDefinition::checkTypeRequirements() baseSpecifier->checkTypeRequirements(); checkIllegalOverrides(); + checkAbstractFunctions(); FunctionDefinition const* constructor = getConstructor(); if (constructor && !constructor->getReturnParameters().empty()) @@ -60,6 +62,7 @@ void ContractDefinition::checkTypeRequirements() FunctionDefinition const* fallbackFunction = nullptr; for (ASTPointer const& function: getDefinedFunctions()) + { if (function->getName().empty()) { if (fallbackFunction) @@ -71,6 +74,9 @@ void ContractDefinition::checkTypeRequirements() BOOST_THROW_EXCEPTION(fallbackFunction->getParameterList().createTypeError("Fallback function cannot take parameters.")); } } + if (!function->isFullyImplemented()) + setFullyImplemented(false); + } for (ASTPointer const& modifier: getFunctionModifiers()) modifier->checkTypeRequirements(); @@ -124,6 +130,28 @@ FunctionDefinition const* ContractDefinition::getFallbackFunction() const return nullptr; } +void ContractDefinition::checkAbstractFunctions() +{ + map functions; + + // Search from base to derived + for (ContractDefinition const* contract: boost::adaptors::reverse(getLinearizedBaseContracts())) + for (ASTPointer const& function: contract->getDefinedFunctions()) + { + string const& name = function->getName(); + if (!function->isFullyImplemented() && functions.count(name) && functions[name]) + BOOST_THROW_EXCEPTION(function->createTypeError("Redeclaring an already implemented function as abstract")); + functions[name] = function->isFullyImplemented(); + } + + for (auto const& it: functions) + if (!it.second) + { + setFullyImplemented(false); + break; + } +} + void ContractDefinition::checkIllegalOverrides() const { // TODO unify this at a later point. for this we need to put the constness and the access specifier @@ -316,8 +344,8 @@ void FunctionDefinition::checkTypeRequirements() modifier->checkTypeRequirements(isConstructor() ? dynamic_cast(*getScope()).getBaseContracts() : vector>()); - - m_body->checkTypeRequirements(); + if (m_body) + m_body->checkTypeRequirements(); } string FunctionDefinition::externalSignature() const @@ -649,6 +677,8 @@ void NewExpression::checkTypeRequirements() m_contract = dynamic_cast(m_contractName->getReferencedDeclaration()); if (!m_contract) BOOST_THROW_EXCEPTION(createTypeError("Identifier is not a contract.")); + if (!m_contract->isFullyImplemented()) + BOOST_THROW_EXCEPTION(m_contract->createTypeError("Trying to create an instance of an abstract contract.")); shared_ptr contractType = make_shared(*m_contract); TypePointers const& parameterTypes = contractType->getConstructorType()->getParameterTypes(); m_type = make_shared(parameterTypes, TypePointers{contractType}, diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 00ad33073..9f5f9fcb1 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -196,6 +196,22 @@ protected: ASTPointer m_documentation; }; +/** + * Abstract class that is added to AST nodes that can be marked as not being fully implemented + */ +class ImplementationOptional +{ +public: + explicit ImplementationOptional(bool _implemented): m_implemented(_implemented) {} + + /// @return whether this node is fully implemented or not + bool isFullyImplemented() const { return m_implemented; } + void setFullyImplemented(bool _implemented) { m_implemented = _implemented; } + +protected: + bool m_implemented; +}; + /// @} /** @@ -203,20 +219,24 @@ protected: * document order. It first visits all struct declarations, then all variable declarations and * finally all function declarations. */ -class ContractDefinition: public Declaration, public Documented +class ContractDefinition: public Declaration, public Documented, public ImplementationOptional { public: - ContractDefinition(SourceLocation const& _location, - ASTPointer const& _name, - ASTPointer const& _documentation, - std::vector> const& _baseContracts, - std::vector> const& _definedStructs, - std::vector> const& _definedEnums, - std::vector> const& _stateVariables, - std::vector> const& _definedFunctions, - std::vector> const& _functionModifiers, - std::vector> const& _events): - Declaration(_location, _name), Documented(_documentation), + ContractDefinition( + SourceLocation const& _location, + ASTPointer const& _name, + ASTPointer const& _documentation, + std::vector> const& _baseContracts, + std::vector> const& _definedStructs, + std::vector> const& _definedEnums, + std::vector> const& _stateVariables, + std::vector> const& _definedFunctions, + std::vector> const& _functionModifiers, + std::vector> const& _events + ): + Declaration(_location, _name), + Documented(_documentation), + ImplementationOptional(true), m_baseContracts(_baseContracts), m_definedStructs(_definedStructs), m_definedEnums(_definedEnums), @@ -263,6 +283,7 @@ public: private: void checkIllegalOverrides() const; + void checkAbstractFunctions(); std::vector, FunctionTypePointer>> const& getInterfaceFunctionList() const; @@ -378,24 +399,29 @@ private: std::vector> m_parameters; }; -class FunctionDefinition: public Declaration, public VariableScope, public Documented +class FunctionDefinition: public Declaration, public VariableScope, public Documented, public ImplementationOptional { public: - FunctionDefinition(SourceLocation const& _location, ASTPointer const& _name, - Declaration::Visibility _visibility, bool _isConstructor, - ASTPointer const& _documentation, - ASTPointer const& _parameters, - bool _isDeclaredConst, - std::vector> const& _modifiers, - ASTPointer const& _returnParameters, - ASTPointer const& _body): - Declaration(_location, _name, _visibility), Documented(_documentation), - m_isConstructor(_isConstructor), - m_parameters(_parameters), - m_isDeclaredConst(_isDeclaredConst), - m_functionModifiers(_modifiers), - m_returnParameters(_returnParameters), - m_body(_body) + FunctionDefinition( + SourceLocation const& _location, + ASTPointer const& _name, + Declaration::Visibility _visibility, bool _isConstructor, + ASTPointer const& _documentation, + ASTPointer const& _parameters, + bool _isDeclaredConst, + std::vector> const& _modifiers, + ASTPointer const& _returnParameters, + ASTPointer const& _body + ): + Declaration(_location, _name, _visibility), + Documented(_documentation), + ImplementationOptional(_body != nullptr), + m_isConstructor(_isConstructor), + m_parameters(_parameters), + m_isDeclaredConst(_isDeclaredConst), + m_functionModifiers(_modifiers), + m_returnParameters(_returnParameters), + m_body(_body) {} virtual void accept(ASTVisitor& _visitor) override; diff --git a/libsolidity/AST_accept.h b/libsolidity/AST_accept.h index 81ede8fc9..3557f8779 100644 --- a/libsolidity/AST_accept.h +++ b/libsolidity/AST_accept.h @@ -175,7 +175,8 @@ void FunctionDefinition::accept(ASTVisitor& _visitor) if (m_returnParameters) m_returnParameters->accept(_visitor); listAccept(m_functionModifiers, _visitor); - m_body->accept(_visitor); + if (m_body) + m_body->accept(_visitor); } _visitor.endVisit(*this); } @@ -188,7 +189,8 @@ void FunctionDefinition::accept(ASTConstVisitor& _visitor) const if (m_returnParameters) m_returnParameters->accept(_visitor); listAccept(m_functionModifiers, _visitor); - m_body->accept(_visitor); + if (m_body) + m_body->accept(_visitor); } _visitor.endVisit(*this); } diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 55ec0cb59..1301bfa5d 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -138,6 +138,8 @@ void CompilerStack::compile(bool _optimize) for (ASTPointer const& node: source->ast->getNodes()) if (ContractDefinition* contract = dynamic_cast(node.get())) { + if (!contract->isFullyImplemented()) + continue; shared_ptr compiler = make_shared(_optimize); compiler->compileContract(*contract, contractBytecode); Contract& compiledContract = m_contracts[contract->getName()]; diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 2f35a96f6..aacbbfd72 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -175,8 +175,17 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(ContractDefin method["author"] = m_author; Json::Value params(Json::objectValue); + std::vector paramNames = it.second->getParameterNames(); for (auto const& pair: m_params) + { + if (find(paramNames.begin(), paramNames.end(), pair.first) == paramNames.end()) + // LTODO: mismatching parameter name, throw some form of warning and not just an exception + BOOST_THROW_EXCEPTION( + DocstringParsingError() << + errinfo_comment("documented parameter \"" + pair.first + "\" not found found in the function") + ); params[pair.first] = pair.second; + } if (!m_params.empty()) method["params"] = params; diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index 393d2734e..5c7676df5 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -164,8 +164,17 @@ ASTPointer Parser::parseContractDefinition() } nodeFactory.markEndPosition(); expectToken(Token::RBrace); - return nodeFactory.createNode(name, docString, baseContracts, structs, enums, - stateVariables, functions, modifiers, events); + return nodeFactory.createNode( + name, + docString, + baseContracts, + structs, + enums, + stateVariables, + functions, + modifiers, + events + ); } ASTPointer Parser::parseInheritanceSpecifier() @@ -247,8 +256,15 @@ ASTPointer Parser::parseFunctionDefinition(ASTString const* } else returnParameters = createEmptyParameterList(); - ASTPointer block = parseBlock(); - nodeFactory.setEndPositionFromNode(block); + ASTPointer block = ASTPointer(); + nodeFactory.markEndPosition(); + if (m_scanner->getCurrentToken() != Token::Semicolon) + { + block = parseBlock(); + nodeFactory.setEndPositionFromNode(block); + } + else + m_scanner->next(); // just consume the ';' bool const c_isConstructor = (_contractName && *name == *_contractName); return nodeFactory.createNode(name, visibility, c_isConstructor, docstring, parameters, isDeclaredConst, modifiers, diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 3bec70a2b..ddcf36140 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -359,6 +359,63 @@ BOOST_AUTO_TEST_CASE(comparison_bitop_precedence) ETH_TEST_CHECK_NO_THROW(parseTextAndResolveNames(text), "Parsing and Name Resolving Failed"); } +BOOST_AUTO_TEST_CASE(function_no_implementation) +{ + ASTPointer sourceUnit; + char const* text = "contract test {\n" + " function functionName(bytes32 input) returns (bytes32 out);\n" + "}\n"; + ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); + std::vector> nodes = sourceUnit->getNodes(); + ContractDefinition* contract = dynamic_cast(nodes[0].get()); + BOOST_CHECK(contract); + BOOST_CHECK(!contract->isFullyImplemented()); + BOOST_CHECK(!contract->getDefinedFunctions()[0]->isFullyImplemented()); +} + +BOOST_AUTO_TEST_CASE(abstract_contract) +{ + ASTPointer sourceUnit; + char const* text = R"( + contract base { function foo(); } + contract derived is base { function foo() {} } + )"; + ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); + std::vector> nodes = sourceUnit->getNodes(); + ContractDefinition* base = dynamic_cast(nodes[0].get()); + ContractDefinition* derived = dynamic_cast(nodes[1].get()); + BOOST_CHECK(base); + BOOST_CHECK(!base->isFullyImplemented()); + BOOST_CHECK(!base->getDefinedFunctions()[0]->isFullyImplemented()); + BOOST_CHECK(derived); + BOOST_CHECK(derived->isFullyImplemented()); + BOOST_CHECK(derived->getDefinedFunctions()[0]->isFullyImplemented()); +} + +BOOST_AUTO_TEST_CASE(create_abstract_contract) +{ + ASTPointer sourceUnit; + char const* text = R"( + contract base { function foo(); } + contract derived { + base b; + function foo() { b = new base();} + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + +BOOST_AUTO_TEST_CASE(redeclare_implemented_abstract_function_as_abstract) +{ + ASTPointer sourceUnit; + char const* text = R"( + contract base { function foo(); } + contract derived is base { function foo() {} } + contract wrong is derived { function foo(); } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + BOOST_AUTO_TEST_CASE(function_canonical_signature) { ASTPointer sourceUnit; diff --git a/test/SolidityNatspecJSON.cpp b/test/SolidityNatspecJSON.cpp index edfe89861..aeaad1966 100644 --- a/test/SolidityNatspecJSON.cpp +++ b/test/SolidityNatspecJSON.cpp @@ -176,7 +176,6 @@ BOOST_AUTO_TEST_CASE(dev_and_user_no_doc) "}\n"; char const* devNatspec = "{\"methods\":{}}"; - char const* userNatspec = "{\"methods\":{}}"; checkNatspec(sourceCode, devNatspec, false); @@ -230,6 +229,18 @@ BOOST_AUTO_TEST_CASE(dev_multiple_params) checkNatspec(sourceCode, natspec, false); } +BOOST_AUTO_TEST_CASE(dev_documenting_nonexistant_param) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter\n" + " /// @param not_existing Documentation for the second parameter\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + BOOST_CHECK_THROW(checkNatspec(sourceCode, "", false), DocstringParsingError); +} + BOOST_AUTO_TEST_CASE(dev_mutiline_param_description) { char const* sourceCode = "contract test {\n" @@ -487,17 +498,7 @@ BOOST_AUTO_TEST_CASE(dev_title_at_function_error) " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" "}\n"; - char const* natspec = "{" - " \"author\": \"Lefteris\"," - " \"title\": \"Just a test contract\"," - " \"methods\":{" - " \"mul(uint256,uint256)\":{ \n" - " \"details\": \"Mul function\"\n" - " }\n" - " }\n" - "}"; - - BOOST_CHECK_THROW(checkNatspec(sourceCode, natspec, false), DocstringParsingError); + BOOST_CHECK_THROW(checkNatspec(sourceCode, "", false), DocstringParsingError); } BOOST_AUTO_TEST_CASE(natspec_notice_without_tag) diff --git a/test/SolidityParser.cpp b/test/SolidityParser.cpp index 392d9ac47..7640f91ad 100644 --- a/test/SolidityParser.cpp +++ b/test/SolidityParser.cpp @@ -108,6 +108,14 @@ BOOST_AUTO_TEST_CASE(single_function_param) ETH_TEST_CHECK_NO_THROW(parseText(text), "Parsing failed."); } +BOOST_AUTO_TEST_CASE(function_no_body) +{ + char const* text = "contract test {\n" + " function functionName(bytes32 input) returns (bytes32 out);\n" + "}\n"; + ETH_TEST_CHECK_NO_THROW(parseText(text), "Parsing failed."); +} + BOOST_AUTO_TEST_CASE(missing_parameter_name_in_named_args) { char const* text = "contract test {\n"