Typing changes in 8.1.4 cause failure of mypy check

[sgillies]

Click's simple example https://github.com/pallets/click#a-simple-example doesn't pass mypy 1.4.1 since 8.1.4. With click 8.1.3 the example did pass. This affects not only the simple example but also existing projects that use click. I ran mypy on one of my projects and everything passed, ran it 30 minutes later and saw failures out of the blue. Click 8.1.4 was published to PyPI between my checks.

To reproduce:

• Save the simple example as click.py
• pip install mypy==1.4.1 click==8.1.3
• mypy click.py

This is the output:

click.py:3: error: Argument 1 has incompatible type "Callable[[Any, Any], Any]"; expected <nothing>  [arg-type]
click.py:12: error: <nothing> not callable  [misc]

Environment:

• Python version: 3.11
• Click version: 8.1.4

[davidism]

To anyone seeing or voting on this issue: Happy to review a PR.

[sirosen]

We ran into this as well. Unfortunately I won't be able to work on a fix for several hours at least, but I'm happy to try things out and submit a PR if I can. If someone else gets to it first, I'd also be happy to help review and check the fix against our codebase if that would be useful.

I think the issue stems from the definition of the _Decorator type alias in the decorators. I'm not quite sure how the types should be shaped, but I tried a change which swaps an underlying TypeVar for a TypeAlias.

