python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.28k stars 2.79k forks source link

Type of default value for `attrs.field()` is revealed as `Union[builtins.int, None]` #16174

Open gertvdijk opened 1 year ago

gertvdijk commented 1 year ago

Bug Report

Default (default=) value for attrs.field() type is revealed as Union[builtins.int, None] where the Union with None is unexpected. While this used to be revealed as Any, this shows up as a regression, bisected to https://github.com/python/mypy/commit/391ed853f (PR https://github.com/python/mypy/pull/15021, mypy master, unreleased).

To Reproduce

import attrs

@attrs.define()
class MyDataClass:
    some_int_field: int = attrs.field(default=123)

# Somewhere else in the project I would like to reference that default:
reveal = attrs.fields(MyDataClass).some_int_field.default
reveal_type(reveal)

assignment_to_int_fails: int = attrs.fields(MyDataClass).some_int_field.default

Expected Behavior

$ mypy --strict myfile.py
myfile.py:9:13: note: Revealed type is "builtins.int"
Success: no issues found in 1 source file

Actual Behavior

on master (tested 9edda9a79790d8f7263234eca9509657ea0c37f0):

$ mypy --strict --show-error-context --show-column-numbers myfile.py
myfile.py:9:13: note: Revealed type is "Union[builtins.int, None]"
myfile.py:11:32: error: Incompatible types in assignment (expression has type "int | None", variable has type "int")  [assignment]
Found 1 error in 1 file (checked 1 source file)

(my project fails, suspected mypy bug / imperfect type inference)

On 1.5.1:

$ mypy --strict myfile.py
myfile.py:9:13: note: Revealed type is "Any"
Success: no issues found in 1 source file

(my project passes, albeit without a proper type check)

Your Environment

Project to test on if you want (100% mypy-strict compatible on 1.5.1 and master otherwise): https://github.com/gertvdijk/PyKMP

gertvdijk commented 1 year ago

Hi @Tinche! I noticed you sumitted a PR to revert some problematic behaviour introduced by that commit I bisected this to (https://github.com/python/mypy/pull/15393) and then @hauntsaninja proposed an alternative (https://github.com/python/mypy/pull/15688), so I wanted you guys to get this friendly ping.

Am I missing something or is there already some fix in the works for this? Thanks!

Tinche commented 1 year ago

Hi,

this is the first I'm hearing of this so I don't think we have a fix in the works.

That said, I'm not sure this is easily fixable (except that the default type should actually be int | Literal[attrs.NOTHING] instead of int | None).

The type of fields(MyDataClass).some_int_field is attrs.Attribute[int], and there isn't enough information in that type signature for Mypy to determine whether the type of attrs.Attribute[int].default should be int or Literal[attrs.NOTHING] (i.e. whether the field has a default isn't knowable from the type).

I see two solutions:

  1. We change the Mypy plugin to make the type of Attribute.default be Any. This will remove your error but it's basically false type safety.
  2. Maybe we can change the signature of attrs.Attribute to be generic over two type variables; so in your case it'd be attrs.Attribute[int, Literal[123]]. (Or at least attrs.Attribute[int, int].) We aren't necessarily bound to the amount of type variables the actual attrs.Attribute class has; at runtime it has 0 currently. This approach is much more work but actually pushes the attrs typing boundary further in the right direction.

EDIT: The default value could also be an instance of attrs.Factory.

gertvdijk commented 11 months ago

Thanks for the thoughts and info. For now I've worked around the error and just made it a simple constant reused by both cases (see commit referenced above) and I think it's not a big deal.

Tinche commented 11 months ago

I actually put in some work in the meantime to see if this is achievable. I think it's probably worthwhile but it'll need an attrs release as well, and I'm not sure how to coordinate exactly.