Skip to content

Refine non-empty f-string detection - #26526

Merged
MichaReiser merged 2 commits into
astral-sh:mainfrom
velikodniy:fix-non-empty-f-string-truthiness
Jul 6, 2026
Merged

Refine non-empty f-string detection#26526
MichaReiser merged 2 commits into
astral-sh:mainfrom
velikodniy:fix-non-empty-f-string-truthiness

Conversation

@velikodniy

@velikodniy velikodniy commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

is_non_empty_f_string required all elements of an f-string part to be provably non-empty, but a concatenation is guaranteed non-empty if any element is. This made Truthiness::from_expr both miss and over-report:

  • f"{x}abc" or "bar" was not flagged by SIM222, although the "abc" literal guarantees truthiness.
  • f"" f"{x}" or "bar" was falsely flagged: all is true for the zero-element f"" part (because all is always true for empty collections), yet the value is str(x), which may be empty, so the unsafe fix could change runtime behaviour.

Also closes #21048

Test Plan

New SIM222 fixture cases cover both directions.
The snapshot has been updated and now it correctly flags the pre-existing f"{1}{''}" or "bar" case.

@astral-sh-bot
astral-sh-bot Bot requested a review from ntBre July 3, 2026 13:23
@astral-sh-bot

astral-sh-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot

astral-sh-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

ecosystem-analyzer results

No diagnostic changes detected ✅

Flaky changes detected. This PR summary excludes flaky changes; see the HTML report for details.

Full report with detailed diff (timing results)

@MichaReiser
MichaReiser requested review from MichaReiser and removed request for ntBre July 6, 2026 13:42
@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Jul 6, 2026
@astral-sh-bot

astral-sh-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 1 projects; 55 projects unchanged)

home-assistant/core (+1 -0 violations, +0 -0 fixes)

+ homeassistant/components/emby/media_player.py:182:16: SIM222 Use `f"Emby {self.device.name}"` instead of `f"Emby {self.device.name}" or ...`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
SIM222 1 1 0 0 0

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 1 projects; 55 projects unchanged)

home-assistant/core (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

+ homeassistant/components/emby/media_player.py:182:16: expr-or-true Use `f"Emby {self.device.name}"` instead of `f"Emby {self.device.name}" or ...`

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
expr-or-true 1 1 0 0 0

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser changed the title Fix non-empty f-string detection in Truthiness::from_expr Refine non-empty f-string detection Jul 6, 2026
@MichaReiser MichaReiser added the bug Something isn't working label Jul 6, 2026
@MichaReiser

Copy link
Copy Markdown
Member

Nice, the ecosystem finding looks like a real bug. Thank you

@MichaReiser
MichaReiser enabled auto-merge (squash) July 6, 2026 14:01
@MichaReiser
MichaReiser merged commit 1e9287d into astral-sh:main Jul 6, 2026
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

is_non_empty_f_string has false positives

2 participants