[ty] Fix overload filtering to prefer more "precise" match by dhruvmanila · Pull Request #21859 · astral-sh/ruff

dhruvmanila · 2025-12-09T05:49:19Z

Summary

I took this chance to add some debug level tracing logs for overload call evaluation similar to Doug's implementation in constraints.rs.

Test Plan

Add new mdtests
Tested it against sqlalchemy.select in pyx which results in the correct overload being matched

astral-sh-bot · 2025-12-09T05:51:26Z

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

astral-sh-bot · 2025-12-09T05:53:14Z

`mypy_primer` results

Changes were detected when running on open source projects

scipy-stubs (https://github.com/scipy/scipy-stubs)
+ tests/stats/test_rv_frozen.pyi:35:1: error[type-assertion-failure] Type `int | float` does not match asserted type `Unknown`
+ tests/stats/test_rv_frozen.pyi:40:1: error[type-assertion-failure] Type `int | float` does not match asserted type `Unknown`
- Found 1212 diagnostics
+ Found 1214 diagnostics

pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
- pandas-stubs/_typing.pyi:1217:16: warning[unused-ignore-comment] Unused blanket `type: ignore` directive
- Found 5545 diagnostics
+ Found 5544 diagnostics

No memory usage changes detected ✅

dhruvmanila · 2025-12-09T09:33:42Z

crates/ty_python_semantic/src/types/call/bind.rs

+        tracing::debug!(
+            target: "ty_python_semantic::types::call::bind",
+            matching_overload_index = ?self.matching_overload_index(),
+            signature = %self.signature_type.display(db),
+            "after step 1",
+        );


The signature is important because we want to know which message is this for but this will be too verbose for functions containing multiple overloads. Maybe, it might be better to add some information about the call site instead like which call expression this is for.

dhruvmanila · 2025-12-09T09:34:42Z

crates/ty_python_semantic/src/types/call/bind.rs

+        // The following loop is trying to construct a tuple of argument types that correspond to
+        // the participating parameter indexes. Considering the following example:
+        //
+        // ```python
+        // @overload
+        // def f(x: Literal[1], y: Literal[2]) -> tuple[int, int]: ...
+        // @overload
+        // def f(*args: Any) -> tuple[Any, ...]: ...
+        //
+        // f(1, 2)
+        // ```
+        //
+        // Here, only the first parameter participates in the filtering process because only one
+        // overload has the second parameter. So, while going through the argument types, the
+        // second argument needs to be skipped but for the second overload both arguments map to
+        // the first parameter and that parameter is considered for the filtering process. This
+        // flag is to handle that special case of many-to-one mapping from arguments to parameters.
+        let mut variadic_parameter_handled = false;
+
        for (argument_index, argument_type) in arguments.iter_types().enumerate() {
+            if variadic_parameter_handled {
+                continue;
+            }
            for overload_index in matching_overload_indexes {
                let overload = &self.overloads[*overload_index];
                for (parameter_index, variadic_argument_type) in
                    overload.argument_matches[argument_index].iter()
                {
+                    if overload.signature.parameters()[parameter_index].is_variadic() {
+                        variadic_parameter_handled = true;
+                    }


This is the diff that fixes the issue. I don't really like the change that much, I tried to see if this could be simplified and I have some ideas but might require some more time so I'll punt that on for post-beta stuff to do.

Is there a risk of skipping keyword-variadic parameters here? If so, would it be useful to add a variation of the test case where the def f(*args: Any) -> tuple[Any, ...]: ... overload also takes keyword arguments (and we would make sure that this overload is still selected if keyword arguments are also passed in)?

Yeah, this doesn't work well when keyword arguments are involved.

Going to track the follow-up in astral-sh/ty#1825

sharkdp

Thank you!!

sharkdp · 2025-12-09T09:47:19Z

crates/ty_python_semantic/src/types/call/bind.rs

+        // The following loop is trying to construct a tuple of argument types that correspond to
+        // the participating parameter indexes. Considering the following example:
+        //
+        // ```python
+        // @overload
+        // def f(x: Literal[1], y: Literal[2]) -> tuple[int, int]: ...
+        // @overload
+        // def f(*args: Any) -> tuple[Any, ...]: ...
+        //
+        // f(1, 2)
+        // ```
+        //
+        // Here, only the first parameter participates in the filtering process because only one
+        // overload has the second parameter. So, while going through the argument types, the
+        // second argument needs to be skipped but for the second overload both arguments map to
+        // the first parameter and that parameter is considered for the filtering process. This
+        // flag is to handle that special case of many-to-one mapping from arguments to parameters.
+        let mut variadic_parameter_handled = false;
+
        for (argument_index, argument_type) in arguments.iter_types().enumerate() {
+            if variadic_parameter_handled {
+                continue;
+            }
            for overload_index in matching_overload_indexes {
                let overload = &self.overloads[*overload_index];
                for (parameter_index, variadic_argument_type) in
                    overload.argument_matches[argument_index].iter()
                {
+                    if overload.signature.parameters()[parameter_index].is_variadic() {
+                        variadic_parameter_handled = true;
+                    }


Is there a risk of skipping keyword-variadic parameters here? If so, would it be useful to add a variation of the test case where the def f(*args: Any) -> tuple[Any, ...]: ... overload also takes keyword arguments (and we would make sure that this overload is still selected if keyword arguments are also passed in)?

This reverts commit c0864b7.

…return * dcreager/die-die-intersections: (29 commits) simpler bounds [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) [ty] Remove legacy `concise_message` fallback behavior (#21847) [ty] Make Python-version subdiagnostics less verbose (#21849) [ty] Supress inlay hints when assigning a trivial initializer call (#21848) ...

…return * dcreager/die-die-intersections: (31 commits) clippy reword comment simpler bounds [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816) [ty] Remove legacy `concise_message` fallback behavior (#21847) ...

* origin/main: (33 commits) [ty] Simplify union lower bounds and intersection upper bounds in constraint sets (#21871) [ty] Collapse `never` paths in constraint set BDDs (#21880) Fix leading comment formatting for lambdas with multiple parameters (#21879) [ty] Type inference for `@asynccontextmanager` (#21876) Fix comment placement in lambda parameters (#21868) [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) ...

dhruvmanila added bug Something isn't working ty Multi-file analysis & type inference labels Dec 9, 2025

dhruvmanila force-pushed the dhruv/fix-overload-filtering branch from 85aa937 to ee8b013 Compare December 9, 2025 08:52

[ty] Fix overload filtering

a04f52f

dhruvmanila force-pushed the dhruv/fix-overload-filtering branch from ee8b013 to a04f52f Compare December 9, 2025 08:55

dhruvmanila commented Dec 9, 2025

View reviewed changes

dhruvmanila changed the title ~~[ty] Fix overload filtering~~ [ty] Fix overload filtering to prefer more "precise" match Dec 9, 2025

dhruvmanila marked this pull request as ready for review December 9, 2025 09:35

dhruvmanila requested review from AlexWaygood, carljm, dcreager and sharkdp as code owners December 9, 2025 09:35

sharkdp approved these changes Dec 9, 2025

View reviewed changes

Skip step 5 for overloads with *args / **kwargs

c0864b7

dhruvmanila marked this pull request as draft December 9, 2025 12:01

dhruvmanila added 2 commits December 9, 2025 20:20

Revert "Skip step 5 for overloads with *args / **kwargs"

d8eb515

This reverts commit c0864b7.

Add TODO comments for failing test cases

b21d520

dhruvmanila marked this pull request as ready for review December 9, 2025 14:53

dhruvmanila mentioned this pull request Dec 9, 2025

Correctly handle variadic parameters for overload step 5 filtering astral-sh/ty#1825

Open

dhruvmanila merged commit c35bf8f into main Dec 9, 2025
40 checks passed

dhruvmanila deleted the dhruv/fix-overload-filtering branch December 9, 2025 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ty] Fix overload filtering to prefer more "precise" match#21859

[ty] Fix overload filtering to prefer more "precise" match#21859
dhruvmanila merged 4 commits intomainfrom
dhruv/fix-overload-filtering

dhruvmanila commented Dec 9, 2025 •

edited

Loading

Uh oh!

astral-sh-bot bot commented Dec 9, 2025 •

edited

Loading

Uh oh!

astral-sh-bot bot commented Dec 9, 2025 •

edited

Loading

Uh oh!

dhruvmanila Dec 9, 2025

Uh oh!

dhruvmanila Dec 9, 2025

Uh oh!

sharkdp Dec 9, 2025

Uh oh!

dhruvmanila Dec 9, 2025

Uh oh!

dhruvmanila Dec 9, 2025

Uh oh!

sharkdp left a comment

Uh oh!

sharkdp Dec 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dhruvmanila commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test Plan

Uh oh!

astral-sh-bot bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Diagnostic diff on typing conformance tests

Uh oh!

astral-sh-bot bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

mypy_primer results

Uh oh!

dhruvmanila Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

sharkdp Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

dhruvmanila Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

sharkdp left a comment

Choose a reason for hiding this comment

Uh oh!

sharkdp Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dhruvmanila commented Dec 9, 2025 •

edited

Loading

astral-sh-bot bot commented Dec 9, 2025 •

edited

Loading

astral-sh-bot bot commented Dec 9, 2025 •

edited

Loading

`mypy_primer` results