Skip to content

Add Class Decorator/Metaclass/Base Class plugin #4328

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

Merged
merged 13 commits into from
Dec 14, 2017
102 changes: 97 additions & 5 deletions mypy/plugin.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
"""Plugin system for extending mypy."""

from collections import OrderedDict
from abc import abstractmethod
from typing import Callable, List, Tuple, Optional, NamedTuple, TypeVar
from typing import Callable, List, Tuple, Optional, NamedTuple, TypeVar, Dict

from mypy.nodes import Expression, StrExpr, IntExpr, UnaryExpr, Context, DictExpr
from mypy.nodes import Expression, StrExpr, IntExpr, UnaryExpr, Context, \
DictExpr, TypeInfo, ClassDef, ARG_POS, ARG_OPT, Var, Argument, FuncDef, \
Block, SymbolTableNode, MDEF
from mypy.types import (
Type, Instance, CallableType, TypedDictType, UnionType, NoneTyp, FunctionLike, TypeVarType,
Type, Instance, CallableType, TypedDictType, UnionType, NoneTyp, TypeVarType,
AnyType, TypeList, UnboundType, TypeOfAny
)
from mypy.messages import MessageBuilder
Expand Down Expand Up @@ -53,6 +54,14 @@ def named_generic_type(self, name: str, args: List[Type]) -> Instance:
raise NotImplementedError


class SemanticAnalyzerPluginInterface:
"""Interface for accessing semantic analyzer functionality in plugins."""

@abstractmethod
def named_type(self, qualified_name: str, args: Optional[List[Type]] = None) -> Instance:
Copy link
Member

Choose a reason for hiding this comment

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

Is this the only method that would be potentially used by plugins? Do we need to add some more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My current explorations also need parse_bool and anal_type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fail could be useful too. Should I add them as needed or add them now?

Copy link
Member

Choose a reason for hiding this comment

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

Should I add them as needed or add them now?

I am not sure why we actually need this class. Can't we just pass everything? Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm following the pattern of the other plugins. Technically the object that's passed in is the full object. I believe this base class is here to limit what methods the plugins should be calling. i.e. if the method isn't listed here then the mypy checker will complain about it.

Copy link
Member

Choose a reason for hiding this comment

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

Then we probably need to think what to add here. My expectation is that many methods of SemanticAnalyzer may be useful. You can play more with your implementation for attrs and add here whatever you ever used.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can keep only these methods, and then add more in your PR with the actual plugin (if necessary).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is why we are using an ABC instead exposing the entire semantic analyzer interface:

  • By only giving access to a subset of an interface, we declare the public part of the semantic analyzer interface. This has a few benefits related to modularity and loose coupling:
    • Everything else in the semantic analyzer can be more freely modified without breaking plugins. If we change the public interface, we may break 3rd party plugins. (Currently the plugin interface is still not officially supported so we can change things without worries, but I'd like to make the interface more stable in the future.)
    • It's less likely that plugins will rely on internal implementation details that are likely to change, or on poorly thought-out internal APIs that are hard to use correctly.
    • It's easier to reason about plugins in general, as the plugins are expected to stick to the exposed public interface instead of freely accessing mypy internals.
  • This can avoid a cyclic import dependency (maybe not right now, but it will help untangle cyclic dependencies). They slow down incremental checking.

raise NotImplementedError


# A context for a function hook that infers the return type of a function with
# a special signature.
#
Expand Down Expand Up @@ -98,6 +107,11 @@ def named_generic_type(self, name: str, args: List[Type]) -> Instance:
('context', Context),
('api', CheckerPluginInterface)])

ClassDefContext = NamedTuple(
'ClassDecoratorContext', [
('cls', ClassDef),
('api', SemanticAnalyzerPluginInterface)
])

class Plugin:
"""Base class of all type checker plugins.
Expand Down Expand Up @@ -136,7 +150,17 @@ def get_attribute_hook(self, fullname: str
) -> Optional[Callable[[AttributeContext], Type]]:
return None

# TODO: metaclass / class decorator hook
def get_class_decorator_hook(self, fullname: str
) -> Optional[Callable[[ClassDefContext], None]]:
return None

def get_class_metaclass_hook(self, fullname: str
Copy link
Member

Choose a reason for hiding this comment

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

I think you can call this just get_metaclass_hook.

) -> Optional[Callable[[ClassDefContext], None]]:
return None

def get_class_base_hook(self, fullname: str
Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_base_class_hook maybe?

Copy link
Member

Choose a reason for hiding this comment

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

get_base_class_hook maybe?

As you prefer, both are fine with me.

) -> Optional[Callable[[ClassDefContext], None]]:
return None


T = TypeVar('T')
Expand Down Expand Up @@ -182,6 +206,18 @@ def get_attribute_hook(self, fullname: str
) -> Optional[Callable[[AttributeContext], Type]]:
return self._find_hook(lambda plugin: plugin.get_attribute_hook(fullname))

def get_class_decorator_hook(self, fullname: str
) -> Optional[Callable[[ClassDefContext], None]]:
return self._find_hook(lambda plugin: plugin.get_class_decorator_hook(fullname))

def get_class_metaclass_hook(self, fullname: str
) -> Optional[Callable[[ClassDefContext], None]]:
return self._find_hook(lambda plugin: plugin.get_class_metaclass_hook(fullname))

def get_class_base_hook(self, fullname: str
) -> Optional[Callable[[ClassDefContext], None]]:
return self._find_hook(lambda plugin: plugin.get_class_base_hook(fullname))

def _find_hook(self, lookup: Callable[[Plugin], T]) -> Optional[T]:
for plugin in self._plugins:
hook = lookup(plugin)
Expand Down Expand Up @@ -215,6 +251,12 @@ def get_method_hook(self, fullname: str
return int_pow_callback
return None

def get_class_decorator_hook(self, fullname: str
) -> Optional[Callable[[ClassDefContext], None]]:
if fullname == 'attr.s':
return attr_s_callback
return None


def open_callback(ctx: FunctionContext) -> Type:
"""Infer a better return type for 'open'.
Expand Down Expand Up @@ -332,3 +374,53 @@ def int_pow_callback(ctx: MethodContext) -> Type:
else:
return ctx.api.named_generic_type('builtins.float', [])
return ctx.default_return_type


