Skip to content

Commit 4d92e20

Browse files
VascoSch92ntBre
VascoSch92
and
ntBre
authored
[pylint] Convert a code keyword argument to a positional argument (PLR1722) (#16424)
The PR addresses issue #16396 . Specifically: - If the exit statement contains a code keyword argument, it is converted into a positional argument. - If retrieving the code from the exit statement is not possible, a violation is raised without suggesting a fix. --------- Co-authored-by: Brent Westbrook <[email protected]>
1 parent c457816 commit 4d92e20

11 files changed

+194
-11
lines changed

‎crates/ruff_linter/resources/test/fixtures/pylint/sys_exit_alias_13.py

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
exit(code=2)
2+

‎crates/ruff_linter/resources/test/fixtures/pylint/sys_exit_alias_14.py

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
code = {"code": 2}
2+
exit(**code)

‎crates/ruff_linter/resources/test/fixtures/pylint/sys_exit_alias_15.py

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
success = True
2+
if success:
3+
code = 0
4+
else:
5+
code = 1
6+
exit(code)

‎crates/ruff_linter/resources/test/fixtures/pylint/sys_exit_alias_16.py

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
def test_case_1():
2+
# comments preserved with a positional argument
3+
exit( # comment
4+
1, # 2
5+
) # 3
6+
7+
8+
def test_case_2():
9+
# comments preserved with a single keyword argument
10+
exit( # comment
11+
code=1, # 2
12+
) # 3
13+
14+
15+
def test_case_3():
16+
# no diagnostic for multiple arguments
17+
exit(2, 3, 4)
18+
19+
20+
def test_case_4():
21+
# this should now be fixable
22+
codes = [1]
23+
exit(*codes)

‎crates/ruff_linter/src/checkers/ast/analyze/expression.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,7 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
900900
pylint::rules::unnecessary_direct_lambda_call(checker, expr, func);
901901
}
902902
if checker.enabled(Rule::SysExitAlias) {
903-
pylint::rules::sys_exit_alias(checker, func);
903+
pylint::rules::sys_exit_alias(checker, call);
904904
}
905905
if checker.enabled(Rule::BadOpenMode) {
906906
pylint::rules::bad_open_mode(checker, call);

‎crates/ruff_linter/src/rules/pylint/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ mod tests {
6464
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_10.py"))]
6565
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_11.py"))]
6666
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_12.py"))]
67+
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_13.py"))]
68+
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_14.py"))]
69+
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_15.py"))]
70+
#[test_case(Rule::SysExitAlias, Path::new("sys_exit_alias_16.py"))]
6771
#[test_case(Rule::ContinueInFinally, Path::new("continue_in_finally.py"))]
6872
#[test_case(Rule::GlobalStatement, Path::new("global_statement.py"))]
6973
#[test_case(

‎crates/ruff_linter/src/rules/pylint/rules/sys_exit_alias.rs

+29-10
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1+
use crate::checkers::ast::Checker;
2+
use crate::importer::ImportRequest;
13
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
24
use ruff_macros::{derive_message_formats, ViolationMetadata};
3-
use ruff_python_ast::Expr;
4-
use ruff_text_size::Ranged;
55

6-
use crate::checkers::ast::Checker;
7-
use crate::importer::ImportRequest;
6+
use ruff_python_ast::ExprCall;
7+
use ruff_text_size::Ranged;
88

99
/// ## What it does
1010
/// Checks for uses of the `exit()` and `quit()`.
@@ -56,8 +56,8 @@ impl Violation for SysExitAlias {
5656
}
5757

5858
/// PLR1722
59-
pub(crate) fn sys_exit_alias(checker: &Checker, func: &Expr) {
60-
let Some(builtin) = checker.semantic().resolve_builtin_symbol(func) else {
59+
pub(crate) fn sys_exit_alias(checker: &Checker, call: &ExprCall) {
60+
let Some(builtin) = checker.semantic().resolve_builtin_symbol(&call.func) else {
6161
return;
6262
};
6363
if !matches!(builtin, "exit" | "quit") {
@@ -67,16 +67,35 @@ pub(crate) fn sys_exit_alias(checker: &Checker, func: &Expr) {
6767
SysExitAlias {
6868
name: builtin.to_string(),
6969
},
70-
func.range(),
70+
call.func.range(),
7171
);
72+
73+
let has_star_kwargs = call
74+
.arguments
75+
.keywords
76+
.iter()
77+
.any(|kwarg| kwarg.arg.is_none());
78+
// only one optional argument allowed, and we can't convert **kwargs
79+
if call.arguments.len() > 1 || has_star_kwargs {
80+
checker.report_diagnostic(diagnostic);
81+
return;
82+
};
83+
7284
diagnostic.try_set_fix(|| {
7385
let (import_edit, binding) = checker.importer().get_or_import_symbol(
7486
&ImportRequest::import("sys", "exit"),
75-
func.start(),
87+
call.func.start(),
7688
checker.semantic(),
7789
)?;
78-
let reference_edit = Edit::range_replacement(binding, func.range());
79-
Ok(Fix::unsafe_edits(import_edit, [reference_edit]))
90+
let reference_edit = Edit::range_replacement(binding, call.func.range());
91+
let mut edits = vec![reference_edit];
92+
if let Some(kwarg) = call.arguments.find_keyword("code") {
93+
edits.push(Edit::range_replacement(
94+
checker.source()[kwarg.value.range()].to_string(),
95+
kwarg.range,
96+
));
97+
};
98+
Ok(Fix::unsafe_edits(import_edit, edits))
8099
});
81100
checker.report_diagnostic(diagnostic);
82101
}

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1722_sys_exit_alias_13.py.snap

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pylint/mod.rs
3+
---
4+
sys_exit_alias_13.py:1:1: PLR1722 [*] Use `sys.exit()` instead of `exit`
5+
|
6+
1 | exit(code=2)
7+
| ^^^^ PLR1722
8+
|
9+
= help: Replace `exit` with `sys.exit()`
10+
11+
ℹ Unsafe fix
12+
1 |-exit(code=2)
13+
1 |+import sys
14+
2 |+sys.exit(2)
15+
2 3 |

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1722_sys_exit_alias_14.py.snap

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pylint/mod.rs
3+
---
4+
sys_exit_alias_14.py:2:1: PLR1722 Use `sys.exit()` instead of `exit`
5+
|
6+
1 | code = {"code": 2}
7+
2 | exit(**code)
8+
| ^^^^ PLR1722
9+
|
10+
= help: Replace `exit` with `sys.exit()`

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1722_sys_exit_alias_15.py.snap

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pylint/mod.rs
3+
---
4+
sys_exit_alias_15.py:6:1: PLR1722 [*] Use `sys.exit()` instead of `exit`
5+
|
6+
4 | else:
7+
5 | code = 1
8+
6 | exit(code)
9+
| ^^^^ PLR1722
10+
|
11+
= help: Replace `exit` with `sys.exit()`
12+
13+
ℹ Unsafe fix
14+
1 |+import sys
15+
1 2 | success = True
16+
2 3 | if success:
17+
3 4 | code = 0
18+
4 5 | else:
19+
5 6 | code = 1
20+
6 |-exit(code)
21+
7 |+sys.exit(code)

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1722_sys_exit_alias_16.py.snap

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
---
2+
source: crates/ruff_linter/src/rules/pylint/mod.rs
3+
---
4+
sys_exit_alias_16.py:3:5: PLR1722 [*] Use `sys.exit()` instead of `exit`
5+
|
6+
1 | def test_case_1():
7+
2 | # comments preserved with a positional argument
8+
3 | exit( # comment
9+
| ^^^^ PLR1722
10+
4 | 1, # 2
11+
5 | ) # 3
12+
|
13+
= help: Replace `exit` with `sys.exit()`
14+
15+
ℹ Unsafe fix
16+
1 |+import sys
17+
1 2 | def test_case_1():
18+
2 3 | # comments preserved with a positional argument
19+
3 |- exit( # comment
20+
4 |+ sys.exit( # comment
21+
4 5 | 1, # 2
22+
5 6 | ) # 3
23+
6 7 |
24+
25+
sys_exit_alias_16.py:10:5: PLR1722 [*] Use `sys.exit()` instead of `exit`
26+
|
27+
8 | def test_case_2():
28+
9 | # comments preserved with a single keyword argument
29+
10 | exit( # comment
30+
| ^^^^ PLR1722
31+
11 | code=1, # 2
32+
12 | ) # 3
33+
|
34+
= help: Replace `exit` with `sys.exit()`
35+
36+
ℹ Unsafe fix
37+
1 |+import sys
38+
1 2 | def test_case_1():
39+
2 3 | # comments preserved with a positional argument
40+
3 4 | exit( # comment
41+
--------------------------------------------------------------------------------
42+
7 8 |
43+
8 9 | def test_case_2():
44+
9 10 | # comments preserved with a single keyword argument
45+
10 |- exit( # comment
46+
11 |- code=1, # 2
47+
11 |+ sys.exit( # comment
48+
12 |+ 1, # 2
49+
12 13 | ) # 3
50+
13 14 |
51+
14 15 |
52+
53+
sys_exit_alias_16.py:17:5: PLR1722 Use `sys.exit()` instead of `exit`
54+
|
55+
15 | def test_case_3():
56+
16 | # no diagnostic for multiple arguments
57+
17 | exit(2, 3, 4)
58+
| ^^^^ PLR1722
59+
|
60+
= help: Replace `exit` with `sys.exit()`
61+
62+
sys_exit_alias_16.py:23:5: PLR1722 [*] Use `sys.exit()` instead of `exit`
63+
|
64+
21 | # this should now be fixable
65+
22 | codes = [1]
66+
23 | exit(*codes)
67+
| ^^^^ PLR1722
68+
|
69+
= help: Replace `exit` with `sys.exit()`
70+
71+
ℹ Unsafe fix
72+
1 |+import sys
73+
1 2 | def test_case_1():
74+
2 3 | # comments preserved with a positional argument
75+
3 4 | exit( # comment
76+
--------------------------------------------------------------------------------
77+
20 21 | def test_case_4():
78+
21 22 | # this should now be fixable
79+
22 23 | codes = [1]
80+
23 |- exit(*codes)
81+
24 |+ sys.exit(*codes)

0 commit comments

Comments
 (0)