From b2b45cc97bd02de1461cd14fe867aa3e03749c29 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Mon, 2 Mar 2020 16:21:03 +0100 Subject: [PATCH] Ensure contract order in suggestion is equal to inheritance order --- libsolidity/analysis/OverrideChecker.cpp | 21 +++++++++++++------ libsolidity/analysis/OverrideChecker.h | 2 +- .../interface/overrides_multiple.sol | 7 ++++--- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index 67862f7cd..f5c247afb 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -565,18 +565,17 @@ void OverrideChecker::checkOverride(OverrideProxy const& _overriding, OverridePr void OverrideChecker::overrideListError( OverrideProxy const& _item, - set _secondary, + vector _secondary, string const& _message1, string const& _message2 ) { - // Using a set rather than a vector so the order is always the same - set names; + vector names; SecondarySourceLocation ssl; for (Declaration const* c: _secondary) { ssl.append("This contract: ", c->location()); - names.insert("\"" + c->name() + "\""); + names.emplace_back("\"" + c->name() + "\""); } string contractSingularPlural = "contract "; if (_secondary.size() > 1) @@ -823,18 +822,28 @@ void OverrideChecker::checkOverrideList(OverrideProxy _item, OverrideProxyBySign missingContracts = expectedContracts - specifiedContracts; if (!missingContracts.empty()) + { + vector sortedMissingContracts; + for (size_t i = _item.contract().annotation().linearizedBaseContracts.size() - 1; i > 0; i--) + { + auto contract = _item.contract().annotation().linearizedBaseContracts[i]; + if (missingContracts.count(contract)) + sortedMissingContracts.emplace_back(contract); + } + overrideListError( _item, - missingContracts, + sortedMissingContracts, _item.astNodeNameCapitalized() + " needs to specify overridden ", "" ); + } auto surplusContracts = specifiedContracts - expectedContracts; if (!surplusContracts.empty()) overrideListError( _item, - surplusContracts, + util::convertContainer>(surplusContracts), "Invalid ", "specified in override list: " ); diff --git a/libsolidity/analysis/OverrideChecker.h b/libsolidity/analysis/OverrideChecker.h index 0c57ccd65..b78aa3c84 100644 --- a/libsolidity/analysis/OverrideChecker.h +++ b/libsolidity/analysis/OverrideChecker.h @@ -157,7 +157,7 @@ private: void checkOverride(OverrideProxy const& _overriding, OverrideProxy const& _super); void overrideListError( OverrideProxy const& _item, - std::set _secondary, + std::vector _secondary, std::string const& _message1, std::string const& _message2 ); diff --git a/test/libsolidity/syntaxTests/inheritance/interface/overrides_multiple.sol b/test/libsolidity/syntaxTests/inheritance/interface/overrides_multiple.sol index 36d76f3a8..c037eb54d 100644 --- a/test/libsolidity/syntaxTests/inheritance/interface/overrides_multiple.sol +++ b/test/libsolidity/syntaxTests/inheritance/interface/overrides_multiple.sol @@ -14,7 +14,7 @@ interface SuperB { function test5() external returns (uint256); } -interface Sub is SuperA, SuperB { +interface Sub is SuperB, SuperA { function test1() external returns (uint256); function test2() external override returns (uint256); function test3() external override(SuperA) returns (uint256); @@ -25,7 +25,8 @@ interface Sub is SuperA, SuperB { // ---- // TypeError: (572-616): Overriding function is missing "override" specifier. // TypeError: (572-616): Overriding function is missing "override" specifier. -// TypeError: (572-616): Function needs to specify overridden contracts "SuperA" and "SuperB". -// TypeError: (647-655): Function needs to specify overridden contracts "SuperA" and "SuperB". +// TypeError: (572-616): Function needs to specify overridden contracts "SuperB" and "SuperA". +// TypeError: (647-655): Function needs to specify overridden contracts "SuperB" and "SuperA". // TypeError: (705-721): Function needs to specify overridden contract "SuperB". // TypeError: (771-787): Function needs to specify overridden contract "SuperA". +// Warning: (837-861): Override specifier list order differs from inheritance order.