Skip to content

Conversation

@asukaminato0721
Copy link
Contributor

fix #1175

@meta-cla meta-cla bot added the cla signed label Oct 16, 2025
@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 16, 2025 18:44
@asukaminato0721 asukaminato0721 marked this pull request as draft October 16, 2025 18:48
@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 16, 2025 19:18
@asukaminato0721 asukaminato0721 changed the title fix 1175 fix report unsafe __get__ return types Oct 16, 2025
@yangdanny97 yangdanny97 requested a review from stroxler October 17, 2025 03:52
@meta-codesync
Copy link

meta-codesync bot commented Oct 20, 2025

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D85052586.

Copy link
Contributor

@stroxler stroxler left a comment

Choose a reason for hiding this comment

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

Hi @asukaminato0721 -

Thanks for the PR! I'm sorry to say I think we may have mis-marked this issue as a good first issue.

It's actually pretty tricky and I'm not sure we're ready for an implementation. Your code looks right to me in the sense that it implements the requested behavior, but I don't think we're at a point where we can actually commit to much here yet.

I've commented on the original issue, but there are a host of questions we need to answer before we can implement and ship a solution:

  • First off, do we want to diverge from other type checkers at all? At the moment, no other type checker implements the proposed check
  • What do we do if the annotated type is actually the descriptor type itself? I think this code as written would complain, but the conformance tests explicitly say we are expected to allow this (see https://github.com/python/typing/blob/main/conformance/tests/dataclasses_descriptors.py)
  • How do we want to handle built-in types that run into the same kinds of behaviors, e.g. a Callable that gets assigned a function def (which has method binding descriptors)

While we almost certainly could implement some kind of best-effort check here, I think we may just not be ready to do that yet.

Apologies for the churn, the change itself looks like it could be a great starting point whenever we're ready to put in (and document the behavior of) best-effort checks for incompatible descriptor behaviors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

report unsafe __get__ return types

2 participants