Skip to content

getComputedType: route all declaration-name ranges through the binding path (#4091)#4091

Closed
kinto0 wants to merge 2 commits into
facebook:mainfrom
kinto0:export-D111155589
Closed

getComputedType: route all declaration-name ranges through the binding path (#4091)#4091
kinto0 wants to merge 2 commits into
facebook:mainfrom
kinto0:export-D111155589

Conversation

@kinto0

@kinto0 kinto0 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary:

fix #4042

by using the classifications in classify_surface

Reviewed By: stroxler

Differential Revision: D111155589

@meta-cla meta-cla Bot added the cla signed label Jul 9, 2026
@meta-codesync

meta-codesync Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@kinto0 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D111155589.

@stroxler stroxler left a comment

Copy link
Copy Markdown
Contributor

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.

}

// =======================================================================
// getComputedType range routing: an identifier-name range resolves through

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This range-routing section covers def/class/param/typevar/import/except/global/match, but not a keyword-argument label range — which is the one case whose routing actually changed in this PR. Previously the ExprName-only guard in get_computed_type_at_range sent the x range in f(x=1) to the trace path; now classify_surface maps KeywordArgument to a Key (the matched parameter's declaration), so it routes through the declaration-preserving path instead (or NoType → trace when no parameter matches).

Since that behavior change isn't pinned by any test, could we add one here? Something like def f(x: int): ... then f(x=1), asserting the x= label range resolves to the parameter's type — plus a no-match case (e.g. f(y=1)) that should still fall back to the trace path.

kinto0 added 2 commits July 9, 2026 10:20
Summary: See last diff for context. reproduces facebook#4042 with more examples

Reviewed By: maggiemoss

Differential Revision: D111155587
…g path (facebook#4091)

Summary:

fix facebook#4042

by using the classifications in classify_surface

Reviewed By: stroxler

Differential Revision: D111155589
@meta-codesync meta-codesync Bot changed the title getComputedType: route all declaration-name ranges through the binding path getComputedType: route all declaration-name ranges through the binding path (#4091) Jul 9, 2026
@kinto0 kinto0 force-pushed the export-D111155589 branch from ff97ba6 to e4d8f08 Compare July 9, 2026 17:21
@github-actions github-actions Bot added size/xl and removed size/xl labels Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@meta-codesync meta-codesync Bot closed this in 7e636d1 Jul 9, 2026
@meta-codesync meta-codesync Bot added the Merged label Jul 9, 2026
@meta-codesync

meta-codesync Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in 7e636d1.

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.

TSP: inferred return type dropped at definition-site requests (unannotated functions / methods / @property getters render -> Unknown)

3 participants