ty icon indicating copy to clipboard operation
ty copied to clipboard

add an option to disable `Unknown` type from from being added where there is no type annotation

Open DetachHead opened this issue 3 months ago • 6 comments

the current default behavior is unsafe, see https://play.ty.dev/b92b01eb-09da-48ea-8e4b-02812d27a803

a = [1] # inferred as list[Unknown | int]
a.append("i'm not an int") # no error
a.pop() + 1 # runtime crash

as mentioned in https://github.com/astral-sh/ty/issues/1211#issuecomment-3322024482:

I do think we will likely add an option to, or even default to, inferring the type of un-annotated generic container literals without the union with Unknown.

DetachHead avatar Sep 23 '25 02:09 DetachHead

i believe this case is essential:

class A:
    length = 100  # inferred as Unknown | Literal[100]

def get_length() -> int | None:
    return None

def f(a: A):
    a.length + 1

a = A()
a.length = get_length()  # oops!
f(a)

KotlinIsland avatar Sep 23 '25 02:09 KotlinIsland

As a note for readers who are not aware about the full context: our current behavior is very much intentional and documented here (with an example similar to the one in the second post). This does not mean that we are opposed to future changes or opt-ins to stricter behavior. I just want to emphasize that this is not a bug.

There is a tradeoff between "fewer false positives on correct untyped code", and "fewer false negatives on incorrect untyped code". And ty currently chooses to lean more towards the former behavior (more so than mypy and pyright).

Also, please note that both issues above can be caught by adding a single type annotation (a: list[int] and length: int / length: int | None).

sharkdp avatar Sep 23 '25 09:09 sharkdp

Also, please note that both issues above can be caught by adding a single type annotation (a: list[int] and length: int / length: int | None).

as i said in https://github.com/astral-sh/ty/issues/1211#issuecomment-3321964618, i don't think this is an optimal solution:

But doesn’t that mean users who want full strictness will have to annotate everything to opt out of this behavior, and therefore can’t rely on type inference at all?

I realize that people like us who want the type checker to be as strict as possible are in the minority, but this sounds like ty will be a downgrade for us, which is a shame because everything else astral has made has been a massive improvement over the existing solutions.

If gradual adoption of type safety is the goal, what does that mean for people who complete the “gradual” journey and end up at the same place as us? Their experience will be degraded by being forced to annotate everything, which is a major reason why people don’t like the idea of type checkers in the first place.

i'm glad you guys are open to adding an option to change this behavior, i just figured it was worth re-iterating my stance from the other issue for those that didn't see that converation

DetachHead avatar Sep 23 '25 13:09 DetachHead

this case is essential

as this is not @Todo, i am assuming that it is intentional, and not incomplete functionality

class C:
    def f(self):
        self  # Unknown

KotlinIsland avatar Sep 25 '25 00:09 KotlinIsland

i am assuming that it is intentional, and not incomplete functionality

It is just incomplete functionality and unrelated to this issue. It doesn't use @Todo simply because it dates back to before the existence of that type. There's ongoing work in progress to fix this, it just uncovers some other issues that needed resolution first.

carljm avatar Sep 25 '25 00:09 carljm

The plan outlined in https://github.com/astral-sh/ty/issues/1473 would mean that we don't need to union in Unknown, and instead extend bidirectional inference to ensure type-safety at all uses of the collection (or any generic class). However, we may still want a strict option to opt out of that behaviour and error more eagerly.

Also note that our current behavior is somewhat inconsistent in that we only add Unknown to collection literals, and not generic class constructors in general. It's not clear we want to change that right now, as it tends to work well in practice, and will eventually be solved by https://github.com/astral-sh/ty/issues/1473 .

ibraheemdev avatar Nov 04 '25 20:11 ibraheemdev