From 8b07fe59d53498de89e5e79527c126bf0ac1f20a Mon Sep 17 00:00:00 2001 From: Simone <79767264+smonicas@users.noreply.github.com> Date: Fri, 8 Sep 2023 06:39:49 +0200 Subject: [PATCH] improve name resolution of type aliases (#2061) * BREAKING CHANGE: Renamed user_defined_types to type_aliases so it's less confusing with what we call UserDefinedType. * Added type aliased at the Contract level so now at the file scope there are only top level aliasing and fully qualified contract aliasing like C.myAlias. * Fix #1809 --- slither/core/compilation_unit.py | 6 ++-- slither/core/declarations/contract.py | 34 ++++++++++++++++++ slither/core/scope/scope.py | 6 ++-- slither/solc_parsing/declarations/contract.py | 8 ++--- .../declarations/using_for_top_level.py | 2 +- .../solc_parsing/expressions/find_variable.py | 9 +++-- .../slither_compilation_unit_solc.py | 8 ++--- .../solidity_types/type_parsing.py | 30 ++++++++-------- .../visitors/slithir/expression_to_slithir.py | 4 +-- tests/e2e/solc_parsing/test_ast_parsing.py | 1 + .../type-aliases.sol-0.8.19-compact.zip | Bin 0 -> 2111 bytes .../type-aliases.sol-0.8.19-compact.json | 6 ++++ .../solc_parsing/test_data/type-aliases.sol | 20 +++++++++++ tests/unit/core/test_source_mapping.py | 6 ++-- 14 files changed, 101 insertions(+), 39 deletions(-) create mode 100644 tests/e2e/solc_parsing/test_data/compile/type-aliases.sol-0.8.19-compact.zip create mode 100644 tests/e2e/solc_parsing/test_data/expected/type-aliases.sol-0.8.19-compact.json create mode 100644 tests/e2e/solc_parsing/test_data/type-aliases.sol diff --git a/slither/core/compilation_unit.py b/slither/core/compilation_unit.py index 6d24786eb..35180cefc 100644 --- a/slither/core/compilation_unit.py +++ b/slither/core/compilation_unit.py @@ -47,7 +47,7 @@ class SlitherCompilationUnit(Context): self._pragma_directives: List[Pragma] = [] self._import_directives: List[Import] = [] self._custom_errors: List[CustomErrorTopLevel] = [] - self._user_defined_value_types: Dict[str, TypeAliasTopLevel] = {} + self._type_aliases: Dict[str, TypeAliasTopLevel] = {} self._all_functions: Set[Function] = set() self._all_modifiers: Set[Modifier] = set() @@ -220,8 +220,8 @@ class SlitherCompilationUnit(Context): return self._custom_errors @property - def user_defined_value_types(self) -> Dict[str, TypeAliasTopLevel]: - return self._user_defined_value_types + def type_aliases(self) -> Dict[str, TypeAliasTopLevel]: + return self._type_aliases # endregion ################################################################################### diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 9b1488db3..f9bf15306 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -45,6 +45,7 @@ if TYPE_CHECKING: from slither.core.compilation_unit import SlitherCompilationUnit from slither.core.scope.scope import FileScope from slither.core.cfg.node import Node + from slither.core.solidity_types import TypeAliasContract LOGGER = logging.getLogger("Contract") @@ -81,6 +82,7 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods self._functions: Dict[str, "FunctionContract"] = {} self._linearizedBaseContracts: List[int] = [] self._custom_errors: Dict[str, "CustomErrorContract"] = {} + self._type_aliases: Dict[str, "TypeAliasContract"] = {} # The only str is "*" self._using_for: Dict[USING_FOR_KEY, USING_FOR_ITEM] = {} @@ -364,6 +366,38 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods def custom_errors_as_dict(self) -> Dict[str, "CustomErrorContract"]: return self._custom_errors + # endregion + ################################################################################### + ################################################################################### + # region Custom Errors + ################################################################################### + ################################################################################### + + @property + def type_aliases(self) -> List["TypeAliasContract"]: + """ + list(TypeAliasContract): List of the contract's custom errors + """ + return list(self._type_aliases.values()) + + @property + def type_aliases_inherited(self) -> List["TypeAliasContract"]: + """ + list(TypeAliasContract): List of the inherited custom errors + """ + return [s for s in self.type_aliases if s.contract != self] + + @property + def type_aliases_declared(self) -> List["TypeAliasContract"]: + """ + list(TypeAliasContract): List of the custom errors declared within the contract (not inherited) + """ + return [s for s in self.type_aliases if s.contract == self] + + @property + def type_aliases_as_dict(self) -> Dict[str, "TypeAliasContract"]: + return self._type_aliases + # endregion ################################################################################### ################################################################################### diff --git a/slither/core/scope/scope.py b/slither/core/scope/scope.py index cafeb3585..937a05136 100644 --- a/slither/core/scope/scope.py +++ b/slither/core/scope/scope.py @@ -52,7 +52,7 @@ class FileScope: # User defined types # Name -> type alias - self.user_defined_types: Dict[str, TypeAlias] = {} + self.type_aliases: Dict[str, TypeAlias] = {} def add_accesible_scopes(self) -> bool: """ @@ -95,8 +95,8 @@ class FileScope: if not _dict_contain(new_scope.renaming, self.renaming): self.renaming.update(new_scope.renaming) learn_something = True - if not _dict_contain(new_scope.user_defined_types, self.user_defined_types): - self.user_defined_types.update(new_scope.user_defined_types) + if not _dict_contain(new_scope.type_aliases, self.type_aliases): + self.type_aliases.update(new_scope.type_aliases) learn_something = True return learn_something diff --git a/slither/solc_parsing/declarations/contract.py b/slither/solc_parsing/declarations/contract.py index 74a1cbcf0..a118b1e65 100644 --- a/slither/solc_parsing/declarations/contract.py +++ b/slither/solc_parsing/declarations/contract.py @@ -291,10 +291,10 @@ class ContractSolc(CallerContextExpression): alias = item["name"] alias_canonical = self._contract.name + "." + item["name"] - user_defined_type = TypeAliasContract(original_type, alias, self.underlying_contract) - user_defined_type.set_offset(item["src"], self.compilation_unit) - self._contract.file_scope.user_defined_types[alias] = user_defined_type - self._contract.file_scope.user_defined_types[alias_canonical] = user_defined_type + type_alias = TypeAliasContract(original_type, alias, self.underlying_contract) + type_alias.set_offset(item["src"], self.compilation_unit) + self._contract.type_aliases_as_dict[alias] = type_alias + self._contract.file_scope.type_aliases[alias_canonical] = type_alias def _parse_struct(self, struct: Dict) -> None: diff --git a/slither/solc_parsing/declarations/using_for_top_level.py b/slither/solc_parsing/declarations/using_for_top_level.py index fe72e5780..bfed3c417 100644 --- a/slither/solc_parsing/declarations/using_for_top_level.py +++ b/slither/solc_parsing/declarations/using_for_top_level.py @@ -152,7 +152,7 @@ class UsingForTopLevelSolc(CallerContextExpression): # pylint: disable=too-few- if self._global: for scope in self.compilation_unit.scopes.values(): if isinstance(type_name, TypeAliasTopLevel): - for alias in scope.user_defined_types.values(): + for alias in scope.type_aliases.values(): if alias == type_name: scope.using_for_directives.add(self._using_for) elif isinstance(type_name, UserDefinedType): diff --git a/slither/solc_parsing/expressions/find_variable.py b/slither/solc_parsing/expressions/find_variable.py index 32f5afc58..3bbdd268e 100644 --- a/slither/solc_parsing/expressions/find_variable.py +++ b/slither/solc_parsing/expressions/find_variable.py @@ -114,6 +114,8 @@ def find_top_level( :return: :rtype: """ + if var_name in scope.type_aliases: + return scope.type_aliases[var_name], False if var_name in scope.structures: return scope.structures[var_name], False @@ -205,6 +207,10 @@ def _find_in_contract( if sig == var_name: return modifier + type_aliases = contract.type_aliases_as_dict + if var_name in type_aliases: + return type_aliases[var_name] + # structures are looked on the contract declarer structures = contract.structures_as_dict if var_name in structures: @@ -362,9 +368,6 @@ def find_variable( if var_name in current_scope.renaming: var_name = current_scope.renaming[var_name] - if var_name in current_scope.user_defined_types: - return current_scope.user_defined_types[var_name], False - # Use ret0/ret1 to help mypy ret0 = _find_variable_from_ref_declaration( referenced_declaration, direct_contracts, direct_functions diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index 00ac3a519..ecfecb4f0 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -344,10 +344,10 @@ class SlitherCompilationUnitSolc(CallerContextExpression): original_type = ElementaryType(underlying_type["name"]) - user_defined_type = TypeAliasTopLevel(original_type, alias, scope) - user_defined_type.set_offset(top_level_data["src"], self._compilation_unit) - self._compilation_unit.user_defined_value_types[alias] = user_defined_type - scope.user_defined_types[alias] = user_defined_type + type_alias = TypeAliasTopLevel(original_type, alias, scope) + type_alias.set_offset(top_level_data["src"], self._compilation_unit) + self._compilation_unit.type_aliases[alias] = type_alias + scope.type_aliases[alias] = type_alias else: raise SlitherException(f"Top level {top_level_data[self.get_key()]} not supported") diff --git a/slither/solc_parsing/solidity_types/type_parsing.py b/slither/solc_parsing/solidity_types/type_parsing.py index e12290722..c03b8e656 100644 --- a/slither/solc_parsing/solidity_types/type_parsing.py +++ b/slither/solc_parsing/solidity_types/type_parsing.py @@ -235,7 +235,7 @@ def parse_type( sl: "SlitherCompilationUnit" renaming: Dict[str, str] - user_defined_types: Dict[str, TypeAlias] + type_aliases: Dict[str, TypeAlias] enums_direct_access: List["Enum"] = [] # Note: for convenicence top level functions use the same parser than function in contract # but contract_parser is set to None @@ -247,13 +247,13 @@ def parse_type( sl = caller_context.compilation_unit next_context = caller_context renaming = {} - user_defined_types = sl.user_defined_value_types + type_aliases = sl.type_aliases else: assert isinstance(caller_context, FunctionSolc) sl = caller_context.underlying_function.compilation_unit next_context = caller_context.slither_parser renaming = caller_context.underlying_function.file_scope.renaming - user_defined_types = caller_context.underlying_function.file_scope.user_defined_types + type_aliases = caller_context.underlying_function.file_scope.type_aliases structures_direct_access = sl.structures_top_level all_structuress = [c.structures for c in sl.contracts] all_structures = [item for sublist in all_structuress for item in sublist] @@ -299,7 +299,7 @@ def parse_type( functions = list(scope.functions) renaming = scope.renaming - user_defined_types = scope.user_defined_types + type_aliases = scope.type_aliases elif isinstance(caller_context, (ContractSolc, FunctionSolc)): sl = caller_context.compilation_unit if isinstance(caller_context, FunctionSolc): @@ -329,7 +329,7 @@ def parse_type( functions = contract.functions + contract.modifiers renaming = scope.renaming - user_defined_types = scope.user_defined_types + type_aliases = scope.type_aliases else: raise ParsingError(f"Incorrect caller context: {type(caller_context)}") @@ -343,8 +343,8 @@ def parse_type( name = t.name if name in renaming: name = renaming[name] - if name in user_defined_types: - return user_defined_types[name] + if name in type_aliases: + return type_aliases[name] return _find_from_type_name( name, functions, @@ -365,9 +365,9 @@ def parse_type( name = t["typeDescriptions"]["typeString"] if name in renaming: name = renaming[name] - if name in user_defined_types: - _add_type_references(user_defined_types[name], t["src"], sl) - return user_defined_types[name] + if name in type_aliases: + _add_type_references(type_aliases[name], t["src"], sl) + return type_aliases[name] type_found = _find_from_type_name( name, functions, @@ -386,9 +386,9 @@ def parse_type( name = t["attributes"][type_name_key] if name in renaming: name = renaming[name] - if name in user_defined_types: - _add_type_references(user_defined_types[name], t["src"], sl) - return user_defined_types[name] + if name in type_aliases: + _add_type_references(type_aliases[name], t["src"], sl) + return type_aliases[name] type_found = _find_from_type_name( name, functions, @@ -407,8 +407,8 @@ def parse_type( name = t["name"] if name in renaming: name = renaming[name] - if name in user_defined_types: - return user_defined_types[name] + if name in type_aliases: + return type_aliases[name] type_found = _find_from_type_name( name, functions, diff --git a/slither/visitors/slithir/expression_to_slithir.py b/slither/visitors/slithir/expression_to_slithir.py index 005ad81a4..a1dadbb63 100644 --- a/slither/visitors/slithir/expression_to_slithir.py +++ b/slither/visitors/slithir/expression_to_slithir.py @@ -516,8 +516,8 @@ class ExpressionToSlithIR(ExpressionVisitor): # contract A { type MyInt is int} # contract B { function f() public{ A.MyInt test = A.MyInt.wrap(1);}} # The logic is handled by _post_call_expression - if expression.member_name in expr.file_scope.user_defined_types: - set_val(expression, expr.file_scope.user_defined_types[expression.member_name]) + if expression.member_name in expr.file_scope.type_aliases: + set_val(expression, expr.file_scope.type_aliases[expression.member_name]) return # Lookup errors referred to as member of contract e.g. Test.myError.selector if expression.member_name in expr.custom_errors_as_dict: diff --git a/tests/e2e/solc_parsing/test_ast_parsing.py b/tests/e2e/solc_parsing/test_ast_parsing.py index 307e6736f..28ac79986 100644 --- a/tests/e2e/solc_parsing/test_ast_parsing.py +++ b/tests/e2e/solc_parsing/test_ast_parsing.py @@ -459,6 +459,7 @@ ALL_TESTS = [ ["0.6.9", "0.7.6", "0.8.16"], ), Test("user_defined_operators-0.8.19.sol", ["0.8.19"]), + Test("type-aliases.sol", ["0.8.19"]), ] # create the output folder if needed try: diff --git a/tests/e2e/solc_parsing/test_data/compile/type-aliases.sol-0.8.19-compact.zip b/tests/e2e/solc_parsing/test_data/compile/type-aliases.sol-0.8.19-compact.zip new file mode 100644 index 0000000000000000000000000000000000000000..0c4d2b1c5e2a155c49cf64c3815266e1b19dc65c GIT binary patch literal 2111 zcma);c{~%01I9OJxsQm+D3WuIO=yngT$3ZTFiXQoIT}L9BFBu}xd~_y6zn`904c-+!L}zmKIUGm9R83BUvJOW8wPCc@A!*#UsH%K(5h z008iN@(8bl^Ki%c;(Z~$-X4zlz(?LbevUXF=lkydc*p}^Z?Cf~tN<(k@C*RZA(7mm zzo3MIk-<7RcTSg=`{h<}<8_p<#y;otLnW1npKSTWw~f zBa8XyV}68OuQM$JorFk_!s@gN;3P;l+y(BnDnyfl4kkAS`vo*Ma{A=wTpBg0$Ay>o z3-uHV%T+$-vSxLP?B0_|c3p6-ITv0jnLO9<&3+woic|Ep-Xn)} zM@b=K%e93)mX0S(p}N0n^gG*J4w(KXju6RV4#S6+?gsVPPkrA#D@_?6qn7H&9=ZJ! z=~uLW^wC>^Xwurpy*dq!5f%AaMR;Nw}WPdo=paOK7wNJQj8sd)A z(c%O>+;+^{~yroyf>)K}#v%W(-L+m5kT?$l}GW5*ylr3eb|t#Fm}7ZroySAPZ6To~5Rh z9H?q>UVPF~au$?j!J6nF+|9SC8kxc5ywCA_47jFn2x;^J+)D)JA7*# zyv}r%eesE!N0uq6-{M4ElI|)bj}~aPCal-QXVZ<($k=*6*5Hxbs8yK`hJ)r37N1#s zS*q0^2<+uzzTpI=*Hga*HS=+Ol2CLv{{k_ZX&*H7hL735m;T^pXTdrkU0voWhMBW! zz!wg_$yRFI^XJZh@}BE^Lp)8O3YHbCaE6i`qHk7?=#9o*vwEQVz~#o!cndx=B~=}) zT!%~=x*|G&i~#ZCOtA)Z+xSvd$Q)rD<+0v;3>UD{SH_BTTM~ZM^dz!MYO-{c)*MRT|}$l zZ0{y$#>g^Q_Rz6W*6Xg|?~IR7Wl5}00 zlO3D6$-)NmXPH|7Blvz1u+KQ%NU-90C)pJ5`#lgVQX5oehE;Q|Q!n>pA07QBjH}Wn z4+?m1=N;3G5Hxx3PyeVbqz77NrAEM>Yb+Dk7JyaPj&>&jFd1JG`#xf5WJ*R3c^=yA z^c1C(d`+h~+JX{DtV$uK4^zcL_dSQ$^-rCXQj5BHzl8Q+MHw&$4@z`99Hq~5Ujbv9 z?D+xX7DgwAG8T7NUvRe1(%d)=aBDIc(Nl+;7{t&u=0(JMRhY6^64O>L`{j4I(4XK%6J!mO%ueAL?(?-2om<8Od!|^T6Os znLvRB>A3lU9z6TPKPhi#D=2Dx@>0d%IQ-(*w40I!C9WukjBHu!RP2^;Z{{5YGvW52 z4D0XthxB2a`(=m2z*lU)ekQyPY;n61riRLo%`nEjEQx@9h8Kzoy7<(+|J^cY5^LQw zrEF6NcYn(1^6q^G2O|)e3E>h8D(nmS(p*-#OR;^RLc2hB-AJ2$l38L*IMEfzM7&6H zuuU%Q2*2w82(4h?qV)~2H2pibUnn7OwxR<901V9}euaH#5bA|vT5@Ei^1wS$%B^}* z6b_iiqIm8exPe8aXQzz0W`!Uq73BBJEH1yVd$z8fFX~+tvI!>5sN9(Ww?|>bF9ZDrut|7S&tv#F`21SA{*q2}QkJ8PU1Iw0d=Ne$?udB}g9Y;4K{( z(9V~nr9s1$4|NxO(mjnSr8MAm{uYV1;\n1[label=\"Node Type: NEW VARIABLE 1\n\"];\n}\n" + }, + "DeleteTest": {} +} \ No newline at end of file diff --git a/tests/e2e/solc_parsing/test_data/type-aliases.sol b/tests/e2e/solc_parsing/test_data/type-aliases.sol new file mode 100644 index 000000000..53fdaabeb --- /dev/null +++ b/tests/e2e/solc_parsing/test_data/type-aliases.sol @@ -0,0 +1,20 @@ + +struct Z { + int x; + int y; +} + +contract OtherTest { + struct Z { + int x; + int y; + } + + function myfunc() external { + Z memory z = Z(2,3); + } +} + +contract DeleteTest { + type Z is int; +} diff --git a/tests/unit/core/test_source_mapping.py b/tests/unit/core/test_source_mapping.py index fe5335977..55eb08295 100644 --- a/tests/unit/core/test_source_mapping.py +++ b/tests/unit/core/test_source_mapping.py @@ -85,15 +85,13 @@ def test_references_user_defined_aliases(solc_binary_path): file = Path(SRC_MAPPING_TEST_ROOT, "ReferencesUserDefinedAliases.sol").as_posix() slither = Slither(file, solc=solc_path) - alias_top_level = slither.compilation_units[0].user_defined_value_types["aliasTopLevel"] + alias_top_level = slither.compilation_units[0].type_aliases["aliasTopLevel"] assert len(alias_top_level.references) == 2 lines = _sort_references_lines(alias_top_level.references) assert lines == [12, 16] alias_contract_level = ( - slither.compilation_units[0] - .contracts[0] - .file_scope.user_defined_types["C.aliasContractLevel"] + slither.compilation_units[0].contracts[0].file_scope.type_aliases["C.aliasContractLevel"] ) assert len(alias_contract_level.references) == 2 lines = _sort_references_lines(alias_contract_level.references)