[`perflint`] implement quick-fix for `manual-list-comprehension` (`PERF401`)

[w0nder1ng]

Summary

I implemented a fix to rewrite for-loops where PERF401 applies into list.extends. Since the target is guaranteed to be a list, and the lint (seemingly) guarantees that the iterator doesn't reference itself, this should be valid for all for-loops where the lint is applicable. It would be nice to implement this to rewrite the entire for-loop/variable assignment into a single list comprehensions, but I thought of several cases where this would affect how the code works, so this would need more consideration to write.

Test Plan

I tried this on local files and it seemed to work, but I couldn't find where tests for ruff check --fix lints go. I'd be happy to add tests if someone points me in the right direction.

[MichaReiser]

Nice, thank you. Would you mind adding a few tests showing the fix? Are there cases where a comprehension would be preferred over list.extend. If so, then I don't think we should provide a code fix.

[w0nder1ng]

Again, I'm not sure where to add tests for fixes.

[MichaReiser]

You can extend the existing tests. The testing framework runs your lint rule against the file and snapshots all created diagnostics with their fixes.

https://github.com/astral-sh/ruff/blob/b34278e0cd25a24b69f4786897ca80c19fc3d5f1/crates/ruff_linter/src/rules/perflint/mod.rs#L20

[github-actions[bot]]

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+49 -49 violations, +0 -0 fixes in 4 projects; 50 projects unchanged)

