From 694e7ed52e0293b39f95e066e426f1c310e345b6 Mon Sep 17 00:00:00 2001 From: Dylan Date: Fri, 29 Aug 2025 17:12:54 -0500 Subject: [PATCH] Less confidently mark f-strings as empty when inferring truthiness (#20152) 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 --- .../fixtures/flake8_pytest_style/PT015.py | 8 ++++ .../resources/test/fixtures/ruff/RUF037.py | 6 +++ ...es__flake8_pytest_style__tests__PT015.snap | 2 + ..._rules__ruff__tests__RUF037_RUF037.py.snap | 5 +++ crates/ruff_python_ast/src/helpers.rs | 39 +++++++++++-------- 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT015.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT015.py index 162a6180a1..57769dcfa4 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT015.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT015.py @@ -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}" diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py index e0460df98c..ea639e30db 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF037.py @@ -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}") diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap index 88a035bada..51ddc586d5 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT015.snap @@ -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 | diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap index 7334701e64..97644c428a 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF037_RUF037.py.snap @@ -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"{""=}") diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 4390659858..a26083941a 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -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, + ) -> 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()) } }) }