Skip to content

Commit e85a171

Browse files
committed
a parameter its own body rebinds stands for no one value, so it keeps no bound
`body_parameter_constraints` already computes `single_bindings` — a name bound more than once cannot stand for one value — and applied it to every local except the parameter itself. this applies it there too, which is the rule the file already states rather than an exception carved for this case. measured over the 153 top-level stdlib modules, two binaries from one tree: 5041 diagnostics to 4790. **−254 by location and +3**, across 41 modules. the other 46 of the 49 added lines are the same diagnostic at the same file:line:col with a differently rendered type, and read better for it — `has no attribute 'startswith'` on `object` becomes `not defined on 'None' in union 'Unknown | str | None'`, which names the actual bug in `inspect.getsourcefile`'s None return. all 3 genuinely new ones are correct. of the 254 removed, about 249 are false positives and 5 are real. the strongest thing lost is `code._showtraceback`, where `sys.exc_info()` gives `BaseException | None` against a body calling `value.with_traceback(tb)` — and that only survived because the same body's rebinding was not reachable above it. ⚠️ this masks rather than fixes. two of the three false-positive families it removes have nothing to do with rebinding: a recovered protocol records no `__getitem__`, no operator dunders and no `__iter__`, and a member called twice is pinned to the first call's argument types. both still fire on a parameter nobody rebinds. a third family is genuinely caused by rebinding — a rebind inside a loop retroactively hides a requirement at the loop head, so `ast.visit_If` ends up contradicting its own recovered signature above the line that rebinds. the narrower rule — drop the bound only when the rebinding reads a member off the name — was built and measured: 109 removals, a strict subset, and it loses the two `code.py` signals anyway (`value = value.with_traceback(tb)` is that shape) while keeping ~145 of the false positives. strictly worse on every axis.
1 parent 58ee1ff commit e85a171

3 files changed

Lines changed: 79 additions & 9 deletions

File tree

crates/ty_python_semantic/resources/mdtest/basedpython_sound_types.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,50 @@ def f(x):
567567
f("anything") # ok
568568
```
569569

570+
### a parameter its own body rebinds says nothing
571+
572+
a name bound more than once cannot stand for one value, which is already why a reassigned local
573+
contributes nothing. a parameter is no different once its own body rebinds it: below the rebinding
574+
the name is whatever the rebinding produced, so what is done with it there requires nothing of what
575+
the caller passed
576+
577+
keeping the requirements collected *above* the rebinding would not do either. walking a traceback
578+
requires only that the argument have a `tb_next`, so the rebinding lands on that member's type —
579+
which is `object`, because nothing said what it holds — and the read below it then fails against the
580+
bound the function itself produced
581+
582+
```py
583+
def deepest(tb):
584+
if tb.tb_next:
585+
tb = tb.tb_next
586+
return tb.tb_frame
587+
588+
deepest("anything") # ok
589+
```
590+
591+
the same holds when the member is a method, which is the shape most of these take
592+
593+
```py
594+
def rebound(x):
595+
x.foo()
596+
x = x.foo()
597+
x.foo()
598+
599+
rebound("anything") # ok
600+
```
601+
602+
the rebinding does not have to be reachable, or to come from the parameter, for the name to stop
603+
standing for one value
604+
605+
```py
606+
def maybe(x, flag):
607+
x.foo()
608+
if flag:
609+
x = 1
610+
611+
maybe("anything", True) # ok
612+
```
613+
570614
### a recursive call does not constrain
571615

572616
```py

crates/ty_python_semantic/src/types/inferred_signature.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,8 @@ pub(crate) fn body_parameter_constraints<'db>(
393393

394394
// a name bound more than once cannot stand for one value: which of them a later use is
395395
// about is not a question this can answer
396-
let single_bindings = index
397-
.place_table(body_scope.file_scope_id(db))
396+
let place_table = index.place_table(body_scope.file_scope_id(db));
397+
let single_bindings: FxHashSet<Name> = place_table
398398
.symbols()
399399
.filter(|symbol| symbol.is_bound() && !symbol.is_reassigned())
400400
.map(|symbol| symbol.name().clone())
@@ -431,13 +431,26 @@ pub(crate) fn body_parameter_constraints<'db>(
431431
entries.extend(asserted_parameter_types(db, env, index, body_scope, node));
432432

433433
// a parameter a nested scope captured keeps nothing: that body is checked against this
434-
// bound, and this walk never saw what it does with the name
435-
if !collector.captured.is_empty() {
436-
entries.retain(|(parameter, _)| {
437-
parameter_definition_name(db, *parameter)
438-
.is_none_or(|name| !collector.captured.contains(&name))
439-
});
440-
}
434+
// bound, and this walk never saw what it does with the name.
435+
//
436+
// a parameter its own body rebinds keeps nothing either, for the same reason a rebound
437+
// local does. after
438+
//
439+
// while tb.tb_next:
440+
// tb = tb.tb_next
441+
//
442+
// the name stands for whatever the rebinding produced, not for what the caller passed, so
443+
// the uses below it are no requirement on the argument — and bounding the argument by them
444+
// anyway makes the body fail against its own bound, because the rebinding lands on the
445+
// member type the bound itself invented
446+
entries.retain(|(parameter, _)| {
447+
parameter_definition_name(db, *parameter).is_none_or(|name| {
448+
!collector.captured.contains(&name)
449+
&& place_table
450+
.symbol_id(name.as_str())
451+
.is_some_and(|symbol| !place_table.symbol(symbol).is_reassigned())
452+
})
453+
});
441454

442455
entries.sort_by_key(|(parameter, _)| *parameter);
443456

docs/basedpython/features/sound-types.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,19 @@ that a member's value was given to is read the same way: a name bound more than
174174
for one value, and a use under a narrowing is about something narrower than the value it was bound
175175
to
176176

177+
a parameter its own body rebinds keeps nothing at all, for the same reason. the reads above the
178+
rebinding are not enough on their own: walking a linked structure asks only that the argument have
179+
the member it walks along, so the rebinding lands on that member — whose value nothing described —
180+
and the walk's next step would fail against the signature the function itself produced
181+
182+
```python
183+
def deepest(tb):
184+
if tb.tb_next:
185+
tb = tb.tb_next
186+
return tb.tb_frame
187+
# def deepest(tb)
188+
```
189+
177190
### an `assert` at the top of the body
178191

179192
an `assert` there holds for every call that returns normally, so it is the author saying what they

0 commit comments

Comments
 (0)