Skip to content

Commit c2a80b9

Browse files
dylwil3second-ed
authored andcommitted
[ruff] Handle empty t-strings in unnecessary-empty-iterable-within-deque-call (RUF037) (astral-sh#20045)
Adds a method to `TStringValue` to detect whether the t-string is empty _as an iterable_. Note the subtlety here that, unlike f-strings, an empty t-string is still truthy (i.e. `bool(t"")==True`). Closes astral-sh#19951
1 parent 6e68f91 commit c2a80b9

File tree

4 files changed

+66
-1
lines changed

4 files changed

+66
-1
lines changed

crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,8 @@ def f():
102102
deque(b"abc") # OK
103103
deque(f"" "a") # OK
104104
deque(f"{x}" "") # OK
105+
106+
# https://github.com/astral-sh/ruff/issues/19951
107+
deque(t"")
108+
deque(t"" t"")
109+
deque(t"{""}") # OK

crates/ruff_linter/src/rules/ruff/rules/unnecessary_literal_within_deque_call.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ pub(crate) fn unnecessary_literal_within_deque_call(checker: &Checker, deque: &a
103103
Expr::StringLiteral(string) => string.value.is_empty(),
104104
Expr::BytesLiteral(bytes) => bytes.value.is_empty(),
105105
Expr::FString(fstring) => fstring.value.is_empty_literal(),
106+
Expr::TString(tstring) => tstring.value.is_empty_iterable(),
106107
_ => false,
107108
};
108109
if !is_empty_literal {

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,3 +383,42 @@ help: Replace with `deque()`
383383
101 101 | deque("abc") # OK
384384
102 102 | deque(b"abc") # OK
385385
103 103 | deque(f"" "a") # OK
386+
387+
RUF037 [*] Unnecessary empty iterable within a deque call
388+
--> RUF037.py:107:1
389+
|
390+
106 | # https://github.com/astral-sh/ruff/issues/19951
391+
107 | deque(t"")
392+
| ^^^^^^^^^^
393+
108 | deque(t"" t"")
394+
109 | deque(t"{""}") # OK
395+
|
396+
help: Replace with `deque()`
397+
398+
Safe fix
399+
104 104 | deque(f"{x}" "") # OK
400+
105 105 |
401+
106 106 | # https://github.com/astral-sh/ruff/issues/19951
402+
107 |-deque(t"")
403+
107 |+deque()
404+
108 108 | deque(t"" t"")
405+
109 109 | deque(t"{""}") # OK
406+
407+
RUF037 [*] Unnecessary empty iterable within a deque call
408+
--> RUF037.py:108:1
409+
|
410+
106 | # https://github.com/astral-sh/ruff/issues/19951
411+
107 | deque(t"")
412+
108 | deque(t"" t"")
413+
| ^^^^^^^^^^^^^^^
414+
109 | deque(t"{""}") # OK
415+
|
416+
help: Replace with `deque()`
417+
418+
Safe fix
419+
105 105 |
420+
106 106 | # https://github.com/astral-sh/ruff/issues/19951
421+
107 107 | deque(t"")
422+
108 |-deque(t"" t"")
423+
108 |+deque()
424+
109 109 | deque(t"{""}") # OK

crates/ruff_python_ast/src/nodes.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ impl FStringValue {
515515

516516
/// Returns `true` if the node represents an empty f-string literal.
517517
///
518-
/// Noteh that a [`FStringValue`] node will always have >= 1 [`FStringPart`]s inside it.
518+
/// Note that a [`FStringValue`] node will always have >= 1 [`FStringPart`]s inside it.
519519
/// This method checks whether the value of the concatenated parts is equal to the empty
520520
/// f-string, not whether the f-string has 0 parts inside it.
521521
pub fn is_empty_literal(&self) -> bool {
@@ -681,6 +681,22 @@ impl TStringValue {
681681
pub fn elements(&self) -> impl Iterator<Item = &InterpolatedStringElement> {
682682
self.iter().flat_map(|tstring| tstring.elements.iter())
683683
}
684+
685+
/// Returns `true` if the node represents an empty t-string in the
686+
/// sense that `__iter__` returns an empty iterable.
687+
///
688+
/// Beware that empty t-strings are still truthy, i.e. `bool(t"") == True`.
689+
///
690+
/// Note that a [`TStringValue`] node will always contain at least one
691+
/// [`TString`] node. This method checks whether each of the constituent
692+
/// t-strings (in an implicitly concatenated t-string) are empty
693+
/// in the above sense.
694+
pub fn is_empty_iterable(&self) -> bool {
695+
match &self.inner {
696+
TStringValueInner::Single(tstring) => tstring.is_empty(),
697+
TStringValueInner::Concatenated(tstrings) => tstrings.iter().all(TString::is_empty),
698+
}
699+
}
684700
}
685701

686702
impl<'a> IntoIterator for &'a TStringValue {
@@ -1182,6 +1198,10 @@ impl TString {
11821198
pub fn quote_style(&self) -> Quote {
11831199
self.flags.quote_style()
11841200
}
1201+
1202+
pub fn is_empty(&self) -> bool {
1203+
self.elements.is_empty()
1204+
}
11851205
}
11861206

11871207
impl From<TString> for Expr {

0 commit comments

Comments
 (0)