Skip to content

[ruff] Add fallible-context-manager (RUF075)#22844

Merged
ntBre merged 25 commits into
astral-sh:mainfrom
anishgirianish:add-rule-b036
May 20, 2026
Merged

[ruff] Add fallible-context-manager (RUF075)#22844
ntBre merged 25 commits into
astral-sh:mainfrom
anishgirianish:add-rule-b036

Conversation

@anishgirianish
Copy link
Copy Markdown
Contributor

@anishgirianish anishgirianish commented Jan 25, 2026

Summary

Adds a new rule RUF075 (preview) that detects @contextlib.contextmanager and @contextlib.asynccontextmanager decorated functions where yield is not protected against exceptions.

Closes #15629.

Details

When a context manager yields without exception protection, cleanup code after yield won't run if an exception is raised inside the with block.

# Bad - cleanup won't run on exception
@contextmanager
def bad():
    print("setup")
    yield
    print("cleanup")  # never runs if exception raised

# Good
@contextmanager
def good():
    print("setup")
    try:
        yield
    finally:
        print("cleanup")  # always runs

The rule does not flag:

  • Yields inside try blocks that have a finally or except handler
  • Yields as the last statement of a with block (cleanup is delegated to __exit__)
  • Yields in terminal position (last statement in the function body, or immediately before a return), where there is no trailing cleanup that could be skipped

Terminal position is inherited through if / for / while / match branches: a yield that is the last statement of a branch is terminal only when the enclosing statement is itself terminal.

The rule is in preview mode.

Test Plan

Tests live in crates/ruff_linter/resources/mdtest/ruff.md and are run via:

cargo nextest run -p ruff_mdtest -- mdtest::ruff

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Jan 25, 2026

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+87 -0 violations, +0 -0 fixes in 21 projects; 35 projects unchanged)

DisnakeDev/disnake (+1 -0 violations, +0 -0 fixes)

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

+ tests/ui/test_decorators.py:25:5: RUF075 Context manager does not catch exceptions

RasaHQ/rasa (+3 -0 violations, +0 -0 fixes)

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

+ rasa/engine/storage/local_model_storage.py:165:9: RUF075 Context manager does not catch exceptions
+ rasa/telemetry.py:797:5: RUF075 Context manager does not catch exceptions
+ tests/conftest.py:852:5: RUF075 Context manager does not catch exceptions

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

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

+ airflow-core/src/airflow/migrations/utils.py:27:9: RUF075 Context manager does not catch exceptions
+ airflow-core/src/airflow/utils/sqlalchemy.py:412:5: RUF075 Context manager does not catch exceptions
+ airflow-core/tests/unit/cli/commands/test_task_command.py:76:5: RUF075 Context manager does not catch exceptions
+ airflow-core/tests/unit/utils/test_cli_util.py:257:5: RUF075 Context manager does not catch exceptions
+ dev/breeze/src/airflow_breeze/utils/parallel.py:584:5: RUF075 Context manager does not catch exceptions
+ devel-common/src/sphinx_exts/docs_build/github_action_utils.py:38:5: RUF075 Context manager does not catch exceptions
+ devel-common/src/tests_common/test_utils/azure_system_helpers.py:104:5: RUF075 Context manager does not catch exceptions
+ devel-common/src/tests_common/test_utils/gcp_system_helpers.py:115:5: RUF075 Context manager does not catch exceptions
+ providers/cncf/kubernetes/tests/unit/cncf/kubernetes/operators/test_pod.py:96:5: RUF075 Context manager does not catch exceptions
+ providers/fab/src/airflow/providers/fab/auth_manager/cli_commands/utils.py:69:9: RUF075 Context manager does not catch exceptions
... 5 additional changes omitted for project

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

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

+ superset/utils/core.py:1500:13: RUF075 Context manager does not catch exceptions
+ superset/utils/core.py:1506:9: RUF075 Context manager does not catch exceptions
+ superset/utils/log.py:268:9: RUF075 Context manager does not catch exceptions
+ tests/integration_tests/conftest.py:111:13: RUF075 Context manager does not catch exceptions
+ tests/integration_tests/datasource_tests.py:71:5: RUF075 Context manager does not catch exceptions
+ tests/integration_tests/reports/commands_tests.py:180:5: RUF075 Context manager does not catch exceptions
+ tests/integration_tests/security/migrate_roles_tests.py:60:9: RUF075 Context manager does not catch exceptions

