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

Attempt to make the test suite pass under mypy 1.0.0 #89

Merged
merged 10 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ packages =
zope-stubs
package_dir = =src
install_requires =
mypy==0.981
mypy==1.0.0
zope.interface
zope.schema
include_package_data = True
Expand Down
27 changes: 27 additions & 0 deletions src/mypy_zope/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ def make_simple_type(
}


HACK_IS_ABSTRACT_NON_PROPAGATING = -12345678


class ZopeInterfacePlugin(Plugin):
def __init__(self, options: Options):
super().__init__(options)
Expand Down Expand Up @@ -479,6 +482,9 @@ def _analyze_zope_interface(
replacement = self._adjust_interface_function(api, cls.info, item)
elif isinstance(item, OverloadedFuncDef):
replacement = self._adjust_interface_overload(api, cls.info, item)
elif isinstance(item, Decorator):
replacement = item
replacement.func = self._adjust_interface_function(api, cls.info, item.func)
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be causing error in the description. What this change is designed for?

The following patch to your branch makes all tests pass:

diff --git a/src/mypy_zope/plugin.py b/src/mypy_zope/plugin.py
index e9aeca7..02ab413 100644
--- a/src/mypy_zope/plugin.py
+++ b/src/mypy_zope/plugin.py
@@ -482,9 +482,6 @@ class ZopeInterfacePlugin(Plugin):
                 replacement = self._adjust_interface_function(api, cls.info, item)
             elif isinstance(item, OverloadedFuncDef):
                 replacement = self._adjust_interface_overload(api, cls.info, item)
-            elif isinstance(item, Decorator):
-                replacement = item
-                replacement.func = self._adjust_interface_function(api, cls.info, item.func)
             else:
                 continue
 
diff --git a/tests/samples/interface_meta.py b/tests/samples/interface_meta.py
index 2e04617..94d63fa 100644
--- a/tests/samples/interface_meta.py
+++ b/tests/samples/interface_meta.py
@@ -7,7 +7,7 @@ T = TypeVar('T', bound='zope.interface.Interface')
 class IBookmark(zope.interface.Interface):
     @staticmethod
     def create(url: str) -> 'IBookmark':
-        pass
+        return IBookmark(url)
 
 def createOb(iface: Type[T]) -> T:
     if zope.interface.interfaces.IInterface.providedBy(iface):

Copy link
Member

Choose a reason for hiding this comment

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

Well, actually my patch is stupid, interface cannot have an implementation, disregard it...

Copy link
Member

Choose a reason for hiding this comment

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

That create method didn't make too much sense anyways. I suggest this, unless there's something important about handling decorators:

diff --git a/src/mypy_zope/plugin.py b/src/mypy_zope/plugin.py
index e9aeca7..02ab413 100644
--- a/src/mypy_zope/plugin.py
+++ b/src/mypy_zope/plugin.py
@@ -482,9 +482,6 @@ class ZopeInterfacePlugin(Plugin):
                 replacement = self._adjust_interface_function(api, cls.info, item)
             elif isinstance(item, OverloadedFuncDef):
                 replacement = self._adjust_interface_overload(api, cls.info, item)
-            elif isinstance(item, Decorator):
-                replacement = item
-                replacement.func = self._adjust_interface_function(api, cls.info, item.func)
             else:
                 continue
 
diff --git a/tests/samples/interface_meta.py b/tests/samples/interface_meta.py
index 2e04617..35a7d42 100644
--- a/tests/samples/interface_meta.py
+++ b/tests/samples/interface_meta.py
@@ -5,8 +5,6 @@ T = TypeVar('T', bound='zope.interface.Interface')
 
 
 class IBookmark(zope.interface.Interface):
-    @staticmethod
-    def create(url: str) -> 'IBookmark':
         pass
 
 def createOb(iface: Type[T]) -> T:
@@ -20,6 +18,6 @@ def main(self) -> None:
 
 """
 <output>
-interface_meta.py:19: note: Revealed type is "__main__.IBookmark"
+interface_meta.py:17: note: Revealed type is "__main__.IBookmark"
 </output>
 """

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What this change is designed for?

The IBookmark example that you cite. I thought the staticmethod was intended to be part of the IBookmark interface, so that classes which implement it would have to define their own create method.

If that's not the case then let's apply your patch!

Copy link
Member

Choose a reason for hiding this comment

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

I don't even think interfaces can define static methods... At least it doesn't make sense to me today.

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 suggest this, unless there's something important about handling decorators:

Ahh I see, the test doesn't even make use of create. Let's try the second patch.

else:
continue

Expand Down Expand Up @@ -520,6 +526,27 @@ def _adjust_interface_function(
func_def.arg_kinds.insert(0, ARG_POS)
func_def.arguments.insert(0, selfarg)

# 1: We want mypy to consider this method abstract, so that it allows the
# method to have an empty body without causing a warning.
# 2: We want mypy to consider the interface class NOT abstract, so we can use the
# "adaption" pattern.
# Unfortunately whenever mypy sees (1) it will mark the class as abstract,
# forcing (2) to be false. This seems to be a change in mypy 0.990, namely
# https://github.com/python/mypy/pull/13729
#
# Mypy 1.0.0:
# - allows empty bodies for abstract methods in mypy/checker.py:1240
# by testing if
# abstract_status != NOT_ABSTRACT.
# - marks classes as abstract based on their methods in
# mypy/semanal_classprop.py:79 by testing if
# abstract_status in (IS_ABSTRACT, IMPLICITLY_ABSTRACT)
#
# Thus we can make (1) and (2) true by setting abstract_status to some value
# distinct from these three NOT_ABSTRACT, IS_ABSTRACT and IMPLICITLY_ABSTRACT.
# These are presently the integers 0, 1, and 2 defined in mypy/nodes.py:738-743.
func_def.abstract_status = HACK_IS_ABSTRACT_NON_PROPAGATING
Copy link
Member

Choose a reason for hiding this comment

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

This is very unorthodox, but if it will let us to upgrade to mypy-1.0, I'm all for it! :)


return func_def

def _adjust_interface_overload(
Expand Down
2 changes: 1 addition & 1 deletion tests/samples/contextmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class A(Generic[_T]):

@contextmanager
def m(x: _T) -> Iterator[A[_T]]:
...
return iter([A()])


with m(7) as x:
Expand Down
2 changes: 1 addition & 1 deletion tests/samples/interface_unknown_inherit.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Bookmark(object):
<output>
interface_unknown_inherit.py:8: error: Cannot find implementation or library stub for module named "unknown.interfaces"
interface_unknown_inherit.py:8: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
interface_unknown_inherit.py:11: error: Method must have at least one argument
interface_unknown_inherit.py:11: error: Method must have at least one argument. Did you forget the "self" argument?
interface_unknown_inherit.py:14: error: zope.interface.implementer accepts interface, not __main__.IKnownInterface.
interface_unknown_inherit.py:14: error: Make sure you have stubs for all packages that provide interfaces for __main__.IKnownInterface class hierarchy.
</output>
Expand Down
4 changes: 2 additions & 2 deletions tests/samples/overload_readme.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def say() -> str:
def say(count: int) -> List[str]:
...

def say(count: int = None) -> Union[str, List[str]]:
def say(count: Optional[int] = None) -> Union[str, List[str]]:
pass


Expand All @@ -28,7 +28,7 @@ def say(self) -> str:
def say(self, count: int) -> List[str]:
...

def say(self, count: int = None) -> Union[str, List[str]]:
def say(self, count: Optional[int] = None) -> Union[str, List[str]]:
if count is None:
return "Mooo"
return ["Mooo"] * count
Expand Down
1 change: 1 addition & 0 deletions tests/test_samples.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def test_samples(samplefile, mypy_cache_dir):
opts.cache_dir = mypy_cache_dir
opts.show_traceback = True
opts.namespace_packages = True
opts.hide_error_codes = True
opts.plugins = ['mypy_zope:plugin']
# Config file is needed to load plugins, it doesn't not exist and is not
# supposed to.
Expand Down