astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.4k stars 1.08k forks source link

PERF401: split or improve message? #13791

Open xmo-odoo opened 2 weeks ago

xmo-odoo commented 2 weeks ago

I think PERF401 is a good rule, however while its message works fine for the "transformed copy" case e.g.

original = list(range(10000))
filtered = []
for i in original:
    if i % 2:
        filtered.append(i)

the feedback message of

Use a list comprehension to create a transformed list

is very confusing for the second case of a transforming expansion e.g.

original = list(range(10000))
filtered = get_a_base_list()
for i in original:
    if i % 2:
        filtered.append(i)

in this case the improvement would be

filtered.extend(i for i in original if i % 2)

But this involves neither a creation nor a list comprehension. Before I checked the lint's extended documentation I figured it was a false positive due to how "off" the message is.

Since the codes are already different from perflint source, I think the ideal would be to split this lint in two, and to extend the source span of PERF401 (the bit it says to replace) to encompass the collection instantiation and loop, not just the append call itself. For the other one, it might make sense to include the loop too.

MichaReiser commented 2 weeks ago

Makes sense. We should look into how complicated it would be to detect if the starting list is empty or not and adjust the message based on that. If that's not possible, we should try to refine the message to mention extend or at least update the documentation