Bug: C419 autofix removing list within any/all causes undesirable short circuiting

[jamesbraza]

Please see the below code:

class Worker:
    """Asynchronous worker."""

    def wait(self) -> bool:
        """Wait for worker thread to join."""
        # Code wraps threading.Event.wait()


def wait_all(*workers: Worker) -> bool:
    return all([w.wait() for w in workers])  # List stops short circuit

With ruff==0.0.265, and running ruff --select=C419 --fix a.py, it removes the list cast:

def wait_all(*workers: Worker) -> bool:
    return all(w.wait() for w in workers)  # List stops short circuit

However, this actually added a subtle bug, because now the all statement will be short-circuited. This short circuiting (in my scenario here) will cause all workers beyond the first w.wait() return of False to not be awaited.

I think C4 autofixing should not be applied within all/any, as these can be short circuited, which can cause undesirable side effects.

[charliermarsh]

Ah yeah, this is intentional. In the future we'll probably qualify this as a suggested, and not an automated fix, since it changes the semantics of the program, but I wouldn't want to remove it entirely, since this is the entire rule and it's generally the correct fix.

[zanieb]

We can make that change as part of #4181, specifically #4184 which should be ready to work on once #4303 is merged.

[jamesbraza]

but I wouldn't want to remove it entirely, since this is the entire rule and it's generally the correct fix.

Just to share my two cents here, I agree removing extra list casts is generally the correct fix, especially with argument unpacking into *args.

I would argue though, when short circuiting can happen, that C4 actually changes the core behavior. Imo autofixes should not be able to change fundamental behaviors.