fix: reorder named arguments to match declaration order (#1949)

Store names list in call operations and expressions. Reorder named arguments to match declaration order.

A function call with explicitly named parameters, of the form f({argName1: arg1, ...}), may list the parameters in an order different from that of the function's declaration. In this case, the IR should reorder the arguments to match the function declaration order before generating IR.

This PR implements named argument reordering for the following forms of function calls:
- struct constructors (most important)
- static internal calls

For the following forms of function calls, the order of the declaration arguments is not directly available during IR generation, so the PR did not implement reordering for them:
- external calls (HighLevelCall)
- library calls
- event calls

The get_declared_param_names function returns None for these unimplemented call forms.

slither/core/expressions/call_expression.py

@@ -4,19 +4,47 @@
 
 
 class CallExpression(Expression):  # pylint: disable=too-many-instance-attributes
-    def __init__(self, called: Expression, arguments: List[Any], type_call: str) -> None:
+    def __init__(
+        self,
+        called: Expression,
+        arguments: List[Any],
+        type_call: str,
+        names: Optional[List[str]] = None,
+    ) -> None:
+        """
+        #### Parameters
+        called -
+            The expression denoting the function to be called
+        arguments -
+            List of argument expressions
+        type_call -
+            A string formatting of the called function's return type
+        names -
+            For calls with named fields, list fields in call order.
+            For calls without named fields, None.
+        """
         assert isinstance(called, Expression)
+        assert (names is None) or isinstance(names, list)
         super().__init__()
         self._called: Expression = called
         self._arguments: List[Expression] = arguments
         self._type_call: str = type_call
+        self._names: Optional[List[str]] = names
         # gas and value are only available if the syntax is {gas: , value: }
         # For the .gas().value(), the member are considered as function call
         # And converted later to the correct info (convert.py)
         self._gas: Optional[Expression] = None
         self._value: Optional[Expression] = None
         self._salt: Optional[Expression] = None
 
+    @property
+    def names(self) -> Optional[List[str]]:
+        """
+        For calls with named fields, list fields in call order.
+        For calls without named fields, None.
+        """
+        return self._names
+
     @property
     def call_value(self) -> Optional[Expression]:
         return self._value
@@ -62,4 +90,9 @@ def __str__(self) -> str:
             if gas or value or salt:
                 options = [gas, value, salt]
                 txt += "{" + ",".join([o for o in options if o != ""]) + "}"
-        return txt + "(" + ",".join([str(a) for a in self._arguments]) + ")"
+        args = (
+            "{" + ",".join([f"{n}:{str(a)}" for (a, n) in zip(self._arguments, self._names)]) + "}"
+            if self._names is not None
+            else ",".join([str(a) for a in self._arguments])
+        )
+        return txt + "(" + args + ")"

slither/slithir/convert.py

@@ -385,6 +385,70 @@ def integrate_value_gas(result: List[Operation]) -> List[Operation]:
 ###################################################################################
 
 
+def get_declared_param_names(
+    ins: Union[
+        NewStructure,
+        NewContract,
+        InternalCall,
+        LibraryCall,
+        HighLevelCall,
+        InternalDynamicCall,
+        EventCall,
+    ]
+) -> Optional[List[str]]:
+    """
+    Given a call operation, return the list of parameter names, in the order
+    listed in the function declaration.
+    #### Parameters
+    ins -
+        The call instruction
+    #### Possible Returns
+    List[str] -
+        A list of the parameters in declaration order
+    None -
+        Workaround: Unable to obtain list of parameters in declaration order
+    """
+    if isinstance(ins, NewStructure):
+        return [x.name for x in ins.structure.elems_ordered if not isinstance(x.type, MappingType)]
+    if isinstance(ins, (InternalCall, LibraryCall, HighLevelCall)):
+        if isinstance(ins.function, Function):
+            return [p.name for p in ins.function.parameters]
+        return None
+    if isinstance(ins, InternalDynamicCall):
+        return [p.name for p in ins.function_type.params]
+
+    assert isinstance(ins, (EventCall, NewContract))
+    return None
+
+
+def reorder_arguments(
+    args: List[Variable], call_names: List[str], decl_names: List[str]
+) -> List[Variable]:
+    """
+    Reorder named struct constructor arguments so that they match struct declaration ordering rather
+    than call ordering
+    E.g. for `struct S { int x; int y; }` we reorder `S({y : 2, x : 3})` to `S(3, 2)`
+    #### Parameters
+    args -
+        Arguments to constructor call, in call order
+    names -
+        Parameter names in call order
+    decl_names -
+        Parameter names in declaration order
+    #### Returns
+    Reordered arguments to constructor call, now in declaration order
+    """
+    assert len(args) == len(call_names)
+    assert len(call_names) == len(decl_names)
+
+    args_ret = []
+    for n in decl_names:
+        ind = call_names.index(n)
+        args_ret.append(args[ind])
+
+    return args_ret
+
+
 def propagate_type_and_convert_call(result: List[Operation], node: "Node") -> List[Operation]:
     """
     Propagate the types variables and convert tmp call to real call operation
@@ -434,6 +498,23 @@ def propagate_type_and_convert_call(result: List[Operation], node: "Node") -> Li
             if ins.call_id in calls_gas and isinstance(ins, (HighLevelCall, InternalDynamicCall)):
                 ins.call_gas = calls_gas[ins.call_id]
 
+        if isinstance(ins, Call) and (ins.names is not None):
+            assert isinstance(
+                ins,
+                (
+                    NewStructure,
+                    NewContract,
+                    InternalCall,
+                    LibraryCall,
+                    HighLevelCall,
+                    InternalDynamicCall,
+                    EventCall,
+                ),
+            )
+            decl_param_names = get_declared_param_names(ins)
+            if decl_param_names is not None:
+                call_data = reorder_arguments(call_data, ins.names, decl_param_names)
+
         if isinstance(ins, (Call, NewContract, NewStructure)):
             # We might have stored some arguments for libraries
             if ins.arguments:
@@ -855,7 +936,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
         if isinstance(ins.ori.variable_left, Contract):
             st = ins.ori.variable_left.get_structure_from_name(ins.ori.variable_right)
             if st:
-                op = NewStructure(st, ins.lvalue)
+                op = NewStructure(st, ins.lvalue, names=ins.names)
                 op.set_expression(ins.expression)
                 op.call_id = ins.call_id
                 return op
@@ -892,6 +973,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
                 ins.nbr_arguments,
                 ins.lvalue,
                 ins.type_call,
+                names=ins.names,
             )
             libcall.set_expression(ins.expression)
             libcall.call_id = ins.call_id
@@ -950,6 +1032,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
                     len(lib_func.parameters),
                     ins.lvalue,
                     "d",
+                    names=ins.names,
                 )
                 lib_call.set_expression(ins.expression)
                 lib_call.set_node(ins.node)
@@ -1031,6 +1114,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
             ins.nbr_arguments,
             ins.lvalue,
             ins.type_call,
+            names=ins.names,
         )
         msgcall.call_id = ins.call_id
 
@@ -1082,7 +1166,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
         return n
 
     if isinstance(ins.called, Structure):
-        op = NewStructure(ins.called, ins.lvalue)
+        op = NewStructure(ins.called, ins.lvalue, names=ins.names)
         op.set_expression(ins.expression)
         op.call_id = ins.call_id
         op.set_expression(ins.expression)
@@ -1106,7 +1190,7 @@ def extract_tmp_call(ins: TmpCall, contract: Optional[Contract]) -> Union[Call,
         if len(ins.called.constructor.parameters) != ins.nbr_arguments:
             return Nop()
         internalcall = InternalCall(
-            ins.called.constructor, ins.nbr_arguments, ins.lvalue, ins.type_call
+            ins.called.constructor, ins.nbr_arguments, ins.lvalue, ins.type_call, ins.names
         )
         internalcall.call_id = ins.call_id
         internalcall.set_expression(ins.expression)
@@ -1440,6 +1524,7 @@ def look_for_library_or_top_level(
                 ir.nbr_arguments,
                 ir.lvalue,
                 ir.type_call,
+                names=ir.names,
             )
             lib_call.set_expression(ir.expression)
             lib_call.set_node(ir.node)

slither/slithir/operations/call.py

@@ -6,9 +6,25 @@
 
 
 class Call(Operation):
-    def __init__(self) -> None:
+    def __init__(self, names: Optional[List[str]] = None) -> None:
+        """
+        #### Parameters
+        names -
+            For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+            Otherwise, None.
+        """
+        assert (names is None) or isinstance(names, list)
         super().__init__()
         self._arguments: List[Variable] = []
+        self._names = names
+
+    @property
+    def names(self) -> Optional[List[str]]:
+        """
+        For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+        Otherwise, None.
+        """
+        return self._names
 
     @property
     def arguments(self) -> List[Variable]:

slither/slithir/operations/high_level_call.py

@@ -28,11 +28,18 @@ def __init__(
         nbr_arguments: int,
         result: Optional[Union[TemporaryVariable, TupleVariable, TemporaryVariableSSA]],
         type_call: str,
+        names: Optional[List[str]] = None,
     ) -> None:
+        """
+        #### Parameters
+        names -
+            For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+            Otherwise, None.
+        """
         assert isinstance(function_name, Constant)
         assert is_valid_lvalue(result) or result is None
         self._check_destination(destination)
-        super().__init__()
+        super().__init__(names=names)
         # Contract is only possible for library call, which inherits from highlevelcall
         self._destination: Union[Variable, SolidityVariable, Contract] = destination  # type: ignore
         self._function_name = function_name

slither/slithir/operations/internal_call.py

@@ -20,8 +20,16 @@ def __init__(
             Union[TupleVariableSSA, TemporaryVariableSSA, TupleVariable, TemporaryVariable]
         ],
         type_call: str,
+        names: Optional[List[str]] = None,
     ) -> None:
-        super().__init__()
+        # pylint: disable=too-many-arguments
+        """
+        #### Parameters
+        names -
+            For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+            Otherwise, None.
+        """
+        super().__init__(names=names)
         self._contract_name = ""
         if isinstance(function, Function):
             self._function: Optional[Function] = function

slither/slithir/operations/new_contract.py

@@ -12,11 +12,20 @@
 
 class NewContract(Call, OperationWithLValue):  # pylint: disable=too-many-instance-attributes
     def __init__(
-        self, contract_name: Constant, lvalue: Union[TemporaryVariableSSA, TemporaryVariable]
+        self,
+        contract_name: Constant,
+        lvalue: Union[TemporaryVariableSSA, TemporaryVariable],
+        names: Optional[List[str]] = None,
     ) -> None:
+        """
+        #### Parameters
+        names -
+            For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+            Otherwise, None.
+        """
         assert isinstance(contract_name, Constant)
         assert is_valid_lvalue(lvalue)
-        super().__init__()
+        super().__init__(names=names)
         self._contract_name = contract_name
         # todo create analyze to add the contract instance
         self._lvalue = lvalue

slither/slithir/operations/new_structure.py

@@ -1,4 +1,4 @@
-from typing import List, Union
+from typing import List, Optional, Union
 
 from slither.slithir.operations.call import Call
 from slither.slithir.operations.lvalue import OperationWithLValue
@@ -17,8 +17,15 @@ def __init__(
         self,
         structure: StructureContract,
         lvalue: Union[TemporaryVariableSSA, TemporaryVariable],
+        names: Optional[List[str]] = None,
     ) -> None:
-        super().__init__()
+        """
+        #### Parameters
+        names -
+            For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+            Otherwise, None.
+        """
+        super().__init__(names=names)
         assert isinstance(structure, Structure)
         assert is_valid_lvalue(lvalue)
         self._structure = structure

slither/slithir/tmp_operations/tmp_call.py

@@ -1,4 +1,4 @@
-from typing import Optional, Union
+from typing import List, Optional, Union
 
 from slither.core.declarations import (
     Event,
@@ -25,7 +25,15 @@ def __init__(
         nbr_arguments: int,
         result: Union[TupleVariable, TemporaryVariable],
         type_call: str,
+        names: Optional[List[str]] = None,
     ) -> None:
+        # pylint: disable=too-many-arguments
+        """
+        #### Parameters
+        names -
+            For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+            Otherwise, None.
+        """
         assert isinstance(
             called,
             (
@@ -42,13 +50,22 @@ def __init__(
         self._called = called
         self._nbr_arguments = nbr_arguments
         self._type_call = type_call
+        self._names = names
         self._lvalue = result
         self._ori = None  #
         self._callid = None
         self._gas = None
         self._value = None
         self._salt = None
 
+    @property
+    def names(self) -> Optional[List[str]]:
+        """
+        For calls of the form f({argName1 : arg1, ...}), the names of parameters listed in call order.
+        Otherwise, None.
+        """
+        return self._names
+
     @property
     def call_value(self):
         return self._value