binary-husky/gpt_academic (+1 -0 violations, +0 -0 fixes)

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

+ shared_utils/fastapi_server.py:252:9: RUF075 Context manager does not catch exceptions

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

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

+ src/bokeh/application/handlers/code.py:200:5: RUF075 Context manager does not catch exceptions
+ src/bokeh/document/models.py:134:9: RUF075 Context manager does not catch exceptions
+ src/bokeh/embed/util.py:178:5: RUF075 Context manager does not catch exceptions
+ src/bokeh/io/doc.py:82:5: RUF075 Context manager does not catch exceptions
+ src/bokeh/io/export.py:410:5: RUF075 Context manager does not catch exceptions
+ src/bokeh/models/plots.py:442:13: RUF075 Context manager does not catch exceptions
+ tests/support/plugins/managed_server_loop.py:67:9: RUF075 Context manager does not catch exceptions
+ tests/support/util/env.py:55:5: RUF075 Context manager does not catch exceptions
+ tests/support/util/filesystem.py:162:5: RUF075 Context manager does not catch exceptions

ibis-project/ibis (+4 -0 violations, +0 -0 fixes)

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

+ ibis/backends/datafusion/__init__.py:744:5: RUF075 Context manager does not catch exceptions
+ ibis/backends/duckdb/tests/test_decompile_tpch.py:65:5: RUF075 Context manager does not catch exceptions
+ ibis/backends/tests/test_client.py:1545:5: RUF075 Context manager does not catch exceptions
+ ibis/tests/expr/mocks.py:102:9: RUF075 Context manager does not catch exceptions

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

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

+ libs/core/langchain_core/callbacks/usage.py:118:5: RUF075 Context manager does not catch exceptions

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

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

+ src/latch/account.py:197:9: RUF075 Context manager does not catch exceptions
+ src/latch/registry/project.py:166:9: RUF075 Context manager does not catch exceptions
+ src/latch/registry/table.py:397:9: RUF075 Context manager does not catch exceptions

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

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

+ lnbits/app.py:142:5: RUF075 Context manager does not catch exceptions

pypa/cibuildwheel (+1 -0 violations, +0 -0 fixes)

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

+ cibuildwheel/logger.py:240:9: RUF075 Context manager does not catch exceptions

python-poetry/poetry (+3 -0 violations, +0 -0 fixes)

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

+ src/poetry/config/file_config_source.py:77:9: RUF075 Context manager does not catch exceptions
+ src/poetry/puzzle/provider.py:92:9: RUF075 Context manager does not catch exceptions
+ tests/helpers.py:260:5: RUF075 Context manager does not catch exceptions

reflex-dev/reflex (+7 -0 violations, +0 -0 fixes)

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

+ reflex/app.py:1338:17: RUF075 Context manager does not catch exceptions
+ reflex/istate/manager/disk.py:373:13: RUF075 Context manager does not catch exceptions
+ reflex/istate/manager/redis.py:465:17: RUF075 Context manager does not catch exceptions
+ reflex/istate/manager/redis.py:472:17: RUF075 Context manager does not catch exceptions
+ reflex/istate/manager/redis.py:498:21: RUF075 Context manager does not catch exceptions
+ reflex/istate/manager/redis.py:524:25: RUF075 Context manager does not catch exceptions
+ tests/integration/utils.py:62:5: RUF075 Context manager does not catch exceptions

rotki/rotki (+2 -0 violations, +0 -0 fixes)

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

+ rotkehlchen/chain/evm/transactions.py:205:9: RUF075 Context manager does not catch exceptions
+ rotkehlchen/db/drivers/gevent.py:485:13: RUF075 Context manager does not catch exceptions

scikit-build/scikit-build (+3 -0 violations, +0 -0 fixes)

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

+ tests/__init__.py:44:5: RUF075 Context manager does not catch exceptions
+ tests/__init__.py:57:5: RUF075 Context manager does not catch exceptions
+ tests/__init__.py:70:5: RUF075 Context manager does not catch exceptions

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

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

+ zerver/lib/upload/s3.py:335:9: RUF075 Context manager does not catch exceptions

indico/indico (+6 -0 violations, +0 -0 fixes)

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

+ indico/modules/events/management/util.py:180:5: RUF075 Context manager does not catch exceptions
+ indico/modules/events/static/util.py:190:5: RUF075 Context manager does not catch exceptions
+ indico/testing/fixtures/database.py:139:9: RUF075 Context manager does not catch exceptions
+ indico/testing/fixtures/database.py:55:5: RUF075 Context manager does not catch exceptions
+ indico/web/flask/wrappers.py:169:9: RUF075 Context manager does not catch exceptions
+ indico/web/flask/wrappers.py:235:9: RUF075 Context manager does not catch exceptions

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

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

+ src/trio/_tests/test_dtls.py:101:13: RUF075 Context manager does not catch exceptions
+ src/trio/_tests/test_subprocess.py:106:9: RUF075 Context manager does not catch exceptions

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

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

+ testing/logging/test_reporting.py:949:13: RUF075 Context manager does not catch exceptions
+ testing/test_capture.py:1008:5: RUF075 Context manager does not catch exceptions

pdm-project/pdm (+1 -0 violations, +0 -0 fixes)

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

+ src/pdm/cli/hooks.py:22:9: RUF075 Context manager does not catch exceptions

astropy/astropy (+14 -0 violations, +0 -0 fixes)

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

+ astropy/coordinates/polarization.py:64:5: RUF075 Context manager does not catch exceptions
+ astropy/io/fits/tests/test_table.py:3363:5: RUF075 Context manager does not catch exceptions
+ astropy/io/registry/base.py:204:9: RUF075 Context manager does not catch exceptions
+ astropy/io/votable/util.py:51:17: RUF075 Context manager does not catch exceptions
+ astropy/io/votable/util.py:77:13: RUF075 Context manager does not catch exceptions
+ astropy/io/votable/util.py:79:13: RUF075 Context manager does not catch exceptions
+ astropy/logger.py:447:9: RUF075 Context manager does not catch exceptions
+ astropy/logger.py:489:9: RUF075 Context manager does not catch exceptions
+ astropy/timeseries/core.py:101:9: RUF075 Context manager does not catch exceptions
+ astropy/utils/metadata/merge.py:201:5: RUF075 Context manager does not catch exceptions
... 4 additional changes omitted for project

... Truncated remaining completed project reports due to GitHub comment length restrictions

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
RUF075 87 87 0 0 0

@amyreese
Copy link
Copy Markdown
Contributor

amyreese commented Jan 26, 2026

I looked through the ecosystem results, and there are a number of cases where either yield is the last statement in a branch/body (or immediately followed by a return), or cases where the yield is already wrapped in a try/except block. I think the first case makes sense to detect because it means there's no semantic purpose to adding a try/finally, and in the second case, we should absolutely not complain because the developer has already been explicit about how to handle exceptions.

https://github.com/apache/superset/blob/b09e60c1ec261af5e59b5ad071ec7a4216c4675f/superset/utils/oauth2.py#L212

https://github.com/apache/airflow/blob/e916980ba53cfa8cab51e06bf3d7f62dc51ea885/airflow-core/src/airflow/assets/manager.py#L113

https://github.com/apache/superset/blob/b09e60c1ec261af5e59b5ad071ec7a4216c4675f/tests/integration_tests/conftest.py#L114

https://github.com/bokeh/bokeh/blob/829b2a75c402d0d0abd7e37ff201fbdfd949d857/src/bokeh/command/util.py#L206

@amyreese amyreese added the rule Implementing or modifying a lint rule label Jan 26, 2026
@anishgirianish
Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback, @amyreese! I've updated the rule to no longer flag yields in terminal position (where there's no cleanup code to protect) and to accept try/except as sufficient exception handling. The ecosystem check shows 127 fewer false positives across 20 projects with no lost true positives. Would love your review whenever you get a chance!

Copy link
Copy Markdown
Contributor

@amyreese amyreese left a comment

Choose a reason for hiding this comment

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

Curious why you added this as part of the flake8_bugbear rules, since this doesn't match what upstream B036 is, and as far as I can tell, doesn't match any of the bugbear rules.

@amyreese amyreese requested a review from ntBre January 27, 2026 21:06
Copy link
Copy Markdown
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 for working on this! I agree with Amy, I don't think we can make this a flake8-bugbear rule and especially not B036, which has a different meaning in the upstream linter. I would probably just make this a new RUF rule.

The Visitor feels a little complicated to me, but I'm not sure if it's actually over-complicated yet. I'll have to take a deeper look to be sure I understand it. I think it would help to add a couple of documentation comments on the Visitor struct, its fields, and maybe the methods in its impl block (like on visit_with_body and on visit_body_with_terminal).

The general idea looks reasonable to me, I'm just wishing we could reuse more of the default Visitor behavior instead of having to inline so many parts of walk_stmt.

@ntBre ntBre added the preview Related to preview mode features label Jan 27, 2026
@anishgirianish
Copy link
Copy Markdown
Contributor Author

Hi @ntBre , @amyreese
Thanks for the thorough review! I've addressed all the feedback:

  • Moved from flake8-bugbear (B036) to RUF069
  • Renamed to FallibleContextManager
  • Updated message to "Context manager does not catch exceptions"
  • Embedded Checker in the visitor to emit diagnostics directly
  • Added doc comments to the visitor struct, fields, and methods
  • Set preview_since to NEXT_RUFF_VERSION

Would appreciate another look when you get a chance. Thanks!

@ntBre ntBre changed the title [flake8-bugbear] Add B036 contextmanager without try/finally [ruff] Add RUF069 contextmanager without try/finally Jan 28, 2026
Copy link
Copy Markdown
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 again for working on this. I took a deeper look at this today, playing with the code myself locally. I added a few inline comments as I went, but the very brief summary is that we need a lot more test coverage, or much of this code is unused. I'll put the patch I ended up with locally down below, but I was able to delete a ton of code without failing any existing tests. Some of this I believe was genuinely redundant, like the in_with_last_statement flag, but others like the in_protected_try flag were just casualties of too little test coverage.

Patch
diff --git a/crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs b/crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
index 9024b8f738..0a46de3048 100644
--- a/crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
+++ b/crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
@@ -66,7 +66,7 @@ pub(crate) fn fallible_context_manager(checker: &Checker, function_def: &StmtFun
     }
 
     let mut visitor = YieldFinallyVisitor::new(checker);
