mypy plugin broken with mypy==0.730, works with 0.711 about adt HOT 13 CLOSED

So I have been looking into this for a few days, now. It seems like mypy changed the way and timing of the execution of the hooks. There are three issues (so far) we need to solve:

• error: "Case" expects no type arguments, but 1 given
  The type analyzer stumbles over the Case usage having more type parameters than the definition. This is because the type is analyzed before the get_class_decorator_hook hook is executed. I was able to solve this by special-casing our Case type by providing a get_type_analyze_hook-hook, which returns a CaseConstructor type instead of Case[..].
• error: 'L' is a type variable and only valid in type context
  When using Case[T] with a type variable, the semantic analyzer is executed before the get_class_decorator_hook hook. Since the hook is not executed before, it is not removing the type variable and the semantic analyzer complains about the type variable existing in run-time code. I was able to solve this by introducing a get_customize_class_mro_hook-hook which performs the _get_and_delete_cases-operation on the code and stoes the _ClassDef in the plugin state.
• error: Cannot infer type argument 1 of "match" of "Expression"
  The plugin is unable to correctly resolve the match type when using a type variable in the Case. I'm currently looking into how to solve this.

An alternative would be to request mypy to provide a hook that lets us change class definition in the AST before they run the semantic analyzer. Since I'm still not completely sure how the plugin and mypy works, I have not done this, yet. If I understand more and there is indeed no simpler solution, I will make the feature request. I do expect implementation of the hook to take a while though, so it would be nice if we had a workaround to have adt work with newer mypy versions.

Edit: I realized that some of my test cases are wrong (due to the readme containing a typo) and some are also failing in the current version for mypy 0.711. I'm going to check which errors are actually due to the mypy version and give an update here.

from adt.

Should be fixed with #28

from adt.

Confirmed in a venv, works flawlessly with mypy==0.711, but broken with 0.730.

from adt.

The 0.720 release notes say:

Plugin API Changes
The new semantic analyzer may require changes to mypy plugins. We have a GitHub issue with more information for maintainers of plugins.

They moved to their new semantic analyzer as the default in this version.

from adt.

Thanks for trying it out! Sorry for my slow replies (past and future).

Just for due diligence' sake, do you see these issues if you manually install mypy==0.711, like in the requirements.txt? Of course, this would not be a permanent fix, it's just helpful to isolate the problem.

from adt.

Ah details: python/mypy#6617

from adt.

probably the same issue, but updated console log for mypy 0.750:

$ mypy --version
mypy 0.750

$ mypy part1.py 
part1.py:24: error: "Case" expects no type arguments, but 1 given
part1.py:25: error: "Case" expects no type arguments, but 1 given
part1.py:20: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.rtfd.io/en/latest/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 0.750
part1.py:20: : note: please use --show-traceback to print a traceback when reporting a bug

$ mypy --show-traceback part1.py 
part1.py:24: error: "Case" expects no type arguments, but 1 given
part1.py:25: error: "Case" expects no type arguments, but 1 given
part1.py:20: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.rtfd.io/en/latest/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 0.750
Traceback (most recent call last):
  File "mypy/semanal.py", line 4656, in accept
  File "mypy/nodes.py", line 939, in accept
  File "mypy/semanal.py", line 1029, in visit_class_def
  File "mypy/semanal.py", line 1106, in analyze_class
  File "mypy/semanal.py", line 1115, in analyze_class_body_common
  File "mypy/semanal.py", line 1161, in apply_class_plugin_hooks
  File "/home/zack/.local/lib/python3.7/site-packages/adt/mypy_plugin.py", line 115, in _transform_class
    cases = _get_and_delete_cases(context)
  File "/home/zack/.local/lib/python3.7/site-packages/adt/mypy_plugin.py", line 149, in _get_and_delete_cases
    _CaseDef(context=context, name=var.name(),
TypeError: 'str' object is not callable
part1.py:20: : note: use --pdb to drop into pdb

adt is great, so it's kinda of a pity it cannot be properly type checked right now with mypy :-( Is there any workaround for this?

thanks a lot for adt !

from adt.

There's a workaround in pinning to mypy==0.711, but otherwise the plugin just needs to be refactored for the latest version of mypy. I'm afraid I haven't had bandwidth to work on this, but I'd be happy to review anyone else's attempt in a pull request.

from adt.

@wchresta I took a cursory glance at these, based on your (very helpful) write-up, and in conjunction with the mypy plugin notes (found via the link @kastiglione shared).

The type analyzer stumbles over the Case usage having more type parameters than the definition. This is because the type is analyzed before the get_class_decorator_hook hook is executed.

I wasn't able to confirm this one--when adding some logging into adt's get_class_decorator_hook and _transform_class, it appears those functions are both invoked before the type errors are printed by mypy. Interestingly, _transform_class never ends up completing on file issue21.py.

My hypothesis is that type analysis is unintentionally getting triggered by adt while the class definition is being modified. I haven't dug deeper than that, though.

Sorry for the poorly-documented code, and wherever conventions in the project might be weird! Ironically, I wasn't very experienced in Python while building this. 😅

from adt.

@jspahrsummers yeah, at first it looked to me like that, too. I then took a debugger to observe mypy's internals. It seems like the output of the error messages might be buffered; the fail method seems to be called before our hook gets executed.

The reason for this is here:
https://github.com/python/mypy/blob/25c993be1007a09baac5d95c1d2bfce779055ad3/mypy/semanal.py#L1114-L1115

The type analyzer with raises the failure is ran when .accept() is called. But the decorator hook is only called after.

Anyway, it seemed like this is the only real problem with the new semantic analyzer. The other problems were either wrong syntax on my part or have already existed.

from adt.

I'm just dipping my toe into type-checking with mypy and adt, but I seem to still be getting this error with mypy 0.782. Can anyone else confirm that this bug is fixed?

stacks/models.py:2627: error: "Case" expects no type arguments, but 1 given
stacks/models.py:2631: error: "Case" expects no type arguments, but 1 given

from adt.

oh, never mind, it's because I didn't have the plugin enabled!

from adt.

@radix oh... can confirm!

ulidtko@pasocon ~ > cat test.py
from adt import adt, Case

@adt
class Cmd:
    READKEY:   Case[str, str]
    WRITEKEY:  Case[str, str, str]
    DELETEKEY: Case[str, str]

ulidtko@pasocon ~> mypy test.py
test.py:5: error: "Case" expects no type arguments, but 2 given
test.py:6: error: "Case" expects no type arguments, but 3 given
test.py:7: error: "Case" expects no type arguments, but 2 given
Found 3 errors in 1 file (checked 1 source file)

ulidtko@pasocon ~ [1]> mypy --version
mypy 0.782

Haha, I've fallen into the same trap again :)

Sure, you need to add this into setup.cfg (which doesn't exist of course when testing on standalone file):

[mypy]
plugins = adt.mypy_plugin

from adt.