apache/airflow (+31 -31 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ dev/breeze/src/airflow_breeze/commands/ci_commands.py:374:25: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/commands/ci_commands.py:374:25: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/commands/release_management_commands.py:3055:9: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/commands/release_management_commands.py:3055:9: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/commands/sbom_commands.py:973:13: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/commands/sbom_commands.py:973:13: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/params/shell_params.py:385:13: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/params/shell_params.py:385:13: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/exclude_from_matrix.py:32:9: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/exclude_from_matrix.py:32:9: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/packages.py:326:9: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/packages.py:326:9: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/reproducible.py:124:17: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/reproducible.py:124:17: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/selective_checks.py:1073:17: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/selective_checks.py:1073:17: PERF401 Use a list comprehension to create a transformed list
+ docs/exts/docs_build/fetch_inventories.py:103:9: PERF401 Use `list.extend` to create a transformed list
- docs/exts/docs_build/fetch_inventories.py:103:9: PERF401 Use a list comprehension to create a transformed list
+ docs/exts/docs_build/fetch_inventories.py:111:9: PERF401 Use `list.extend` to create a transformed list
- docs/exts/docs_build/fetch_inventories.py:111:9: PERF401 Use a list comprehension to create a transformed list
+ docs/exts/docs_build/fetch_inventories.py:119:9: PERF401 Use `list.extend` to create a transformed list
- docs/exts/docs_build/fetch_inventories.py:119:9: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/hooks/s3.py:486:21: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/hooks/s3.py:486:21: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/hooks/s3.py:669:21: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/hooks/s3.py:669:21: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/log/s3_task_handler.py:122:17: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/log/s3_task_handler.py:122:17: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/operators/sagemaker.py:1210:17: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/operators/sagemaker.py:1210:17: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/operators/sagemaker.py:379:17: PERF401 Use `list.extend` to create a transformed list
... 31 additional changes omitted for project

apache/superset (+11 -11 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ scripts/benchmark_migration.py:128:21: PERF401 Use `list.extend` to create a transformed list
- scripts/benchmark_migration.py:128:21: PERF401 Use a list comprehension to create a transformed list
+ superset/db_engine_specs/base.py:107:9: PERF401 Use `list.extend` to create a transformed list
- superset/db_engine_specs/base.py:107:9: PERF401 Use a list comprehension to create a transformed list
+ superset/tasks/cache.py:192:17: PERF401 Use `list.extend` to create a transformed list
- superset/tasks/cache.py:192:17: PERF401 Use a list comprehension to create a transformed list
+ superset/utils/core.py:1184:17: PERF401 Use `list.extend` to create a transformed list
- superset/utils/core.py:1184:17: PERF401 Use a list comprehension to create a transformed list
+ tests/integration_tests/charts/api_tests.py:353:13: PERF401 Use `list.extend` to create a transformed list
- tests/integration_tests/charts/api_tests.py:353:13: PERF401 Use a list comprehension to create a transformed list
+ tests/integration_tests/charts/api_tests.py:467:13: PERF401 Use `list.extend` to create a transformed list
... 11 additional changes omitted for project

bokeh/bokeh (+7 -7 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --no-preview --select ALL

+ src/bokeh/layouts.py:475:25: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/layouts.py:475:25: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/plotting/_figure.py:479:17: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/plotting/_figure.py:479:17: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/plotting/_figure.py:485:17: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/plotting/_figure.py:485:17: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/server/contexts.py:310:17: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/server/contexts.py:310:17: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/settings.py:780:21: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/settings.py:780:21: PERF401 Use a list comprehension to create a transformed list
... 4 additional changes omitted for project

pandas-dev/pandas (+0 -0 violations, +0 -0 fixes)

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PERF401	98	49	49	0	0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+49 -49 violations, +0 -0 fixes in 3 projects; 51 projects unchanged)

apache/airflow (+31 -31 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ dev/breeze/src/airflow_breeze/commands/ci_commands.py:374:25: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/commands/ci_commands.py:374:25: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/commands/release_management_commands.py:3055:9: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/commands/release_management_commands.py:3055:9: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/commands/sbom_commands.py:973:13: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/commands/sbom_commands.py:973:13: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/params/shell_params.py:385:13: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/params/shell_params.py:385:13: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/exclude_from_matrix.py:32:9: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/exclude_from_matrix.py:32:9: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/packages.py:326:9: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/packages.py:326:9: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/reproducible.py:124:17: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/reproducible.py:124:17: PERF401 Use a list comprehension to create a transformed list
+ dev/breeze/src/airflow_breeze/utils/selective_checks.py:1073:17: PERF401 Use `list.extend` to create a transformed list
- dev/breeze/src/airflow_breeze/utils/selective_checks.py:1073:17: PERF401 Use a list comprehension to create a transformed list
+ docs/exts/docs_build/fetch_inventories.py:103:9: PERF401 Use `list.extend` to create a transformed list
- docs/exts/docs_build/fetch_inventories.py:103:9: PERF401 Use a list comprehension to create a transformed list
+ docs/exts/docs_build/fetch_inventories.py:111:9: PERF401 Use `list.extend` to create a transformed list
- docs/exts/docs_build/fetch_inventories.py:111:9: PERF401 Use a list comprehension to create a transformed list
+ docs/exts/docs_build/fetch_inventories.py:119:9: PERF401 Use `list.extend` to create a transformed list
- docs/exts/docs_build/fetch_inventories.py:119:9: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/hooks/s3.py:486:21: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/hooks/s3.py:486:21: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/hooks/s3.py:669:21: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/hooks/s3.py:669:21: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/log/s3_task_handler.py:122:17: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/log/s3_task_handler.py:122:17: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/operators/sagemaker.py:1210:17: PERF401 Use `list.extend` to create a transformed list
- providers/src/airflow/providers/amazon/aws/operators/sagemaker.py:1210:17: PERF401 Use a list comprehension to create a transformed list
+ providers/src/airflow/providers/amazon/aws/operators/sagemaker.py:379:17: PERF401 Use `list.extend` to create a transformed list
... 31 additional changes omitted for project

apache/superset (+11 -11 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ scripts/benchmark_migration.py:128:21: PERF401 Use `list.extend` to create a transformed list
- scripts/benchmark_migration.py:128:21: PERF401 Use a list comprehension to create a transformed list
+ superset/db_engine_specs/base.py:107:9: PERF401 Use `list.extend` to create a transformed list
- superset/db_engine_specs/base.py:107:9: PERF401 Use a list comprehension to create a transformed list
+ superset/tasks/cache.py:192:17: PERF401 Use `list.extend` to create a transformed list
- superset/tasks/cache.py:192:17: PERF401 Use a list comprehension to create a transformed list
+ superset/utils/core.py:1184:17: PERF401 Use `list.extend` to create a transformed list
- superset/utils/core.py:1184:17: PERF401 Use a list comprehension to create a transformed list
+ tests/integration_tests/charts/api_tests.py:353:13: PERF401 Use `list.extend` to create a transformed list
- tests/integration_tests/charts/api_tests.py:353:13: PERF401 Use a list comprehension to create a transformed list
+ tests/integration_tests/charts/api_tests.py:467:13: PERF401 Use `list.extend` to create a transformed list
... 11 additional changes omitted for project

bokeh/bokeh (+7 -7 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL

+ src/bokeh/layouts.py:475:25: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/layouts.py:475:25: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/plotting/_figure.py:479:17: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/plotting/_figure.py:479:17: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/plotting/_figure.py:485:17: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/plotting/_figure.py:485:17: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/server/contexts.py:310:17: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/server/contexts.py:310:17: PERF401 Use a list comprehension to create a transformed list
+ src/bokeh/settings.py:780:21: PERF401 Use `list.extend` to create a transformed list
- src/bokeh/settings.py:780:21: PERF401 Use a list comprehension to create a transformed list
... 4 additional changes omitted for project

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PERF401	98	49	49	0	0

[w0nder1ng]

One thing that I wasn't sure how to handle is cases where there is control flow between the binding and the loop.

def f(early_return):
    result = []
    if early_return:
        return
    for i in range(10):
        result.append(i+1)

Currently, this fix offers to rewrite the code:

def f(early_return):
    result = [i+1 for i in range(10)]
    if early_return:
        return

How do you think this case ought to be handled?

[w0nder1ng]

I've modified the code to preserve comments and not leave a newline when removing the for loop. The tests seem to be failing because the fix is now gated behind a preview--is there a way to allow the test to set the preview flag, or should I temporarily change the fix availability to "sometimes"?

[dhruvmanila]

The tests seem to be failing because the fix is now gated behind a preview--is there a way to allow the test to set the preview flag, or should I temporarily change the fix availability to "sometimes"?

@w0nder1ng You'll need to add a new test case where the preview flag is enabled. Refer to the following as an example:

https://github.com/astral-sh/ruff/blob/8d7dda9fb7066f02ebedc1e63a270861816c4c72/crates/ruff_linter/src/rules/ruff/mod.rs#L386-L402

[w0nder1ng]

How should I deal with the existing test case?

[MichaReiser]

How should I deal with the existing test case?

You would have to mark the rule as sometimes fixable. I suggest to add a TODO comment to the preview check that says that the rule should be changed to always fixable when promoting the fix to stable.

[w0nder1ng]

I changed the diagnostic message, does the new one make more sense? Also, do you know why the tests aren't running?

[dhruvmanila]

@w0nder1ng there are merge conflicts which needs to be resolved to get the CI running

[MichaReiser]

Hmm, what's up with the ecosystem check... Let's re-run to get a better sense of the changes

[MichaReiser]

I suspect that the ecosystem check is panicking during the fix generation. Let me run it locally

[w0nder1ng]

The issue was that having no comments inside the for loop caused an empty insert, and that made a debug assert panic. The reason I didn't catch it before was because every test case for the fix has # PERF401 following the lint target! I added a new test case to hopefully spot this type of error.

[MichaReiser]

Ah, that makes sense. This is great. Thank you so much for working on the fix