diff --git a/Changelog.md b/Changelog.md index f151365ae..72b325070 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ Language Features: * Inline Assembly: Allow assigning to `_slot` of local storage variable pointers. + * Type Checker: Warn if override specifier list order differs from inheritance order. Compiler Features: diff --git a/libsolidity/analysis/OverrideChecker.cpp b/libsolidity/analysis/OverrideChecker.cpp index ae32161af..67862f7cd 100644 --- a/libsolidity/analysis/OverrideChecker.cpp +++ b/libsolidity/analysis/OverrideChecker.cpp @@ -767,6 +767,38 @@ void OverrideChecker::checkOverrideList(OverrideProxy _item, OverrideProxyBySign } } } + else if (specifiedContracts.size() > 1) + { + vector overrideSpecifierContractIds; + vector inheritanceContractsIds; + + for (auto const& overrideName: _item.overrides()->overrides()) + { + int64_t const contractId = overrideName->annotation().referencedDeclaration->id(); + + auto const matchId = [&](ContractDefinition const* _contract) + { + return contractId == _contract->id(); + }; + + // Only compare contracts that exist in the inheritance chain + if (util::contains_if(_item.contract().annotation().linearizedBaseContracts, matchId)) + overrideSpecifierContractIds.emplace_back(contractId); + } + + for (size_t i = _item.contract().annotation().linearizedBaseContracts.size() - 1; i > 0; i--) + { + int64_t contractId = _item.contract().annotation().linearizedBaseContracts[i]->id(); + if (util::contains(overrideSpecifierContractIds, contractId)) + inheritanceContractsIds.emplace_back(contractId); + } + + if (overrideSpecifierContractIds != inheritanceContractsIds) + m_errorReporter.warning( + _item.overrides()->location(), + "Override specifier list order differs from inheritance order." + ); + } set expectedContracts; diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_interface_multiple.sol b/test/libsolidity/syntaxTests/inheritance/override/override_interface_multiple.sol index 9b82be89f..e2f8b882d 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_interface_multiple.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_interface_multiple.sol @@ -12,3 +12,4 @@ contract X is A, B { function test2() external override(B, A) returns (uint256) {} } // ---- +// Warning: (325-339): Override specifier list order differs from inheritance order. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol b/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol index ec20bd729..0f8743ffa 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_multiple_missing.sol @@ -18,5 +18,6 @@ abstract contract X is A, B, C, D { function foo() internal override(A, C) virtual returns (uint256); } // ---- +// Warning: (533-550): Override specifier list order differs from inheritance order. // TypeError: (533-550): Invalid contract specified in override list: "C". // TypeError: (603-617): Function needs to specify overridden contracts "B" and "D". diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_shared_base_partial.sol b/test/libsolidity/syntaxTests/inheritance/override/override_shared_base_partial.sol index bb955d13a..c433c6d39 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_shared_base_partial.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_shared_base_partial.sol @@ -8,3 +8,5 @@ contract B is I {} contract C is A, B { function f() external override(A, I) {} } +// ---- +// Warning: (178-192): Override specifier list order differs from inheritance order. diff --git a/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol b/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol index 833743f75..99f785c64 100644 --- a/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol +++ b/test/libsolidity/syntaxTests/inheritance/override/override_unimplemented_fail.sol @@ -12,4 +12,5 @@ abstract contract D is B, A { } // ---- // TypeError: (154-199): Overriding an implemented function with an unimplemented function is not allowed. +// Warning: (266-280): Override specifier list order differs from inheritance order. // TypeError: (236-281): Overriding an implemented function with an unimplemented function is not allowed. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/055_inheritance_diamond_basic.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/055_inheritance_diamond_basic.sol index 8fda79901..0650145e5 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/055_inheritance_diamond_basic.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/055_inheritance_diamond_basic.sol @@ -3,6 +3,6 @@ contract inter1 is root { function f() public virtual {} } contract inter2 is root { function f() public virtual {} } contract derived is root, inter2, inter1 { function g() public { f(); rootFunction(); } - function f() override(inter1, inter2) public {} + function f() override(inter2, inter1) public {} } // ----