I put together a reproducer for the expected <nothing> behavior which tries to ignore most of the details, and a possible fix (though I'm not 100% sure that the types I played with are correct):

import typing as t 

from click import Command


T = t.TypeVar("T")
_AnyCallable: t.TypeAlias = t.Callable[..., t.Any]
_Decorator: t.TypeAlias = t.Callable[[T], T]
FC = t.TypeVar("FC", bound=t.Union[_AnyCallable, Command])


def option() -> _Decorator[FC]:
    def decorator(f: FC) -> FC:
        return f 

    return decorator


FC2: t.TypeAlias = t.Union[_AnyCallable, Command]
FC2_Decorator: t.TypeAlias = t.Callable[[FC2], FC2]


def option2() -> FC2_Decorator:
    def decorator(f: FC) -> FC:
        return f 

    return decorator


def foo() -> None:
    pass


reveal_type(option)
reveal_type(option2)
foo = option()(foo)
foo2 = option2()(foo)

[sirosen]

A minor note: I'm now fairly certain that my "solution" above is incorrect.

If FC2 is defined as an alias for _AnyCallable | Command, then it won't allow subtypes of _AnyCallable, which is not acceptable. (It would mean that a command whose callback is defined with a return type other than Any gets rejected.)

It might be okay to use Callable (with no parameters) in lieu of _AnyCallable, but I haven't yet learned why _AnyCallable is being used rather than Callable.

[rra]

I'm guessing the right way to type these decorators is to use the facilities described here, which are intended to deal with exactly this problem of decorating functions that may have arbitrary signatures:

https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators

[davidism]

That was what I used originally. It was apparently lacking, because we continue to get reports from users saying that their type checker doesn't like something, then we add more complexity to the type, then something else isn't correct, etc. Python's and mypy's implementation of typing can be a huge mess sometimes.

Whoever is going to change this, be sure to follow git blame (or GitHub's view of it) back in time until you get to the original addition of types. Make sure to take into account why the various commits were made along the way.

[Tinche]

Taking a quick look at this.

Would you consider changing the signature of option (and maybe others) to:

def option(
    *param_decls: str, cls: t.Optional[t.Type[Option]] = None, **attrs: t.Any
) -> t.Callable[[FC], FC]:

? This syntax is equivalent to what we have today.

The current implementation with a generic type alias might be a little too much for Mypy at this time (I'm getting note: Revealed type is "<nothing>"). This by itself won't solve this issue but it might be a necessary step.

After that, I think this overload is the issue:

# variant: name omitted, cls _must_ be a keyword argument, @command(cmd=CommandCls, ...)
# The correct way to spell this overload is to use keyword-only argument syntax:
# def command(*, cls: t.Type[CmdType], **attrs: t.Any) -> ...
# However, mypy thinks this doesn't fit the overloaded function. Pyright does
# accept that spelling, and the following work-around makes pyright issue a
# warning that CmdType could be left unsolved, but mypy sees it as fine. *shrug*
@t.overload
def command(
    name: None = None,
    cls: t.Type[CmdType] = ...,
    **attrs: t.Any,
) -> t.Callable[[_AnyCallable], CmdType]:
    ...

If I comment it out, the example type-checks. So we need to figure out how to fix it. Given the comment, it might not be super easy.

[Tinche]

Like the command comment mentions, the proper way to type that overload is:

@t.overload
def command(
    name: None = None,
    *,
    cls: t.Type[CmdType],
    **attrs: t.Any,
) -> t.Callable[[_AnyCallable], CmdType]:
    ...

The comment also mentions this doesn't actually pass Mypy. Now I haven't really used custom command in click, so I googled and found this example from the official docs: https://click.palletsprojects.com/en/8.1.x/advanced/#command-aliases. It looks like this example might be exercising the problematic overload?

If I try pasting the first part of the example into a file, Mypy seems to work fine with it:

import click


class AliasedGroup(click.Group):
    def get_command(self, ctx, cmd_name):
        rv = click.Group.get_command(self, ctx, cmd_name)
        if rv is not None:
            return rv
        matches = [x for x in self.list_commands(ctx) if x.startswith(cmd_name)]
        if not matches:
            return None
        elif len(matches) == 1:
            return click.Group.get_command(self, ctx, matches[0])
        ctx.fail(f"Too many matches: {', '.join(sorted(matches))}")

    def resolve_command(self, ctx, args):
        # always return the full command name
        _, cmd, args = super().resolve_command(ctx, args)
        return cmd.name if cmd is not None else None, cmd, args


@click.command(cls=AliasedGroup)
def cli():
    pass


reveal_type(cli)  # note: Revealed type is "a02.AliasedGroup"

So maybe Mypy fixed this in the meantime and we can use the proper overload variant?

[davidism]

I'm fine with changing signatures. I'm also fine with writing the "right" type for people using Click, even if mypy doesn't like it when checking Click itself; we can just add ignores and comments.

Another wrinkle in this, as shown by that comment, is that people want both mypy and pyright to work, and they have different behaviors regarding decorators.

[Tinche]

I can put together a PR over the weekend. How about I set up some typing tests too (by stealing them from @hynek in attrs)?

[davidism]

That would be great!

We added some typing tests to Flask after talking with Hynek at PyCon. The tests/typing directory has files with different examples that should pass (they don't have to be sensible/useful code). MyPy is configured to type check that directory as well, so it runs as part of tox and CI.

[davidism]

Happy to review a PR.

[ofek]

I think this is still broken but maybe I'm doing something incorrectly:

~\Desktop\j ddev{repo: core, org: staging}
❯ pip freeze | rg "click|mypy"
click==8.1.5
mypy==1.4.1
mypy-extensions==1.0.0

~\Desktop\j ddev{repo: core, org: staging}
❯ mypy .
t.py:17: error: <nothing> has no attribute "add_command"  [attr-defined]
Found 1 error in 1 file (checked 1 source file)

import click


@click.group(context_settings={'help_option_names': ['-h', '--help']}, invoke_without_command=True)
@click.version_option(version='0.1.0', prog_name='ddev')
@click.pass_context
def ddev(ctx: click.Context):
    print('ddev')


@click.command(name='ci')
@click.pass_context
def ci(ctx: click.Context):
    print('ci')


ddev.add_command(ci)


def main():
    return ddev(prog_name='ddev', windows_expand_args=False)


if __name__ == '__main__':
    main()

[Tinche]

@ofek And if you try with #2565?

[ofek]

Oh indeed that fixes it:

~\Desktop\j ddev{repo: core, org: staging}
❯ pip install -q -I "click @ git+https://github.com/layday/click.git@typing-fixes"

[notice] A new release of pip is available: 23.1.1 -> 23.1.2
[notice] To update, run: C:\USERS\OFEK\APPDATA\LOCAL\PROGRAMS\PYTHON\PYTHON311\PYTHON.EXE -m pip install --upgrade pip

~\Desktop\j ddev{repo: core, org: staging} took 5s
❯ pip freeze | rg "click|mypy"
click @ git+https://github.com/layday/click.git@dc597ba60ead6dac09c7858e18074b3dae7f1273
mypy==1.4.1
mypy-extensions==1.0.0

~\Desktop\j ddev{repo: core, org: staging}
❯ mypy .
Success: no issues found in 1 source file

[blue42u]

I'm still experiencing this issue, the examples in https://click.palletsprojects.com/en/8.1.x/quickstart/#nesting-commands fail to typecheck under MyPy 1.4.1. I imagine whatever type adjustments were done to click.command for 8.1.5 also needs to be done to click.group.

import click

@click.group()
def cli():
    pass

@cli.command()
def initdb():
    click.echo('Initialized the database')

@cli.command()
def dropdb():
    click.echo('Dropped the database')

❯ python3 -m pip list
Package           Version
----------------- -------
click             8.1.5
mypy              1.4.1
mypy-extensions   1.0.0
pip               23.1.2
setuptools        68.0.0
typing_extensions 4.7.1

❯ mypy --pretty example.py
example.py:7: error: <nothing> has no attribute "command"  [attr-defined]
    @cli.command()
     ^~~~~~~~~~~
example.py:11: error: <nothing> has no attribute "command"  [attr-defined]
    @cli.command()
     ^~~~~~~~~~~
Found 2 errors in 1 file (checked 1 source file)

[layday]

Can you try #2565?

[blue42u]

Can confirm #2565 resolves this issue for me. Thanks! Now to wait for 8.1.6...

[davidism]

Locking to avoid "me too" comments.

[davidism]

Click 8.1.6 is available on PyPI.

[aarnphm]

For anyone that runs mypy with pre-commit, make sure to add click>=8.1.6 to additional_dependencies