Repository: mandiant/capa
File: capa/engine.py (Python)

Code before:
#
# used throughout matching as the context in which features are searched:
# to check if a feature exists, do: `Number(0x10) in features`.
# to collect the locations of a feature, do: `features[Number(0x10)]`
#
# aliased here so that the type can be documented and xref'd.
FeatureSet = Dict[Feature, Set[Address]]


class Statement:
    """
    superclass for structural nodes, such as and/or/not.
    this exists to provide a default impl for `__str__` and `__repr__`,
     and to declare the interface method `evaluate`
    """

    def __init__(self, description=None):
        super().__init__()
        self.name = self.__class__.__name__
        self.description = description

    def __str__(self):
        if self.description:
            return f"{self.name.lower()}({','.join(map(str, self.get_children()))} = {self.description})"
        else:
            return f"{self.name.lower()}({','.join(map(str, self.get_children()))})"

    def __repr__(self):
        return str(self)

    def evaluate(self, features: FeatureSet, short_circuit=True) -> Result:
        """
        classes that inherit `Statement` must implement `evaluate`

        args:
            short_circuit (bool): if true, then statements like and/or/some may short circuit.
        """
        raise NotImplementedError()

    def get_children(self) -> Iterator[Union["Statement", Feature]]:
        if hasattr(self, "child"):
            # this really confuses mypy because the property may not exist
            # since its defined in the subclasses.
            child = self.child  # type: ignore
            assert isinstance(child, (Statement, Feature))
            yield child

        if hasattr(self, "children"):
            for child in getattr(self, "children"):
                assert isinstance(child, (Statement, Feature))
                yield child

Review comment (suggestion):
neither the old nor the new are very readable imho.

i think we should avoid putting much logic within the `{...}` blocks because its hard for humans to keep track of the context: its a string, no an interpolation, no function call, etc.

if the substitutions are complex, lets do something like:

```py
name = self.name.lower()
children = ','.join(map(str, self.get_children()))
return f"{name}({children} = {self.description})"
```

Code after:
#
# used throughout matching as the context in which features are searched:
# to check if a feature exists, do: `Number(0x10) in features`.
# to collect the locations of a feature, do: `features[Number(0x10)]`
#
# aliased here so that the type can be documented and xref'd.
FeatureSet = Dict[Feature, Set[Address]]


class Statement:
    """
    superclass for structural nodes, such as and/or/not.
    this exists to provide a default impl for `__str__` and `__repr__`,
     and to declare the interface method `evaluate`
    """

    def __init__(self, description=None):
        super().__init__()
        self.name = self.__class__.__name__
        self.description = description

    def __str__(self):
        name = self.name.lower()
        children = ",".join(map(str, self.get_children()))
        if self.description:
            return f"{name}({children} = {self.description})"
        else:
            return f"{name}({children})"

    def __repr__(self):
        return str(self)

    def evaluate(self, features: FeatureSet, short_circuit=True) -> Result:
        """
        classes that inherit `Statement` must implement `evaluate`

        args:
            short_circuit (bool): if true, then statements like and/or/some may short circuit.
        """
        raise NotImplementedError()

    def get_children(self) -> Iterator[Union["Statement", Feature]]:
        if hasattr(self, "child"):
            # this really confuses mypy because the property may not exist
            # since its defined in the subclasses.
            child = self.child  # type: ignore
            assert isinstance(child, (Statement, Feature))
            yield child

        if hasattr(self, "children"):
            for child in getattr(self, "children"):