Skip to content

Conversation

@thejchap
Copy link
Contributor

@thejchap thejchap commented Jun 7, 2025

Summary

Related to: #18347

This PR adds tests for some of the current behavior of validate_attribute_assignment, to help with the refactor. This catches some issues that surfaced in the ecosystem check.

Opening as a separate PR so I can see these pass against main then rebase #18347 on top of this and fail them

Projects

Test Plan

New mdtests

# instead of unresolved-attribute, this should report possibly-unbound-attribute
# TODO: error: [possibly-unbound-attribute]
# error: [unresolved-attribute]
a.x = 42
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mypy:

../test.py:12: error: Item "A" of "A | B" has no attribute "x" [union-attr]

@thejchap thejchap force-pushed the thejchap/union-attribute-assignment-possibly-undefined branch from ce040d2 to ca79111 Compare June 7, 2025 02:33
@thejchap thejchap changed the title [ty] Add attribute assignment tests for unions [ty] Add attribute assignment tests Jun 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2025

mypy_primer results

No ecosystem changes detected ✅

@thejchap thejchap force-pushed the thejchap/union-attribute-assignment-possibly-undefined branch 2 times, most recently from 4cf497e to a74c196 Compare June 7, 2025 02:55
class B:
x: int

C = TypeVar("C", bound=Union[A, B])
Copy link
Contributor Author

@thejchap thejchap Jun 7, 2025

Choose a reason for hiding this comment

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

this should address the beartype regression this doesn't catch the issue/fail on the other branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sharkdp

making slow progress on these - limited time :)

i'm still unclear on why the warning in beartype (warning[possibly-unbound-attribute] beartype/_util/api/external/utilclick.py:108:5: Attribute callbackon typeBeartypeableT is possibly unbound) is no longer being emitted on #18347

however while investigating i did find what i think is an issue on main with setting attributes on generics where the upper bound is a union, after narrowing. added a test to illustrate

this results in a possibly-unbound-attribute from ty but no diagnostic from mypy

mypy

../test.py:16: note: Revealed type is "test.B"
Success: no issues found in 1 source file

ty

info[revealed-type]: Revealed type
  --> /Users/justinchapman/src/test.py:16:23
   |
14 |     if not isinstance(b, B):
15 |         raise TypeError("Expected instance of B")
16 |     print(reveal_type(b))
   |                       ^ `C & B`
17 |     b.x = 42
   |

code

from typing import Union, TypeVar
from typing_extensions import reveal_type

class A:
    pass

class B:
    def __init__(self) -> None:
        self.x: int = 0

C = TypeVar("C", bound=Union[A, B])

def _(b: C):
    if not isinstance(b, B):
        raise TypeError("Expected instance of B")
    print(reveal_type(b))
    b.x = 42

b = B()
_(b)

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks like a bug, yes. A similar bug was mentioned on Discord the other day. Would you mind reporting that as a separate issue? A simplified example would be https://play.ty.dev/62d91bdb-cbf1-4758-8f2c-6a26722c9f47

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thejchap thejchap force-pushed the thejchap/union-attribute-assignment-possibly-undefined branch from a74c196 to ea2d263 Compare June 7, 2025 03:09
a.x = 42 # error: [possibly-unbound-attribute]
```

### Assigning to a data descriptor attribute
Copy link
Contributor Author

@thejchap thejchap Jun 7, 2025

Choose a reason for hiding this comment

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

this should address the datadog regression doesn't seem to

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Jun 7, 2025
@thejchap thejchap force-pushed the thejchap/union-attribute-assignment-possibly-undefined branch from ea2d263 to 5f6f995 Compare June 11, 2025 23:26
@thejchap thejchap force-pushed the thejchap/union-attribute-assignment-possibly-undefined branch from 5f6f995 to 933a21e Compare June 12, 2025 00:49
@thejchap thejchap closed this Jul 20, 2025
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.

3 participants