Less confidently mark f-strings as empty when inferring truthiness (#20152)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / Fuzz for new ty panics (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks-instrumented (push) Blocked by required conditions
CI / benchmarks-walltime (push) Blocked by required conditions
[ty Playground] Release / publish (push) Waiting to run

When computing the boolean value of an f-string, we over-eagerly
interpreted some f-string interpolations as empty. In this PR we now
mark the truthiness of f-strings involving format specs, debug text, and
bytes literals as "unknown".

This will probably result in some false negatives, which may be further
refined (for example - there are probably many cases where
`is_not_empty_f_string` should be modified to return `true`), but for
now at least we should have fewer false positives.

Affected rules (may not be an exhaustive list):

- [unnecessary-empty-iterable-within-deque-call
(RUF037)](https://docs.astral.sh/ruff/rules/unnecessary-empty-iterable-within-deque-call/#unnecessary-empty-iterable-within-deque-call-ruf037)
- [falsy-dict-get-fallback
(RUF056)](https://docs.astral.sh/ruff/rules/falsy-dict-get-fallback/#falsy-dict-get-fallback-ruf056)
- [pytest-assert-always-false
(PT015)](https://docs.astral.sh/ruff/rules/pytest-assert-always-false/#pytest-assert-always-false-pt015)
- [expr-or-not-expr
(SIM221)](https://docs.astral.sh/ruff/rules/expr-or-not-expr/#expr-or-not-expr-sim221)
- [expr-or-true
(SIM222)](https://docs.astral.sh/ruff/rules/expr-or-true/#expr-or-true-sim222)
- [expr-and-false
(SIM223)](https://docs.astral.sh/ruff/rules/expr-and-false/#expr-and-false-sim223)

Closes #19935
This commit is contained in:
Dylan 2025-08-29 17:12:54 -05:00 committed by GitHub
parent fe953e5c5c
commit 694e7ed52e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 44 additions and 16 deletions

View file

@ -23,3 +23,11 @@ def test_error():
assert list([])
assert set(set())
assert tuple("")
# https://github.com/astral-sh/ruff/issues/19935
def test_all_ok():
assert f"{b""}"
assert f"{""=}"
assert f"{""!a}"
assert f"{""!r}"
assert f"{"":1}"

View file

@ -110,3 +110,9 @@ deque(t"{""}") # OK
# https://github.com/astral-sh/ruff/issues/20050
deque(f"{""}") # RUF037
deque(f"{b""}")
deque(f"{""=}")
deque(f"{""!a}")
deque(f"{""!r}")
deque(f"{"":1}")

View file

@ -182,4 +182,6 @@ PT015 Assertion always fails, replace with `pytest.fail()`
24 | assert set(set())
25 | assert tuple("")
| ^^^^^^^^^^^^^^^^
26 |
27 | # https://github.com/astral-sh/ruff/issues/19935
|

View file

@ -396,6 +396,8 @@ RUF037 [*] Unnecessary empty iterable within a deque call
111 | # https://github.com/astral-sh/ruff/issues/20050
112 | deque(f"{""}") # RUF037
| ^^^^^^^^^^^^^^
113 |
114 | deque(f"{b""}")
|
help: Replace with `deque()`
109 | deque(t"{""}") # OK
@ -403,3 +405,6 @@ help: Replace with `deque()`
111 | # https://github.com/astral-sh/ruff/issues/20050
- deque(f"{""}") # RUF037
112 + deque() # RUF037
113 |
114 | deque(f"{b""}")
115 | deque(f"{""=}")

View file

@ -1410,31 +1410,38 @@ pub fn is_empty_f_string(expr: &ast::ExprFString) -> bool {
fn inner(expr: &Expr) -> bool {
match expr {
Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => value.is_empty(),
Expr::BytesLiteral(ast::ExprBytesLiteral { value, .. }) => value.is_empty(),
// Confusingly, `bool(f"{b""}") == True` even though
// `bool(b"") == False`. This is because `f"{b""}"`
// evaluates as the string `'b""'` of length 3.
Expr::BytesLiteral(_) => false,
Expr::FString(ast::ExprFString { value, .. }) => {
value
.elements()
.all(|f_string_element| match f_string_element {
InterpolatedStringElement::Literal(
ast::InterpolatedStringLiteralElement { value, .. },
) => value.is_empty(),
InterpolatedStringElement::Interpolation(ast::InterpolatedElement {
expression,
..
}) => inner(expression),
})
is_empty_interpolated_elements(value.elements())
}
_ => false,
}
}
fn is_empty_interpolated_elements<'a>(
mut elements: impl Iterator<Item = &'a InterpolatedStringElement>,
) -> bool {
elements.all(|element| match element {
InterpolatedStringElement::Literal(ast::InterpolatedStringLiteralElement {
value,
..
}) => value.is_empty(),
InterpolatedStringElement::Interpolation(f_string) => {
f_string.debug_text.is_none()
&& f_string.conversion.is_none()
&& f_string.format_spec.is_none()
&& inner(&f_string.expression)
}
})
}
expr.value.iter().all(|part| match part {
ast::FStringPart::Literal(string_literal) => string_literal.is_empty(),
ast::FStringPart::FString(f_string) => {
f_string.elements.iter().all(|element| match element {
InterpolatedStringElement::Literal(string_literal) => string_literal.is_empty(),
InterpolatedStringElement::Interpolation(f_string) => inner(&f_string.expression),
})
is_empty_interpolated_elements(f_string.elements.iter())
}
})
}