Skip to content

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Oct 26, 2025

fix #238

Taught the narrowing logic to treat dict.get("literal") calls as key facets, so truthiness and is not None checks now refine the corresponding dictionary entry

Added regression coverage for both explicit is not None and plain truthiness checks on literal-key .get calls to confirm the dictionary entry narrows as expected

@meta-cla meta-cla bot added the cla signed label Oct 26, 2025
@asukaminato0721 asukaminato0721 changed the title fix fix narrowing on dict .get Oct 26, 2025
@asukaminato0721 asukaminato0721 changed the title fix narrowing on dict .get fix narrowing on dict (and typed dict) .get Oct 26, 2025
@asukaminato0721 asukaminato0721 marked this pull request as ready for review October 26, 2025 09:27
@yangdanny97 yangdanny97 self-assigned this Oct 30, 2025
@yangdanny97
Copy link
Contributor

Awesome, thanks!

@yangdanny97
Copy link
Contributor

yangdanny97 commented Oct 30, 2025

The concern I have here is that this would apply to get methods on other types as well, not just dict and TypedDict. We might need a different type of Facet or Subject that somehow checks if we're calling a method on dict when we solve the narrow - if it's not a dict or TypedDict then we should not apply any narrowing.

Copy link
Contributor

@yangdanny97 yangdanny97 left a comment

Choose a reason for hiding this comment

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

Back to you with comments, I think this is a great start, just needs one more piece to make sure it doesn't have any unintended effects

@asukaminato0721
Copy link
Contributor Author

Introduced FacetOrigin/FacetSubject to track whether facet chains came from direct subscripts or .get calls, and only reuse them when the base is a dict-like source

Added runtime checks so .get-derived narrows only fire when the underlying type is a builtin dict or (partial) TypedDict

@yangdanny97
Copy link
Contributor

Thanks!

@meta-codesync
Copy link

meta-codesync bot commented Oct 31, 2025

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

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.

Review automatically exported from Phabricator review in Meta.

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.

[mypy parity] We should look at narrowing on dict (and typed dict) .get

4 participants