Skip to content

Commit 53962dc

Browse files
authored
[refurb] Make fix unsafe if it deletes comments (FURB105) (#22767)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> ## Test Plan <!-- How was it tested? -->
1 parent b1bcee2 commit 53962dc

3 files changed

Lines changed: 44 additions & 4 deletions

File tree

crates/ruff_linter/resources/test/fixtures/refurb/FURB105.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,9 @@
3737
print("", "", **kwargs)
3838
print(*args, sep=",")
3939
print(f"foo")
40+
41+
42+
print(
43+
# text
44+
""
45+
)

crates/ruff_linter/src/rules/refurb/rules/print_empty_string.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use ruff_python_ast::helpers::{contains_effect, is_empty_f_string};
33
use ruff_python_ast::{self as ast, Expr};
44
use ruff_python_codegen::Generator;
55
use ruff_python_semantic::SemanticModel;
6+
use ruff_python_trivia::CommentRanges;
67
use ruff_text_size::Ranged;
78

89
use crate::checkers::ast::Checker;
@@ -31,7 +32,7 @@ use crate::{Applicability, Edit, Fix, FixAvailability, Violation};
3132
/// ```
3233
///
3334
/// ## Fix safety
34-
/// This fix is marked as unsafe if it removes an unused `sep` keyword argument
35+
/// This fix is marked as unsafe if it removes comments or an unused `sep` keyword argument
3536
/// that may have side effects. Removing such arguments may change the program's
3637
/// behavior by skipping the execution of those side effects.
3738
///
@@ -99,6 +100,7 @@ pub(crate) fn print_empty_string(checker: &Checker, call: &ast::ExprCall) {
99100
Separator::Remove,
100101
checker.semantic(),
101102
checker.generator(),
103+
checker.comment_ranges(),
102104
)
103105
.into_fix(),
104106
);
@@ -125,6 +127,7 @@ pub(crate) fn print_empty_string(checker: &Checker, call: &ast::ExprCall) {
125127
Separator::Remove,
126128
checker.semantic(),
127129
checker.generator(),
130+
checker.comment_ranges(),
128131
)
129132
.into_fix(),
130133
);
@@ -185,8 +188,14 @@ pub(crate) fn print_empty_string(checker: &Checker, call: &ast::ExprCall) {
185188
);
186189

187190
diagnostic.set_fix(
188-
EmptyStringFix::from_call(call, separator, checker.semantic(), checker.generator())
189-
.into_fix(),
191+
EmptyStringFix::from_call(
192+
call,
193+
separator,
194+
checker.semantic(),
195+
checker.generator(),
196+
checker.comment_ranges(),
197+
)
198+
.into_fix(),
190199
);
191200
}
192201
}
@@ -218,11 +227,16 @@ impl EmptyStringFix {
218227
separator: Separator,
219228
semantic: &SemanticModel,
220229
generator: Generator,
230+
comment_ranges: &CommentRanges,
221231
) -> Self {
222232
let range = call.range();
223233
let mut call = call.clone();
224234
let mut applicability = Applicability::Safe;
225235

236+
if comment_ranges.intersects(range) {
237+
applicability = Applicability::Unsafe;
238+
}
239+
226240
// Remove all empty string positional arguments.
227241
call.arguments.args = call
228242
.arguments

crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB105_FURB105.py.snap

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,4 +417,24 @@ help: Remove empty string
417417
24 + print(end="bar")
418418
25 |
419419
26 | # OK.
420-
27 |
420+
27 |
421+
422+
FURB105 [*] Unnecessary empty string passed to `print`
423+
--> FURB105.py:42:1
424+
|
425+
42 | / print(
426+
43 | | # text
427+
44 | | ""
428+
45 | | )
429+
| |_^
430+
|
431+
help: Remove empty string
432+
39 | print(f"foo")
433+
40 |
434+
41 |
435+
- print(
436+
- # text
437+
- ""
438+
- )
439+
42 + print()
440+
note: This is an unsafe fix and may change runtime behavior

0 commit comments

Comments
 (0)