False positive when a function accepts **kwargs after arguments with default values

[MaigoAkisame]

Pyre Bug

Bug description

Pyre mistakenly reports an "incompatible parameter type [6]" error when a function's signature declares **kwargs after arguments with default values, and a caller tries to provide values for kwargs with a ** expansion.

Reproduction steps

Here's a minimal example:

def foo(a: int = 1, b: float = 2.0, c: bool = True, **kwargs: Any) -> None:
    pass

dic = {"key": "value", "foo": [1, 2, 3]}
foo(**dic)

Run pyre, and you'll get the following errors:

Incompatible parameter type [6]: In call `foo`, for 1st positional argument, expected `bool` but got `Union[List[int], str]`.
Incompatible parameter type [6]: In call `foo`, for 1st positional argument, expected `float` but got `Union[List[int], str]`.
Incompatible parameter type [6]: In call `foo`, for 1st positional argument, expected `int` but got `Union[List[int], str]`.

Apparently, pyre is matching the values in dic with the arguments a, b, and c.
Also, the word 1st in the error messages isn't always accurate: b and c are the 2nd and 3rd positional arguments.

Expected behavior

There should be no error, because the contents of dic should be matched with **kwargs.

Logs

N/A

Additional context

Maybe a duplicate of #242, but it's rather hard to read, and I'm not sure if I understand it.

Another Python type checker, mypy, has this bug, too. It has been reported repeatedly in the last 8 years (examples below), but hasn't been fixed yet.
python/mypy#8485 (reported by the same user as the issue above)
python/mypy#1969 (first report in 2016)
python/mypy#5382 (contains links to all similar reports)
python/mypy#8862

[stroxler]

Thanks @MaigoAkisame for the detailed report.

Function argument matching is pretty tricky to get right, and in general has false negative / false positive tradeoffs (in this particular example the dict happens to a literal so we could in principle synthesize an exact typed dict match, but currently Pyre doesn't automatically syntheseize typed dicts and I think neither does mypy)

We may be able to do better though, this is a good example. And you're certainly write that the repeated use of "1st" in error messages is strange and a bit confusing

[MaigoAkisame]

@stroxler

Inferring the type of the values in dic is one thing; the other is matching the keys with the argument list.

In my example, the important thing is to figure out key and foo don't match a, b or c, so they should go into kwargs.
Because kwargs is annotated as Any, it can accept the values in dic regardless of their types.

How difficult do you estimate the fix will be?

[yangdanny97]

I think this is a duplicate of #242, or at least the solution is the same.

Either you can declare the dictionary as a typed dict:

class MyTypedDict(TypedDict):
    key: str
    foo: list[int]

def foo(a: int = 1, b: float = 2.0, c: bool = True, **kwargs: Any) -> None:
    pass

dic: MyTypedDict = {"key": "value", "foo": [1, 2, 3]}
foo(**dic)

or you can unpack the dictionary literal directly in the call

def foo(a: int = 1, b: float = 2.0, c: bool = True, **kwargs: Any) -> None:
    pass
foo(**{"key": "value", "foo": [1, 2, 3]})

Both of those would work on the latest version of Pyre.

When you unpack a dictionary literal directly in the call, we have a special case that expands it to named arguments. Outside of that, we don't model heterogeneous dictionaries that aren't a typed dict.

[MaigoAkisame]

@yangdanny97 For more context, I'm writing a function like foo for any user to use.
I'd like to write the foo function in a way so that users won't be troubled by this bug of pyre, instead of having users tweak their code to avoid the error.

Currently my solution is to write the foo function like this:

def foo(**kwargs: Any) -> None:
    a = kwargs.pop("a", 1)
    b = kwargs.pop("b", 2.0)
    c = kwargs.pop("c", True)
    ...

Admittedly, this makes the signature more obscure, and loses the type checking on a, b, and c, but at least the user can call foo(**dic) without a problem.

And BTW, the issue I'd like to have resolved is not having pyre infer the types of the values in dic, but rather having pyre understand that the content of dic should be matched with kwargs, not a, b, or c.
This only requires comparing the keys in dic with the named arguments.
Since kwargs is annotated as Any, there should be no error even if we don't know the exact types of the values in dic.

Will this be easier to fix?

[yangdanny97]

So technically it's not a "bug" in the sense that it doesn't violate the typing specification for Python; the spec mentions that the behavior for checking non-typed dictionaries varies by typechecker, and the behavior of Pyre v.s. Mypy v.s. Pyright are all different here.

https://typing.readthedocs.io/en/latest/spec/callables.html#function-calls-with-standard-dictionaries

What you're asking for sounds like structural typing for non-typed dictionaries, which would be a nontrivial feature to implement since we'd need to track the set of known keys through del statements, assignments, spreads, etc.

[MaigoAkisame]

I'm not asking to infer the types of the elements in dic; I'm just asking to analyze the keys in dic.
In other words, since dic does not contain the keys a, b, or c, it shouldn't be compared to these arguments at all.
If dic had a key named a, then it's OK to assume dic["a"] has type Any since dic is unannotated. I don't expect pyre to infer that dic["a"] is actually an int.

Are you saying even inferring which keys exist in dic could be difficult due to del statements, assignments, spreads, etc?

[yangdanny97]

Are you saying even inferring which keys exist in dic could be difficult due to del statements, assignments, spreads, etc?

Yes, because there's no mechanism in Pyre to track known keys in a non-typed dictionary, and the behavior isn't standardized so even if we implemented something the other typecheckers would give different results depending on their implementation details.

The easiest solution would be to ignore operations like del, assignment, spread, and just drop all the known keys and fall back to dict whenever we encounter something like that, but that makes the feature much less useful.

[MaigoAkisame]

Thanks for the explanation! I think I'll use the kwargs.pop solution for now.
You can decide if you want to keep the issue open for tracking purposes.