Skip to content

fix Odd warning on shutil.rmtree and ExitStack.callback #2105#2340

Open
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:2105
Open

fix Odd warning on shutil.rmtree and ExitStack.callback #2105#2340
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:2105

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Feb 6, 2026

Summary

Fixes #2105

Fixed ParamSpec inference for forwarded callbacks to select the overload that matches the forwarded arguments, even when the callback is a bound __call__ (protocol/class instance).

Test Plan

Added a regression test for ExitStack.callback(shutil.rmtree, ..., ignore_errors=True).

@meta-cla meta-cla bot added the cla signed label Feb 6, 2026
@github-actions
Copy link

github-actions bot commented Feb 6, 2026

Diff from mypy_primer, showing the effect of this PR on open source code:

pwndbg (https://github.com/pwndbg/pwndbg)
- ERROR pwndbg/dbg_mod/lldb/repl/fuzzy.py:127:20-45: Missing argument `ignore_errors` in function `atexit.register` [missing-argument]
- ERROR pwndbg/dbg_mod/lldb/repl/fuzzy.py:127:20-45: Missing argument `onerror` in function `atexit.register` [missing-argument]
- ::error file=pwndbg/dbg_mod/lldb/repl/fuzzy.py,line=127,col=20,endLine=127,endColumn=45,title=Pyrefly missing-argument::Missing argument `ignore_errors` in function `atexit.register`
- ::error file=pwndbg/dbg_mod/lldb/repl/fuzzy.py,line=127,col=20,endLine=127,endColumn=45,title=Pyrefly missing-argument::Missing argument `onerror` in function `atexit.register`

tornado (https://github.com/tornadoweb/tornado)
+ ERROR tornado/gen.py:255:62-69: Argument `Awaitable[Unknown] | Future[Unknown] | dict[Any, Awaitable[Unknown]] | list[Awaitable[Unknown]] | _T | None` is not assignable to parameter `first_yielded` with type `Awaitable[Unknown] | Future[Unknown] | dict[Any, Awaitable[Unknown]] | list[Awaitable[Unknown]] | None` in function `Runner.__init__` [bad-argument-type]
+ ::error file=tornado/gen.py,line=255,col=62,endLine=255,endColumn=69,title=Pyrefly bad-argument-type::Argument `Awaitable[Unknown] | Future[Unknown] | dict[Any, Awaitable[Unknown]] | list[Awaitable[Unknown]] | _T | None` is not assignable to parameter `first_yielded` with type `Awaitable[Unknown] | Future[Unknown] | dict[Any, Awaitable[Unknown]] | list[Awaitable[Unknown]] | None` in function `Runner.__init__`

starlette (https://github.com/encode/starlette)
+ ERROR starlette/testclient.py:112:28-39: Argument `BoundMethod[BlockingPortal, Overload[
+   [PosArgsT, T_Retval](self: BlockingPortal, func: (**tuple[*PosArgsT]) -> Awaitable[T_Retval], *args: *PosArgsT) -> T_Retval
+   [PosArgsT, T_Retval](self: BlockingPortal, func: (**tuple[*PosArgsT]) -> T_Retval, *args: *PosArgsT) -> T_Retval
+ ]]` is not assignable to parameter with type `(func: (**tuple[*PosArgsT]) -> Awaitable[T_Retval], *args: *PosArgsT) -> @_` in function `contextlib._BaseExitStack.callback` [bad-argument-type]
+ ::error file=starlette/testclient.py,line=112,col=28,endLine=112,endColumn=39,title=Pyrefly bad-argument-type::Argument `BoundMethod[BlockingPortal, Overload[%0A  [PosArgsT, T_Retval](self: BlockingPortal, func: (**tuple[*PosArgsT]) -> Awaitable[T_Retval], *args: *PosArgsT) -> T_Retval%0A  [PosArgsT, T_Retval](self: BlockingPortal, func: (**tuple[*PosArgsT]) -> T_Retval, *args: *PosArgsT) -> T_Retval%0A]]` is not assignable to parameter with type `(func: (**tuple[*PosArgsT]) -> Awaitable[T_Retval], *args: *PosArgsT) -> @_` in function `contextlib._BaseExitStack.callback`

ibis (https://github.com/ibis-project/ibis)
- ERROR ibis/backends/clickhouse/tests/conftest.py:146:32-156:18: Missing argument `text` in function `concurrent.futures._base.Executor.submit` [missing-argument]
- ERROR ibis/backends/tests/base.py:362:25-372:18: Missing argument `text` in function `concurrent.futures._base.Executor.submit` [missing-argument]
- ::error file=ibis/backends/clickhouse/tests/conftest.py,line=146,col=32,endLine=156,endColumn=18,title=Pyrefly missing-argument::Missing argument `text` in function `concurrent.futures._base.Executor.submit`
- ::error file=ibis/backends/tests/base.py,line=362,col=25,endLine=372,endColumn=18,title=Pyrefly missing-argument::Missing argument `text` in function `concurrent.futures._base.Executor.submit`

@asukaminato0721 asukaminato0721 marked this pull request as ready for review February 7, 2026 00:29
Copilot AI review requested due to automatic review settings February 7, 2026 00:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes pyrefly’s ParamSpec inference when forwarding arguments through callback-style helpers (e.g., ExitStack.callback), ensuring the selected callable overload matches the forwarded arguments (including for bound __call__ callables), addressing issue #2105.

Changes:

  • Update ParamSpec inference in call analysis to prefer the callable overload that matches forwarded *args/**kwargs.
  • Add a regression test covering ExitStack.callback(shutil.rmtree, ..., ignore_errors=True).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
pyrefly/lib/alt/callable.rs Adds logic to select an overload for a forwarded callable to constrain the ParamSpec based on forwarded arguments.
pyrefly/lib/test/paramspec.rs Adds a regression test reproducing the ExitStack.callback(shutil.rmtree, ..., ignore_errors=True) scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +548 to +552
let call_errors_local = self.error_collector();
let _ = self.callable_infer(
sig.clone(),
None,
None,
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

Overload selection here uses a “first signature with zero call errors wins” approach by iterating sigs and breaking on the first match. This can diverge from the project’s overload call evaluation logic (see pyrefly/lib/alt/overload.rs, which applies additional disambiguation steps when multiple overloads match), and could pin the ParamSpec to the wrong parameter list when multiple signatures accept the forwarded args. Consider reusing the existing overload-selection machinery (or at least its disambiguation heuristic) to choose the same overload that a normal call would select, before constraining paramspec_var.

Copilot uses AI. Check for mistakes.
Comment on lines +544 to +548
if sigs.len() > 1 {
let forwarded_args = args.get(arg_index + 1..).unwrap_or(&[]);
let mut selected = None;
for sig in sigs {
let call_errors_local = self.error_collector();
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

This loop potentially calls callable_infer once per candidate signature, which re-infers forwarded_args/keywords each time. In hot paths with large overload sets this can become noticeably expensive. If possible, precompute argument types once (similar to CallWithTypes in call_overloads) and/or delegate to call_overloads to benefit from its argument-flattening and matching heuristics.

Copilot uses AI. Check for mistakes.
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.

Odd warning on shutil.rmtree and ExitStack.callback

1 participant