Skip to content

Commit ebd2504

Browse files
author
Scott Sanderson
committed
ENH: Warn when defaults use non-interface attrs.
Warn if an interface default uses a non-interface member in its implementation.
1 parent 44b461c commit ebd2504

13 files changed

+290
-23
lines changed

interface/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
from .interface import default, implements, InvalidImplementation, Interface
1+
from .interface import implements, InvalidImplementation, Interface
2+
from .default import default
23

34
__all__ = [
45
'default',

interface/default.py

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
import dis
2+
import warnings
3+
4+
from .compat import PY3
5+
from .formatting import bulleted_list
6+
from .functional import keysorted, sliding_window
7+
8+
19
class default(object):
210
"""Default implementation of a function in terms of interface methods.
311
"""
@@ -6,3 +14,83 @@ def __init__(self, implementation):
614

715
def __repr__(self):
816
return "{}({})".format(type(self).__name__, self.implementation)
17+
18+
19+
class DefaultUsesNonInterfaceMembers(UserWarning):
20+
pass
21+
22+
23+
if PY3: # pragma: nocover-py2
24+
_DEFAULT_USES_NON_INTERFACE_MEMBER_TEMPLATE = (
25+
"Default implementation of {iface}.{method} uses non-interface attributes.\n\n"
26+
"The following attributes may be accessed but are not part of "
27+
"the interface:\n"
28+
"{non_members}\n\n"
29+
"Consider changing the implementation of {method} or making these attributes"
30+
" part of {iface}."
31+
)
32+
33+
def warn_if_defaults_use_non_interface_members(interface_name,
34+
defaults,
35+
members):
36+
"""Warn if an interface default uses non-interface members of self.
37+
"""
38+
for method_name, attrs in non_member_attributes(defaults, members):
39+
warnings.warn(_DEFAULT_USES_NON_INTERFACE_MEMBER_TEMPLATE.format(
40+
iface=interface_name,
41+
method=method_name,
42+
non_members=bulleted_list(attrs),
43+
))
44+
45+
def non_member_attributes(defaults, members):
46+
from .typed_signature import TypedSignature
47+
48+
for default_name, default in keysorted(defaults):
49+
impl = default.implementation
50+
51+
if isinstance(impl, staticmethod):
52+
# staticmethods can't use attributes of the interface.
53+
continue
54+
55+
self_name = TypedSignature(impl).first_argument_name
56+
if self_name is None:
57+
# No parameters.
58+
# TODO: This is probably a bug in the interface, since a method
59+
# with no parameters that's not a staticmethod probably can't
60+
# be called in any natural way.
61+
continue
62+
63+
used = accessed_attributes_of_local(impl, self_name)
64+
non_interface_usages = used - members
65+
66+
if non_interface_usages:
67+
yield default_name, sorted(non_interface_usages)
68+
69+
def accessed_attributes_of_local(f, local_name):
70+
"""
71+
Get a list of attributes of ``local_name`` accessed by ``f``.
72+
73+
The analysis performed by this function is conservative, meaning that
74+
it's not guaranteed to find **all** attributes used.
75+
"""
76+
used = set()
77+
# Find sequences of the form: LOAD_FAST(local_name), LOAD_ATTR(<name>).
78+
# This will find all usages of the form ``local_name.<name>``.
79+
#
80+
# It will **NOT** find usages in which ``local_name`` is aliased to
81+
# another name.
82+
for first, second in sliding_window(dis.get_instructions(f), 2):
83+
if first.opname == 'LOAD_FAST' and first.argval == local_name:
84+
if second.opname == 'LOAD_ATTR':
85+
used.add(second.argval)
86+
return used
87+
88+
else: # pragma: nocover-py3
89+
def warn_if_defaults_use_non_interface_members(*args, **kwargs):
90+
pass
91+
92+
def non_member_warnings(*args, **kwargs):
93+
return iter(())
94+
95+
def accessed_attributes_of_local(*args, **kwargs):
96+
return set()

interface/formatting.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
"""String formatting utilities.
2+
"""
3+
4+
5+
def bulleted_list(items):
6+
return "\n".join(map(" - {}".format, items))

interface/functional.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
----------
44
Functional programming utilities.
55
"""
6+
from collections import deque
7+
from operator import itemgetter
68
from .compat import viewkeys
79

810

@@ -16,6 +18,10 @@ def keyfilter(f, d):
1618
return {k: v for k, v in d.items() if f(k)}
1719

1820

21+
def keysorted(d):
22+
return sorted(d.items(), key=itemgetter(0))
23+
24+
1925
def valfilter(f, d):
2026
return {k: v for k, v in d.items() if f(v)}
2127

@@ -25,3 +31,19 @@ def dzip(left, right):
2531
k: (left.get(k), right.get(k))
2632
for k in viewkeys(left) & viewkeys(right)
2733
}
34+
35+
36+
def sliding_window(iterable, n):
37+
it = iter(iterable)
38+
items = deque(maxlen=n)
39+
try:
40+
for i in range(n):
41+
items.append(next(it))
42+
except StopIteration:
43+
return
44+
45+
yield tuple(items)
46+
47+
for item in it:
48+
items.append(item)
49+
yield tuple(items)

interface/interface.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@
33
---------
44
"""
55
from collections import defaultdict
6-
from functools import wraps
7-
import inspect
86
from operator import attrgetter, itemgetter
97
from textwrap import dedent
108
from weakref import WeakKeyDictionary
119

12-
from .compat import raise_from, viewkeys, with_metaclass
13-
from .default import default # noqa reexport
10+
from .compat import raise_from, with_metaclass
11+
from .default import default, warn_if_defaults_use_non_interface_members
12+
from .formatting import bulleted_list
1413
from .functional import complement, keyfilter, valfilter
1514
from .typecheck import compatible
1615
from .typed_signature import TypedSignature
@@ -75,9 +74,7 @@ def _conflicting_defaults(typename, conflicts):
7574
{interfaces}"""
7675
).format(
7776
attr=attrname,
78-
interfaces="\n".join(sorted([
79-
" - {name}".format(name=iface.__name__) for iface in interfaces
80-
]))
77+
interfaces=bulleted_list(sorted(map(getname, interfaces))),
8178
)
8279
return InvalidImplementation(message)
8380

@@ -108,6 +105,12 @@ def __new__(mcls, name, bases, clsdict):
108105
if isinstance(v, default):
109106
defaults[k] = v
110107

108+
warn_if_defaults_use_non_interface_members(
109+
name,
110+
defaults,
111+
set(signatures.keys())
112+
)
113+
111114
clsdict['_signatures'] = signatures
112115
clsdict['_defaults'] = defaults
113116
return super(InterfaceMeta, mcls).__new__(mcls, name, bases, clsdict)
@@ -284,7 +287,7 @@ def __new__(mcls, name, bases, clsdict, interfaces=empty_set):
284287
errors = []
285288
default_impls = {}
286289
default_providers = defaultdict(list)
287-
for iface in newtype.interfaces():
290+
for iface in sorted(newtype.interfaces(), key=getname):
288291
try:
289292
defaults_from_iface = iface.verify(newtype)
290293
for name, impl in defaults_from_iface.items():
@@ -306,7 +309,7 @@ def __new__(mcls, name, bases, clsdict, interfaces=empty_set):
306309
elif len(errors) == 1:
307310
raise errors[0]
308311
else:
309-
raise InvalidImplementation("\n\n".join(map(str, errors)))
312+
raise InvalidImplementation("\n".join(map(str, errors)))
310313

311314
def __init__(mcls, name, bases, clsdict, interfaces=empty_set):
312315
mcls._interfaces = interfaces
@@ -372,7 +375,7 @@ def implements(*interfaces):
372375
ordered_ifaces = tuple(sorted(interfaces, key=getname))
373376
iface_names = list(map(getname, ordered_ifaces))
374377

375-
name = "Implements{I}".format(I="_".join(iface_names))
378+
name = "Implements{}".format("_".join(iface_names))
376379
doc = dedent(
377380
"""\
378381
Implementation of {interfaces}.

interface/tests/test_functional.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
from functools import total_ordering
2+
3+
import pytest
4+
5+
from ..functional import keysorted, sliding_window
6+
7+
8+
def test_sliding_window():
9+
assert list(sliding_window([], 2)) == []
10+
assert list(sliding_window([1, 2, 3], 2)) == [(1, 2), (2, 3)]
11+
assert list(sliding_window([1, 2, 3, 4], 2)) == [(1, 2), (2, 3), (3, 4)]
12+
assert list(sliding_window([1, 2, 3, 4], 3)) == [(1, 2, 3), (2, 3, 4)]
13+
14+
15+
def test_keysorted():
16+
17+
@total_ordering
18+
class Unorderable(object):
19+
20+
def __init__(self, obj):
21+
self.obj = obj
22+
23+
def __eq__(self, other):
24+
raise AssertionError("Can't compare this.")
25+
26+
__ne__ = __lt__ = __eq__
27+
28+
with pytest.raises(AssertionError):
29+
sorted([Unorderable(0), Unorderable(0)])
30+
31+
d = {'c': Unorderable(3), 'b': Unorderable(2), 'a': Unorderable(1)}
32+
items = keysorted(d)
33+
34+
assert items[0][0] == 'a'
35+
assert items[0][1].obj == 1
36+
37+
assert items[1][0] == 'b'
38+
assert items[1][1].obj == 2
39+
40+
assert items[2][0] == 'c'
41+
assert items[2][1].obj == 3

interface/tests/test_interface.py

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
import pytest
22
from textwrap import dedent
33

4+
from ..compat import PY3
45
from ..interface import implements, InvalidImplementation, Interface, default
56

67

8+
py3_only = pytest.mark.skipif(not PY3, reason="Python 3 Only")
9+
10+
711
def test_valid_implementation():
812

913
class I(Interface): # pragma: nocover
@@ -220,12 +224,12 @@ class D failed to implement interface I:
220224
The following methods of I were not implemented:
221225
- m0(self, a)
222226
223-
class D failed to implemente interface J:
227+
class D failed to implement interface J:
224228
225229
The following methods of J were implemented with invalid signatures:
226-
- m1(self, z) != m1(self, a)
227-
"""
230+
- m1(self, z) != m1(self, a)"""
228231
)
232+
assert actual == expected
229233

