From 6661be2c30948ba0113cfd3973463c348450de1d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 7 Sep 2023 17:16:43 +0200 Subject: [PATCH] Use full range for SIM105 fixes (#7221) Avoids inserting an accidental extra newline after the fix _and_ addresses the case of a trailing semicolon. --- .../test/fixtures/flake8_simplify/SIM105_0.py | 12 +- .../rules/suppressible_exception.rs | 17 +- ...8_simplify__tests__SIM105_SIM105_0.py.snap | 320 +++++++++++------- ...8_simplify__tests__SIM105_SIM105_1.py.snap | 1 - ...8_simplify__tests__SIM105_SIM105_2.py.snap | 1 - 5 files changed, 211 insertions(+), 140 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_0.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_0.py index 78d338875d..7d63806c61 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_0.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM105_0.py @@ -8,6 +8,7 @@ try: except ValueError: pass + # SIM105 try: foo() @@ -111,10 +112,19 @@ try: except "not an exception": pass - # Regression test for: https://github.com/astral-sh/ruff/issues/7123 def write_models(directory, Models): try: os.makedirs(model_dir); except OSError: pass; + + try: os.makedirs(model_dir); + except OSError: + pass; + + try: os.makedirs(model_dir); + except OSError: + pass; \ + \ + # diff --git a/crates/ruff/src/rules/flake8_simplify/rules/suppressible_exception.rs b/crates/ruff/src/rules/flake8_simplify/rules/suppressible_exception.rs index 599659dc63..03e4b2f08c 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/suppressible_exception.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/suppressible_exception.rs @@ -1,11 +1,10 @@ -use ruff_python_ast::{self as ast, Constant, ExceptHandler, Expr, Stmt}; -use ruff_text_size::{TextLen, TextRange}; - use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::compose_call_path; use ruff_python_ast::helpers; +use ruff_python_ast::{self as ast, Constant, ExceptHandler, Expr, Stmt}; use ruff_text_size::Ranged; +use ruff_text_size::{TextLen, TextRange}; use crate::checkers::ast::Checker; use crate::importer::ImportRequest; @@ -134,12 +133,10 @@ pub(crate) fn suppressible_exception( stmt.range(), ); if checker.patch(diagnostic.kind.rule()) { - if !checker.indexer().has_comments(stmt, checker.locator()) - && !checker - .indexer() - .in_multi_statement_line(stmt, checker.locator()) - { + if !checker.indexer().has_comments(stmt, checker.locator()) { diagnostic.try_set_fix(|| { + // let range = statement_range(stmt, checker.locator(), checker.indexer()); + let (import_edit, binding) = checker.importer().get_or_import_symbol( &ImportRequest::import("contextlib", "suppress"), stmt.start(), @@ -149,8 +146,8 @@ pub(crate) fn suppressible_exception( format!("with {binding}({exception})"), TextRange::at(stmt.start(), "try".text_len()), ); - let handler_line_begin = checker.locator().line_start(range.start()); - let remove_handler = Edit::deletion(handler_line_begin, range.end()); + let remove_handler = + Edit::range_deletion(checker.locator().full_lines_range(*range)); Ok(Fix::suggested_edits( import_edit, [replace_try, remove_handler], diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_0.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_0.py.snap index 483f5ea126..f11cb66d1d 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_0.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_0.py.snap @@ -2,17 +2,15 @@ source: crates/ruff/src/rules/flake8_simplify/mod.rs --- SIM105_0.py:6:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `try`-`except`-`pass` - | - 5 | # SIM105 - 6 | / try: - 7 | | foo() - 8 | | except ValueError: - 9 | | pass - | |________^ SIM105 -10 | -11 | # SIM105 - | - = help: Replace with `contextlib.suppress(ValueError)` + | +5 | # SIM105 +6 | / try: +7 | | foo() +8 | | except ValueError: +9 | | pass + | |________^ SIM105 + | + = help: Replace with `contextlib.suppress(ValueError)` ℹ Suggested fix 1 |+import contextlib @@ -27,21 +25,19 @@ SIM105_0.py:6:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `tr 8 |-except ValueError: 9 |- pass 10 9 | - 10 |+ -11 11 | # SIM105 -12 12 | try: -13 13 | foo() +11 10 | +12 11 | # SIM105 -SIM105_0.py:12:1: SIM105 [*] Use `contextlib.suppress(ValueError, OSError)` instead of `try`-`except`-`pass` +SIM105_0.py:13:1: SIM105 [*] Use `contextlib.suppress(ValueError, OSError)` instead of `try`-`except`-`pass` | -11 | # SIM105 -12 | / try: -13 | | foo() -14 | | except (ValueError, OSError): -15 | | pass +12 | # SIM105 +13 | / try: +14 | | foo() +15 | | except (ValueError, OSError): +16 | | pass | |________^ SIM105 -16 | -17 | # SIM105 +17 | +18 | # SIM105 | = help: Replace with `contextlib.suppress(ValueError, OSError)` @@ -51,30 +47,28 @@ SIM105_0.py:12:1: SIM105 [*] Use `contextlib.suppress(ValueError, OSError)` inst 2 3 | pass 3 4 | -------------------------------------------------------------------------------- -9 10 | pass 10 11 | -11 12 | # SIM105 -12 |-try: - 13 |+with contextlib.suppress(ValueError, OSError): -13 14 | foo() -14 |-except (ValueError, OSError): -15 |- pass -16 15 | - 16 |+ -17 17 | # SIM105 -18 18 | try: -19 19 | foo() +11 12 | +12 13 | # SIM105 +13 |-try: + 14 |+with contextlib.suppress(ValueError, OSError): +14 15 | foo() +15 |-except (ValueError, OSError): +16 |- pass +17 16 | +18 17 | # SIM105 +19 18 | try: -SIM105_0.py:18:1: SIM105 [*] Use `contextlib.suppress(ValueError, OSError)` instead of `try`-`except`-`pass` +SIM105_0.py:19:1: SIM105 [*] Use `contextlib.suppress(ValueError, OSError)` instead of `try`-`except`-`pass` | -17 | # SIM105 -18 | / try: -19 | | foo() -20 | | except (ValueError, OSError) as e: -21 | | pass +18 | # SIM105 +19 | / try: +20 | | foo() +21 | | except (ValueError, OSError) as e: +22 | | pass | |________^ SIM105 -22 | -23 | # SIM105 +23 | +24 | # SIM105 | = help: Replace with `contextlib.suppress(ValueError, OSError)` @@ -84,30 +78,28 @@ SIM105_0.py:18:1: SIM105 [*] Use `contextlib.suppress(ValueError, OSError)` inst 2 3 | pass 3 4 | -------------------------------------------------------------------------------- -15 16 | pass -16 17 | -17 18 | # SIM105 -18 |-try: - 19 |+with contextlib.suppress(ValueError, OSError): -19 20 | foo() -20 |-except (ValueError, OSError) as e: -21 |- pass -22 21 | - 22 |+ -23 23 | # SIM105 -24 24 | try: -25 25 | foo() +16 17 | pass +17 18 | +18 19 | # SIM105 +19 |-try: + 20 |+with contextlib.suppress(ValueError, OSError): +20 21 | foo() +21 |-except (ValueError, OSError) as e: +22 |- pass +23 22 | +24 23 | # SIM105 +25 24 | try: -SIM105_0.py:24:1: SIM105 [*] Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass` +SIM105_0.py:25:1: SIM105 [*] Use `contextlib.suppress(Exception)` instead of `try`-`except`-`pass` | -23 | # SIM105 -24 | / try: -25 | | foo() -26 | | except: -27 | | pass +24 | # SIM105 +25 | / try: +26 | | foo() +27 | | except: +28 | | pass | |________^ SIM105 -28 | -29 | # SIM105 +29 | +30 | # SIM105 | = help: Replace with `contextlib.suppress(Exception)` @@ -117,30 +109,28 @@ SIM105_0.py:24:1: SIM105 [*] Use `contextlib.suppress(Exception)` instead of `tr 2 3 | pass 3 4 | -------------------------------------------------------------------------------- -21 22 | pass -22 23 | -23 24 | # SIM105 -24 |-try: - 25 |+with contextlib.suppress(Exception): -25 26 | foo() -26 |-except: -27 |- pass -28 27 | - 28 |+ -29 29 | # SIM105 -30 30 | try: -31 31 | foo() +22 23 | pass +23 24 | +24 25 | # SIM105 +25 |-try: + 26 |+with contextlib.suppress(Exception): +26 27 | foo() +27 |-except: +28 |- pass +29 28 | +30 29 | # SIM105 +31 30 | try: -SIM105_0.py:30:1: SIM105 [*] Use `contextlib.suppress(a.Error, b.Error)` instead of `try`-`except`-`pass` +SIM105_0.py:31:1: SIM105 [*] Use `contextlib.suppress(a.Error, b.Error)` instead of `try`-`except`-`pass` | -29 | # SIM105 -30 | / try: -31 | | foo() -32 | | except (a.Error, b.Error): -33 | | pass +30 | # SIM105 +31 | / try: +32 | | foo() +33 | | except (a.Error, b.Error): +34 | | pass | |________^ SIM105 -34 | -35 | # OK +35 | +36 | # OK | = help: Replace with `contextlib.suppress(a.Error, b.Error)` @@ -150,29 +140,27 @@ SIM105_0.py:30:1: SIM105 [*] Use `contextlib.suppress(a.Error, b.Error)` instead 2 3 | pass 3 4 | -------------------------------------------------------------------------------- -27 28 | pass -28 29 | -29 30 | # SIM105 -30 |-try: - 31 |+with contextlib.suppress(a.Error, b.Error): -31 32 | foo() -32 |-except (a.Error, b.Error): -33 |- pass -34 33 | - 34 |+ -35 35 | # OK -36 36 | try: -37 37 | foo() +28 29 | pass +29 30 | +30 31 | # SIM105 +31 |-try: + 32 |+with contextlib.suppress(a.Error, b.Error): +32 33 | foo() +33 |-except (a.Error, b.Error): +34 |- pass +35 34 | +36 35 | # OK +37 36 | try: -SIM105_0.py:84:5: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `try`-`except`-`pass` +SIM105_0.py:85:5: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `try`-`except`-`pass` | -82 | def with_ellipsis(): -83 | # OK -84 | try: +83 | def with_ellipsis(): +84 | # OK +85 | try: | _____^ -85 | | foo() -86 | | except ValueError: -87 | | ... +86 | | foo() +87 | | except ValueError: +88 | | ... | |___________^ SIM105 | = help: Replace with `contextlib.suppress(ValueError)` @@ -183,36 +171,33 @@ SIM105_0.py:84:5: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `t 2 3 | pass 3 4 | -------------------------------------------------------------------------------- -81 82 | -82 83 | def with_ellipsis(): -83 84 | # OK -84 |- try: - 85 |+ with contextlib.suppress(ValueError): -85 86 | foo() -86 |- except ValueError: -87 |- ... -88 87 | +82 83 | +83 84 | def with_ellipsis(): +84 85 | # OK +85 |- try: + 86 |+ with contextlib.suppress(ValueError): +86 87 | foo() +87 |- except ValueError: +88 |- ... 89 88 | - 89 |+ -90 90 | def with_ellipsis_and_return(): -91 91 | # OK -92 92 | try: +90 89 | +91 90 | def with_ellipsis_and_return(): -SIM105_0.py:99:5: SIM105 Use `contextlib.suppress(ValueError, OSError)` instead of `try`-`except`-`pass` +SIM105_0.py:100:5: SIM105 Use `contextlib.suppress(ValueError, OSError)` instead of `try`-`except`-`pass` | - 98 | def with_comment(): - 99 | try: + 99 | def with_comment(): +100 | try: | _____^ -100 | | foo() -101 | | except (ValueError, OSError): -102 | | pass # Trailing comment. +101 | | foo() +102 | | except (ValueError, OSError): +103 | | pass # Trailing comment. | |____________^ SIM105 -103 | -104 | try: +104 | +105 | try: | = help: Replace with `contextlib.suppress(ValueError, OSError)` -SIM105_0.py:117:5: SIM105 Use `contextlib.suppress(OSError)` instead of `try`-`except`-`pass` +SIM105_0.py:117:5: SIM105 [*] Use `contextlib.suppress(OSError)` instead of `try`-`except`-`pass` | 115 | # Regression test for: https://github.com/astral-sh/ruff/issues/7123 116 | def write_models(directory, Models): @@ -222,7 +207,88 @@ SIM105_0.py:117:5: SIM105 Use `contextlib.suppress(OSError)` instead of `try`-`e 119 | | except OSError: 120 | | pass; | |____________^ SIM105 +121 | +122 | try: os.makedirs(model_dir); | = help: Replace with `contextlib.suppress(OSError)` +ℹ Suggested fix + 1 |+import contextlib +1 2 | def foo(): +2 3 | pass +3 4 | +-------------------------------------------------------------------------------- +114 115 | +115 116 | # Regression test for: https://github.com/astral-sh/ruff/issues/7123 +116 117 | def write_models(directory, Models): +117 |- try: + 118 |+ with contextlib.suppress(OSError): +118 119 | os.makedirs(model_dir); +119 |- except OSError: +120 |- pass; +121 120 | +122 121 | try: os.makedirs(model_dir); +123 122 | except OSError: + +SIM105_0.py:122:5: SIM105 [*] Use `contextlib.suppress(OSError)` instead of `try`-`except`-`pass` + | +120 | pass; +121 | +122 | try: os.makedirs(model_dir); + | _____^ +123 | | except OSError: +124 | | pass; + | |____________^ SIM105 +125 | +126 | try: os.makedirs(model_dir); + | + = help: Replace with `contextlib.suppress(OSError)` + +ℹ Suggested fix + 1 |+import contextlib +1 2 | def foo(): +2 3 | pass +3 4 | +-------------------------------------------------------------------------------- +119 120 | except OSError: +120 121 | pass; +121 122 | +122 |- try: os.makedirs(model_dir); +123 |- except OSError: +124 |- pass; + 123 |+ with contextlib.suppress(OSError): os.makedirs(model_dir); +125 124 | +126 125 | try: os.makedirs(model_dir); +127 126 | except OSError: + +SIM105_0.py:126:5: SIM105 [*] Use `contextlib.suppress(OSError)` instead of `try`-`except`-`pass` + | +124 | pass; +125 | +126 | try: os.makedirs(model_dir); + | _____^ +127 | | except OSError: +128 | | pass; \ + | |____________^ SIM105 +129 | \ +130 | # + | + = help: Replace with `contextlib.suppress(OSError)` + +ℹ Suggested fix + 1 |+import contextlib +1 2 | def foo(): +2 3 | pass +3 4 | +-------------------------------------------------------------------------------- +123 124 | except OSError: +124 125 | pass; +125 126 | +126 |- try: os.makedirs(model_dir); +127 |- except OSError: +128 |- pass; \ + 127 |+ with contextlib.suppress(OSError): os.makedirs(model_dir); +129 128 | \ +130 129 | # + diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_1.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_1.py.snap index e9fd647daf..2d8426b90e 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_1.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_1.py.snap @@ -23,6 +23,5 @@ SIM105_1.py:5:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `tr 6 7 | math.sqrt(-1) 7 |-except ValueError: 8 |- pass - 8 |+ diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_2.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_2.py.snap index 5eee10ea83..6a41bb27e1 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_2.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM105_SIM105_2.py.snap @@ -21,6 +21,5 @@ SIM105_2.py:10:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `t 11 11 | foo() 12 |-except ValueError: 13 |- pass - 12 |+