Skip to content

Commit 44ddd98

Browse files
[pyupgrade] Better messages and diagnostic range (UP015) (#15872)
## Summary Resolves #15863. In preview, diagnostic ranges will now be limited to that of the argument. Rule documentation, variable names, error messages and fix titles have all been modified to use "argument" consistently. ## Test Plan `cargo nextest run` and `cargo insta test`.
1 parent 82cb867 commit 44ddd98

6 files changed

+1147
-127
lines changed

crates/ruff_linter/src/rules/pyupgrade/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,25 @@ mod tests {
118118
Ok(())
119119
}
120120

121+
#[test_case(Rule::RedundantOpenModes, Path::new("UP015.py"))]
122+
#[test_case(Rule::RedundantOpenModes, Path::new("UP015_1.py"))]
123+
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
124+
let snapshot = format!(
125+
"preview__{}_{}",
126+
rule_code.noqa_code(),
127+
path.to_string_lossy()
128+
);
129+
let diagnostics = test_path(
130+
Path::new("pyupgrade").join(path).as_path(),
131+
&settings::LinterSettings {
132+
preview: PreviewMode::Enabled,
133+
..settings::LinterSettings::for_rule(rule_code)
134+
},
135+
)?;
136+
assert_messages!(snapshot, diagnostics);
137+
Ok(())
138+
}
139+
121140
#[test]
122141
fn up007_preview() -> Result<()> {
123142
let diagnostics = test_path(

crates/ruff_linter/src/rules/pyupgrade/rules/redundant_open_modes.rs

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,17 @@ use anyhow::Result;
22
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
33
use ruff_macros::{derive_message_formats, ViolationMetadata};
44
use ruff_python_ast::{self as ast, Expr};
5-
use ruff_python_codegen::Stylist;
65
use ruff_python_parser::{TokenKind, Tokens};
76
use ruff_python_stdlib::open_mode::OpenMode;
87
use ruff_text_size::{Ranged, TextSize};
98

109
use crate::checkers::ast::Checker;
1110

1211
/// ## What it does
13-
/// Checks for redundant `open` mode parameters.
12+
/// Checks for redundant `open` mode arguments.
1413
///
1514
/// ## Why is this bad?
16-
/// Redundant `open` mode parameters are unnecessary and should be removed to
15+
/// Redundant `open` mode arguments are unnecessary and should be removed to
1716
/// avoid confusion.
1817
///
1918
/// ## Example
@@ -40,18 +39,18 @@ impl AlwaysFixableViolation for RedundantOpenModes {
4039
fn message(&self) -> String {
4140
let RedundantOpenModes { replacement } = self;
4241
if replacement.is_empty() {
43-
"Unnecessary open mode parameters".to_string()
42+
"Unnecessary mode argument".to_string()
4443
} else {
45-
format!("Unnecessary open mode parameters, use \"{replacement}\"")
44+
format!("Unnecessary modes, use `{replacement}`")
4645
}
4746
}
4847

4948
fn fix_title(&self) -> String {
5049
let RedundantOpenModes { replacement } = self;
5150
if replacement.is_empty() {
52-
"Remove open mode parameters".to_string()
51+
"Remove mode argument".to_string()
5352
} else {
54-
format!("Replace with \"{replacement}\"")
53+
format!("Replace with `{replacement}`")
5554
}
5655
}
5756
}
@@ -71,68 +70,71 @@ pub(crate) fn redundant_open_modes(checker: &mut Checker, call: &ast::ExprCall)
7170
return;
7271
}
7372

74-
let Some(mode_param) = call.arguments.find_argument_value("mode", 1) else {
73+
let Some(mode_arg) = call.arguments.find_argument_value("mode", 1) else {
7574
return;
7675
};
77-
let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &mode_param else {
76+
let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = &mode_arg else {
7877
return;
7978
};
8079
let Ok(mode) = OpenMode::from_chars(value.chars()) else {
8180
return;
8281
};
8382
let reduced = mode.reduce();
8483
if reduced != mode {
85-
checker.diagnostics.push(create_diagnostic(
86-
call,
87-
mode_param,
88-
reduced,
89-
checker.tokens(),
90-
checker.stylist(),
91-
));
84+
checker
85+
.diagnostics
86+
.push(create_diagnostic(call, mode_arg, reduced, checker));
9287
}
9388
}
9489

9590
fn create_diagnostic(
9691
call: &ast::ExprCall,
97-
mode_param: &Expr,
92+
mode_arg: &Expr,
9893
mode: OpenMode,
99-
tokens: &Tokens,
100-
stylist: &Stylist,
94+
checker: &Checker,
10195
) -> Diagnostic {
96+
let range = if checker.settings.preview.is_enabled() {
97+
mode_arg.range()
98+
} else {
99+
call.range
100+
};
101+
102102
let mut diagnostic = Diagnostic::new(
103103
RedundantOpenModes {
104104
replacement: mode.to_string(),
105105
},
106-
call.range(),
106+
range,
107107
);
108108

109109
if mode.is_empty() {
110-
diagnostic
111-
.try_set_fix(|| create_remove_param_fix(call, mode_param, tokens).map(Fix::safe_edit));
110+
diagnostic.try_set_fix(|| {
111+
create_remove_argument_fix(call, mode_arg, checker.tokens()).map(Fix::safe_edit)
112+
});
112113
} else {
114+
let stylist = checker.stylist();
113115
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
114116
format!("{}{mode}{}", stylist.quote(), stylist.quote()),
115-
mode_param.range(),
117+
mode_arg.range(),
116118
)));
117119
}
118120

119121
diagnostic
120122
}
121123

122-
fn create_remove_param_fix(
124+
fn create_remove_argument_fix(
123125
call: &ast::ExprCall,
124-
mode_param: &Expr,
126+
mode_arg: &Expr,
125127
tokens: &Tokens,
126128
) -> Result<Edit> {
127-
// Find the last comma before mode_param and create a deletion fix
128-
// starting from the comma and ending after mode_param.
129+
// Find the last comma before mode_arg and create a deletion fix
130+
// starting from the comma and ending after mode_arg.
129131
let mut fix_start: Option<TextSize> = None;
130132
let mut fix_end: Option<TextSize> = None;
131133
let mut is_first_arg: bool = false;
132134
let mut delete_first_arg: bool = false;
133135

134136
for token in tokens.in_range(call.range()) {
135-
if token.start() == mode_param.start() {
137+
if token.start() == mode_arg.start() {
136138
if is_first_arg {
137139
delete_first_arg = true;
138140
continue;

0 commit comments

Comments
 (0)