diff --git a/Changelog.md b/Changelog.md index 0bedeb7d6..e2ae7bc67 100644 --- a/Changelog.md +++ b/Changelog.md @@ -21,6 +21,7 @@ Bugfixes: * Control Flow Graph: Take internal calls to functions that always revert into account for reporting unused or unassigned variables. * Control Flow Graph: Assume unimplemented modifiers use a placeholder. * Function Call Graph: Fix internal error connected with circular constant references. + * Name Resolver: Do not issue shadowing warning if the shadowing name is not directly accessible. * Natspec: Allow multiple ``@return`` tags on public state variable documentation. * SMTChecker: Fix internal error on struct constructor with fixed bytes member initialized with string literal. * SMTChecker: Fix internal error on external calls from the constructor. diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index e7b23f7dd..7ca134c5d 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -26,6 +26,9 @@ #include #include +#include +#include + using namespace std; using namespace solidity; using namespace solidity::frontend; @@ -120,11 +123,7 @@ bool DeclarationContainer::registerDeclaration( if (conflictingDeclaration(_declaration, _name)) return false; - // Do not warn about shadowing for structs and enums because their members are - // not accessible without prefixes. Also do not warn about event parameters - // because they do not participate in any proper scope. - bool special = _declaration.scope() && (_declaration.isStructMember() || _declaration.isEnumValue() || _declaration.isEventOrErrorParameter()); - if (m_enclosingContainer && !special) + if (m_enclosingContainer && _declaration.isVisibleAsUnqualifiedName()) m_homonymCandidates.emplace_back(*_name, _location ? _location : &_declaration.location()); } @@ -143,16 +142,35 @@ bool DeclarationContainer::registerDeclaration( return registerDeclaration(_declaration, nullptr, nullptr, _invisible, _update); } -vector DeclarationContainer::resolveName(ASTString const& _name, bool _recursive, bool _alsoInvisible) const +vector DeclarationContainer::resolveName( + ASTString const& _name, + bool _recursive, + bool _alsoInvisible, + bool _onlyVisibleAsUnqualifiedNames +) const { solAssert(!_name.empty(), "Attempt to resolve empty name."); vector result; + if (m_declarations.count(_name)) - result = m_declarations.at(_name); + { + if (_onlyVisibleAsUnqualifiedNames) + result += m_declarations.at(_name) | ranges::views::filter(&Declaration::isVisibleAsUnqualifiedName) | ranges::to_vector; + else + result += m_declarations.at(_name); + } + if (_alsoInvisible && m_invisibleDeclarations.count(_name)) - result += m_invisibleDeclarations.at(_name); + { + if (_onlyVisibleAsUnqualifiedNames) + result += m_invisibleDeclarations.at(_name) | ranges::views::filter(&Declaration::isVisibleAsUnqualifiedName) | ranges::to_vector; + else + result += m_invisibleDeclarations.at(_name); + } + if (result.empty() && _recursive && m_enclosingContainer) - result = m_enclosingContainer->resolveName(_name, true, _alsoInvisible); + result = m_enclosingContainer->resolveName(_name, true, _alsoInvisible, _onlyVisibleAsUnqualifiedNames); + return result; } diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h index 2f29425a3..2dbed0098 100644 --- a/libsolidity/analysis/DeclarationContainer.h +++ b/libsolidity/analysis/DeclarationContainer.h @@ -39,11 +39,10 @@ class DeclarationContainer public: using Homonyms = std::vector>>; - explicit DeclarationContainer( - ASTNode const* _enclosingNode = nullptr, - DeclarationContainer* _enclosingContainer = nullptr - ): - m_enclosingNode(_enclosingNode), m_enclosingContainer(_enclosingContainer) + DeclarationContainer() = default; + explicit DeclarationContainer(ASTNode const* _enclosingNode, DeclarationContainer* _enclosingContainer): + m_enclosingNode(_enclosingNode), + m_enclosingContainer(_enclosingContainer) { if (_enclosingContainer) _enclosingContainer->m_innerContainers.emplace_back(this); @@ -57,7 +56,20 @@ public: bool registerDeclaration(Declaration const& _declaration, ASTString const* _name, langutil::SourceLocation const* _location, bool _invisible, bool _update); bool registerDeclaration(Declaration const& _declaration, bool _invisible, bool _update); - std::vector resolveName(ASTString const& _name, bool _recursive = false, bool _alsoInvisible = false) const; + /// Finds all declarations that in the current scope can be referred to using specified name. + /// @param _name the name to look for. + /// @param _recursive if true and there are no matching declarations in the current container, + /// recursively searches the enclosing containers as well. + /// @param _alsoInvisible if true, include invisible declaration in the results. + /// @param _onlyVisibleAsUnqualifiedNames if true, do not include declarations which can never + /// actually be referenced using their name alone (without being qualified with the name + /// of scope in which they are declared). + std::vector resolveName( + ASTString const& _name, + bool _recursive = false, + bool _alsoInvisible = false, + bool _onlyVisibleAsUnqualifiedNames = false + ) const; ASTNode const* enclosingNode() const { return m_enclosingNode; } DeclarationContainer const* enclosingContainer() const { return m_enclosingContainer; } std::map> const& declarations() const { return m_declarations; } @@ -80,8 +92,8 @@ public: void populateHomonyms(std::back_insert_iterator _it) const; private: - ASTNode const* m_enclosingNode; - DeclarationContainer const* m_enclosingContainer; + ASTNode const* m_enclosingNode = nullptr; + DeclarationContainer const* m_enclosingContainer = nullptr; std::vector m_innerContainers; std::map> m_declarations; std::map> m_invisibleDeclarations; diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 5645111f1..97308f7e7 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -182,7 +182,13 @@ vector NameAndTypeResolver::nameFromCurrentScope(ASTString c Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector const& _path) const { solAssert(!_path.empty(), ""); - vector candidates = m_currentScope->resolveName(_path.front(), true); + vector candidates = m_currentScope->resolveName( + _path.front(), + /* _recursive */ true, + /* _alsoInvisible */ false, + /* _onlyVisibleAsUnqualifiedNames */ true + ); + for (size_t i = 1; i < _path.size() && candidates.size() == 1; i++) { if (!m_scopes.count(candidates.front())) @@ -627,7 +633,10 @@ void DeclarationRegistrationHelper::enterNewSubScope(ASTNode& _subScope) solAssert(dynamic_cast(&_subScope), "Unexpected scope type."); else { - bool newlyAdded = m_scopes.emplace(&_subScope, make_shared(m_currentScope, m_scopes[m_currentScope].get())).second; + bool newlyAdded = m_scopes.emplace( + &_subScope, + make_shared(m_currentScope, m_scopes[m_currentScope].get()) + ).second; solAssert(newlyAdded, "Unable to add new scope."); } m_currentScope = &_subScope; diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 29137c1b6..6d5e89ce2 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -606,6 +606,18 @@ bool Declaration::isEventOrErrorParameter() const return dynamic_cast(scope()) || dynamic_cast(scope()); } +bool Declaration::isVisibleAsUnqualifiedName() const +{ + if (!scope()) + return true; + if (isStructMember() || isEnumValue() || isEventOrErrorParameter()) + return false; + if (auto const* functionDefinition = dynamic_cast(scope())) + if (!functionDefinition->isImplemented()) + return false; // parameter of a function without body + return true; +} + DeclarationAnnotation& Declaration::annotation() const { return initAnnotation(); diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 84981daae..f02e93599 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -265,6 +265,12 @@ public: /// @returns true if this is a declaration of a parameter of an event. bool isEventOrErrorParameter() const; + /// @returns false if the declaration can never be referenced without being qualified with a scope. + /// Usually the name alone can be used to refer to the corresponding entity. + /// But, for example, struct member names or enum member names always require a prefix. + /// Another example is event parameter names, which do not participate in any proper scope. + bool isVisibleAsUnqualifiedName() const; + /// @returns the type of expressions referencing this declaration. /// This can only be called once types of variable declarations have already been resolved. virtual Type const* type() const = 0; diff --git a/test/libsolidity/semanticTests/errors/named_parameters_shadowing_types.sol b/test/libsolidity/semanticTests/errors/named_parameters_shadowing_types.sol new file mode 100644 index 000000000..5af0205c6 --- /dev/null +++ b/test/libsolidity/semanticTests/errors/named_parameters_shadowing_types.sol @@ -0,0 +1,26 @@ +pragma abicoder v2; + +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + error E1(StructType StructType); + error E2(EnumType StructType, StructType EnumType); + + function f() public { + revert E1({StructType: StructType(42)}); + } + + function g() public { + revert E2({EnumType: StructType(42), StructType: EnumType.B}); + } +} +// ==== +// compileToEwasm: also +// compileViaYul: also +// ---- +// f() -> FAILURE, hex"33a54193", hex"000000000000000000000000000000000000000000000000000000000000002a" +// g() -> FAILURE, hex"374b9387", hex"0000000000000000000000000000000000000000000000000000000000000001", hex"000000000000000000000000000000000000000000000000000000000000002a" diff --git a/test/libsolidity/syntaxTests/enums/enum_member_shadowing.sol b/test/libsolidity/syntaxTests/enums/enum_member_shadowing.sol new file mode 100644 index 000000000..dbbb8dc91 --- /dev/null +++ b/test/libsolidity/syntaxTests/enums/enum_member_shadowing.sol @@ -0,0 +1,12 @@ +contract C { + struct S { + uint x; + } + + enum E {E, S, C, a, f} + + uint a; + + function f() public pure {} +} +// ---- diff --git a/test/libsolidity/syntaxTests/errors/error_param_type_shadowed_by_builtin_type_used_as_param_name.sol b/test/libsolidity/syntaxTests/errors/error_param_type_shadowed_by_builtin_type_used_as_param_name.sol new file mode 100644 index 000000000..c8f9ca65b --- /dev/null +++ b/test/libsolidity/syntaxTests/errors/error_param_type_shadowed_by_builtin_type_used_as_param_name.sol @@ -0,0 +1,5 @@ +contract C { + error E(int bytes, bytes x); +} +// ---- +// ParserError 2314: (29-34): Expected ',' but got 'bytes' diff --git a/test/libsolidity/syntaxTests/errors/error_param_type_shadowed_by_param_name.sol b/test/libsolidity/syntaxTests/errors/error_param_type_shadowed_by_param_name.sol new file mode 100644 index 000000000..4650ed6d4 --- /dev/null +++ b/test/libsolidity/syntaxTests/errors/error_param_type_shadowed_by_param_name.sol @@ -0,0 +1,12 @@ +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + error E1(StructType StructType); + error E2(EnumType EnumType); + error E3(EnumType StructType, StructType EnumType); +} +// ---- diff --git a/test/libsolidity/syntaxTests/events/event_param_type_shadowed_by_param_name.sol b/test/libsolidity/syntaxTests/events/event_param_type_shadowed_by_param_name.sol new file mode 100644 index 000000000..b59e67ece --- /dev/null +++ b/test/libsolidity/syntaxTests/events/event_param_type_shadowed_by_param_name.sol @@ -0,0 +1,13 @@ +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + event E1(StructType StructType); + event E2(EnumType EnumType); + event E3(EnumType StructType, StructType EnumType); + event E4(StructType indexed StructType) anonymous; +} +// ---- diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_name_vs_contract.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_name_vs_contract.sol new file mode 100644 index 000000000..1ae8ed754 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_name_vs_contract.sol @@ -0,0 +1,20 @@ +interface I { + function f(uint I) external; // OK +} + +library L { + function f(uint L) public pure {} // warning +} + +abstract contract A { + function f(uint A) public pure {} // warning + function g(uint A) public virtual; // OK +} + +contract C { + function f(uint C) public pure {} // warning +} +// ---- +// Warning 2519: (91-97): This declaration shadows an existing declaration. +// Warning 2519: (168-174): This declaration shadows an existing declaration. +// Warning 2519: (283-289): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_vs_its_function.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_vs_its_function.sol new file mode 100644 index 000000000..a17a74353 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_vs_its_function.sol @@ -0,0 +1,5 @@ +contract C { + function f(uint f) pure public {} +} +// ---- +// Warning 2519: (28-34): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_vs_struct_enum.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_vs_struct_enum.sol new file mode 100644 index 000000000..61441ece6 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameter_vs_struct_enum.sol @@ -0,0 +1,20 @@ +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + function f(StructType memory StructType) external {} + function g(EnumType EnumType) external {} + function h(EnumType StructType, StructType memory EnumType) external {} + + function z(EnumType e) external returns (uint EnumType) {} +} +// ---- +// Warning 2519: (104-132): This declaration shadows an existing declaration. +// Warning 2519: (161-178): This declaration shadows an existing declaration. +// Warning 2519: (207-226): This declaration shadows an existing declaration. +// Warning 2519: (228-254): This declaration shadows an existing declaration. +// Warning 2519: (314-327): This declaration shadows an existing declaration. +// TypeError 5172: (104-114): Name has to refer to a struct, enum or contract. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameters_parameter_vs_struct_enum.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameters_parameter_vs_struct_enum.sol new file mode 100644 index 000000000..fcba51e57 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_parameters_parameter_vs_struct_enum.sol @@ -0,0 +1,20 @@ +library C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + function f1(function (StructType memory StructType) external f) external {} + function f2(function (EnumType EnumType) external f) external {} + function f3(function (EnumType StructType, StructType memory EnumType) external f) external {} +} +// ---- +// Warning 6162: (114-142): Naming function type parameters is deprecated. +// Warning 6162: (194-211): Naming function type parameters is deprecated. +// Warning 6162: (263-282): Naming function type parameters is deprecated. +// Warning 6162: (284-310): Naming function type parameters is deprecated. +// Warning 2519: (114-142): This declaration shadows an existing declaration. +// Warning 2519: (194-211): This declaration shadows an existing declaration. +// Warning 2519: (263-282): This declaration shadows an existing declaration. +// Warning 2519: (284-310): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_return_parameter_vs_struct_enum.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_return_parameter_vs_struct_enum.sol new file mode 100644 index 000000000..ed4b2e0d2 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_return_parameter_vs_struct_enum.sol @@ -0,0 +1,20 @@ +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + function f() external returns (StructType memory StructType) {} + function g() external returns (EnumType EnumType) {} + function h() external returns (EnumType StructType, StructType memory EnumType) {} + + function z(uint EnumType) external returns (EnumType e) {} +} +// ---- +// Warning 2519: (124-152): This declaration shadows an existing declaration. +// Warning 2519: (192-209): This declaration shadows an existing declaration. +// Warning 2519: (249-268): This declaration shadows an existing declaration. +// Warning 2519: (270-296): This declaration shadows an existing declaration. +// Warning 2519: (317-330): This declaration shadows an existing declaration. +// TypeError 5172: (124-134): Name has to refer to a struct, enum or contract. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_type_parameter.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_type_parameter.sol new file mode 100644 index 000000000..a82f054e5 --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_type_parameter.sol @@ -0,0 +1,20 @@ +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + function (StructType memory StructType) external ext1; + function (EnumType EnumType) external ext2; + function (EnumType StructType, StructType memory EnumType) external ext3; +} +// ---- +// Warning 6162: (103-131): Naming function type parameters is deprecated. +// Warning 6162: (162-179): Naming function type parameters is deprecated. +// Warning 6162: (210-229): Naming function type parameters is deprecated. +// Warning 6162: (231-257): Naming function type parameters is deprecated. +// Warning 2519: (103-131): This declaration shadows an existing declaration. +// Warning 2519: (162-179): This declaration shadows an existing declaration. +// Warning 2519: (210-229): This declaration shadows an existing declaration. +// Warning 2519: (231-257): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing_function_type_return_parameter.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_type_return_parameter.sol new file mode 100644 index 000000000..5c260ceae --- /dev/null +++ b/test/libsolidity/syntaxTests/scoping/name_shadowing_function_type_return_parameter.sol @@ -0,0 +1,20 @@ +contract C { + enum EnumType {A, B, C} + + struct StructType { + uint x; + } + + function () external returns (StructType memory StructType) ext1; + function () external returns (EnumType EnumType) ext2; + function () external returns (EnumType StructType, StructType memory EnumType) ext3; +} +// ---- +// SyntaxError 7304: (123-151): Return parameters in function types may not be named. +// SyntaxError 7304: (193-210): Return parameters in function types may not be named. +// SyntaxError 7304: (252-271): Return parameters in function types may not be named. +// SyntaxError 7304: (273-299): Return parameters in function types may not be named. +// Warning 2519: (123-151): This declaration shadows an existing declaration. +// Warning 2519: (193-210): This declaration shadows an existing declaration. +// Warning 2519: (252-271): This declaration shadows an existing declaration. +// Warning 2519: (273-299): This declaration shadows an existing declaration. diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing2.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_local_variable_vs_free_function.sol similarity index 100% rename from test/libsolidity/syntaxTests/scoping/name_shadowing2.sol rename to test/libsolidity/syntaxTests/scoping/name_shadowing_local_variable_vs_free_function.sol diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing3.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_local_variable_vs_functions.sol similarity index 100% rename from test/libsolidity/syntaxTests/scoping/name_shadowing3.sol rename to test/libsolidity/syntaxTests/scoping/name_shadowing_local_variable_vs_functions.sol diff --git a/test/libsolidity/syntaxTests/scoping/name_shadowing.sol b/test/libsolidity/syntaxTests/scoping/name_shadowing_local_variable_vs_state_variable.sol similarity index 100% rename from test/libsolidity/syntaxTests/scoping/name_shadowing.sol rename to test/libsolidity/syntaxTests/scoping/name_shadowing_local_variable_vs_state_variable.sol diff --git a/test/libsolidity/syntaxTests/structs/member_type_eq_name.sol b/test/libsolidity/syntaxTests/structs/member_type_eq_name.sol index 525185edd..8871e3f82 100644 --- a/test/libsolidity/syntaxTests/structs/member_type_eq_name.sol +++ b/test/libsolidity/syntaxTests/structs/member_type_eq_name.sol @@ -3,4 +3,4 @@ contract C { function f(function(S memory) external) public {} } // ---- -// TypeError 5172: (25-26): Name has to refer to a struct, enum or contract. +// DeclarationError 7920: (25-26): Identifier not found or not unique. diff --git a/test/libsolidity/syntaxTests/structs/member_type_eq_name_2.sol b/test/libsolidity/syntaxTests/structs/member_type_eq_name_2.sol new file mode 100644 index 000000000..12abeb28e --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/member_type_eq_name_2.sol @@ -0,0 +1,6 @@ +contract C { + enum E {a, b, c} + struct S {E X; uint E;} + struct T {E T; uint E;} + struct U {E E;} +} diff --git a/test/libsolidity/syntaxTests/structs/member_type_eq_name_3.sol b/test/libsolidity/syntaxTests/structs/member_type_eq_name_3.sol new file mode 100644 index 000000000..cd09c1d74 --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/member_type_eq_name_3.sol @@ -0,0 +1,12 @@ +contract C { + enum E {a, b, c} + struct S {function (E X) external f; uint E;} + struct T {function (E T) external f; uint E;} + struct U {function (E E) external f;} +} +// ---- +// Warning 6162: (58-61): Naming function type parameters is deprecated. +// Warning 6162: (108-111): Naming function type parameters is deprecated. +// Warning 6162: (158-161): Naming function type parameters is deprecated. +// Warning 2519: (108-111): This declaration shadows an existing declaration. +// Warning 2519: (158-161): This declaration shadows an existing declaration.