Skip to content

Conversation

@sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Apr 4, 2025

Summary

Fix assignability of tuple[()] to AlwaysFalsy.

closes #17202

Test Plan

Ran the property tests for a while

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Apr 4, 2025
.is_assignable_to(db, target) =>
{
true
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that we now allow a fall-through to the catch-all "self is subtype of target" branch.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2025

mypy_primer results

No ecosystem changes detected ✅

@carljm
Copy link
Contributor

carljm commented Apr 4, 2025

I have a vague memory of intentionally making the opposite decision here.

I think the key question we need to ensure we are answering consistently is, do instances of subclasses of the built-in tuple type also inhabit our Type::Tuple types? My memory is that in other places we assume they can, but I would have to double-check. If they do, then we cannot say that the empty Type::Tuple is always-falsy, since the empty instance of a subclass of tuple could be truthy.

The potential problem with saying they do not, is that we do need to allow code like this:

def f(t: tuple[int, str]): ...

class MyTuple(tuple[int, str]): pass

f(MyTuple((1, "foo")))

In other words, if we say they do not, then our Type::Tuple can no longer correspond to the semantics of a tuple annotation.

@AlexWaygood
Copy link
Member

I have a vague memory of intentionally making the opposite decision here.

Ah yes, astral-sh/ty#215 is still unresolved... I have a long reply in my head I've been meaning to write up there for a while :-) maybe I should set an explicit goal for myself to do that on Monday...

Anyway, I think the change to the code here is correct even if we disagree on the assertion being made in the mdtest!

@carljm
Copy link
Contributor

carljm commented Apr 4, 2025

Yes, I think the code change is correct, but we should add a TODO comment above the assertions (and maybe also above similar assertions for subtyping, if we have those already?) that we either need to special-case enforcement of Liskov on __bool__ and __len__ for tuple subclasses, or stop assuming that empty tuples are falsy and non-empty tuples are truthy.

@sharkdp sharkdp merged commit 1a6a10b into main Apr 4, 2025
23 checks passed
@sharkdp sharkdp deleted the david/fix-empty-tuple-always-falsy branch April 4, 2025 20:00
dcreager added a commit that referenced this pull request Apr 4, 2025
* main:
  [red-knot] Empty tuple is always-falsy (#17213)
  Run fuzzer with `--preview` (#17210)
  Bump 0.11.4 (#17212)
  [syntax-errors] Allow `yield` in base classes and annotations (#17206)
  Don't skip visiting non-tuple slice in `typing.Annotated` subscripts (#17201)
  [red-knot] mypy_primer: do not specify Python version (#17200)
  [red-knot] Add `Type.definition` method (#17153)
  Implement `Invalid rule provided` as rule RUF102 with `--fix` (#17138)
  [red-knot] Add basic on-hover to playground and LSP (#17057)
  [red-knot] don't remove negations when simplifying constrained typevars (#17189)
  [minor] Fix extra semicolon for clippy (#17188)
  [syntax-errors] Invalid syntax in annotations (#17101)
  [syntax-errors] Duplicate attributes in match class pattern (#17186)
  [syntax-errors] Fix multiple assignment for class keyword argument (#17184)
  use astral-sh/cargo-dist instead (#17187)
  Enable overindented docs lint (#17182)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[red-knot] is_subtype_of no longer implies is_assignable_to for all Type::Tuple types

3 participants