Skip to content

Stabilize for-loop-set-mutations (FURB142) #18557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented Jun 8, 2025

No description provided.

@dylwil3 dylwil3 added this to the v0.12 milestone Jun 8, 2025
@dylwil3 dylwil3 added the rule Implementing or modifying a lint rule label Jun 8, 2025
Copy link
Contributor

github-actions bot commented Jun 8, 2025

ruff-ecosystem results

Linter (stable)

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

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

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

+ airflow-core/src/airflow/utils/dag_edges.py:74:17: FURB142 [*] Use of `set.add()` in a for loop
+ dev/breeze/src/airflow_breeze/utils/docker_command_utils.py:525:5: FURB142 [*] Use of `set.add()` in a for loop
+ dev/breeze/src/airflow_breeze/utils/provider_dependencies.py:53:9: FURB142 [*] Use of `set.add()` in a for loop
+ dev/breeze/src/airflow_breeze/utils/selective_checks.py:952:17: FURB142 [*] Use of `set.add()` in a for loop
+ dev/breeze/src/airflow_breeze/utils/selective_checks.py:958:17: FURB142 [*] Use of `set.add()` in a for loop
+ dev/breeze/src/airflow_breeze/utils/selective_checks.py:986:17: FURB142 [*] Use of `set.add()` in a for loop
+ dev/stats/calculate_statistics_provider_testing_issues.py:110:5: FURB142 [*] Use of `set.add()` in a for loop
+ dev/stats/calculate_statistics_provider_testing_issues.py:117:5: FURB142 [*] Use of `set.add()` in a for loop
+ providers/amazon/src/airflow/providers/amazon/aws/hooks/glue_catalog.py:124:13: FURB142 [*] Use of `set.add()` in a for loop
+ providers/amazon/src/airflow/providers/amazon/aws/hooks/glue_catalog.py:82:13: FURB142 [*] Use of `set.add()` in a for loop
+ scripts/tools/list-integrations.py:67:9: FURB142 [*] Use of `set.add()` in a for loop

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

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

+ superset/extensions/__init__.py:81:13: FURB142 [*] Use of `set.add()` in a for loop
+ superset/utils/dashboard_import_export.py:30:5: FURB142 [*] Use of `set.add()` in a for loop
+ superset/utils/pandas_postprocessing/pivot.py:89:13: FURB142 [*] Use of `set.add()` in a for loop
+ superset/viz.py:1662:13: FURB142 [*] Use of `set.add()` in a for loop
+ tests/integration_tests/security_tests.py:1504:9: FURB142 [*] Use of `set.add()` in a for loop
+ tests/integration_tests/security_tests.py:86:5: FURB142 [*] Use of `set.add()` in a for loop

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

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

+ tests/unit/bokeh/document/test_models.py:113:9: FURB142 [*] Use of `set.add()` in a for loop

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

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

+ zerver/lib/export.py:1852:9: FURB142 [*] Use of `set.add()` in a for loop
+ zerver/lib/streams.py:1668:9: FURB142 [*] Use of `set.add()` in a for loop
+ zerver/lib/subscription_info.py:690:13: FURB142 [*] Use of `set.add()` in a for loop
+ zerver/lib/thumbnail.py:295:5: FURB142 [*] Use of `set.add()` in a for loop
+ zerver/lib/user_groups.py:676:13: FURB142 [*] Use of `set.add()` in a for loop
+ zerver/tests/test_docs.py:335:9: FURB142 [*] Use of `set.add()` in a for loop
+ zerver/tests/test_docs.py:342:13: FURB142 [*] Use of `set.add()` in a for loop

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
FURB142 25 25 0 0 0

Linter (preview)

✅ ecosystem check detected no linter changes.

@ntBre ntBre force-pushed the brent/release-0.12.0 branch from 0370d8a to 040fb6b Compare June 8, 2025 19:55
@ntBre ntBre requested a review from AlexWaygood as a code owner June 8, 2025 19:55
@ntBre ntBre force-pushed the brent/release-0.12.0 branch from 040fb6b to 9252447 Compare June 8, 2025 20:18
@AlexWaygood AlexWaygood removed their request for review June 8, 2025 20:30
@ntBre
Copy link
Contributor

ntBre commented Jun 9, 2025

Not sure if it's a blocker, but there's a new issue on this one: #18575

@ntBre ntBre force-pushed the brent/release-0.12.0 branch from 9252447 to 829acf4 Compare June 9, 2025 00:22
@dylwil3 dylwil3 force-pushed the dylan/stabilize-furb142 branch from 58c3afd to 8316b90 Compare June 9, 2025 13:04
@dylwil3
Copy link
Collaborator Author

dylwil3 commented Jun 9, 2025

Hmm... I think it could be a blocker, actually. The fix would be to check whether the loop variable is used outside of the loop (this is already done in for-loop-writes so we can steal another helper function).

But I think this ends up being a fairly significant change if someone has a lot of violations in the same scope that re-use the loop variable name. (For example - our test fixture fails to converge after 10 iterations if we make this behavior change).

Not sure how much of a big deal that is in practice, but probably worth delaying stabilization until that behavior has sat in preview for a round.

@dylwil3 dylwil3 closed this Jun 9, 2025
@dylwil3 dylwil3 mentioned this pull request Jun 9, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants