[`flake8-simplify`] Preserve original behavior for `except ()` and bare `except` (`SIM105`) #18213

VascoSch92 · 2025-05-20T06:45:03Z

The PR addresses issue #18209.

github-actions · 2025-05-20T06:51:23Z

`ruff-ecosystem` results

Linter (stable)

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

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

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

+ superset/migrations/versions/2015-12-04_09-42_1a48a5411020_adding_slug_to_dash.py:35:5: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- superset/migrations/versions/2015-12-04_09-42_1a48a5411020_adding_slug_to_dash.py:35:5: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`

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

+ crazy_functions/Latex_Function.py:76:5: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- crazy_functions/Latex_Function.py:76:5: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ multi_language.py:363:9: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- multi_language.py:363:9: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ request_llms/com_qwenapi.py:57:21: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- request_llms/com_qwenapi.py:57:21: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ request_llms/oai_std_model_template.py:72:5: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- request_llms/oai_std_model_template.py:72:5: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ toolbox.py:463:13: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- toolbox.py:463:13: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
SIM105	12	6	6	0	0

Linter (preview)

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

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

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

+ superset/migrations/versions/2015-12-04_09-42_1a48a5411020_adding_slug_to_dash.py:35:5: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- superset/migrations/versions/2015-12-04_09-42_1a48a5411020_adding_slug_to_dash.py:35:5: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`

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

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

+ crazy_functions/Latex_Function.py:76:5: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- crazy_functions/Latex_Function.py:76:5: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ multi_language.py:363:9: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- multi_language.py:363:9: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ request_llms/com_qwenapi.py:57:21: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- request_llms/com_qwenapi.py:57:21: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ request_llms/oai_std_model_template.py:72:5: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- request_llms/oai_std_model_template.py:72:5: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`
+ toolbox.py:463:13: SIM105 Use `contextlib.suppress(BaseException)` instead of `try`-`except`-`pass`
- toolbox.py:463:13: SIM105 Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass`

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
SIM105	12	6	6	0	0

dscorbett · 2025-05-20T13:11:58Z

crates/ruff_linter/src/rules/flake8_simplify/rules/suppressible_exception.rs

-        "Exception".to_string()
+        if type_.is_none() {
+            // case where there are no handler names provided at all
+            "BaseException".to_string()


This won’t work if BaseException is shadowed. Here is an example of how to safely insert a builtin symbol:

ruff/crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs

Lines 78 to 82 in b35bf8a

let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol(

"callable",

expr.start(),

checker.semantic(),

)?;

VascoSch92 · 2025-05-21T06:10:37Z

@ntBre I have a problem. How I could import more stuff at the same time? Do you have an example where it was made in another rule?

I'm asking because here I would like to import subprocess from contextlib and also BaseException

MichaReiser · 2025-05-21T07:30:04Z

Would this work

ruff/crates/ruff_linter/src/rules/ruff/rules/quadratic_list_summation.rs

Lines 105 to 115 in 9ae698f

    
           let (reduce_edit, reduce_binding) = checker.importer().get_or_import_symbol( 
        
               &ImportRequest::import("functools", "reduce"), 
        
               call.start(), 
        
               checker.semantic(), 
        
           )?; 
        
           let (iadd_edit, iadd_binding) = checker.importer().get_or_import_symbol( 
        
               &ImportRequest::import("operator", "iadd"), 
        
               iterable.start(), 
        
               checker.semantic(), 
        
           )?;

VascoSch92 · 2025-05-21T16:49:52Z

Hey,
thanks for the help.

I hope understood what you were asking ;-)

dscorbett · 2025-05-21T17:50:12Z

crates/ruff_linter/src/rules/flake8_simplify/rules/suppressible_exception.rs

-                format!("with {binding}({exception})"),
+            let mut rest: Vec<Edit> = Vec::new();
+            let content: String;
+            if exception == "BaseException" {


Is this condition correct when BaseException is shadowed and caught explicitly?

BaseException = ValueError try: int("a") except BaseException: pass

The fixed version should use suppress(BaseException), not suppress(builtins.BaseException).

Good catch. No, it was't. I added a new test case and fix that

VascoSch92 · 2025-05-23T15:09:57Z

It should be fine now

ntBre

Thanks! The implementation looks reasonable to me, I just had a couple of vague ideas for your consideration.

I'm also kind of on the fence about whether we should offer the fix for the completely empty case. As mentioned in the issue, the fix could obscure a potential problem that would be caught by other lint rules.

I think the empty tuple case is okay because useless-contextlib-suppress (B022) will catch the empty suppress, but I'm not seeing any diagnostics on contextlib.suppress(BaseException) with all rules selected (playground).

If we still offer the fix, we could consider adding another rule similar to blind-except (BLE001) that applies to contextlib.suppress (or extend one of these existing rules).

What do you think?

ntBre · 2025-06-05T02:15:01Z

crates/ruff_linter/src/rules/flake8_simplify/rules/suppressible_exception.rs

@@ -115,7 +118,13 @@ pub(crate) fn suppressible_exception(
    };

    let exception = if handler_names.is_empty() {
-        "Exception".to_string()
+        if type_.is_none() {


I think we could collapse this check like the one below to

if handler_names.is_empty() && type_.is_none() {

because join on an empty Vec should also give the empty string. Maybe that's getting too clever though.

But then we lose one else or?

the complete snippet is

let exception = if handler_names.is_empty() { if type_.is_none() { // case where there are no handler names provided at all "BaseException".to_string() } else { // case where handler names is an empty tuple String::new() } } else { handler_names.join(", ") };

If I remember correctly, I was suggesting this:

let exception = if handler_names.is_empty() && type_.is_none() { "BaseException".to_string() } else { handler_names.join(", ") };

That does drop the middle else, but String::new() and handler_names.join(", ") are equivalent when handler_names is empty, so it's safe to combine the two else branches. However, I think this is evidence that your current code is more clear 😄

crates/ruff_linter/src/rules/flake8_simplify/rules/suppressible_exception.rs

ntBre · 2025-06-05T02:50:52Z

If we still offer the fix, we could consider adding another rule similar to blind-except (BLE001) that applies to contextlib.suppress (or extend one of these existing rules).

There's a somewhat related request for this in #4923, except it's for Exception rather than BaseException and try-except-pass (S110) instead of BLE001. There are a lot of rules about exceptions!

VascoSch92 · 2025-06-09T19:58:36Z

Thanks! The implementation looks reasonable to me, I just had a couple of vague ideas for your consideration.

I'm also kind of on the fence about whether we should offer the fix for the completely empty case. As mentioned in the issue, the fix could obscure a potential problem that would be caught by other lint rules.

I think the empty tuple case is okay because useless-contextlib-suppress (B022) will catch the empty suppress, but I'm not seeing any diagnostics on contextlib.suppress(BaseException) with all rules selected (playground).

If we still offer the fix, we could consider adding another rule similar to blind-except (BLE001) that applies to contextlib.suppress (or extend one of these existing rules).

What do you think?

I don't have strong opinions :-) let me know which solution you like and I would change the code accordingly

ntBre · 2025-06-09T21:06:12Z

I'll have to think a bit more about this. I'm mostly working on the 0.12 release this week, but I'm putting this on my todo list for afterward! Thanks for following up.

ntBre

Let's move ahead with the fix and open an issue to add or extend a rule to catch contextlib.suppress(BaseException).

I think that means all of the questions are resolved and this is good to go?

VascoSch92 · 2025-06-22T09:06:52Z

@ntBre Yes good to go ;-)

fix SIM105

71f563a

dscorbett reviewed May 20, 2025

View reviewed changes

import BaseException

f5f117d

VascoSch92 requested a review from dscorbett May 21, 2025 16:49

dscorbett reviewed May 21, 2025

View reviewed changes

new test case and fix

5792784

ntBre added bug Something isn't working fixes Related to suggested fixes for violations labels May 29, 2025

ntBre self-requested a review May 29, 2025 19:19

ntBre reviewed Jun 5, 2025

View reviewed changes

ntBre approved these changes Jun 18, 2025

View reviewed changes

ntBre linked an issue Jun 23, 2025 that may be closed by this pull request

SIM105 fix changes behavior for except (): and except: #18209

Closed

ntBre changed the title ~~[flake8-simplify] fix handlers corner cases (SIM105)~~ [flake8-simplify] Preserve original behavior for except () and bare except (SIM105) Jun 23, 2025

ntBre merged commit ca8ed35 into astral-sh:main Jun 23, 2025
34 checks passed

VascoSch92 deleted the issue-18209 branch June 23, 2025 20:45

BrewTestBot mentioned this pull request Jun 26, 2025

ruff 0.12.1 Homebrew/homebrew-core#228300

Merged

ntBre mentioned this pull request Jul 14, 2025

BLE001: detect contextlib.suppress ? #19306

Open

	let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol(
	"callable",
	expr.start(),
	checker.semantic(),
	)?;

[flake8-simplify] Preserve original behavior for except () and bare except (SIM105) #18213

[flake8-simplify] Preserve original behavior for except () and bare except (SIM105) #18213

Uh oh!

Conversation

VascoSch92 commented May 20, 2025

Uh oh!

github-actions bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

dscorbett May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VascoSch92 commented May 21, 2025

Uh oh!

MichaReiser commented May 21, 2025

Uh oh!

VascoSch92 commented May 21, 2025

Uh oh!

dscorbett May 21, 2025

Choose a reason for hiding this comment

Uh oh!

VascoSch92 May 22, 2025

Choose a reason for hiding this comment

Uh oh!

VascoSch92 commented May 23, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre Jun 5, 2025

Choose a reason for hiding this comment

Uh oh!

VascoSch92 Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ntBre commented Jun 5, 2025

Uh oh!

VascoSch92 commented Jun 9, 2025

Uh oh!

ntBre commented Jun 9, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

VascoSch92 commented Jun 22, 2025

Uh oh!

Uh oh!

Uh oh!

[`flake8-simplify`] Preserve original behavior for `except ()` and bare `except` (`SIM105`) #18213

[`flake8-simplify`] Preserve original behavior for `except ()` and bare `except` (`SIM105`) #18213

github-actions bot commented May 20, 2025 •

edited

Loading

`ruff-ecosystem` results

dscorbett May 20, 2025 •

edited

Loading