230234

231235
def test_implements_memoization():
@@ -570,3 +574,67 @@ def foo(a, b): # pragma: nocover
570574
pass
571575

572576
assert repr(foo) == "default({})".format(foo.implementation)
577+
578+
579+
@py3_only
580+
def test_default_warns_if_method_uses_non_interface_methods(): # pragma: nocover-py2
581+
582+
with pytest.warns(UserWarning) as warns:
583+
584+
class HasDefault(Interface):
585+
586+
def interface_method(self, x):
587+
pass
588+
589+
@default
590+
def foo(self, a, b):
591+
# Shouldn't cause warnings.
592+
self.default_method()
593+
self.default_classmethod()
594+
self.default_staticmethod()
595+
self.interface_method(a)
596+
597+
@default
598+
def probably_broken_method_with_no_args():
599+
pass # This eex
600+
601+
@default
602+
@staticmethod
603+
def default_staticmethod():
604+
# Shouldn't cause warnings.
605+
global some_nonexistent_method
606+
return some_nonexistent_method()
607+
608+
@default
609+
def default_method(self, x):
610+
foo = self.foo(1, 2) # Should be fine.
611+
wut = self.not_in_interface(2, 3) # Should cause a warning.
612+
return foo + wut
613+
614+
@default
615+
@classmethod
616+
def default_classmethod(cls, x):
617+
return cls.class_bar(2, 3) # Should cause a warning.
618+
619+
messages = warns.list
620+
assert len(messages) == 2
621+
622+
first = str(messages[0].message)
623+
expected_first = """\
624+
Default implementation of HasDefault.default_classmethod uses non-interface attributes.
625+
626+
The following attributes may be accessed but are not part of the interface:
627+
- class_bar
628+
629+
Consider changing the implementation of default_classmethod or making these attributes part of HasDefault.""" # noqa
630+
assert first == expected_first
631+
632+
second = str(messages[1].message)
633+
expected_second = """\
634+
Default implementation of HasDefault.default_method uses non-interface attributes.
635+
636+
The following attributes may be accessed but are not part of the interface:
637+
- not_in_interface
638+
639+
Consider changing the implementation of default_method or making these attributes part of HasDefault.""" # noqa
640+
assert second == expected_second

interface/tests/test_typecheck.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from ..compat import PY3, signature
22
from ..typecheck import compatible
3+
from ..typed_signature import TypedSignature
34

45

56
def test_compatible_when_equal():
@@ -72,5 +73,10 @@ def impl(a, b, c, d=3, e=5, f=5): # pragma: nocover
7273
assert not compatible(iface, impl)
7374

7475

76+
def test_first_argument_name():
77+
assert TypedSignature(lambda x, y, z: x).first_argument_name == 'x'
78+
assert TypedSignature(lambda: 0).first_argument_name is None
79+
80+
7581
if PY3: # pragma: nocover
7682
from ._py3_typecheck_tests import *

0 commit comments

Comments
 (0)