`F821`: Declared only variables

[eltoder]

Summary

Local variable annotations make annotated variables local even if there is no direct assignment. This is useful to create a local variable without an initial value. Ruff appears to not understand this usage. For example:

from typing import Callable

def make_cell() -> tuple[Callable[[], int], Callable[[int], None]]:
    x: int

    def getter() -> int:
        return x

    def setter(xx: int) -> None:
        nonlocal x
        x = xx

    return getter, setter

getter, setter = make_cell()
setter(123)
print(getter())

(playground link: https://play.ruff.rs/3bd90cac-2c87-48b0-9cb4-64fe2a53bf72)

This runs fine and passes mypy, however ruff produces a F821:

$ ruff check --isolated t.py
F821 Undefined name `x`
 --> t.py:7:16
  |
6 |     def getter() -> int:
7 |         return x
  |                ^
8 |
9 |     def setter(xx: int) -> None:

Version

0.14.5

[MichaReiser]

Thank you. Yeah, this is a bug. A variable that is declared by using an annotated assignment without a value shouldn't raise an error.

[Ruchir28]

How should this be handled?

While looking into the code, it seems we could suppress this by checking
if the annotation-only binding is inside a function and then returning
ReadResult::Resolved(binding_id). But I'm not sure if returning Resolved
is the correct behavior here, since the variable may still be unassigned.

ruff/crates/ruff_python_semantic/src/model.rs

Lines 467 to 485 in fffbe5a

	match self.bindings[binding_id].kind {
	// If it's a type annotation, don't treat it as resolved. For example, given:
	//
	// ```python
	// name: str
	// print(name)
	// ```
	//
	// The `name` in `print(name)` should be treated as unresolved, but the `name` in
	// `name: str` should be treated as used.
	//
	// Stub files are an exception. In a stub file, it _is_ considered valid to
	// resolve to a type annotation.
	BindingKind::Annotation if !self.in_stub_file() => continue,
	// If it's a deletion, don't treat it as resolved, since the name is now
	// unbound. For example, given:
	//
	// ```python

[eltoder]

@Ruchir28 it's not any different than

if False:
    name = something

[ntBre]

@Ruchir28, from a very quick look, that seems like a reasonable place to mark the binding as resolved. Do other things break when you do that?

I'm also curious about the BindingKind::Nonlocal branch in resolve_load. It seems like x should possibly be marked as resolved after the nonlocal statement in setter, but I might be misunderstanding without playing with the code.

[Ruchir28]

@ntBre No it doesn't break anything - and looks like it does resolve the issue

and I checked the BindingKind::Nonlocal branch ,and from what i understand it is indeed marking x as resolved in setter function.

Although not related to the fix, after fixing this, the below code won't fire F821
but still there should be F823 / UnboundLocalError right (which it doesn't currently)?
Should this be addressed separately or in this issue?

def annotated_local_function():
    x: int
    print(x)

[eltoder]

@Ruchir28 nonlocal does not appear to mark x as resolved:

def make_cell():
    def setter():
        nonlocal x
        print(x)

This produces F821. I think it would make sense to mark it as resolved, but it also does not materially change anything, because a variable declared as nonlocal must be already visible in the current scope (#21495).

For the F823 / UnboundLocalError -- it appears to be raised only in a very specific pattern. For example, this does not raise it:

def annotated_local_function():
    if False:
        x = 1
    print(x)

My impression is that F823 is only raised when the issue is evident from a single statement like += and does not require any control flow analysis. For the local annotation neither of the statements are wrong. You need some control flow analysis to discover that there is no assignment between the declaration and the print.

I suppose you can keep a state per variable of whether it is only declared or assigned, so you will report F823 on

def annotated_local_function():
    x: int
    print(x)

but not on

def annotated_local_function():
    x: int
    print(x)
    x = 1

I'm not sure how useful this is, but it is probably easy to implement.

[Ruchir28]

Interesting looking at the code it looks like it's marking the nonlocal as resolved , but this is weird #21495

ruff/crates/ruff_python_semantic/src/model.rs

Lines 566 to 581 in 78ce17c

	BindingKind::Global(Some(binding_id))
	| BindingKind::Nonlocal(binding_id, _) => {
	// Mark the shadowed binding as used.
	let reference_id = self.resolved_references.push(
	self.scope_id,
	self.node_id,
	ExprContext::Load,
	self.flags,
	name.range,
	);
	self.bindings[binding_id].references.push(reference_id);
	// Treat it as resolved.
	self.resolved_names.insert(name.into(), binding_id);
	return ReadResult::Resolved(binding_id);
	}

Also regarding F823 i am not so sure about it and it's implemenation details rn but looks like it can be reported in both the cases here even in the second one [as the load reference will be attached to the initial BindingKind::Annotation id] so it can be detected there too