From 936ea6f950ffd9b4d4bfaa9cd5ae949be0df58fd Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 21 May 2020 03:49:31 +0200 Subject: [PATCH] Refactor TypeChecker to assign different IDs to different errors --- libsolidity/analysis/TypeChecker.cpp | 289 +++++++++++++++------------ 1 file changed, 161 insertions(+), 128 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index ce090101a..606689368 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1994,19 +1994,16 @@ void TypeChecker::typeCheckFunctionGeneralChecks( bool const isStructConstructorCall = _functionCall.annotation().kind == FunctionCallKind::StructConstructorCall; - string msg; - - if (isVariadic) - msg += + auto [errorId, description] = [&]() -> tuple { + string msg = isVariadic ? "Need at least " + toString(parameterTypes.size()) + " arguments for " + string(isStructConstructorCall ? "struct constructor" : "function call") + ", but provided only " + toString(arguments.size()) + - "."; - else - msg += + "." + : "Wrong argument count for " + string(isStructConstructorCall ? "struct constructor" : "function call") + ": " + @@ -2016,49 +2013,64 @@ void TypeChecker::typeCheckFunctionGeneralChecks( toString(parameterTypes.size()) + "."; - // Extend error message in case we try to construct a struct with mapping member. - if (isStructConstructorCall) - { - /// For error message: Struct members that were removed during conversion to memory. - TypePointer const expressionType = type(_functionCall.expression()); - TypeType const& t = dynamic_cast(*expressionType); - auto const& structType = dynamic_cast(*t.actualType()); - set membersRemovedForStructConstructor = structType.membersMissingInMemory(); - - if (!membersRemovedForStructConstructor.empty()) + // Extend error message in case we try to construct a struct with mapping member. + if (isStructConstructorCall) { - msg += " Members that have to be skipped in memory:"; - for (auto const& member: membersRemovedForStructConstructor) - msg += " " + member; + /// For error message: Struct members that were removed during conversion to memory. + TypePointer const expressionType = type(_functionCall.expression()); + auto const& t = dynamic_cast(*expressionType); + auto const& structType = dynamic_cast(*t.actualType()); + set membersRemovedForStructConstructor = structType.membersMissingInMemory(); + + if (!membersRemovedForStructConstructor.empty()) + { + msg += " Members that have to be skipped in memory:"; + for (auto const& member: membersRemovedForStructConstructor) + msg += " " + member; + } + + return { isVariadic ? 1123_error : 9755_error, msg }; } - } - else if ( - _functionType->kind() == FunctionType::Kind::BareCall || - _functionType->kind() == FunctionType::Kind::BareCallCode || - _functionType->kind() == FunctionType::Kind::BareDelegateCall || - _functionType->kind() == FunctionType::Kind::BareStaticCall - ) - { - if (arguments.empty()) - msg += + else if ( + _functionType->kind() == FunctionType::Kind::BareCall || + _functionType->kind() == FunctionType::Kind::BareCallCode || + _functionType->kind() == FunctionType::Kind::BareDelegateCall || + _functionType->kind() == FunctionType::Kind::BareStaticCall + ) + { + if (arguments.empty()) + return { + isVariadic ? 7653_error : 6138_error, + msg + + " This function requires a single bytes argument." + " Use \"\" as argument to provide empty calldata." + }; + else + return { + isVariadic ? 9390_error : 8922_error, + msg + + " This function requires a single bytes argument." + " If all your arguments are value types, you can use" + " abi.encode(...) to properly generate it." + }; + } + else if ( + _functionType->kind() == FunctionType::Kind::KECCAK256 || + _functionType->kind() == FunctionType::Kind::SHA256 || + _functionType->kind() == FunctionType::Kind::RIPEMD160 + ) + return { + isVariadic ? 1220_error : 4323_error, + msg + " This function requires a single bytes argument." - " Use \"\" as argument to provide empty calldata."; + " Use abi.encodePacked(...) to obtain the pre-0.5.0" + " behaviour or abi.encode(...) to use ABI encoding." + }; else - msg += - " This function requires a single bytes argument." - " If all your arguments are value types, you can use" - " abi.encode(...) to properly generate it."; - } - else if ( - _functionType->kind() == FunctionType::Kind::KECCAK256 || - _functionType->kind() == FunctionType::Kind::SHA256 || - _functionType->kind() == FunctionType::Kind::RIPEMD160 - ) - msg += - " This function requires a single bytes argument." - " Use abi.encodePacked(...) to obtain the pre-0.5.0" - " behaviour or abi.encode(...) to use ABI encoding."; - m_errorReporter.typeError(1093_error, _functionCall.location(), msg); + return { isVariadic ? 9308_error : 6160_error, msg }; + }(); + + m_errorReporter.typeError(errorId, _functionCall.location(), description); return; } @@ -2134,33 +2146,43 @@ void TypeChecker::typeCheckFunctionGeneralChecks( solAssert(!!paramArgMap[i], "unmapped parameter"); if (!type(*paramArgMap[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) { - string msg = - "Invalid type for argument in function call. " - "Invalid implicit conversion from " + - type(*paramArgMap[i])->toString() + - " to " + - parameterTypes[i]->toString() + - " requested."; - if ( - _functionType->kind() == FunctionType::Kind::BareCall || - _functionType->kind() == FunctionType::Kind::BareCallCode || - _functionType->kind() == FunctionType::Kind::BareDelegateCall || - _functionType->kind() == FunctionType::Kind::BareStaticCall - ) - msg += - " This function requires a single bytes argument." - " If all your arguments are value types, you can" - " use abi.encode(...) to properly generate it."; - else if ( - _functionType->kind() == FunctionType::Kind::KECCAK256 || - _functionType->kind() == FunctionType::Kind::SHA256 || - _functionType->kind() == FunctionType::Kind::RIPEMD160 - ) - msg += - " This function requires a single bytes argument." - " Use abi.encodePacked(...) to obtain the pre-0.5.0" - " behaviour or abi.encode(...) to use ABI encoding."; - m_errorReporter.typeError(6706_error, paramArgMap[i]->location(), msg); + auto [errorId, description] = [&]() -> tuple { + string msg = + "Invalid type for argument in function call. " + "Invalid implicit conversion from " + + type(*paramArgMap[i])->toString() + + " to " + + parameterTypes[i]->toString() + + " requested."; + if ( + _functionType->kind() == FunctionType::Kind::BareCall || + _functionType->kind() == FunctionType::Kind::BareCallCode || + _functionType->kind() == FunctionType::Kind::BareDelegateCall || + _functionType->kind() == FunctionType::Kind::BareStaticCall + ) + return { + 8051_error, + msg + + " This function requires a single bytes argument." + " If all your arguments are value types, you can" + " use abi.encode(...) to properly generate it." + }; + else if ( + _functionType->kind() == FunctionType::Kind::KECCAK256 || + _functionType->kind() == FunctionType::Kind::SHA256 || + _functionType->kind() == FunctionType::Kind::RIPEMD160 + ) + return { + 7556_error, + msg + + " This function requires a single bytes argument." + " Use abi.encodePacked(...) to obtain the pre-0.5.0" + " behaviour or abi.encode(...) to use ABI encoding." + }; + else + return { 9553_error, msg }; + }(); + m_errorReporter.typeError(errorId, paramArgMap[i]->location(), description); } } } @@ -2549,61 +2571,70 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) " outside of storage." ); } - string errorMsg = "Member \"" + memberName + "\" not found or not visible " + + auto [errorId, description] = [&]() -> tuple { + string errorMsg = "Member \"" + memberName + "\" not found or not visible " "after argument-dependent lookup in " + exprType->toString() + "."; - if (auto const& funType = dynamic_cast(exprType)) - { - auto const& t = funType->returnParameterTypes(); - - if (memberName == "value") + if (auto const* funType = dynamic_cast(exprType)) { - if (funType->kind() == FunctionType::Kind::Creation) - errorMsg = "Constructor for " + t.front()->toString() + " must be payable for member \"value\" to be available."; - else if ( - funType->kind() == FunctionType::Kind::DelegateCall || - funType->kind() == FunctionType::Kind::BareDelegateCall - ) - errorMsg = "Member \"value\" is not allowed in delegated calls due to \"msg.value\" persisting."; - else - errorMsg = "Member \"value\" is only available for payable functions."; - } - else if ( - t.size() == 1 && - (t.front()->category() == Type::Category::Struct || - t.front()->category() == Type::Category::Contract) - ) - errorMsg += " Did you intend to call the function?"; - } - else if (exprType->category() == Type::Category::Contract) - { - for (auto const& addressMember: TypeProvider::payableAddress()->nativeMembers(nullptr)) - if (addressMember.name == memberName) + TypePointers const& t = funType->returnParameterTypes(); + + if (memberName == "value") { - Identifier const* var = dynamic_cast(&_memberAccess.expression()); - string varName = var ? var->name() : "..."; - errorMsg += " Use \"address(" + varName + ")." + memberName + "\" to access this address member."; - break; + if (funType->kind() == FunctionType::Kind::Creation) + return { + 8827_error, + "Constructor for " + t.front()->toString() + " must be payable for member \"value\" to be available." + }; + else if ( + funType->kind() == FunctionType::Kind::DelegateCall || + funType->kind() == FunctionType::Kind::BareDelegateCall + ) + return { 8477_error, "Member \"value\" is not allowed in delegated calls due to \"msg.value\" persisting." }; + else + return { 8820_error, "Member \"value\" is only available for payable functions." }; } - } - else if (auto addressType = dynamic_cast(exprType)) - { - // Trigger error when using send or transfer with a non-payable fallback function. - if (memberName == "send" || memberName == "transfer") - { - solAssert( - addressType->stateMutability() != StateMutability::Payable, - "Expected address not-payable as members were not found" - ); - - errorMsg = "\"send\" and \"transfer\" are only available for objects of type \"address payable\", not \"" + exprType->toString() + "\"."; + else if ( + t.size() == 1 && ( + t.front()->category() == Type::Category::Struct || + t.front()->category() == Type::Category::Contract + ) + ) + return { 6005_error, errorMsg + " Did you intend to call the function?" }; } - } + else if (exprType->category() == Type::Category::Contract) + { + for (MemberList::Member const& addressMember: TypeProvider::payableAddress()->nativeMembers(nullptr)) + if (addressMember.name == memberName) + { + auto const* var = dynamic_cast(&_memberAccess.expression()); + string varName = var ? var->name() : "..."; + errorMsg += " Use \"address(" + varName + ")." + memberName + "\" to access this address member."; + return { 5256_error, errorMsg }; + } + } + else if (auto const* addressType = dynamic_cast(exprType)) + { + // Trigger error when using send or transfer with a non-payable fallback function. + if (memberName == "send" || memberName == "transfer") + { + solAssert( + addressType->stateMutability() != StateMutability::Payable, + "Expected address not-payable as members were not found" + ); + + return { 2604_error, "\"send\" and \"transfer\" are only available for objects of type \"address payable\", not \"" + exprType->toString() + "\"." }; + } + } + + return { 5856_error, errorMsg }; + }(); m_errorReporter.fatalTypeError( - 4035_error, + errorId, _memberAccess.location(), - errorMsg + description ); } else if (possibleMembers.size() > 1) @@ -3165,17 +3196,17 @@ void TypeChecker::requireLValue(Expression const& _expression, bool _ordinaryAss if (_expression.annotation().isLValue) return; - return m_errorReporter.typeError(1123_error, _expression.location(), [&]() { + auto [errorId, description] = [&]() -> tuple { if (_expression.annotation().isConstant) - return "Cannot assign to a constant variable."; + return { 6520_error, "Cannot assign to a constant variable." }; if (auto indexAccess = dynamic_cast(&_expression)) { if (type(indexAccess->baseExpression())->category() == Type::Category::FixedBytes) - return "Single bytes in fixed bytes arrays cannot be modified."; + return { 9222_error, "Single bytes in fixed bytes arrays cannot be modified." }; else if (auto arrayType = dynamic_cast(type(indexAccess->baseExpression()))) if (arrayType->dataStoredIn(DataLocation::CallData)) - return "Calldata arrays are read-only."; + return { 3335_error, "Calldata arrays are read-only." }; } if (auto memberAccess = dynamic_cast(&_expression)) @@ -3183,18 +3214,20 @@ void TypeChecker::requireLValue(Expression const& _expression, bool _ordinaryAss if (auto structType = dynamic_cast(type(memberAccess->expression()))) { if (structType->dataStoredIn(DataLocation::CallData)) - return "Calldata structs are read-only."; + return { 9942_error, "Calldata structs are read-only." }; } else if (dynamic_cast(type(memberAccess->expression()))) if (memberAccess->memberName() == "length") - return "Member \"length\" is read-only and cannot be used to resize arrays."; + return { 7567_error, "Member \"length\" is read-only and cannot be used to resize arrays." }; } if (auto identifier = dynamic_cast(&_expression)) if (auto varDecl = dynamic_cast(identifier->annotation().referencedDeclaration)) if (varDecl->isExternalCallableParameter() && dynamic_cast(identifier->annotation().type)) - return "External function arguments of reference type are read-only."; + return { 7128_error, "External function arguments of reference type are read-only." }; - return "Expression has to be an lvalue."; - }()); + return { 4247_error, "Expression has to be an lvalue." }; + }(); + + m_errorReporter.typeError(errorId, _expression.location(), description); }