Use full range for SIM105 fixes (#7221)

Avoids inserting an accidental extra newline after the fix _and_
addresses the case of a trailing semicolon.
This commit is contained in:
Charlie Marsh 2023-09-07 17:16:43 +02:00 committed by GitHub
parent 97f945651d
commit 6661be2c30
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 211 additions and 140 deletions

View File

@ -8,6 +8,7 @@ try:
except ValueError: except ValueError:
pass pass
# SIM105 # SIM105
try: try:
foo() foo()
@ -111,10 +112,19 @@ try:
except "not an exception": except "not an exception":
pass pass
# Regression test for: https://github.com/astral-sh/ruff/issues/7123 # Regression test for: https://github.com/astral-sh/ruff/issues/7123
def write_models(directory, Models): def write_models(directory, Models):
try: try:
os.makedirs(model_dir); os.makedirs(model_dir);
except OSError: except OSError:
pass; pass;
try: os.makedirs(model_dir);
except OSError:
pass;
try: os.makedirs(model_dir);
except OSError:
pass; \
\
#

View File

@ -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_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation}; use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::call_path::compose_call_path; use ruff_python_ast::call_path::compose_call_path;
use ruff_python_ast::helpers; 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::Ranged;
use ruff_text_size::{TextLen, TextRange};
use crate::checkers::ast::Checker; use crate::checkers::ast::Checker;
use crate::importer::ImportRequest; use crate::importer::ImportRequest;
@ -134,12 +133,10 @@ pub(crate) fn suppressible_exception(
stmt.range(), stmt.range(),
); );
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {
if !checker.indexer().has_comments(stmt, checker.locator()) if !checker.indexer().has_comments(stmt, checker.locator()) {
&& !checker
.indexer()
.in_multi_statement_line(stmt, checker.locator())
{
diagnostic.try_set_fix(|| { diagnostic.try_set_fix(|| {
// let range = statement_range(stmt, checker.locator(), checker.indexer());
let (import_edit, binding) = checker.importer().get_or_import_symbol( let (import_edit, binding) = checker.importer().get_or_import_symbol(
&ImportRequest::import("contextlib", "suppress"), &ImportRequest::import("contextlib", "suppress"),
stmt.start(), stmt.start(),
@ -149,8 +146,8 @@ pub(crate) fn suppressible_exception(
format!("with {binding}({exception})"), format!("with {binding}({exception})"),
TextRange::at(stmt.start(), "try".text_len()), TextRange::at(stmt.start(), "try".text_len()),
); );
let handler_line_begin = checker.locator().line_start(range.start()); let remove_handler =
let remove_handler = Edit::deletion(handler_line_begin, range.end()); Edit::range_deletion(checker.locator().full_lines_range(*range));
Ok(Fix::suggested_edits( Ok(Fix::suggested_edits(
import_edit, import_edit,
[replace_try, remove_handler], [replace_try, remove_handler],

View File

@ -2,17 +2,15 @@
source: crates/ruff/src/rules/flake8_simplify/mod.rs source: crates/ruff/src/rules/flake8_simplify/mod.rs
--- ---
SIM105_0.py:6:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `try`-`except`-`pass` SIM105_0.py:6:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `try`-`except`-`pass`
| |
5 | # SIM105 5 | # SIM105
6 | / try: 6 | / try:
7 | | foo() 7 | | foo()
8 | | except ValueError: 8 | | except ValueError:
9 | | pass 9 | | pass
| |________^ SIM105 | |________^ SIM105
10 | |
11 | # SIM105 = help: Replace with `contextlib.suppress(ValueError)`
|
= help: Replace with `contextlib.suppress(ValueError)`
Suggested fix Suggested fix
1 |+import contextlib 1 |+import contextlib
@ -27,21 +25,19 @@ SIM105_0.py:6:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `tr
8 |-except ValueError: 8 |-except ValueError:
9 |- pass 9 |- pass
10 9 | 10 9 |
10 |+ 11 10 |
11 11 | # SIM105 12 11 | # SIM105
12 12 | try:
13 13 | foo()
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 | # SIM105
12 | / try: 13 | / try:
13 | | foo() 14 | | foo()
14 | | except (ValueError, OSError): 15 | | except (ValueError, OSError):
15 | | pass 16 | | pass
| |________^ SIM105 | |________^ SIM105
16 | 17 |
17 | # SIM105 18 | # SIM105
| |
= help: Replace with `contextlib.suppress(ValueError, OSError)` = 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 2 3 | pass
3 4 | 3 4 |
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
9 10 | pass
10 11 | 10 11 |
11 12 | # SIM105 11 12 |
12 |-try: 12 13 | # SIM105
13 |+with contextlib.suppress(ValueError, OSError): 13 |-try:
13 14 | foo() 14 |+with contextlib.suppress(ValueError, OSError):
14 |-except (ValueError, OSError): 14 15 | foo()
15 |- pass 15 |-except (ValueError, OSError):
16 15 | 16 |- pass
16 |+ 17 16 |
17 17 | # SIM105 18 17 | # SIM105
18 18 | try: 19 18 | try:
19 19 | foo()
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 | # SIM105
18 | / try: 19 | / try:
19 | | foo() 20 | | foo()
20 | | except (ValueError, OSError) as e: 21 | | except (ValueError, OSError) as e:
21 | | pass 22 | | pass
| |________^ SIM105 | |________^ SIM105
22 | 23 |
23 | # SIM105 24 | # SIM105
| |
= help: Replace with `contextlib.suppress(ValueError, OSError)` = 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 2 3 | pass
3 4 | 3 4 |
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
15 16 | pass 16 17 | pass
16 17 | 17 18 |
17 18 | # SIM105 18 19 | # SIM105
18 |-try: 19 |-try:
19 |+with contextlib.suppress(ValueError, OSError): 20 |+with contextlib.suppress(ValueError, OSError):
19 20 | foo() 20 21 | foo()
20 |-except (ValueError, OSError) as e: 21 |-except (ValueError, OSError) as e:
21 |- pass 22 |- pass
22 21 | 23 22 |
22 |+ 24 23 | # SIM105
23 23 | # SIM105 25 24 | try:
24 24 | try:
25 25 | foo()
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 | # SIM105
24 | / try: 25 | / try:
25 | | foo() 26 | | foo()
26 | | except: 27 | | except:
27 | | pass 28 | | pass
| |________^ SIM105 | |________^ SIM105
28 | 29 |
29 | # SIM105 30 | # SIM105
| |
= help: Replace with `contextlib.suppress(Exception)` = 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 2 3 | pass
3 4 | 3 4 |
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
21 22 | pass 22 23 | pass
22 23 | 23 24 |
23 24 | # SIM105 24 25 | # SIM105
24 |-try: 25 |-try:
25 |+with contextlib.suppress(Exception): 26 |+with contextlib.suppress(Exception):
25 26 | foo() 26 27 | foo()
26 |-except: 27 |-except:
27 |- pass 28 |- pass
28 27 | 29 28 |
28 |+ 30 29 | # SIM105
29 29 | # SIM105 31 30 | try:
30 30 | try:
31 31 | foo()
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 | # SIM105
30 | / try: 31 | / try:
31 | | foo() 32 | | foo()
32 | | except (a.Error, b.Error): 33 | | except (a.Error, b.Error):
33 | | pass 34 | | pass
| |________^ SIM105 | |________^ SIM105
34 | 35 |
35 | # OK 36 | # OK
| |
= help: Replace with `contextlib.suppress(a.Error, b.Error)` = 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 2 3 | pass
3 4 | 3 4 |
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
27 28 | pass 28 29 | pass
28 29 | 29 30 |
29 30 | # SIM105 30 31 | # SIM105
30 |-try: 31 |-try:
31 |+with contextlib.suppress(a.Error, b.Error): 32 |+with contextlib.suppress(a.Error, b.Error):
31 32 | foo() 32 33 | foo()
32 |-except (a.Error, b.Error): 33 |-except (a.Error, b.Error):
33 |- pass 34 |- pass
34 33 | 35 34 |
34 |+ 36 35 | # OK
35 35 | # OK 37 36 | try:
36 36 | try:
37 37 | foo()
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 | def with_ellipsis():
83 | # OK 84 | # OK
84 | try: 85 | try:
| _____^ | _____^
85 | | foo() 86 | | foo()
86 | | except ValueError: 87 | | except ValueError:
87 | | ... 88 | | ...
| |___________^ SIM105 | |___________^ SIM105
| |
= help: Replace with `contextlib.suppress(ValueError)` = 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 2 3 | pass
3 4 | 3 4 |
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
81 82 | 82 83 |
82 83 | def with_ellipsis(): 83 84 | def with_ellipsis():
83 84 | # OK 84 85 | # OK
84 |- try: 85 |- try:
85 |+ with contextlib.suppress(ValueError): 86 |+ with contextlib.suppress(ValueError):
85 86 | foo() 86 87 | foo()
86 |- except ValueError: 87 |- except ValueError:
87 |- ... 88 |- ...
88 87 |
89 88 | 89 88 |
89 |+ 90 89 |
90 90 | def with_ellipsis_and_return(): 91 90 | def with_ellipsis_and_return():
91 91 | # OK
92 92 | try:
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 | def with_comment():
99 | try: 100 | try:
| _____^ | _____^
100 | | foo() 101 | | foo()
101 | | except (ValueError, OSError): 102 | | except (ValueError, OSError):
102 | | pass # Trailing comment. 103 | | pass # Trailing comment.
| |____________^ SIM105 | |____________^ SIM105
103 | 104 |
104 | try: 105 | try:
| |
= help: Replace with `contextlib.suppress(ValueError, OSError)` = 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 115 | # Regression test for: https://github.com/astral-sh/ruff/issues/7123
116 | def write_models(directory, Models): 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: 119 | | except OSError:
120 | | pass; 120 | | pass;
| |____________^ SIM105 | |____________^ SIM105
121 |
122 | try: os.makedirs(model_dir);
| |
= help: Replace with `contextlib.suppress(OSError)` = 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 | #

View File

@ -23,6 +23,5 @@ SIM105_1.py:5:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `tr
6 7 | math.sqrt(-1) 6 7 | math.sqrt(-1)
7 |-except ValueError: 7 |-except ValueError:
8 |- pass 8 |- pass
8 |+

View File

@ -21,6 +21,5 @@ SIM105_2.py:10:1: SIM105 [*] Use `contextlib.suppress(ValueError)` instead of `t
11 11 | foo() 11 11 | foo()
12 |-except ValueError: 12 |-except ValueError:
13 |- pass 13 |- pass
12 |+