sixty-north / cosmic-ray

Mutation testing for Python
MIT License
556 stars 54 forks source link

ExceptionReplacer crashes on unexpected except clauses #527

Open BarrensZeppelin opened 2 years ago

BarrensZeppelin commented 2 years ago

The implementation of ExceptionReplacer._name_nodes expects exception clauses to either contain a single Name ast node or be a tuple of such nodes. However, the Python specification allows arbitrary expressions for exception clauses. This means that you can make the function crash if it is given an unexpected exception clause, such as:

try:
    pass
except IndexError or KeyError:
    pass

(which is nonsense and only catches IndexError, but is valid Python nonetheless)

This is the traceback when running cosmic-ray init ...:

Traceback (most recent call last):
  File "/home/zii/.virtualenvs/mutcr/bin/cosmic-ray", line 8, in <module>
    sys.exit(main())
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/cli.py", line 289, in main
    return cli(argv)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/cli.py", line 100, in init
    cosmic_ray.commands.init(modules, database)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/commands/init.py", line 56, in init
    work_db.add_work_items(_all_work_items(module_paths, operator_names))
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/work_db.py", line 99, in add_work_items
    session.add_all(storage)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/sqlalchemy/orm/session.py", line 2634, in add_all
    for instance in instances:
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/work_db.py", line 96, in <genexpr>
    storage = (_work_item_to_storage(work_item) for work_item in work_items)
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/commands/init.py", line 29, in _all_work_items
    for occurrence, (start_pos, end_pos) in enumerate(positions):
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/commands/init.py", line 23, in <genexpr>
    positions = (
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/operators/exception_replacer.py", line 16, in mutation_positions
    for name in self._name_nodes(node):
  File "/home/zii/.virtualenvs/mutcr/lib/python3.10/site-packages/cosmic_ray/operators/exception_replacer.py", line 36, in _name_nodes
    return test_list.children[::2]
AttributeError: 'Keyword' object has no attribute 'children'

And the relevant part of ExceptionReplacer:

class ExceptionReplacer(Operator):
    """An operator that modifies exception handlers."""

    def mutation_positions(self, node):
        if isinstance(node, PythonNode):
            if node.type == "except_clause":
                for name in self._name_nodes(node):
                    yield (name.start_pos, name.end_pos)

     # ...

    @staticmethod
    def _name_nodes(node):
        if isinstance(node.children[1], Name):
            return (node.children[1],)

        atom = node.children[1]
        test_list = atom.children[1]
        return test_list.children[::2]

I imagine the fix would be to return an empty list in _name_nodes if the node does not match one of the two supported scenarios.

abingham commented 2 years ago

I imagine the fix would be to return an empty list in _name_nodes if the node does not match one of the two supported scenarios.

Yes, this does sound like a reasonable approach. We probably don't want to get into the business of trying to examine arbitrary expressions to find exception names to change.