def add_method(
info: TypeInfo,
method_name: str,
args: List[Argument],
ret_type: Type,
self_type: Type,
function_type: Instance) -> None:
from mypy.semanal import set_callable_name

first = [Argument(Var('self'), self_type, None, ARG_POS)]
args = first + args

arg_types = [arg.type_annotation for arg in args]
arg_names = [arg.variable.name() for arg in args]
arg_kinds = [arg.kind for arg in args]
assert None not in arg_types
signature = CallableType(arg_types, arg_kinds, arg_names,
ret_type, function_type)
func = FuncDef(method_name, args, Block([]))
func.info = info
func.is_class = False
func.type = set_callable_name(signature, func)
func._fullname = info.fullname() + '.' + method_name
info.names[method_name] = SymbolTableNode(MDEF, func)


def attr_s_callback(ctx: ClassDefContext) -> None:
"""Add an __init__ method to classes decorated with attr.s."""
info = ctx.cls.info
# TODO: Handle default arguments
has_default = {} # type: Dict[str, Expression]
args = []

for name, table in info.names.items():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just discovered this is overly broad, but you get the idea.

if table.type:
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be treated differently. You should look at the right hand sides for assignments in the class and take those that have attr.ib (note however, that I am not an expert in attrs so I would like to see many tests for this reviewed by an attrs maintainer).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If possible I'd like to remove all this code from the PR. The actual plugin will require possibly more thought. Is it weird to just have a commit that creates a plugin but doesn't actually use it?

Also it turns out I can't actually use a plugin unless attr.pyi exists. So I'd have to figure out how to bring in python-attrs/attrs#238

Copy link
Member

Choose a reason for hiding this comment

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

If possible I'd like to remove all this code from the PR. The actual plugin will require possibly more thought. Is it weird to just have a commit that creates a plugin but doesn't actually use it?

It is totally OK. I would say it is even preferable to split this PR in two parts:

  • new plugin API
  • actual attrs plugin (with tests)

var = Var(name.lstrip("_"), table.type)
Copy link
Member

Choose a reason for hiding this comment

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

Variable name table is a bit misleading, I would call this symbol or similar.

default = has_default.get(var.name(), None)
kind = ARG_POS if default is None else ARG_OPT
args.append(Argument(var, var.type, default, kind))

add_method(
info=info,
method_name='__init__',
args=args,
ret_type=NoneTyp(),
self_type=ctx.api.named_type(info.name()),
function_type=ctx.api.named_type('__builtins__.function'),
)
33 changes: 31 additions & 2 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
from mypy.sametypes import is_same_type
from mypy.options import Options
from mypy import experiments
from mypy.plugin import Plugin
from mypy.plugin import Plugin, ClassDefContext, SemanticAnalyzerPluginInterface
from mypy import join
from mypy.util import get_prefix

Expand Down Expand Up @@ -172,7 +172,7 @@
}


class SemanticAnalyzerPass2(NodeVisitor[None]):
class SemanticAnalyzerPass2(NodeVisitor[None], SemanticAnalyzerPluginInterface):
"""Semantically analyze parsed mypy files.

The analyzer binds names and does various consistency checks for a
Expand Down Expand Up @@ -720,6 +720,35 @@ def analyze_class_body(self, defn: ClassDef) -> Iterator[bool]:
self.calculate_abstract_status(defn.info)
self.setup_type_promotion(defn)

for decorator in defn.decorators:
fullname = None
if isinstance(decorator, CallExpr):
if isinstance(decorator.callee, RefExpr):
fullname = decorator.callee.fullname
elif isinstance(decorator, NameExpr):
fullname = decorator.fullname

if fullname:
hook = self.plugin.get_class_decorator_hook(fullname)
if hook:
hook(ClassDefContext(defn, self))

if defn.metaclass:
metaclass_name = None
if isinstance(defn.metaclass, NameExpr):
metaclass_name = defn.metaclass.name
elif isinstance(defn.metaclass, MemberExpr):
metaclass_name = get_member_expr_fullname(
defn.metaclass)
hook = self.plugin.get_class_metaclass_hook(metaclass_name)
if hook:
hook(ClassDefContext(defn, self))

for type_info in defn.info.bases:
hook = self.plugin.get_class_base_hook(type_info.type.fullname())
if hook:
hook(ClassDefContext(defn, self))

Copy link
Member

Choose a reason for hiding this comment

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

I would factor out this whole block into a separate method, and call it here like self.apply_plugin_hooks(defn) or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Will do.

self.leave_class()

def analyze_class_keywords(self, defn: ClassDef) -> None:
Expand Down