Skip to content

Conversation

@K-dash
Copy link
Contributor

@K-dash K-dash commented Jun 1, 2025

Summary

Fixes false positive in B909 (loop-iterator-mutation) where mutations inside return/break statements were incorrectly flagged as violations.
The fix adds tracking for when mutations occur within return/break statements and excludes them from violation detection, as they don't cause the iteration issues B909 is designed to prevent.

Test Plan

  • Added test cases covering the reported false positive scenarios to B909.py
  • Verified existing B909 tests continue to pass (no regressions)
  • Ran cargo test -p ruff_linter --lib flake8_bugbear successfully

Fixes #18399

@ntBre ntBre added the rule Implementing or modifying a lint rule label Jun 1, 2025
@ntBre ntBre self-requested a review June 1, 2025 14:22
@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

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

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

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

+ airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py:78:22: PYI059 [*] `Generic[]` should always be the last base class
- airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py:78:22: PYI059 `Generic[]` should always be the last base class
+ airflow-core/src/airflow/api_fastapi/common/exceptions.py:37:23: PYI059 [*] `Generic[]` should always be the last base class
- airflow-core/src/airflow/api_fastapi/common/exceptions.py:37:23: PYI059 `Generic[]` should always be the last base class
+ airflow-core/src/airflow/api_fastapi/core_api/base.py:52:16: PYI059 [*] `Generic[]` should always be the last base class
- airflow-core/src/airflow/api_fastapi/core_api/base.py:52:16: PYI059 `Generic[]` should always be the last base class
+ airflow-core/src/airflow/api_fastapi/core_api/services/public/common.py:37:18: PYI059 [*] `Generic[]` should always be the last base class
- airflow-core/src/airflow/api_fastapi/core_api/services/public/common.py:37:18: PYI059 `Generic[]` should always be the last base class

langchain-ai/langchain (+0 -0 violations, +8 -0 fixes)

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

+ libs/core/langchain_core/output_parsers/base.py:31:26: PYI059 [*] `Generic[]` should always be the last base class
- libs/core/langchain_core/output_parsers/base.py:31:26: PYI059 `Generic[]` should always be the last base class
+ libs/core/langchain_core/prompts/base.py:45:25: PYI059 [*] `Generic[]` should always be the last base class
- libs/core/langchain_core/prompts/base.py:45:25: PYI059 `Generic[]` should always be the last base class
+ libs/core/langchain_core/runnables/base.py:111:15: PYI059 [*] `Generic[]` should always be the last base class
- libs/core/langchain_core/runnables/base.py:111:15: PYI059 `Generic[]` should always be the last base class
+ libs/core/langchain_core/stores.py:26:16: PYI059 [*] `Generic[]` should always be the last base class
- libs/core/langchain_core/stores.py:26:16: PYI059 `Generic[]` should always be the last base class

latchbio/latch (+0 -0 violations, +4 -0 fixes)

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

+ src/latch/types/metadata.py:440:25: PYI059 [*] `Generic[]` should always be the last base class
- src/latch/types/metadata.py:440:25: PYI059 `Generic[]` should always be the last base class
+ src/latch/types/metadata.py:489:24: PYI059 [*] `Generic[]` should always be the last base class
- src/latch/types/metadata.py:489:24: PYI059 `Generic[]` should always be the last base class

python/typeshed (+0 -0 violations, +2 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/asyncio/queues.pyi:28:12: PYI059 [*] `Generic[]` should always be the last base class
- stdlib/asyncio/queues.pyi:28:12: PYI059 `Generic[]` should always be the last base class

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

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

+ zerver/lib/notes.py:12:16: PYI059 [*] `Generic[]` should always be the last base class
- zerver/lib/notes.py:12:16: PYI059 `Generic[]` should always be the last base class
+ zerver/lib/queue.py:35:18: PYI059 [*] `Generic[]` should always be the last base class
- zerver/lib/queue.py:35:18: PYI059 `Generic[]` should always be the last base class

pytest-dev/pytest (+0 -1 violations, +0 -0 fixes)

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

- src/_pytest/recwarn.py:214:24: B909 Mutation to loop iterable `self._list` during iteration

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
PYI059 26 0 0 26 0
B909 1 0 1 0 0

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The new tests look great, but I think there might be a simpler fix for the issue.

@K-dash K-dash requested a review from ntBre June 3, 2025 22:27
@K-dash K-dash requested a review from ntBre June 8, 2025 01:24
@K-dash
Copy link
Contributor Author

K-dash commented Jun 13, 2025

@ntBre
I was wondering if you've had a chance to review this yet?
I would greatly appreciate your feedback when you have the opportunity.

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thank you! It's always fun to fix a bug by deleting code :)

@ntBre ntBre merged commit 793ff9b into astral-sh:main Jun 13, 2025
34 checks passed
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.

B909 false positive when directly returning mutation

2 participants