Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mypy 0.930: regression: no longer keeping contextmanager generic return value #11852

Closed
asottile opened this issue Dec 27, 2021 · 4 comments · Fixed by #11870
Closed

mypy 0.930: regression: no longer keeping contextmanager generic return value #11852

asottile opened this issue Dec 27, 2021 · 4 comments · Fixed by #11870
Labels
bug mypy got something wrong

Comments

@asottile
Copy link
Contributor

Bug Report

I bisected this, the regression was introduced in #11352 CC @BarnabyShearer @sobolevn

here's a minimal case split out from pre-commit:

import concurrent.futures
import contextlib
from typing import Callable
from typing import Generator
from typing import Iterable
from typing import TypeVar

TArg = TypeVar('TArg')
TRet = TypeVar('TRet')

@contextlib.contextmanager
def _thread_mapper(maxsize: int) -> Generator[
    Callable[[Callable[[TArg], TRet], Iterable[TArg]], Iterable[TRet]],
    None, None,
]:
    if maxsize == 1:
        yield map
    else:
        with concurrent.futures.ThreadPoolExecutor(maxsize) as ex:
            yield ex.map


def double(x: int) -> int: return x * 2

with _thread_mapper(1) as m:
    print(list(m(double, [2, 3])))

To Reproduce

  1. mypy t.py

Expected Behavior

I expect it to pass (as it did with 0.920)

Actual Behavior

$ mypy ../t.py
../t.py:25: error: Need type annotation for "m"
Found 1 error in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: 0.930 (regression from 0.920)
  • Mypy command-line flags: none
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.8.10
  • Operating system and version: ubuntu 20.04
@asottile asottile added the bug mypy got something wrong label Dec 27, 2021
@sobolevn
Copy link
Member

Thanks, I will have a look! 👍

@asottile
Copy link
Contributor Author

@sobolevn I think it was a misunderstanding about send type in that PR -- there's no meaning of send types for @contextmanager since it only uses the yield type -- removing detach_callable fixes my snippet above

let me know if you want me to turn this into a patch:

diff --git a/mypy/plugins/default.py b/mypy/plugins/default.py
index 67587090e..c57c5f9a1 100644
--- a/mypy/plugins/default.py
+++ b/mypy/plugins/default.py
@@ -15,7 +15,6 @@ from mypy.types import (
 from mypy.subtypes import is_subtype
 from mypy.typeops import make_simplified_union
 from mypy.checkexpr import is_literal_type_like
-from mypy.checker import detach_callable
 
 
 class DefaultPlugin(Plugin):
@@ -192,12 +191,12 @@ def contextmanager_callback(ctx: FunctionContext) -> Type:
                 and isinstance(default_return, CallableType)):
             # The stub signature doesn't preserve information about arguments so
             # add them back here.
-            return detach_callable(default_return.copy_modified(
+            return default_return.copy_modified(
                 arg_types=arg_type.arg_types,
                 arg_kinds=arg_type.arg_kinds,
                 arg_names=arg_type.arg_names,
                 variables=arg_type.variables,
-                is_ellipsis_args=arg_type.is_ellipsis_args))
+                is_ellipsis_args=arg_type.is_ellipsis_args)
     return ctx.default_return_type
 
 
diff --git a/test-data/unit/check-default-plugin.test b/test-data/unit/check-default-plugin.test
index fc9f132ab..6a89aa266 100644
--- a/test-data/unit/check-default-plugin.test
+++ b/test-data/unit/check-default-plugin.test
@@ -29,10 +29,9 @@ from contextlib import contextmanager
 from typing import TypeVar, Generator
 
 T = TypeVar('T')
-S = TypeVar('S')
 
 @contextmanager
-def yield_id(item: T) -> Generator[T, S, None]:
+def yield_id(item: T) -> Generator[T, None, None]:
     yield item
 
 reveal_type(yield_id) # N: Revealed type is "def [T] (item: T`-1) -> contextlib.GeneratorContextManager[T`-1]"
@@ -70,10 +69,9 @@ from contextlib import asynccontextmanager
 from typing import TypeVar, AsyncGenerator
 
 T = TypeVar('T')
-S = TypeVar('S')
 
 @asynccontextmanager
-async def yield_id(item: T) -> AsyncGenerator[T, S]:
+async def yield_id(item: T) -> AsyncGenerator[T, None]:
     yield item
 
 reveal_type(yield_id) # N: Revealed type is "def [T] (item: T`-1) -> typing.AsyncContextManager[T`-1]"

asottile added a commit to pre-commit/pre-commit that referenced this issue Dec 27, 2021
asottile added a commit to asottile/git-code-debt that referenced this issue Dec 28, 2021
@sobolevn
Copy link
Member

@asottile please, go ahead. Let's try this solution.
Can you also attach the failing case as a test?

@asottile
Copy link
Contributor Author

@sobolevn yep! done -- #11870

asottile added a commit to pre-commit/pre-commit that referenced this issue Jan 10, 2022
asottile added a commit to asottile/git-code-debt that referenced this issue Jan 10, 2022
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this issue Jan 20, 2022
clrpackages pushed a commit to clearlinux-pkgs/pypi-pre_commit that referenced this issue Jan 20, 2022
…version 2.17.0

Anthony Sottile (11):
      add setuptools to the zipapp.  resolves #2122
      minor py2 cleanup for sys.stderr.buffer
      replace echo image with focal
      run dead, remove dead code
      work around python/mypy#11852
      forbid overriding `entry` for meta hooks
      always use #!/bin/sh on windows
      Revert "work around python/mypy#11852"
      add git version to error output
      get lua version from luarocks itself
      v2.17.0

Jamie Alessio (1):
      Update rbenv / ruby-build versions

Lorenz Walthert (3):
      better r path detection
      unset renv project
      no docs

Matt Layman (1):
      Add naive and untested version of Lua language support.

Ralf Schmitt (1):
      Use 'go install' instead of 'go get'

Tony Rintala (2):
      fix: Add missing warning for regular expression with [\\/]
      fix: regex lists to regex tuples

Uwe L. Korn (1):
      Add mamba support to `language: conda`

pre-commit-ci[bot] (4):
      [pre-commit.ci] pre-commit autoupdate
      [pre-commit.ci] pre-commit autoupdate
      [pre-commit.ci] pre-commit autoupdate
      [pre-commit.ci] pre-commit autoupdate
saravankrish pushed a commit to saravankrish/pre-commit that referenced this issue Jul 7, 2023
saravankrish pushed a commit to saravankrish/pre-commit that referenced this issue Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants