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

Add mypy plugin for NewType #55

Merged
merged 7 commits into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Auto-generated from tests
test-output

# Byte-compiled / optimized / DLL files
__pycache__/
*.py[cod]
Expand Down
65 changes: 65 additions & 0 deletions marshmallow_dataclass/mypy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import inspect
from typing import Callable, Optional, Type

from mypy import nodes
from mypy.plugin import DynamicClassDefContext, Plugin

import marshmallow_dataclass

_NEW_TYPE_SIG = inspect.signature(marshmallow_dataclass.NewType)


def plugin(version: str) -> Type[Plugin]:
return MarshmallowDataclassPlugin


class MarshmallowDataclassPlugin(Plugin):
def get_dynamic_class_hook(
self, fullname: str
) -> Optional[Callable[[DynamicClassDefContext], None]]:
if fullname == "marshmallow_dataclass.NewType":
return new_type_hook
return None


def new_type_hook(ctx: DynamicClassDefContext) -> None:
"""
Dynamic class hook for :func:`marshmallow_dataclass.NewType`.

Uses the type of the ``typ`` argument.
"""
typ = _get_arg_by_name(ctx.call, "typ", _NEW_TYPE_SIG)
if not isinstance(typ, nodes.RefExpr):
return
info = typ.node
if not isinstance(info, nodes.TypeInfo):
return
ctx.api.add_symbol_table_node(ctx.name, nodes.SymbolTableNode(nodes.GDEF, info))


def _get_arg_by_name(
call: nodes.CallExpr, name: str, sig: inspect.Signature
) -> Optional[nodes.Expression]:
"""
Get value of argument from a call.

:return: The argument value, or ``None`` if it cannot be found.

.. warning::
This probably doesn't yet work for calls with ``*args`` and/or ``*kwargs``.
"""
args = []
kwargs = {}
for arg_name, arg_value in zip(call.arg_names, call.args):
if arg_name is None:
args.append(arg_value)
else:
kwargs[arg_name] = arg_value
try:
bound_args = sig.bind(*args, **kwargs)
except TypeError:
return None
try:
return bound_args.arguments[name]
except KeyError:
return None
2 changes: 2 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
':python_version == "3.6"': ["dataclasses"],
"lint": ["pre-commit~=1.18"],
"docs": ["sphinx"],
"tests": ["mypy; implementation_name != 'pypy'"],
selimb marked this conversation as resolved.
Show resolved Hide resolved
}
EXTRAS_REQUIRE["dev"] = (
EXTRAS_REQUIRE["enum"]
+ EXTRAS_REQUIRE["union"]
+ EXTRAS_REQUIRE["lint"]
+ EXTRAS_REQUIRE["docs"]
+ EXTRAS_REQUIRE["tests"]
)

setup(
Expand Down
93 changes: 93 additions & 0 deletions tests/test_mypy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import shutil
import textwrap
import os
import unittest

mypy_installed = True
try:
import mypy.api
except ImportError:
# mypy not installed on pypy (see setup.py)
mypy_installed = False

HERE = os.path.dirname(__file__)
TEST_OUTPUT_DIR = os.path.join(HERE, "test-output")
MYPY_INI = """\
[mypy]
follow_imports = silent
plugins = marshmallow_dataclass.mypy
"""


@unittest.skipUnless(mypy_installed, "mypy required")
class TestMypyPlugin(unittest.TestCase):
maxDiff = None

def setUp(self):
"""
Prepare a clean test directory at tests/test-output/test_mypy-{testname}.
Also cd into it for the duration of the test to get simple filenames in mypy output.
"""
testname = self.id().split(".")[-1]
self.testdir = os.path.join(TEST_OUTPUT_DIR, f"test_mypy-{testname}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use tempfile.TemporaryDirectory for this?

Sidenote: this is another good time to re-evaluate pytest, which has built-in functionality for this sort of thing. Not a blocker, but worth considering. @lovasoa

Copy link
Owner

Choose a reason for hiding this comment

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

I argued against pytest in the past because I felt like it wasn't providing anything we needed for this project. But if there is a plugin that allows testing the mypy plugin easily, then I have nothing against switching to pytest and using it. It would definitely make the transition worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh. Deleted previous comment by accident. Copied below for reference.

Hah, yeah I must admit I was bit surprised that this was using unittest. Note that there's also a pytest plugin to easily test mypy plugins, which would effectively remove the need for all the code in setup, teardown and assert_mypy_output.
Do you want me to leave a TODO somewhere, as a reminder to switch to (or at least consider) pytest-mypy-plugins if the project moves to pytest?

Also note that pytest supports unittest, so you could use the pytest runner without necessarily "rewriting" the existing tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lovasoa Cool. How do you want to proceed re: existing tests and re: this PR?

Copy link
Owner

Choose a reason for hiding this comment

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

Since switching to pytest doesn't require rewriting any of the existing tests, I think we can do it in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright! Will work on that later today.

if os.path.exists(self.testdir):
shutil.rmtree(self.testdir)
os.makedirs(self.testdir)
self.old_cwd = os.getcwd()
os.chdir(self.testdir)

def tearDown(self):
os.chdir(self.old_cwd)

def mypy_test(self, contents: str, expected: str):
selimb marked this conversation as resolved.
Show resolved Hide resolved
"""
Run mypy and assert output matches ``expected``.

The file with ``contents`` is always named ``main.py``.
"""
config_path = os.path.join(self.testdir, "mypy.ini")
script_path = os.path.join(self.testdir, "main.py")
with open(config_path, "w") as f:
f.write(MYPY_INI)
with open(script_path, "w") as f:
f.write(textwrap.dedent(contents).strip())
command = [script_path, "--config-file", config_path, "--no-error-summary"]
out, err, returncode = mypy.api.run(command)
err_msg = "\n".join(
["", f"returncode: {returncode}", "stdout:", out, "stderr:", err]
)
self.assertEqual(out.strip(), textwrap.dedent(expected).strip(), err_msg)

def test_basic(self):
self.mypy_test(
"""
from dataclasses import dataclass
import marshmallow as ma
from marshmallow_dataclass import NewType

Email = NewType("Email", str, validate=ma.validate.Email)
UserID = NewType("UserID", validate=ma.validate.Length(equal=32), typ=str)

@dataclass
class User:
id: UserID
email: Email

user = User(id="a"*32, email="user@email.com")
reveal_type(user.id)
reveal_type(user.email)

User(id=42, email="user@email.com")
User(id="a"*32, email=["not", "a", "string"])
""",
"""
main.py:14: note: Revealed type is 'builtins.str'
main.py:15: note: Revealed type is 'builtins.str'
main.py:17: error: Argument "id" to "User" has incompatible type "int"; expected "str"
main.py:18: error: Argument "email" to "User" has incompatible type "List[str]"; expected "str"
""",
)


if __name__ == "__main__":
unittest.main()