[Analysis] Fix false positive unused-variable reporting in f-string format specifiers

[Arths17]

Summary

This PR resolves a false positive where variables used exclusively within an f-string format specifier (e.g., width or precision nested expressions) were incorrectly flagged as unused (Issue #3009).
The Root Cause

The AST visitor logic in pyrefly_util was previously limited to visiting the main interpolation expression of a FormattedValue node. It was not recursing into the format_spec attribute.

In an expression like f"{key:<{max_len}}", the Python AST structures max_len as a nested expression inside the format_spec. Because the visitor skipped this node, the usage collector never "saw" the reference to max_len, leading the analyzer to conclude the variable was defined but never used.

The Fix

I updated the f-string traversal logic in crates/pyrefly_util/src/ruff_visitors.rs to ensure that both the mutable and read-only visitors now explicitly descend into the format_spec node if it exists.

This ensures that any nested expressions used for dynamic formatting (alignment, padding, width, or precision) are correctly identified as variable usages.
Changes

crates/pyrefly_util/src/ruff_visitors.rs: Updated visit_format_value (or equivalent visitor method) to recurse into format_spec.

pyrefly/lib/test/lsp/diagnostic.rs: Added a regression test case using the reporter's example: f"{key:<{max_len}}" to verify max_len is no longer flagged.

Verification

Environment: Verified on macOS (M4 Apple Silicon).

Rust Tests: Ran cargo test -p pyrefly_util to confirm the visitor correctly identifies the nested name nodes.

LSP Tests: Ran cargo test -p pyrefly_lib --test lsp to verify the diagnostic is no longer emitted for the provided example.

[Copilot]

Pull request overview

This PR fixes a false-positive unused-variable diagnostic by ensuring the AST visitor traverses nested expressions inside f-string format_spec nodes (e.g., dynamic width/precision), and adds a regression test for the reported case (Issue #3009).

Changes:

• Update Rust f-string traversal to recurse into format_spec interpolations for both Visit and VisitMut.
• Add an LSP diagnostic regression test covering f"{key:<{max_len}}" so max_len is counted as “used”.
• Ignore common Python virtualenv directories in .gitignore.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
crates/pyrefly_util/src/ruff_visitors.rs	Adds shared helpers to recurse through interpolation expressions and nested format_spec elements, fixing the missed-usage traversal.
pyrefly/lib/test/lsp/diagnostic.rs	Adds a regression test asserting no unused-variable diagnostics when a variable is used only in an f-string format specifier.
.gitignore	Adds .venv/ and venv/ to ignore local virtual environments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yangdanny97]

this change looks unrelated

[Arths17]

My apologies! I added those while setting up the dev environment on my Mac. I'll revert the .gitignore changes and keep the PR focused strictly on the f-string visitor fix.

[Arths17]

Reverted the .gitignore changes and pushed the update, Sorry about the noise.

[Arths17]

I noticed mypy_primer (7) and test (ubuntu-latest) were failing, so I investigated the logs. The issue wasn't a crash, but rather a stale test expectation that my changes accidentally "fixed."

What Changed:

• crates/pyrefly_util/src/ruff_visitors.rs: Implemented recursive traversal for InterpolatedStringElement::Interpolation. This ensures that expressions inside format_spec (like nested width or precision variables) are no longer ignored by the AST visitors.
  

• pyrefly/lib/test/lsp/diagnostic.rs: Added a regression test verifying that variables used exclusively in f-string specifiers (e.g., max_len in {key:<{max_len}}) are correctly identified as "used."
  

• pyrefly/lib/test/lsp/definition.rs: Updated the existing goto_def_in_fstring_format_specifier test. This test was previously written to expect a None result because traversal was missing. Since the new visitor now correctly finds these definitions, I’ve updated the assertion to expect the successful resolution to def f().
  

Summary:

This PR now correctly handles both unused-variable diagnostics and Go-To-Definition support for nested expressions in f-strings. All local tests and CI checks are now passing.

[meta-codesync]

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D99678402.

[stroxler]

Review automatically exported from Phabricator review in Meta.

[meta-codesync]

@yangdanny97 merged this pull request in ea98e57.