-    visitor.visit_body_with_terminal(&function_def.body, true);
+    visitor.visit_body_with_terminal(&function_def.body);
 }
 
 fn has_contextmanager_decorator(checker: &Checker, function_def: &StmtFunctionDef) -> bool {
@@ -94,12 +94,6 @@ fn has_contextmanager_decorator(checker: &Checker, function_def: &StmtFunctionDe
 struct YieldFinallyVisitor<'a, 'b> {
     /// The checker used to emit diagnostics.
     checker: &'a Checker<'b>,
-    /// Whether the visitor is currently inside a `try` block that has
-    /// `finally` or `except` handlers.
-    in_protected_try: bool,
-    /// Whether the visitor is at the last statement in a `with` block body,
-    /// where the `with` statement's `__exit__` provides exception handling.
-    in_with_last_statement: bool,
     /// Whether the visitor is at a terminal position: the last statement in
     /// the function body, or a `yield` immediately before a `return`.
     in_terminal_position: bool,
@@ -110,9 +104,7 @@ impl<'a, 'b> YieldFinallyVisitor<'a, 'b> {
     fn new(checker: &'a Checker<'b>) -> Self {
         Self {
             checker,
-            in_protected_try: false,
-            in_with_last_statement: false,
-            in_terminal_position: false,
+            in_terminal_position: true,
         }
     }
 }
@@ -122,79 +114,10 @@ impl Visitor<'_> for YieldFinallyVisitor<'_, '_> {
         match stmt {
             Stmt::FunctionDef(_) | Stmt::ClassDef(_) => {}
 
-            Stmt::Try(ast::StmtTry {
-                body,
-                handlers,
-                orelse,
-                finalbody,
-                ..
-            }) if !finalbody.is_empty() || !handlers.is_empty() => {
-                let prev = self.in_protected_try;
-                self.in_protected_try = true;
-                self.visit_body(body);
-                for handler in handlers {
-                    self.visit_except_handler(handler);
-                }
-                self.visit_body(orelse);
-                self.in_protected_try = prev;
-                self.visit_body(finalbody);
-            }
-
             Stmt::With(ast::StmtWith { body, .. }) => {
                 self.visit_with_body(body);
             }
 
-            Stmt::If(ast::StmtIf {
-                test,
-                body,
-                elif_else_clauses,
-                ..
-            }) => {
-                self.visit_expr(test);
-                let terminal = self.in_terminal_position;
-                self.visit_body_with_terminal(body, terminal);
-                for clause in elif_else_clauses {
-                    if let Some(test) = &clause.test {
-                        self.visit_expr(test);
-                    }
-                    self.visit_body_with_terminal(&clause.body, terminal);
-                }
-            }
-
-            Stmt::For(ast::StmtFor {
-                target,
-                iter,
-                body,
-                orelse,
-                ..
-            }) => {
-                self.visit_expr(iter);
-                self.visit_expr(target);
-                let terminal = self.in_terminal_position;
-                self.visit_body_with_terminal(body, terminal);
-                self.visit_body_with_terminal(orelse, terminal);
-            }
-
-            Stmt::While(ast::StmtWhile {
-                test, body, orelse, ..
-            }) => {
-                self.visit_expr(test);
-                let terminal = self.in_terminal_position;
-                self.visit_body_with_terminal(body, terminal);
-                self.visit_body_with_terminal(orelse, terminal);
-            }
-
-            Stmt::Match(ast::StmtMatch { subject, cases, .. }) => {
-                self.visit_expr(subject);
-                let terminal = self.in_terminal_position;
-                for case in cases {
-                    if let Some(guard) = &case.guard {
-                        self.visit_expr(guard);
-                    }
-                    self.visit_body_with_terminal(&case.body, terminal);
-                }
-            }
-
             _ => walk_stmt(self, stmt),
         }
     }
@@ -202,10 +125,7 @@ impl Visitor<'_> for YieldFinallyVisitor<'_, '_> {
     fn visit_expr(&mut self, expr: &Expr) {
         match expr {
             Expr::Yield(_) | Expr::YieldFrom(_) => {
-                if !self.in_protected_try
-                    && !self.in_with_last_statement
-                    && !self.in_terminal_position
-                {
+                if !self.in_terminal_position {
                     self.checker
                         .report_diagnostic(FallibleContextManager, expr.range());
                 }
@@ -221,7 +141,7 @@ impl YieldFinallyVisitor<'_, '_> {
     ///
     /// A statement is considered terminal if it is the last in the body (when `terminal` is true)
     /// or if it is a yield statement immediately followed by a `return`.
-    fn visit_body_with_terminal(&mut self, body: &[Stmt], terminal: bool) {
+    fn visit_body_with_terminal(&mut self, body: &[Stmt]) {
         for (i, stmt) in body.iter().enumerate() {
             let is_last = i == body.len() - 1;
             let is_yield_before_return = Self::is_yield_statement(stmt)
@@ -230,7 +150,7 @@ impl YieldFinallyVisitor<'_, '_> {
                     .is_some_and(|next| matches!(next, Stmt::Return(_)));
 
             let prev = self.in_terminal_position;
-            self.in_terminal_position = (is_last && terminal) || is_yield_before_return;
+            self.in_terminal_position &= is_last || is_yield_before_return;
             self.visit_stmt(stmt);
             self.in_terminal_position = prev;
         }
@@ -241,27 +161,26 @@ impl YieldFinallyVisitor<'_, '_> {
     /// The last statement in a `with` block inherits terminal status from the parent context
     /// and is additionally marked as a "with last statement" if it is a yield.
     fn visit_with_body(&mut self, body: &[Stmt]) {
-        if body.is_empty() {
+        let [rest @ .., last] = body else {
             return;
-        }
+        };
 
         let parent_terminal = self.in_terminal_position;
 
         // Non-last statements: delegate terminal tracking to visit_body_with_terminal
-        if body.len() > 1 {
-            self.visit_body_with_terminal(&body[..body.len() - 1], false);
-        }
+        self.in_terminal_position = false;
+        self.visit_body_with_terminal(rest);
+        self.in_terminal_position = parent_terminal;
 
         // Last statement: inherit terminal from parent, set with-last-statement if yield
-        let last = &body[body.len() - 1];
         let prev_terminal = self.in_terminal_position;
         self.in_terminal_position = parent_terminal;
 
         if Self::is_yield_statement(last) {
-            let prev_with = self.in_with_last_statement;
-            self.in_with_last_statement = true;
+            let prev_with = self.in_terminal_position;
+            self.in_terminal_position = true;
             self.visit_stmt(last);
-            self.in_with_last_statement = prev_with;
+            self.in_terminal_position = prev_with;
         } else {
             self.visit_stmt(last);
         }

Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
@anishgirianish
Copy link
Copy Markdown
Contributor Author

anishgirianish commented Feb 6, 2026

Hi @ntBre, thank you so much for the detailed review! I really appreciate the thoughtful feedback. I've addressed all the comments:

  • Removed the Try guard since Python grammar guarantees handlers/finally
  • Added items visiting for the with foo((yield)): case
  • Used the slice pattern as suggested
  • Renumbered to RUF070 since RUF069 was taken by float-comparison

For the code I kept, I added test coverage to demonstrate why it's needed:

  • in_protected_try: Required for non-terminal try blocks:
    try:
    yield
    finally:
    cleanup()
    print("after try") # try isn't terminal, but yield is still protected

  • in_with_last_statement: Required when with block has code after it — yield is protected by exit even though with isn't terminal.

  • If/For/While/Match handling: Required for proper terminal propagation. Added tests for while, elif, match, and loop else clauses.

  • Try visit order: Kept explicit order, so yields in finally aren't marked as protected, they're already in the cleanup phase.

Please let me know if there's anything else I can improve. Thank you again for your time!

@anishgirianish anishgirianish requested a review from ntBre February 6, 2026 05:29
@ntBre
Copy link
Copy Markdown
Contributor

ntBre commented Feb 6, 2026

Thanks for the update, I'll try to take another look soon.

Apologies in advance, I don't usually like to meddle with contributors' workflows, but in the future it would be preferable to push additional commits instead of rebasing. GitHub's review interface doesn't work very well with rebases (the "commits since your last review" button shows an error), and we squash-merge PRs at the end anyway, so we don't need to keep a clean history in the PR. For a big PR like this, that would have been particularly helpful.

@anishgirianish
Copy link
Copy Markdown
Contributor Author

Thanks for the update, I'll try to take another look soon.

Apologies in advance, I don't usually like to meddle with contributors' workflows, but in the future it would be preferable to push additional commits instead of rebasing. GitHub's review interface doesn't work very well with rebases (the "commits since your last review" button shows an error), and we squash-merge PRs at the end anyway, so we don't need to keep a clean history in the PR. For a big PR like this, that would have been particularly helpful.

Thank you so much for letting me know! I'll definitely stick to pushing additional commits going forward. Thanks for your patience, and I look forward to your next review whenever you get the chance.

@anishgirianish anishgirianish changed the title [ruff] Add RUF071 contextmanager without try/finally [ruff] Add RUF072 contextmanager without try/finally Mar 14, 2026
# Conflicts:
#	crates/ruff_linter/resources/test/fixtures/ruff/RUF072.py
#	crates/ruff_linter/src/codes.rs
#	ruff.schema.json
@anishgirianish anishgirianish changed the title [ruff] Add RUF072 contextmanager without try/finally [ruff] Add RUF074 contextmanager without try/finally Apr 6, 2026
Copy link
Copy Markdown
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.

Thank you! This looks very close to ready to go. I only had a few minor comments, mostly around testing some edge cases. This branch will also need a merge with main now that your other RUF074 PR landed first.

Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs
Comment thread crates/ruff_linter/resources/test/fixtures/ruff/RUF074.py
Resolve conflicts caused by upstream landing RUF074
(`incorrect-decorator-order`, astral-sh#23461). Renumber the in-progress
`FallibleContextManager` rule from RUF074 to RUF075:

- codes.rs: keep `IncorrectDecoratorOrder` at 074 (landed), add
  `FallibleContextManager` at 075
- mod.rs: keep both `test_case` entries; FallibleContextManager now
  references `RUF075.py`
- Rename fixture: HEAD's `RUF074.py` content moves to new `RUF075.py`
  with `# RUF074` annotations updated to `# RUF075`
- Rename snapshot: HEAD's snapshot becomes `RUF075_RUF075.py.snap` with
  `RUF074` references rewritten to `RUF075`
- fallible_context_manager.rs: rule-id comment updated to RUF075
- ruff.schema.json regenerated via `cargo dev generate-all`
@anishgirianish anishgirianish changed the title [ruff] Add RUF074 contextmanager without try/finally [ruff] Add RUF075 contextmanager without try/finally May 20, 2026
@anishgirianish
Copy link
Copy Markdown
Contributor Author

@ntBre thank you so much for the re-review. I have addressed all the feedbacks and updated the pr. Would like to request you for your rer-review, whenever you get a chance. thank you

@anishgirianish anishgirianish requested a review from ntBre May 20, 2026 04:16
Copy link
Copy Markdown
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 again for all of your work here! I managed to find a few more suggestions, but again this is very close.

Comment thread crates/ruff_linter/resources/mdtest/ruff.md Outdated
Comment thread crates/ruff_linter/src/rules/ruff/mod.rs Outdated
Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
Comment thread crates/ruff_linter/resources/mdtest/ruff.md Outdated
@anishgirianish anishgirianish requested a review from ntBre May 20, 2026 17:56
@anishgirianish
Copy link
Copy Markdown
Contributor Author

@ntBre thank you so much for the re-review. I have addressed all the feedbacks in the latest push. Would like to request you for your another look, whenever you get a chance. thank you

@ntBre
Copy link
Copy Markdown
Contributor

ntBre commented May 20, 2026

Comment thread crates/ruff_linter/src/rules/ruff/rules/fallible_context_manager.rs Outdated
@anishgirianish anishgirianish requested a review from ntBre May 20, 2026 20:10
Copy link
Copy Markdown
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 work here, thank you!

@ntBre ntBre changed the title [ruff] Add RUF075 contextmanager without try/finally [ruff] Add fallible-context-manager (RUF075) May 20, 2026
@ntBre ntBre merged commit e1cdbba into astral-sh:main May 20, 2026
45 checks passed
thejchap pushed a commit to thejchap/ruff that referenced this pull request May 23, 2026
## Summary

Adds a new rule `RUF075` (preview) that detects
`@contextlib.contextmanager` and `@contextlib.asynccontextmanager`
decorated functions where `yield` is not protected against exceptions.

Closes astral-sh#15629.

## Details

When a context manager yields without exception protection, cleanup code
after `yield` won't run if an exception is raised inside the `with`
block.

```python
# Bad - cleanup won't run on exception
@contextmanager
def bad():
    print("setup")
    yield
    print("cleanup")  # never runs if exception raised

# Good
@contextmanager
def good():
    print("setup")
    try:
        yield
    finally:
        print("cleanup")  # always runs
```

The rule does **not** flag:
- Yields inside `try` blocks that have a `finally` or `except` handler
- Yields as the last statement of a `with` block (cleanup is delegated
to `__exit__`)
- Yields in terminal position (last statement in the function body, or
immediately before a `return`), where there is no trailing cleanup that
could be skipped

Terminal position is inherited through `if` / `for` / `while` / `match`
branches: a yield that is the last statement of a branch is terminal
only when the enclosing statement is itself terminal.

The rule is in preview mode.

## Test Plan

Tests live in `crates/ruff_linter/resources/mdtest/ruff.md` and are run
via:

```sh
cargo nextest run -p ruff_mdtest -- mdtest::ruff
```
anishgirianish added a commit to anishgirianish/ruff that referenced this pull request May 28, 2026
## Summary

Adds a new rule `RUF075` (preview) that detects
`@contextlib.contextmanager` and `@contextlib.asynccontextmanager`
decorated functions where `yield` is not protected against exceptions.

Closes astral-sh#15629.

## Details

When a context manager yields without exception protection, cleanup code
after `yield` won't run if an exception is raised inside the `with`
block.

```python
# Bad - cleanup won't run on exception
@contextmanager
def bad():
    print("setup")
    yield
    print("cleanup")  # never runs if exception raised

# Good
@contextmanager
def good():
    print("setup")
    try:
        yield
    finally:
        print("cleanup")  # always runs
```

The rule does **not** flag:
- Yields inside `try` blocks that have a `finally` or `except` handler
- Yields as the last statement of a `with` block (cleanup is delegated
to `__exit__`)
- Yields in terminal position (last statement in the function body, or
immediately before a `return`), where there is no trailing cleanup that
could be skipped

Terminal position is inherited through `if` / `for` / `while` / `match`
branches: a yield that is the last statement of a branch is terminal
only when the enclosing statement is itself terminal.

The rule is in preview mode.

## Test Plan

Tests live in `crates/ruff_linter/resources/mdtest/ruff.md` and are run
via:

```sh
cargo nextest run -p ruff_mdtest -- mdtest::ruff
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new rule - function decorated with @contextlib.contextmanager that does not wrap its yield statement in a try/finally block

3 participants